cpu: Metric 'package_throttles_total' is per package.

[knweiss]

'package_throttles_total' is per package, not per cpu. This also reduces
the total number of cpu time series a lot (esp for multi core cpus).

Also add some missing fixtures for cpu2 and cpu3 for consistency.

[knweiss]

@rtreffer @SuperQ This PR has only limited testing (among other things because of Issue #644). Please review it.

Also, please notice that I decided to use the label node instead of package for the package as node is already used by other collectors:

node_cpu_package_throttles_total{node="0"} 30

Feedback is welcome!

BTW: IMHO the cpu label values should also be changed from

node_cpu_core_throttles_total{cpu="cpu0"} 5

to

node_cpu_core_throttles_total{cpu="0"} 5

for consistency.

[SuperQ]

I think those missing fixtures were explicitly missing for some tests.

[knweiss]

@SuperQ Hm, I didn't catch that (but also didn't notice any test failures after adding them). Feel free to drop this hunk from the PR if you want.

[SuperQ]

Adding the metrics to the e2e output masks the error. 😜

[knweiss]

Any thoughts on my comment regarding the cpu label values? This is something that should be fixed before node-exporter 1.0 - if you agree that it's an inconsistency worth fixing.

I can open a separate issue for this if you want.

[SuperQ]

Yes, please open an issue to talk about the cpu labels.

[rtreffer]

Oh, the cpu0 -> 0 sounds like a good idea. I can also live with the lower cardinality, so given that it will reduce cardinality I'm in favor of that change.
Thank you for the PR and bringing this up!

[SuperQ]

For the cpu0 -> 0 change, I would like to do that as part of the planned rename of node_cpu to node_cpu_time_seconds.

[knweiss]

I've opened issue #661 (cpu collector: Change cpu label values from "cpu0" to "0" ).

[knweiss]

I've removed the cpu2/cpu3 fixture changes and added two minor improvements as separate commits. I've also rebased against master.

[SuperQ]

Can you add a cpu1 fixture to verify that the 2nd cpu in a node is ignored?
Like /sys/bus/node/devices/node0/cpu1/thermal_throttle/package_throttle_count. I think this makes sense.

[knweiss]

@SuperQ I've added the node 0 cpu1 fixture and rebased against master.

[SuperQ]

LGTM

[discordianfish]

Thanks, LGTM!