Skip to content

Comments

Add index to non unique hwmon sensor names#408

Closed
discordianfish wants to merge 1 commit intoprometheus:masterfrom
discordianfish:hwmon-add-label-index
Closed

Add index to non unique hwmon sensor names#408
discordianfish wants to merge 1 commit intoprometheus:masterfrom
discordianfish:hwmon-add-label-index

Conversation

@discordianfish
Copy link
Member

This fixes #406.

Arguably this could be fix more efficient but code could be made more idiomatic but I think it's good enough for now.

@grobie
Copy link
Member

grobie commented Jan 7, 2017

@rtreffer can you take a look?

@SuperQ
Copy link
Member

SuperQ commented Jan 7, 2017

I'd rather name the sensor after the filename, like temp1 and temp2 since these will be unique.

@rtreffer
Copy link
Contributor

rtreffer commented Jan 7, 2017 via email

@rtreffer
Copy link
Contributor

rtreffer commented Jan 7, 2017 via email

Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The longer I think about it, the more I agree with @SuperQ on using the filename instead. Introducing our own index means users will need to understand the implementation in node_exporter in order to match our names to the their system.

I just checked and many of my sensors don't provide a <sensor>_label file. I'm not it's a good idea to change the value of the sensor label depending on whether a label is available or not, it should be always the same.

Maybe @brian-brazil has another idea, but one option would be to always use the sensor file prefix (temp1, fan2, etc.) as sensor label value and introduce a new metric like node_hwmon_sensor_label with the labels hwmon, sensor, label to export the human readable labels where available.

for sensor, sensorData := range data {

// - we need to give duplicated labels an index, so sort sensors first
sensorIndex := map[string]int{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we use the hwmon name as label as well, the index structure should be relative to the hwmon group. There is no need to attach a custom index to a sensor name in hwmon4 if there exists a sensor with the same label in hwmon3.

@brian-brazil
Copy link
Contributor

Filename sounds the most practical here.

@discordianfish
Copy link
Member Author

@grobie I wish you thought about those things when reviewing the initial PR.. I'll see if I can find an easy fix, otherwise I'm going to revert the whole hwmon collector since right now it breaks the node-exporter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue collecting duplicate entries from hwmon

5 participants