fix(chart): preserve HAMi workload labels in ServiceMonitor - #126
Conversation
HAMi's vgpu-monitor exports the GPU workload's identity as namespace, pod and container. Prometheus attaches labels of the same names for the scrape target itself, so with honorLabels disabled the workload's values are renamed to exported_namespace / exported_pod / exported_container and the WebUI's queries match nothing. Task compute and memory utilization then read as 0 rather than as missing data. Setting honorLabels on the HAMi ServiceMonitor lets the workload's own labels win. Series that do not collide are unaffected: device-level hami_host_gpu_* keeps the target labels, and the synthetic up series still carries the device plugin's own namespace/pod/container. Verified on k3s with HAMi 2.10 and a CUDA workload. Before the change the query used by taskCoreUsed matched 0 series; after it matched 1 and hami_container_core_used / core_util / memory_used are published with real values. No Go change is needed: the existing queries are correct once the scrape contract is. Signed-off-by: Nimbus318 <136771156+Nimbus318@users.noreply.github.com>
|
@Nimbus318: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Nimbus318 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Helm chart now enables ChangesPrometheus workload label preservation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The chart now preserves HAMi workload labels by default so existing utilization queries continue to return metrics, while operators can still disable the behavior explicitly. No actionable merge-blocking risk remains after normal checks and review. Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What type of PR is this?
/kind bug
What this PR does / why we need it:
With
honorLabels: false, Prometheus renames colliding HAMi workload labels(
namespace,pod, andcontainer) toexported_*. The NVIDIA task queriesuse the original names, so they return no series and
queryInstantValreports0.This PR sets
hamiServiceMonitor.honorLabels: true, preserving the workloadlabels and allowing the existing PromQL to work unchanged.
Verification
Tested with k3s, HAMi 2.10, and a CUDA workload:
0series before,1afterhelm lintpassedhelm templaterenderstrueby default--set hamiServiceMonitor.honorLabels=falsestill worksWhich issue(s) this PR fixes:
Supersedes #114. It correctly identified the label mismatch, but only rewrites
namespaceandpod. After Project-HAMi/HAMi#1990,containercan alsocollide, so that query still returns no series on the tested HAMi 2.10 setup.
Thanks to @Shenhan11 for identifying the issue.
Special notes for your reviewer:
Prometheus.spec.overrideHonorLabels: trueoverrides this setting. If the samePrometheus selects both HAMi ServiceMonitors, the endpoint is scraped twice.
Both cases are documented in the installation guide.
The empty-result-to-
0behavior should be addressed separately.Summary by CodeRabbit
New Features
Documentation