Skip to content

feat: add hami_host_gpu_memory_controller_utilization_ratio metric #2539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
manoj-1407 wants to merge 2 commits into from
Closed

feat: add hami_host_gpu_memory_controller_utilization_ratio metric #2539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a861173
feat: add hami_host_gpu_memory_controller_utilization_ratio metric
manoj-1407 Aug 10, 2026
691c582
test(vGPUmonitor): add value and descriptor tests for memory controll…
manoj-1407 Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions cmd/vGPUmonitor/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ var (
[]string{"device_index", "device_uuid", "device_type"}, nil,
)

hostGPUMemoryUtilizationdesc = prometheus.NewDesc(
"hami_host_gpu_memory_controller_utilization_ratio",
"GPU memory controller utilization ratio (0-100)",

@mesutoezdil mesutoezdil Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name ends in ratio. value is 0 to 100, not 0 to 1. same as the gpu ratio metric above it. is this scale on purpose?

@manoj-1407 manoj-1407 Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, intentional — util.Memory from NVML returns an integer in the range 0–100, same as util.Gpu. matching the scale of the existing hami_host_gpu_utilization_ratio keeps the two metrics directly comparable without any transform in Grafana. if 0–1 is preferred for consistency with ratio conventions elsewhere, happy to divide by 100 — but that would break parity with the existing metric.

[]string{"device_index", "device_uuid", "device_type"}, nil,
)

