Add chip to the DRM parser class #672
Conversation
Signed-off-by: Deezzir <[email protected]>
Signed-off-by: Deezzir <[email protected]>
Deezzir
force-pushed
the
drm-hwmon-name
branch
from
2b6737d
to
3705157
Compare
Signed-off-by: Deezzir <[email protected]>
Signed-off-by: Deezzir <[email protected]>
Signed-off-by: Deezzir <[email protected]>
|
@discordianfish and @SuperQ please review! Thank you |
| HWmonChip string // The hwmon chip. | ||
| } | ||
|
|
||
| func cleanChipName(name string) string { |
There was a problem hiding this comment.
Why do we need to sanitize the name?
There was a problem hiding this comment.
It's something that the hwmon does with the chip names as well, so the format is similar.
There was a problem hiding this comment.
Yes but in that case its used to construct the metric name, which has more limitations. Here its being used as label value, where I don't think we need this
There was a problem hiding this comment.
Do you think it will still be the same and relatable by the chip from the hwmon? It seems like a good idea to align the result to what the hwmon does with its chip names
There was a problem hiding this comment.
I guess in the node-exporter we should also just use the raw chip name and put it in a label instead of the metric name. But I think either way, here, this being used as label value, it should not get santized. But want tohave @SuperQ opinion on this as well.
There was a problem hiding this comment.
A clarification - I don't care if it is sanitized or not; the only reason I am doing a sanitization here is to align the label chip coming from the hwmon metrics (which is also sanitized as per the permalink I provided above) to a new label I've added to the node_drm_card_info metric.
I want to match, aka join a metric like node_hwmon_fan_rpm with node_drm_card_info and have a connection between the two collectors. RN, there is no way to match the metrics coming from those collectors, although both provide metrics for GPUs.
Sanitizing the label before the match makes sense because this is what's being done for the chip label in the hwmon collector.
If you think we shouldn't sanitize the hwmon chip label in the node_exporter, I might provide a PR for that. As of now, I am aligning the behaviour with the existing solution.
There was a problem hiding this comment.
Yeah makes sense, let's see what @SuperQ is thinking about this. I'm fine either way but moving chip to a label in the hwmon collector makes most sense to me.
Adds a chip label to the
DRMparser class so the data can be related tohwmonmetrics when exported by the node_exporter.For example,
node_exporterproduces the following metric:So, by having a
chiplabel coming from the DRM parser, DRM metrics can be related to hwmon sensor metrics to build a better dashboard with clear AMD GPU filters.For example:
Also closes: #673