Page MenuHomeFreeBSD

prometheus_sysctl_exporter: Improve sysctl OID names rewriting.
AbandonedPublic

Authored by delphij on Nov 8 2021, 7:03 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 12:53 PM
Unknown Object (File)
Thu, Oct 31, 4:27 PM
Unknown Object (File)
Oct 4 2024, 2:23 PM
Unknown Object (File)
Oct 2 2024, 9:29 AM
Unknown Object (File)
Oct 1 2024, 11:26 PM
Unknown Object (File)
Oct 1 2024, 4:46 PM
Unknown Object (File)
Sep 30 2024, 9:12 AM
Unknown Object (File)
Sep 30 2024, 2:17 AM
Subscribers

Details

Summary

The current code converts all unsupported (!isalnum()) characters
to '_'. Unfortunately, among all sysctl variables, more than
3/4 of themhave at least one underscore, and more than 99.9%
of them have a dot.On the other hand, some sysctl variables
are named very similarly. For example, in recent ZFS versions,
both the legacy "vfs.zfs.arc_max" and the new "vfs.zfs.arc.max"
are provided, and under the current rewriting rule, both of
them would be rewritten as "vfs_zfs_arc_max".

A full list of existing sysctl's that would create conflicting
names can be generated with:

    sysctl -da | grep -E \("$(sysctl -Na | tr .-%\  _ | sort | \
        uniq -c | sort -n | awk '{ if ($1 >1) print $2; }' | \
        sed -e s,_,.,g | paste -sd \| -)"\):

Note that the other unsupported characters are making up only a
very small (slightly more than 5%) fraction of sysctl's, so
performing afull encode of every character like URLencode seems
to be not needed.

Solve this by repeating underscore one more time when
transforming.

PR: 259607

Test Plan

Examine output of prometheus_sysctl_exporter

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 42649
Build 39537: arc lint + arc unit

Event Timeline

usr.sbin/prometheus_sysctl_exporter/prometheus_sysctl_exporter.c
400

Perhaps an explicit comment about doubling _s?

(It took me a moment to figure out that buf contains the _.)

Won't this change the name of the metric being exported? If that's correct, then wouldn't this patch break existing grafana setups?

For example, if I'm collecting sysctl_vm_stats_vm_v_wire_count, I'd have to update my grafana dashboard to collect sysctl_vm_stats_vm_v__wire__count instead.

asomers requested changes to this revision.Apr 13 2022, 5:07 PM
asomers added a reviewer: asomers.
asomers added a subscriber: asomers.

The code change looks sensible to me. But what happens if there are two sysctls named foo..bar and foo_bar? Won't this still result in a collision? Or are such cases considered too rare to worry about? Also, the man page ought to reflect this change.

This revision now requires changes to proceed.Apr 13 2022, 5:07 PM

If the name of the metric being exported changes...how will that not break existing setups?

In D32886#791065, @rew wrote:

If the name of the metric being exported changes...how will that not break existing setups?

It will break them. But I think that's unavoidable, since they're broken already.

OTOH, on my system the only collisions are caused by the ZFS sysctls marked "LEGACY", and vm.uma.tcp_log_bucket.size/vm.uma.tcp_log.bucket_size. So a less disruptive option would simply be to ignore the LEGACY sysctls, and rename vm.uma.tcp_log.bucket_size.

From the PR...it looks like ~25 metrics are broken. And compare that to the total number of exported metrics:

% prometheus_sysctl_exporter | wc -l
    13360

It'd be better to find a way to fix this without breaking downstream consumers. Although, I have no immediate ideas - but wonder if the exporter could be taught not to export duplicated metrics instead. That wouldn't be ideal...but neither is breaking 13335 metrics. Which is the worst of two evils?

OTOH, on my system the only collisions are caused by the ZFS sysctls marked "LEGACY", and vm.uma.tcp_log_bucket.size/vm.uma.tcp_log.bucket_size. So a less disruptive option would simply be to ignore the LEGACY sysctls, and rename vm.uma.tcp_log.bucket_size.

This seems reasonable.

(I didn't see this reply of yours before I sent my last response)