Page MenuHomeFreeBSD

Add support for attaching aggregation labels to sysctl objects.
ClosedPublic

Authored by ed on Dec 13 2016, 2:55 PM.

Details

Summary

I'm currently working on writing a metrics exporter for the Prometheus
monitoring system to provide access to sysctl metrics. Prometheus and
sysctl have some structural differences:

  • sysctl is a tree of string component names.
  • Prometheus uses a flat namespace for its metrics, but allows you to attach labels with values to them, so that you can do aggregation.

An initial version of my exporter simply translated

hw.acpi.thermal.tz1.temperature

to

sysctl_hw_acpi_thermal_tz1_temperature_celcius

while we should ideally have

sysctl_hw_acpi_thermal_temperature_celcius{thermal_zone="tz1"}

allowing you to graph all thermal zones on a system in one go.

The change presented in this commit adds support for accomplishing this,
by providing the ability to attach labels to nodes. In the example I
gave above, the label "thermal_zone" would be attached to "tz1". As this
is a feature that will only be used very rarely, I decided to not change
the KPI too aggressively.

While there, I've also added labels to places where I thought it already
makes sense. Some of these are actually needed to make the metrics
exporter work at all, as they get rid of some metrics having a dash in
their name (which Prometheus doesn't allow).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ed updated this revision to Diff 22872.Dec 13 2016, 2:55 PM
ed retitled this revision from to Add support for attaching aggregation labels to sysctl objects..
ed updated this object.
ed edited the test plan for this revision. (Show Details)
ed added reviewers: allanjude, jonathan, peterj, imp, adrian.
rpokala added inline comments.
share/man/man9/sysctl.9
28 ↗(On Diff #22872)

s/2015/2016/ # or maybe 2017 by the time this gets committed ;-)

share/man/man9/sysctl_add_oid.9
30 ↗(On Diff #22872)

s/2015/2016/

sys/sys/sysctl.h
277 ↗(On Diff #22872)

Doesn't the #define for SYSCTL_OID_WITH_LABEL need to come before its use in the #define for SYSCTL_OID?

304 ↗(On Diff #22872)

Same here - SYSCTL_NODE_WITH_LABEL is used in the definition of SYSCTL_NODE before SYSCTL_NODE_WITH_LABEL is defined.

313 ↗(On Diff #22872)

And again.

ed updated this revision to Diff 22888.Dec 13 2016, 6:47 PM
ed edited edge metadata.

Use the proper year.

Spotted by: rpokala

ed marked 4 inline comments as done.Dec 13 2016, 6:50 PM
ed added inline comments.
sys/sys/sysctl.h
277 ↗(On Diff #22872)

Well, that's intersting about macros: it's jut flat substitution. This means that ordering is irrelevant. The entire expansion is determined at invocation. This means that the existing ordering is all right. It's actually intentional, as I'd rather place the _WITH_LABEL() versions below the more commonly used ones.

Your thoughts?

cem accepted this revision.Dec 13 2016, 7:04 PM
cem added a reviewer: cem.
cem added a subscriber: cem.

I'd prefer the use modifications to be a separate commit from the new API.

It looks like these label strings are just blindly passed up to userspace, so it's really up to userspace to do anything with them. Does your userspace program handle multiple labels on a given sysctl, or is it a single tag deal?

This revision is now accepted and ready to land.Dec 13 2016, 7:04 PM
ed added a comment.EditedDec 13 2016, 7:12 PM
In D8775#181837, @cem wrote:

I'd prefer the use modifications to be a separate commit from the new API.

Will do!

It looks like these label strings are just blindly passed up to userspace, so it's really up to userspace to do anything with them. Does your userspace program handle multiple labels on a given sysctl, or is it a single tag deal?

It should deal with that perfectly fine. Pretty nice if you want to keep track of a histogram of packet size distribution per NIC:

net_received_packets_size_bytes_bucket{interface="em0",le="64"} 1200
net_received_packets_size_bytes_bucket{interface="em0",le="128"} 1400
net_received_packets_size_bytes_bucket{interface="em0",le="256"} 1400
net_received_packets_size_bytes_bucket{interface="em0",le="512"} 2000
net_received_packets_size_bytes_bucket{interface="em0",le="1024"} 3000
net_received_packets_size_bytes_bucket{interface="em0",le="+Inf"} 4000
This revision was automatically updated to reflect the committed changes.
wblock added a subscriber: wblock.Dec 16 2016, 7:32 PM
wblock added inline comments.
head/share/man/man9/sysctl.9
482

This sentence doesn't need to repeat that the label is attached to an OID, that was done in the previous sentence.

Labels can make exporting sysctl data to monitoring systems that
support aggregations through labels, like Prometheus.