-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hwmon: Introduce chipName label carrying the human-readable name of the hwmon device #355
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
name
file exists in thehwmon
directory, the same value as for thechip
label is used.@rtreffer @brian-brazil