From f1c545e489c314fb2be858e0260435d2fc380334 Mon Sep 17 00:00:00 2001 From: shivv23 Date: Sun, 2 Aug 2026 00:14:19 +0530 Subject: [PATCH] chore(metrics): remove Prometheus tutorial scaffolding and hoist scheduler descriptors Signed-off-by: shivv23 --- cmd/scheduler/metrics.go | 218 ++++++++++++++++++---------------- cmd/scheduler/metrics_test.go | 1 + cmd/vGPUmonitor/metrics.go | 92 ++------------ 3 files changed, 126 insertions(+), 185 deletions(-) diff --git a/cmd/scheduler/metrics.go b/cmd/scheduler/metrics.go index 1bde90e163..df7a037845 100644 --- a/cmd/scheduler/metrics.go +++ b/cmd/scheduler/metrics.go @@ -49,130 +49,150 @@ type ClusterManagerCollector struct { metricsProvider schedulerMetricsProvider } -// Describe is implemented with DescribeByCollect. That's possible because the -// Collect method will always return the same metrics with the same descriptors. -func (cc ClusterManagerCollector) Describe(ch chan<- *prometheus.Desc) { - prometheus.DescribeByCollect(cc, ch) -} - -// Collect creates constant metrics for each host on the fly based on the returned data. -func (cc ClusterManagerCollector) Collect(ch chan<- prometheus.Metric) { - klog.V(3).Info("Starting to collect metrics for scheduler") - legacy := cc.ClusterManager.LegacyMetrics - - // New metric descriptors - nodevGPUMemoryLimitDesc := prometheus.NewDesc( +// Descriptors used by ClusterManagerCollector below. +// Metric and label names follow Prometheus naming best practices: +// https://prometheus.io/docs/practices/naming/ +var ( + nodevGPUMemoryLimitDesc = prometheus.NewDesc( "hami_gpu_memory_limit_bytes", "Device memory limit for a certain GPU", []string{"node", "device_uuid", "device_index", "device_type"}, nil, ) - nodevGPUCoreLimitDesc := prometheus.NewDesc( + nodevGPUCoreLimitDesc = prometheus.NewDesc( "hami_gpu_core_limit_ratio", "Device core limit for a certain GPU", []string{"node", "device_uuid", "device_index", "device_type"}, nil, ) - nodevGPUMemoryAllocatedDesc := prometheus.NewDesc( + nodevGPUMemoryAllocatedDesc = prometheus.NewDesc( "hami_gpu_memory_allocated_bytes", "Device memory allocated for a certain GPU", []string{"node", "device_uuid", "device_index", "device_cores", "device_type"}, nil, ) - nodevGPUSharedNumDesc := prometheus.NewDesc( + nodevGPUSharedNumDesc = prometheus.NewDesc( "hami_gpu_shared_count", "Number of containers sharing this GPU", []string{"node", "device_uuid", "device_index", "device_type"}, nil, ) - nodeGPUCoreAllocatedDesc := prometheus.NewDesc( + nodeGPUCoreAllocatedDesc = prometheus.NewDesc( "hami_gpu_core_allocated_ratio", "Device core allocated for a certain GPU", []string{"node", "device_uuid", "device_index", "device_type"}, nil, ) - nodeGPUOverview := prometheus.NewDesc( + nodeGPUOverview = prometheus.NewDesc( "hami_node_gpu_overview", "GPU overview on a certain node", []string{"node", "device_uuid", "device_index", "device_cores", "device_memory_limit", "device_type"}, nil, ) - nodeGPUMemoryPercentage := prometheus.NewDesc( + nodeGPUMemoryPercentage = prometheus.NewDesc( "hami_node_gpu_memory_allocated_ratio", "GPU Memory Allocated Percentage on a certain GPU", []string{"node", "device_uuid", "device_index"}, nil, ) - nodeGPUMigInstance := prometheus.NewDesc( + nodeGPUMigInstance = prometheus.NewDesc( "hami_node_gpu_mig_instance_info", "GPU Sharing mode. 0 for hami-core, 1 for mig, 2 for mps", []string{"node", "device_uuid", "device_index", "mig_name"}, nil, ) + ctrvGPUdeviceAllocatedMemoryDesc = prometheus.NewDesc( + "hami_vgpu_memory_allocated_bytes", + "vGPU memory allocated from a container", + []string{"namespace", "node", "pod", "container_index", "device_uuid"}, nil, + ) + ctrvGPUdeviceAllocatedCoreDesc = prometheus.NewDesc( + "hami_vgpu_core_allocated_ratio", + "vGPU core allocated from a container", + []string{"namespace", "node", "pod", "container_index", "device_uuid"}, nil, + ) + quotaUsedDesc = prometheus.NewDesc( + "hami_resource_quota_used", + "resourcequota usage for a certain device", + []string{"namespace", "quota_name", "limit"}, nil, + ) +) + +// Legacy metric descriptors (populated only when legacy mode is enabled). +var ( + legacyMemoryLimitDesc *prometheus.Desc + legacyCoreLimitDesc *prometheus.Desc + legacyMemoryAllocatedDesc *prometheus.Desc + legacySharedNumDesc *prometheus.Desc + legacyCoreAllocatedDesc *prometheus.Desc + legacyOverview *prometheus.Desc + legacyMemoryPercentage *prometheus.Desc + legacyMigInstance *prometheus.Desc + legacyAllocatedMemory *prometheus.Desc + legacyAllocatedCore *prometheus.Desc + legacyQuotaUsed *prometheus.Desc +) - // Legacy metric descriptors (only created when legacy mode is enabled) - var ( - legacyMemoryLimitDesc *prometheus.Desc - legacyCoreLimitDesc *prometheus.Desc - legacyMemoryAllocatedDesc *prometheus.Desc - legacySharedNumDesc *prometheus.Desc - legacyCoreAllocatedDesc *prometheus.Desc - legacyOverview *prometheus.Desc - legacyMemoryPercentage *prometheus.Desc - legacyMigInstance *prometheus.Desc - legacyAllocatedMemory *prometheus.Desc - legacyAllocatedCore *prometheus.Desc - legacyQuotaUsed *prometheus.Desc +func initLegacyDescriptors() { + legacyMemoryLimitDesc = prometheus.NewDesc( + "GPUDeviceMemoryLimit", + "Device memory limit for a certain GPU", + []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, ) - if legacy { - legacyMemoryLimitDesc = prometheus.NewDesc( - "GPUDeviceMemoryLimit", - "Device memory limit for a certain GPU", - []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, - ) - legacyCoreLimitDesc = prometheus.NewDesc( - "GPUDeviceCoreLimit", - "Device memory core limit for a certain GPU", - []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, - ) - legacyMemoryAllocatedDesc = prometheus.NewDesc( - "GPUDeviceMemoryAllocated", - "Device memory allocated for a certain GPU", - []string{"nodeid", "deviceuuid", "deviceidx", "devicecores", "devicetype"}, nil, - ) - legacySharedNumDesc = prometheus.NewDesc( - "GPUDeviceSharedNum", - "Number of containers sharing this GPU", - []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, - ) - legacyCoreAllocatedDesc = prometheus.NewDesc( - "GPUDeviceCoreAllocated", - "Device core allocated for a certain GPU", - []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, - ) - legacyOverview = prometheus.NewDesc( - "nodeGPUOverview", - "GPU overview on a certain node", - []string{"nodeid", "deviceuuid", "deviceidx", "devicecores", "devicememorylimit", "devicetype"}, nil, - ) - legacyMemoryPercentage = prometheus.NewDesc( - "nodeGPUMemoryPercentage", - "GPU Memory Allocated Percentage on a certain GPU", - []string{"nodeid", "deviceuuid", "deviceidx"}, nil, - ) - legacyMigInstance = prometheus.NewDesc( - "nodeGPUMigInstance", - "GPU Sharing mode. 0 for hami-core, 1 for mig, 2 for mps", - []string{"nodeid", "deviceuuid", "deviceidx", "migname"}, nil, - ) - legacyAllocatedMemory = prometheus.NewDesc( - "vGPUMemoryAllocated", - "vGPU memory allocated from a container", - []string{"podnamespace", "nodename", "podname", "containeridx", "deviceuuid"}, nil, - ) - legacyAllocatedCore = prometheus.NewDesc( - "vGPUCoreAllocated", - "vGPU core allocated from a container", - []string{"podnamespace", "nodename", "podname", "containeridx", "deviceuuid"}, nil, - ) - legacyQuotaUsed = prometheus.NewDesc( - "QuotaUsed", - "resourcequota usage for a certain device", - []string{"quotanamespace", "quotaName", "limit"}, nil, - ) - } + legacyCoreLimitDesc = prometheus.NewDesc( + "GPUDeviceCoreLimit", + "Device memory core limit for a certain GPU", + []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, + ) + legacyMemoryAllocatedDesc = prometheus.NewDesc( + "GPUDeviceMemoryAllocated", + "Device memory allocated for a certain GPU", + []string{"nodeid", "deviceuuid", "deviceidx", "devicecores", "devicetype"}, nil, + ) + legacySharedNumDesc = prometheus.NewDesc( + "GPUDeviceSharedNum", + "Number of containers sharing this GPU", + []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, + ) + legacyCoreAllocatedDesc = prometheus.NewDesc( + "GPUDeviceCoreAllocated", + "Device core allocated for a certain GPU", + []string{"nodeid", "deviceuuid", "deviceidx", "devicetype"}, nil, + ) + legacyOverview = prometheus.NewDesc( + "nodeGPUOverview", + "GPU overview on a certain node", + []string{"nodeid", "deviceuuid", "deviceidx", "devicecores", "devicememorylimit", "devicetype"}, nil, + ) + legacyMemoryPercentage = prometheus.NewDesc( + "nodeGPUMemoryPercentage", + "GPU Memory Allocated Percentage on a certain GPU", + []string{"nodeid", "deviceuuid", "deviceidx"}, nil, + ) + legacyMigInstance = prometheus.NewDesc( + "nodeGPUMigInstance", + "GPU Sharing mode. 0 for hami-core, 1 for mig, 2 for mps", + []string{"nodeid", "deviceuuid", "deviceidx", "migname"}, nil, + ) + legacyAllocatedMemory = prometheus.NewDesc( + "vGPUMemoryAllocated", + "vGPU memory allocated from a container", + []string{"podnamespace", "nodename", "podname", "containeridx", "deviceuuid"}, nil, + ) + legacyAllocatedCore = prometheus.NewDesc( + "vGPUCoreAllocated", + "vGPU core allocated from a container", + []string{"podnamespace", "nodename", "podname", "containeridx", "deviceuuid"}, nil, + ) + legacyQuotaUsed = prometheus.NewDesc( + "QuotaUsed", + "resourcequota usage for a certain device", + []string{"quotanamespace", "quotaName", "limit"}, nil, + ) +} + +// Describe is implemented with DescribeByCollect. That's possible because the +// Collect method will always return the same metrics with the same descriptors. +func (cc ClusterManagerCollector) Describe(ch chan<- *prometheus.Desc) { + prometheus.DescribeByCollect(cc, ch) +} + +// Collect creates constant metrics for each host on the fly based on the returned data. +func (cc ClusterManagerCollector) Collect(ch chan<- prometheus.Metric) { + klog.V(3).Info("Starting to collect metrics for scheduler") + legacy := cc.ClusterManager.LegacyMetrics nu := cc.metricsProvider.InspectAllNodesUsage() for nodeID, val := range *nu { @@ -296,21 +316,6 @@ func (cc ClusterManagerCollector) Collect(ch chan<- prometheus.Metric) { } } - ctrvGPUdeviceAllocatedMemoryDesc := prometheus.NewDesc( - "hami_vgpu_memory_allocated_bytes", - "vGPU memory allocated from a container", - []string{"namespace", "node", "pod", "container_index", "device_uuid"}, nil, - ) - ctrvGPUdeviceAllocatedCoreDesc := prometheus.NewDesc( - "hami_vgpu_core_allocated_ratio", - "vGPU core allocated from a container", - []string{"namespace", "node", "pod", "container_index", "device_uuid"}, nil, - ) - quotaUsedDesc := prometheus.NewDesc( - "hami_resource_quota_used", - "resourcequota usage for a certain device", - []string{"namespace", "quota_name", "limit"}, nil, - ) for ns, val := range cc.metricsProvider.GetQuotaManager().GetResourceQuota() { for quotaname, q := range *val { ch <- prometheus.MustNewConstMetric( @@ -396,6 +401,9 @@ func (cc ClusterManagerCollector) Collect(ch chan<- prometheus.Metric) { // NewClusterManager creates a ClusterManager and registers its collector. func NewClusterManager(zone string, reg prometheus.Registerer, metricsProvider schedulerMetricsProvider, legacyMetrics bool) *ClusterManager { + if legacyMetrics { + initLegacyDescriptors() + } c := &ClusterManager{ Zone: zone, LegacyMetrics: legacyMetrics, diff --git a/cmd/scheduler/metrics_test.go b/cmd/scheduler/metrics_test.go index ef0e18f690..e6a855d29e 100644 --- a/cmd/scheduler/metrics_test.go +++ b/cmd/scheduler/metrics_test.go @@ -83,6 +83,7 @@ func TestClusterManagerCollectorSkipsMemoryRatioWithNonPositiveTotalMemory(t *te }, } + initLegacyDescriptors() collector := ClusterManagerCollector{ ClusterManager: &ClusterManager{ LegacyMetrics: true, diff --git a/cmd/vGPUmonitor/metrics.go b/cmd/vGPUmonitor/metrics.go index ea4b4bf0b4..0b9bd8a569 100644 --- a/cmd/vGPUmonitor/metrics.go +++ b/cmd/vGPUmonitor/metrics.go @@ -38,44 +38,19 @@ import ( "k8s.io/klog/v2" ) -// ClusterManager is an example for a system that might have been built without -// Prometheus in mind. It models a central manager of jobs running in a -// cluster. Thus, we implement a custom Collector called -// ClusterManagerCollector, which collects information from a ClusterManager -// using its provided methods and turns them into Prometheus Metrics for -// collection. -// -// An additional challenge is that multiple instances of the ClusterManager are -// run within the same binary, each in charge of a different zone. We need to -// make use of wrapping Registerers to be able to register each -// ClusterManagerCollector instance with Prometheus. +// ClusterManager models the vGPU monitoring state for a single node and holds +// the informers and listers used to discover the pods and containers running +// on that node. A custom Collector called ClusterManagerCollector collects +// information from a ClusterManager using its provided methods and turns them +// into Prometheus Metrics for collection. type ClusterManager struct { Zone string - // Contains many more fields not listed in this example. + // PodLister lists pods assigned to the monitored node. PodLister listerscorev1.PodLister containerLister *nvidia.ContainerLister LegacyMetrics bool } -// ReallyExpensiveAssessmentOfTheSystemState is a mock for the data gathering a -// real cluster manager would have to do. Since it may actually be really -// expensive, it must only be called once per collection. This implementation, -// obviously, only returns some made-up data. -func (c *ClusterManager) ReallyExpensiveAssessmentOfTheSystemState() ( - oomCountByHost map[string]int, ramUsageByHost map[string]float64, -) { - // Just example fake data. - oomCountByHost = map[string]int{ - "foo.example.org": 42, - "bar.example.org": 2001, - } - ramUsageByHost = map[string]float64{ - "foo.example.org": 6.023e23, - "bar.example.org": 3.14, - } - return -} - // ClusterManagerCollector implements the Collector interface. type ClusterManagerCollector struct { ClusterManager *ClusterManager @@ -211,9 +186,8 @@ func sendLegacyMetric(ch chan<- prometheus.Metric, desc *prometheus.Desc, valueT } } -// Describe is implemented with DescribeByCollect. That's possible because the -// Collect method will always return the same two metrics with the same two -// descriptors. +// Describe sends the metric descriptors to the provided channel. The Collect +// method always returns the same metrics with the same descriptors. func (cc ClusterManagerCollector) Describe(ch chan<- *prometheus.Desc) { ch <- hostGPUdesc ch <- ctrvGPUdesc @@ -237,52 +211,10 @@ func (cc ClusterManagerCollector) Describe(ch chan<- *prometheus.Desc) { } } -//func parseidstr(podusage string) (string, string, error) { -// tmp := strings.Split(podusage, "_") -// if len(tmp) > 1 { -// return tmp[0], tmp[1], nil -// } else { -// return "", "", errors.New("parse error") -// } -//} -// -//func gettotalusage(usage podusage, vidx int) (deviceMemory, error) { -// added := deviceMemory{ -// bufferSize: 0, -// contextSize: 0, -// moduleSize: 0, -// offset: 0, -// total: 0, -// } -// for _, val := range usage.sr.procs { -// added.bufferSize += val.used[vidx].bufferSize -// added.contextSize += val.used[vidx].contextSize -// added.moduleSize += val.used[vidx].moduleSize -// added.offset += val.used[vidx].offset -// added.total += val.used[vidx].total -// } -// return added, nil -//} -// -//func getTotalUtilization(usage podusage, vidx int) deviceUtilization { -// added := deviceUtilization{ -// decUtil: 0, -// encUtil: 0, -// smUtil: 0, -// } -// for _, val := range usage.sr.procs { -// added.decUtil += val.deviceUtil[vidx].decUtil -// added.encUtil += val.deviceUtil[vidx].encUtil -// added.smUtil += val.deviceUtil[vidx].smUtil -// } -// return added -//} - -// Collect first triggers the ReallyExpensiveAssessmentOfTheSystemState. Then it -// creates constant metrics for each host on the fly based on the returned data. -// -// Note that Collect could be called concurrently, so we depend on -// ReallyExpensiveAssessmentOfTheSystemState to be concurrency-safe. +// Collect gathers all device, pod and container metrics for the monitored +// node and emits them as constant metrics on the fly based on the returned +// data. Note that Collect could be called concurrently, so all state read +// here is expected to be concurrency-safe. func (cc ClusterManagerCollector) Collect(ch chan<- prometheus.Metric) { klog.Info("Starting to collect metrics for vGPUMonitor")