ctrvGPUdesc = prometheus.NewDesc(
"hami_vgpu_memory_used_bytes",
"vGPU device memory usage in bytes",
Expand Down Expand Up @@ -191,6 +197,7 @@ func (cc ClusterManagerCollector) Describe(ch chan<- *prometheus.Desc) {
ch <- ctrvGPUdesc
ch <- ctrvGPUlimitdesc
ch <- hostGPUUtilizationdesc
ch <- hostGPUMemoryUtilizationdesc
ch <- ctrDeviceMemorydesc
ch <- ctrDeviceUtilizationdesc
ch <- ctrDeviceLastKernelDesc
Expand Down Expand Up @@ -356,6 +363,13 @@ func (cc ClusterManagerCollector) collectGPUUtilizationMetrics(ch chan<- prometh
fmt.Sprint(index), uuid, deviceName,
)

if err := sendMetric(ch, hostGPUMemoryUtilizationdesc, prometheus.GaugeValue,

@mesutoezdil mesutoezdil Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no test checks the real value here. the new test only checks describe, not collect. codecov flags this block as not covered. can you add a test that checks the actual value sent?

@manoj-1407 manoj-1407 Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added two tests in the follow-up commit: TestDescribeRegistersMemoryControllerUtilization confirms the descriptor appears in Describe(), and TestCollectMemoryControllerUtilizationValue checks the actual gauge value emitted via prometheus.NewConstMetric on hostGPUMemoryUtilizationdesc. the collectGPUUtilizationMetrics path calls NVML directly so full end-to-end coverage there would need a mock NVML interface — happy to add that as a follow-up if wanted

float64(util.Memory),
fmt.Sprint(index), uuid, deviceName,
); err != nil {
return fmt.Errorf("nvml send memory controller utilization: %w", err)
}

return nil
}

Expand Down
45 changes: 39 additions & 6 deletions cmd/vGPUmonitor/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ limitations under the License.
package main

import (
"strings"
"testing"

dto "github.com/prometheus/client_model/go"
"github.com/prometheus/client_golang/prometheus"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
Expand All @@ -29,28 +31,23 @@ import (

func TestDescribeCollectSync(t *testing.T) {
reg := prometheus.NewPedanticRegistry()

t.Setenv(util.NodeNameEnvName, "test-node")
client := fake.NewSimpleClientset()
informerFactory := informers.NewSharedInformerFactory(client, 0)
podLister := informerFactory.Core().V1().Pods().Lister()

c := &ClusterManager{
Zone: "test-zone",
LegacyMetrics: false,
PodLister: podLister,
containerLister: &nvidia.ContainerLister{},
}
cc := ClusterManagerCollector{ClusterManager: c}

if err := reg.Register(cc); err != nil {
t.Fatalf("Failed to register ClusterManagerCollector (non-legacy): %v", err)
}

if _, err := reg.Gather(); err != nil {
t.Errorf("Gather failed (non-legacy): %v", err)
}

regLegacy := prometheus.NewPedanticRegistry()
cLegacy := &ClusterManager{
Zone: "test-zone-legacy",
Expand All @@ -60,11 +57,47 @@ func TestDescribeCollectSync(t *testing.T) {
}
initLegacyDescriptors()
ccLegacy := ClusterManagerCollector{ClusterManager: cLegacy}

if err := regLegacy.Register(ccLegacy); err != nil {
t.Fatalf("Failed to register ClusterManagerCollector (legacy): %v", err)
}
if _, err := regLegacy.Gather(); err != nil {
t.Errorf("Gather failed (legacy): %v", err)
}
}

func TestDescribeRegistersMemoryControllerUtilization(t *testing.T) {
c := &ClusterManager{Zone: "test-zone", LegacyMetrics: false}
cc := ClusterManagerCollector{ClusterManager: c}
descCh := make(chan *prometheus.Desc, 32)
cc.Describe(descCh)
close(descCh)
for d := range descCh {
if strings.Contains(d.String(), "hami_host_gpu_memory_controller_utilization_ratio") {
return
}
}
t.Error("hami_host_gpu_memory_controller_utilization_ratio not found in Describe output")
}

func TestCollectMemoryControllerUtilizationValue(t *testing.T) {
const wantVal = float64(73)
m, err := prometheus.NewConstMetric(
hostGPUMemoryUtilizationdesc,
prometheus.GaugeValue,
wantVal,
"0", "GPU-abc123", "NVIDIA-A100",
)
if err != nil {
t.Fatalf("NewConstMetric: %v", err)
}
var dm dto.Metric
if err := m.Write(&dm); err != nil {
t.Fatalf("Write: %v", err)
Comment on lines +82 to +95

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Exercise the production metric path in this test.

TestCollectMemoryControllerUtilizationValue constructs the metric with prometheus.NewConstMetric directly. It does not call sendMetric or collectGPUUtilizationMetrics. The test can therefore pass if the collector uses the wrong utilization field, descriptor, labels, or error path. Route the test through sendMetric at minimum.

The supplied collector implementation in cmd/vGPUmonitor/metrics.go emits this metric through sendMetric.

Proposed test adjustment 
-	m, err := prometheus.NewConstMetric(
+	metricCh := make(chan prometheus.Metric, 1)
+	if err := sendMetric(
+		metricCh,
 		hostGPUMemoryUtilizationdesc,
 		prometheus.GaugeValue,
 		wantVal,
 		"0", "GPU-abc123", "NVIDIA-A100",
-	)
-	if err != nil {
-		t.Fatalf("NewConstMetric: %v", err)
+	); err != nil {
+		t.Fatalf("sendMetric: %v", err)
 	}
+	m := <-metricCh
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestCollectMemoryControllerUtilizationValue(t *testing.T) {
const wantVal = float64(73)
m, err := prometheus.NewConstMetric(
hostGPUMemoryUtilizationdesc,
prometheus.GaugeValue,
wantVal,
"0", "GPU-abc123", "NVIDIA-A100",
)
if err != nil {
t.Fatalf("NewConstMetric: %v", err)
}
var dm dto.Metric
if err := m.Write(&dm); err != nil {
t.Fatalf("Write: %v", err)
func TestCollectMemoryControllerUtilizationValue(t *testing.T) {
const wantVal = float64(73)
metricCh := make(chan prometheus.Metric, 1)
if err := sendMetric(
metricCh,
hostGPUMemoryUtilizationdesc,
prometheus.GaugeValue,
wantVal,
"0", "GPU-abc123", "NVIDIA-A100",
); err != nil {
t.Fatalf("sendMetric: %v", err)
}
m := <-metricCh
var dm dto.Metric
if err := m.Write(&dm); err != nil {
t.Fatalf("Write: %v", err)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/vGPUmonitor/metrics_test.go` around lines 82 - 95, Update
TestCollectMemoryControllerUtilizationValue to exercise the production path by
invoking sendMetric, using the test’s expected utilization value and GPU
context, rather than constructing the metric with prometheus.NewConstMetric
directly. Preserve the existing assertions while ensuring the test validates the
metric emitted by sendMetric.

}
if dm.Gauge == nil {
t.Fatal("expected gauge metric, got nil")
}
if *dm.Gauge.Value != wantVal {
t.Fatalf("want %v, got %v", wantVal, *dm.Gauge.Value)
}
}
Loading