Add hwmon /sensors support #278

rtreffer · 2016-08-09T20:25:52Z

This commit adds initial support for linux hardware sensors, exported through sysfs.
This data is commonly read via lm-sensors sensors command

Details of the interface can be found at https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface

Sensor data is exported as
node_hwmon_<sensor_type><sensor_element>
e.g. node_hwmon_temp_celsius
All extra information, like the original sensor, the input within the sensor and the possible input name are exported as labels.

Reading the data does not require any special permissions (I'm running this as my linux normal user).

On my home machine this exports (full paste at http://pastebin.com/7TR6KKVM )

node_hwmon_in_volt{hwmon="nct6776",label="in0",sensor="in0"} 0.888
node_hwmon_in_volt{hwmon="nct6776",label="in1",sensor="in1"} 1.864
node_hwmon_in_volt{hwmon="nct6776",label="in2",sensor="in2"} 3.408
node_hwmon_in_volt{hwmon="nct6776",label="in3",sensor="in3"} 3.408
node_hwmon_in_volt{hwmon="nct6776",label="in4",sensor="in4"} 0.136
node_hwmon_in_volt{hwmon="nct6776",label="in5",sensor="in5"} 1.688
node_hwmon_in_volt{hwmon="nct6776",label="in6",sensor="in6"} 0.736
node_hwmon_in_volt{hwmon="nct6776",label="in7",sensor="in7"} 3.44
node_hwmon_in_volt{hwmon="nct6776",label="in8",sensor="in8"} 3.2640000000000002

On my laptop I get e.g.

node_hwmon_fan_rpm{hwmon="applesmc_768",label="left_side",sensor="fan1"} 2156
node_hwmon_fan_rpm{hwmon="applesmc_768",label="right_side",sensor="fan2"} 1993

The workflow to generate the data is

Determine all sensors (via reading the directory /sys/class/hwmon/)
Determine a name, either via /sys/class/hwmon/name or by resolving /sys/class/hwmon/device
For each sensors (data is always grouped by input, e.g. fan3)
3.a) read the hwmon ./ folder
3.b) read the hwmon device/ folder
3.c) Dump as prometheus metrics, adding types if known (most of the updateHwmon method)

This patch will most likely need a cleanup (it is too large & could need some tests etc), but I'm opening the PR for discussion as this starts to look interesting and #211 mentions temperature data.

grobie · 2016-08-09T20:45:19Z

collector/hwmon_linux.go

+
+func init() {
+	Factories["hwmon"] = NewhwMonCollector
+	invalidMetricChars, _ = regexp.Compile("[^a-z0-9:_]")


Use MustCompile instead.

rtreffer · 2016-08-23T19:51:59Z

@brian-brazil @grobie I've worked through all the small bugs so can I have another 👀 plz?

grobie · 2016-08-23T20:23:29Z

collector/hwmon_linux.go

+	hwMonSubsystem = "hwmon"
+)
+
+type hwMonCollector struct{}


Nit: Can we keep that directly over NewHwMonCollector? I usually keep that order in go: imports, global const, global var, init, type, NewType, functions.

meta-nit: Can you add that to the contributing docs? It doesn't look like the order is documented anywhere. Maybe even try to get this into https://github.com/golang/go/wiki/CodeReviewComments

grobie · 2016-08-23T20:41:09Z

Can you please add end-to-end tests.

brian-brazil · 2016-08-23T20:41:43Z

collector/hwmon_linux.go

+	if err != nil {
+		return err
+	}
+	_ = collectSensorData(path.Join(dir, "device"), data)


You shouldn't ignore this error

discordianfish · 2016-09-13T12:54:44Z

@rtreffer This is really cool! Any chance you might find time soon to address the comments here? Would love to have this in the node-exporter!

rtreffer · 2016-09-13T13:01:55Z

@discordianfish yes, looked into it yesterday and thought "I should fix this". Will try to get the next round going soon.

