hwmon: Introduce chipName label carrying the human-readable name of the hwmon device#355
hwmon: Introduce chipName label carrying the human-readable name of the hwmon device#355horazont wants to merge 2 commits intoprometheus:masterfrom
Conversation
The chip label generation has been changed in prometheus#334 to prefer the unique device path (e.g. the location on the PCI bus) due to prometheus#333. Here, a new label, chipName, is introduced which, again, carries the human-readable sensor name (e.g. coretemp). It is used in addition to the existing labels. This allows to mitigate the downsides of the solution to prometheus#333 (namely that the device path may not be stable across kernels and reboots) for cases where it does not matter that multiple devices may have the same human-readable name (e.g. aggregation or where at most one device of a type is present).
|
Thanks for the PR. Each label should add distinctness to the timeseries. As |
|
@brian-brazil Thanks for the feedback! The concrete issue we are having is that we want to monitor GPU temperatures, which are available via hwmon. However, since the fix for #333, it is hard to address exactly those, because the PCI path is not guaranteed to be stable (and varies over machines), the I see that this is not a problem for many devices, because you could, for example, easily select all coretemp sensors by matching for chipName adds value here, even if its not distinctness. If this is a hard limit, does it make more sense to prepend the |
|
The way to approach this would be using the machine roles method https://www.robustperception.io/how-to-have-labels-for-machine-roles/ Messing around with label values would just cause confusion, and lead to users using regex. Regexes are a smell. |
|
I’m not sure how machine roles apply here, but you may be able to clarify. My idea to applying machine roles would be to have something like (via textfile-collector, omitting metric metadata): and then query using: Does that make sense? This seems overly cumbersome for me, but the hack with prepending the name to the |
|
You'd have both chip and chipName on the new metric. |
|
So more like: and then query using the |
|
Yes |
|
Okay thanks. We still feel that it makes sense to use labels for these cases, but the other way works too, I guess. Closing as it seems to violate the policy of the project and won’t be merged. |
|
We could add these annotation metrics to the exporter without having to resort to doing these in the textfile. Just not as part of the original metrics themselves. |
|
@SuperQ So a pull request implementing that would be fine? |
|
Yes, I think it would be good. |
* fix crash seen in node_exporter Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
The chip label generation has been changed in #334 to prefer the unique device path (e.g. the location on the PCI bus) due to #333.
We propose the re-introduction of the human-readable device name (e.g.
coretemp) as a separate label.This allows to mitigate the downsides of the solution to #333 for cases where it does not matter that multiple devices may have the same human-readable name (e.g. aggregation or where at most one device of a type is present).
For cases where no
namefile exists in thehwmondirectory, the same value as for thechiplabel is used.@rtreffer @brian-brazil