[GitHub] [incubator-yunikorn-core] chenya-zhang commented on a change in pull request #352: [YUNIKORN-721]Improve YuniKorn core's queue-level and scheduler metrics

[GitBox <gi...@apache.org>]

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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org