discordianfish · 2016-09-13T13:08:48Z

@rtreffer Awesome, looking forward to it!

rtreffer · 2016-09-17T14:22:28Z

Got interrupted yesterday, but managed to go with a split-by-regex approach and fixing all those silly tyops. Still have to build the end-to-end test, though.

juliusv · 2016-09-17T15:04:01Z

@rtreffer Not sure if you've already looked at it, but the mentioned end-to-end tests are super easy. See https://github.com/prometheus/node_exporter/blob/master/end-to-end-test.sh

Just add the relevant sysfs files to https://github.com/prometheus/node_exporter/tree/master/collector/fixtures and update the expected output in https://github.com/prometheus/node_exporter/blob/master/collector/fixtures/e2e-output.txt.

juliusv · 2016-09-17T15:05:05Z

(and enable that collector in the end-to-end-test.sh)

rtreffer · 2016-09-25T21:36:53Z

Ok, I need at least 2 sensors as 2 layouts are possible:

Old driver have their data under the device folder
Newer drivers use the hwmon/hwmonX folder

Besides that it's a mix of some of the reading I got on a few machines.

rtreffer · 2016-09-27T19:01:39Z

@juliusv / @brian-brazil anything else?

brian-brazil · 2016-09-27T19:32:39Z

collector/hwmon_linux.go

+				continue
+			}
+
+			// fallback, just dump


Comments should be full sentences.

brian-brazil · 2016-09-27T19:35:00Z

collector/hwmon_linux.go

+				label = sensor
+			}
+		}
+		labels := []string{hwmonName, sensor, label}


I'm not sure we should expose both the sensor and label, just the label should do.

I'll give it a try on a few machines, if it works with just label I'd change to that. In theory it could fail with "bad" drivers, but I'm not sure if such drivers exist (sensor is guaranteed to be unique, label not).

rtreffer · 2016-09-28T20:22:26Z

@brian-brazil updated. Couldn't see issue when dropping sensor, but I have only a limited sample size.

brian-brazil · 2016-09-28T20:23:22Z

collector/hwmon_linux.go

