chenya-zhang commented on a change in pull request #352:
URL:
https://github.com/apache/incubator-yunikorn-core/pull/352#discussion_r775056804
##########
File path: pkg/metrics/queue.go
##########
@@ -28,60 +28,38 @@ import (
"github.com/apache/incubator-yunikorn-core/pkg/log"
)
+// QueueMetrics to declare queue metrics
type QueueMetrics struct {
- // metrics related to app
- appMetrics *prometheus.CounterVec
-
- // metrics related to resource
- usedResourceMetrics *prometheus.GaugeVec
- pendingResourceMetrics *prometheus.GaugeVec
- availableResourceMetrics *prometheus.GaugeVec
+ appMetrics *prometheus.GaugeVec
Review comment:
I think it is more meaningful to count the current running apps in a
queue not "all the apps that have run in a queue unless the scheduler restarts".
I checked the Prometheus definition on counter v.s. gauge.
> Do not use a counter to expose a value that can decrease. For example, do
not use a counter for the number of currently running processes; instead use a
gauge.
> Gauges are typically used for measured values like temperatures or current
memory usage, but also "counts" that can go up and down, like the number of
concurrent requests.
https://prometheus.io/docs/concepts/metric_types/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
