Add the Prometheus sysctl exporter.

[ed]

Summary

Now that we have our sysctl tree annotated with aggregation labels,
let's go ahead and provide a very simple utility for exporting the
sysctl tree in Prometheus' format. It can either be used in conjunction
with the Prometheus node exporter or run on its own.

The reason why I'm opting for having it in the base system is because it
has a pretty strong integration with some of sysctl's innards, such as
access to iterators, name lookups and metadata. That's why it would be
safer to oversee the development of this exporter ourselves.

The exporter is also remarkably compact, especially when compared to the
official Linux binary of the Prometheus node exporter (16 KB vs 12 MB).
I guess this could be an interesting aspect for monitoring embedded
FreeBSD-based systems.

[ed]

Cross reference the prometheus_sysctl_exporter(8) from sysctl(8).

[jilles]

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.c

105 ↗

(On Diff #22915)

I know that sbin/sysctl uses the same BUFSIZ, but I don't like that BUFSIZ is being used as an arbitrary buffer size for almost any purpose.

571 ↗

(On Diff #22915)

These headers allow the client to send a subsequent request which will not work. A Connection: close or HTTP/1.0 will avoid that.

[ed]

Thanks!

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.c

105 ↗

(On Diff #22915)

Yes, it's quite horrible. I used BUFSIZ exactly for the reason you gave: because sysctl(8) also does that.

What kind of way of solving would you propose here? I'm afraid that using too much dynamic allocation wouldn't make the code more beautiful. I would have used C++ in that case instead. ;-)

571 ↗

(On Diff #22915)

Woah, good catch. Fixed!

[ed]

Use 'Connection: close'.

[ed]

Fix build on MIPS.

[bjk]

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.8

57 ↗

(On Diff #22926)

Put a comma after e.g., and then you won't need the non-breaking space to escape the special mdoc processing for stops.

76 ↗

(On Diff #22926)

"help descriptions" feels odd; maybe just "descriptions"?

104 ↗

(On Diff #22926)

I think we generally try to list the human as well as the copyright holder.
Maybe some of kib's recent work is a good example?

[ed]

Improve man page.

[ed]

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.8

104 ↗

(On Diff #22926)

I search my mailbox for commits made by him to see if I could find any examples, but unfortunately I couldn't find any. Can you still remember which man pages I could use as an example for this?

[manu]

I'm not sure that having an exporter for a specific software is wise, where do we draw the line ?
How hard would it be to have a +/- generic exporter with templates etc ... ?

[ed]

In D8792#182237, @manu wrote:

I'm not sure that having an exporter for a specific software is wise, where do we draw the line ?

I think it's pretty obvious where we should draw the line. Prometheus is one of the very few monitoring systems that has a scraping protocol that:

• is properly documented: https://prometheus.io/docs/instrumenting/exposition_formats/
• is very light-weight to implement
• has a very loose coupling

In addition to that, the Prometheus project also has a *huge* momentum. It's basically an Open Source clone of the monitoring system that companies like Google, etc. have already used for more than a decade. The project recently became a member of the Cloud Native Computing Foundation (https://www.cncf.io/).

That said, if there are other monitoring systems that are commonly used for which we can write an exporter that is ~600 LOC and only has a disk footprint of ~20 KB, then we should consider supporting those as well. As I said in a private discussion the other day: Prometheus has a properly documented API for scraping, but we don't have a properly documented API for all of the sysctl() bits that this exporter uses. That's why it's our duty to interface with Prometheus; not the other way around.

How hard would it be to have a +/- generic exporter with templates etc ... ?

It depends on how different the other formats are. I think that's also outside the scope of this change specifically.

[manu]

The fact that this software clearly defined its protocol doesn't make it suitable for this inclusion in base.
If I follow your argument, if somebody clearly document some monitoring system (munin,collectd,...) we should add some exporter for such software.
It doesn't adds up, this kind of "wrapper" should be in ports, that's what ports are for.
If we were to include some wrapper for monitoring system in base I want it to be templatable, i.e. not made for a specific software.

[cem]

It seems like almost all of the exporter (except main and usage and possibly oid_print) could just be thrown into a libsysctl that is provided from base. Then the exporter could live as a very program and we get a generally useful sysctl library out of it.

I don't have strong feelings about whether this lives in ports or base. We already have some specific exporters in base (snmp, syslog, smtp).

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.c

66 ↗

(On Diff #22926)

nitems() from sys/param.h

87–88 ↗

(On Diff #22926)

Ugh. It'd be good to give these real names. (I know this isn't the first code to reference them just as numbers.) Doesn't need to be fixed in this change.

204–212 ↗

(On Diff #22926)

Does fprintf("%f") not cover these cases?

[ed]

In D8792#182352, @manu wrote:

If we were to include some wrapper for monitoring system in base I want it to be templatable, i.e. not made for a specific software.

Nowhere have I stated that making it templatable is out of the question. The only thing I did state: it's outside the scope of this specific code review. The first step is to get this to work for at least one monitoring system, and in my case I chose Prometheus.

If we get to the point where we want to support other monitoring systems, that's the point where we should start asking ourselves the question how that should be accomplished. Maybe we agree templating is the way to go. Maybe we discover that templating is too constrained. In that case, maybe the right solution is to extend this tool to also support that format. Or maybe we realise that that should be done in a separate utility. That's not for us to decide right now.

Rome wasn't built in a day, nor will this tool be finished in one commit.

[ed]

Use nitems().

[ed]

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.c

66 ↗

(On Diff #22926)

I'm personally not a big fan of nitems(), for the reason that it adds a dependency on a BSD-specific macro, even though a perfectly standard way of phrasing it is not that much longer to type.

That said, considering that this code is intended to be run on FreeBSD exclusively, it makes sense to use it. Will fix.

204–212 ↗

(On Diff #22926)

The Prometheus exposition format documentation mentions that floating point numbers should be formatted like Go's strconv.FormatFloat()/strconv.ParseFloat(), which uses different formatting for Inf/NaN-values. :-/

[cem]

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.c

66 ↗

(On Diff #22926)

I have the opposite approach. Using nitems() is so nice that I'll reimplement it if needed on platforms that don't have it.

204–212 ↗

(On Diff #22926)

Got it.

[bjk]

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.8

104 ↗

(On Diff #22926)

Hmm, it's possible that I'm thinking of the license block and not necessarily the AUTHORS section, but thr_suspend.2 has:

.\" This documentation was written by
.\" Konstantin Belousov <[e-mail]> under sponsorship
.\" from the FreeBSD Foundation.

[ed]

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.8

104 ↗

(On Diff #22926)

I don't think that template would be suitable in this case. When developing software for the FreeBSD foundation, you always assign the copyright to the foundation, but still allow your name to be mentioned below the copyright statements.

In this case I'm the owner of my private consulting company, so there is no need to mention both parties individually.

[ed]

Tidy up the name fetching code a bit.

Due to the code not copying over the individual OID component numbers
over from struct oid to struct oidname, we didn't do any caching. D'oh!

Make this code more understandable by getting rid of the weird way of
keeping track of the length by zero-terminating the name_end array.
Instead, simply embed a struct oid in there, so we can copy over the OID
and its length in one go.

[ed]

Add support for converting timevals and load averages to floats.

While there, clean up the fetching of OID names/labels even more.

[ed]

Err... Sync from the right revision in Git.

[ed]

Gah! Sorry for the noise!