+var (
+	hwmonInvalidMetricChars = regexp.MustCompile("[^a-z0-9:_]")
+	hwmonFilenameFormat     = regexp.MustCompile(`^(?P<type>[^0-9]+)(?P<id>[0-9]*)?(_(?P<property>.+))?$`)
+	hwmonLabelDesc          = []string{"chip", "sensorName"}


it can just be sensor now.

sure thing.

brian-brazil · 2016-09-28T22:16:18Z

node_exporter.go

@@ -31,7 +31,7 @@ import (
 )

 const (
-	defaultCollectors = "conntrack,cpu,diskstats,entropy,filefd,filesystem,loadavg,mdadm,meminfo,netdev,netstat,sockstat,stat,textfile,time,uname,vmstat"
+	defaultCollectors = "conntrack,cpu,diskstats,entropy,filefd,filesystem,hwmon,loadavg,mdadm,meminfo,netdev,netstat,sockstat,stat,textfile,time,uname,vmstat"


The README needs updating too

That's just the one-line I've added, right?

This commit adds initial support for linux hardware sensors, exported through sysfs. Details of the interface can be found at https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface

rtreffer · 2016-10-06T15:29:16Z

So anything else?

brian-brazil · 2016-10-06T15:33:31Z

Thanks!

* Add hwmon support (mainly known from lm-sensors) This commit adds initial support for linux hardware sensors, exported through sysfs. Details of the interface can be found at https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface * Add end-to-end test with some real life data * Cleanup comments on hwmon collector * Drop raw sensor name from hwmon output * Let the sensor label be "sensor" * Add hwmon short description to README.

grobie reviewed Aug 9, 2016
View reviewed changes

rtreffer force-pushed the hwmon_sensors branch from 0c200cf to c4b2a6f Compare August 16, 2016 20:08

grobie reviewed Aug 23, 2016
View reviewed changes

brian-brazil reviewed Aug 23, 2016
View reviewed changes

rtreffer force-pushed the hwmon_sensors branch 2 times, most recently from 67f84a3 to 79ca7f4 Compare September 17, 2016 14:21

rtreffer force-pushed the hwmon_sensors branch from 79ca7f4 to 9567d18 Compare September 25, 2016 21:27

rtreffer changed the title ~~[WIP] Add hwmon /sensors support~~ Add hwmon /sensors support Sep 26, 2016

brian-brazil reviewed Sep 27, 2016

View reviewed changes

collector/hwmon_linux.go

continue

}

// fallback, just dump

Copy link

Contributor

brian-brazil Sep 27, 2016

Choose a reason for hiding this comment

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

Comments should be full sentences.

brian-brazil reviewed Sep 27, 2016

View reviewed changes

brian-brazil reviewed Sep 28, 2016

View reviewed changes

rtreffer force-pushed the hwmon_sensors branch from dad4ead to e65908e Compare September 28, 2016 20:30

brian-brazil reviewed Sep 28, 2016

View reviewed changes

rtreffer-sc added 6 commits October 3, 2016 10:45

Add hwmon support (mainly known from lm-sensors)

aa0dc78

This commit adds initial support for linux hardware sensors, exported through sysfs. Details of the interface can be found at https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface

Add end-to-end test with some real life data

3ff99f2

Cleanup comments on hwmon collector

f7ea84e

Drop raw sensor name from hwmon output

3aaf453

Let the sensor label be "sensor"

e717680

Add hwmon short description to README.

fbcb428

rtreffer force-pushed the hwmon_sensors branch from e65908e to fbcb428 Compare October 3, 2016 08:51

brian-brazil merged commit 081ecc5 into prometheus:master Oct 6, 2016

SuperQ mentioned this pull request Nov 6, 2016

Release v0.13.0 #336

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hwmon /sensors support #278

Add hwmon /sensors support #278

rtreffer commented Aug 9, 2016

grobie Aug 9, 2016

rtreffer commented Aug 23, 2016

grobie Aug 23, 2016

discordianfish Sep 13, 2016

grobie commented Aug 23, 2016

brian-brazil Aug 23, 2016

discordianfish commented Sep 13, 2016

rtreffer commented Sep 13, 2016

discordianfish commented Sep 13, 2016

rtreffer commented Sep 17, 2016

juliusv commented Sep 17, 2016

juliusv commented Sep 17, 2016 •

edited

Loading

rtreffer commented Sep 25, 2016 •

edited

Loading

rtreffer commented Sep 27, 2016

brian-brazil Sep 27, 2016

brian-brazil Sep 27, 2016

rtreffer Sep 28, 2016

rtreffer commented Sep 28, 2016

brian-brazil Sep 28, 2016

rtreffer Sep 28, 2016

brian-brazil Sep 28, 2016

rtreffer Oct 6, 2016

brian-brazil Oct 6, 2016

rtreffer commented Oct 6, 2016

brian-brazil commented Oct 6, 2016

Add hwmon /sensors support #278

Add hwmon /sensors support #278

Conversation

rtreffer commented Aug 9, 2016

Choose a reason for hiding this comment

rtreffer commented Aug 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grobie commented Aug 23, 2016

Choose a reason for hiding this comment

discordianfish commented Sep 13, 2016

rtreffer commented Sep 13, 2016

discordianfish commented Sep 13, 2016

rtreffer commented Sep 17, 2016

juliusv commented Sep 17, 2016

juliusv commented Sep 17, 2016 • edited Loading

rtreffer commented Sep 25, 2016 • edited Loading

rtreffer commented Sep 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtreffer commented Sep 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtreffer commented Oct 6, 2016

brian-brazil commented Oct 6, 2016

juliusv commented Sep 17, 2016 •

edited

Loading

rtreffer commented Sep 25, 2016 •

edited

Loading