Repository navigation
fix: add label node_name to kepler_node_cpu_info metric - #2559
Conversation
bitflicker64
left a comment
There was a problem hiding this comment.
Thanks for this, looks good to me and nothing is blocking. kepler_node_gpu_info and the power metrics already carry node_name as a const label, so this lines kepler_node_cpu_info up with them. Both callers of NewCPUInfoCollector are updated, and the docs/user/metrics.md hunk has the same shape the generator emits for the other metrics.
One test suggestion inline, plus two small notes:
- The commit scope is
fix(device), but the change is in the Prometheus exporter.fix(exporter): ...would describe it better. - A new label changes the series identity, so each
kepler_node_cpu_infoseries ends at upgrade and a new one starts. The bundled dashboards'count by (...)andlabel_values(..., instance)queries don't group onnode_name, so they are fine. A line in the PR description would help whoever writes the release notes.
9b6c103 to
d64920d
Compare
bitflicker64
left a comment
There was a problem hiding this comment.
Thanks, d64920d4 covers all three: node_name is in expectedLabels(), the scope is exporter, and the upgrade note is in the description. LGTM.
|
@vprashar2929 could you take a look at this one? I've reviewed and approved it, but my approval doesn't count toward the merge requirement until #2550 lands. The contributor is new here, so the workflows also need "Approve and run" before |
sthaha
left a comment
There was a problem hiding this comment.
Looks good overall but I think we shouldn't make 'node-name' mandatory.
Signed-off-by: ffais <ffais@fbk.eu>
d64920d to
5d3e7a6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2559 +/- ##
==========================================
+ Coverage 92.41% 92.46% +0.04%
==========================================
Files 58 58
Lines 6121 6121
==========================================
+ Hits 5657 5660 +3
+ Misses 329 327 -2
+ Partials 135 134 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
📊 Profiling reports are ready to be viewed
💻 CPU Comparison with base Kepler💾 Memory Comparison with base Kepler (Inuse)💾 Memory Comparison with base Kepler (Alloc)⬇️ Download the Profiling artifacts from the Actions Summary page 📦 Artifact name: 🔧 Or use GitHub CLI to download artifacts: gh run download 36303206960 -n profile-artifacts-2559 |
Description
This PR add node_name label to the kepler_node_cpu_info metric to make it consistent with the other metrics.
Upgrade note
kepler_node_cpu_infonow carries anode_namelabel, matchingkepler_node_gpu_infoand the power metrics.Because the label set changes, existing
kepler_node_cpu_infoseries end at upgrade and new series (with node_name`) start.The bundled Grafana dashboards are unaffected. Custom queries, recording rules or alerts that match on the
full label set of this metric may need updating.