Add chip label to DRM metrics#3572
Open
Deezzir wants to merge 1 commit into
Open
Conversation
Author
|
@SuperQ and @discordianfish PTAL |
Member
|
This needs a rebase. |
Author
|
@SuperQ done |
Deezzir
force-pushed
the
add-drm-chip
branch
3 times, most recently
from
d3422a4 to
ca0ae3e
Compare
Author
|
@SuperQ PTAL |
Deezzir
force-pushed
the
add-drm-chip
branch
3 times, most recently
from
53101c6 to
8e4666c
Compare
Deezzir
force-pushed
the
add-drm-chip
branch
2 times, most recently
from
209c02c to
9154335
Compare
Signed-off-by: Deezzir <deezzir@gmail.com>
rtreffer
reviewed
Jul 10, 2026
rtreffer
left a comment
rtreffer
left a comment
Contributor
There was a problem hiding this comment.
I think this can be simplified by using just one metric and exporting an empty string if missing (which is a common pattern).
Other than that: having common join keys would be helpful.
| prometheus.BuildFQName(namespace, drmCollectorSubsystem, "card_info"), | ||
| "Card information", | ||
| []string{"card", "memory_vendor", "power_performance_level", "unique_id", "chip", "vendor"}, nil, | ||
| ) |
Contributor
There was a problem hiding this comment.
I don't think we need 2 metrics here. Just add the "chip" label always and keep it as empty string if missing.
Comment on lines
+113
to
+117
| if cleanDevName != "" { | ||
| return cleanDevName | ||
| } | ||
|
|
||
| return "" |
Contributor
There was a problem hiding this comment.
This is the same as
Suggested change
| if cleanDevName != "" { | |
| return cleanDevName | |
| } | |
| return "" | |
| return cleanDevName |
| for _, s := range stats { | ||
| ch <- prometheus.MustNewConstMetric( | ||
| drmCardInfo, prometheus.GaugeValue, 1, | ||
| s.Name, s.MemoryVRAMVendor, s.PowerDPMForcePerformanceLevel, s.UniqueID, vendor) |
Contributor
There was a problem hiding this comment.
This could turn into
ch <- prometheus.MustNewConstMetric(
drmCardInfo, prometheus.GaugeValue, 1,
s.Name, s.MemoryVRAMVendor, s.PowerDPMForcePerformanceLevel,
s.UniqueID, chipName(s), vendor)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Add chip label to the
node_drm_card_infometric.Solution
Extend the DRM collector to have the
chiplabel from theClassDRMCardAMDGPUStats.The change was also proposed in the procfs repo, and the PR was merged.
Context
The DRM metrics are impossible to relate to the
hwmonmetrics, which export other helpful information about AMD GPUs. The extension will allow us to relate metrics from bothhwmonanddrmand, in turn, create a better GPU Dashboard for AMD with proper filtering and labelling.Example metrics: