[Discussion] Appropriate level of normalization in snmp_exporter and the generator

[RichiH]

This should serve as the canonical place for this recurring discussion to collect links to related issues and to keep the overall discussion somewhat concise.

[brian-brazil]

This should follow the same principles as all other non-distinguishing labels, per the examples at https://www.robustperception.io/how-to-have-labels-for-machine-roles/ and https://www.robustperception.io/exposing-the-software-version-to-prometheus/

For why non-distinguishing labels shouldn't be added to time series, see https://www.robustperception.io/target-labels-are-for-life-not-just-for-christmas/ (replace target with time series, same logic applied).

[RichiH]

The obvious counter-point is the same as in all normalization discussions:

Striking the right balance between pristine design and usability.

Having everything in referenced time series quickly descends into a join-fest, which highly complicates promql queries for no end-user benefit.

As per @fabxc , there's no storage penalties either way.

Also, while it's not a Prometheus project, Grafana is the default suggestion for dashboarding and there's no way to handle joins in either templates nor in legends.

As mentioned, this situation has led to people not adapting snmp_exporter or even Prometheus; or to writing their own custom exporters.

All in all, the trade-off seems to be more than valid, to me. I would not object to allowing snmp_exporter to do both, but this opens the question of what the default should be.

[brian-brazil]

The argument you're making is that client libraries should be permitted to allow adding constant labels to a set of exposed metrics for things like version numbers, as that is the same semantic question as what's being discussed here.

We've decided that's not the way we're going there (and we call it out in the exporter guidelines), and I see no reason to make a different decision here.

Having everything in referenced time series quickly descends into a join-fest, which highly complicates promql queries for no end-user benefit.

That it does not benefit your exact use case does not mean there is no end-user benefit. What you suggest breaks common use cases due to discontinuity of time series, and makes life harder for other use cases due to superfluous labels.

Grafana is the default suggestion for dashboarding and there's no way to handle joins in either templates nor in legends.

An exact type of tooling support not currently existing is not an argument I accept, as that leads to technical and design debt when that feature comes along. It's also not required, you'd do this in the expression as we do with version numbers (which users appear to be fine with).

I'd suggest filing a bug with Grafana if this is something you want, though I'm unsure how this would work. I think this might require a custom plugin.

As mentioned, this situation has led to people not adapting snmp_exporter or even Prometheus; or to writing their own custom exporters.

Many users chose to ignore Prometheus guidelines and/or philosophy. That doesn't mean that we then throw away those guidelines when it is explained to a user how to solve their particular problem, but decide they don't want to.

I would not object to allowing snmp_exporter to do both, but this opens the question of what the default should be.

We do not generally offer users incorrect ways of doing things as options, though sometimes incorrect usage is possible due to other features we do provide. For example, that it's possible to do this currently with hand written configs is an unintended consequence of the design decision that the snmp_exporter implementation be dumb and that all the smarts live in the generator.

[RichiH]

The argument you're making is that client libraries should be permitted to allow adding constant labels to a set of exposed metrics for things like version numbers, as that is the same semantic question as what's being discussed here.

That is not the argument I was making even if the semantics are the same; the argument I was making is specific to MIB tables and the snmp_exporter.

It's also not required, you'd do this in the expression as we do with version numbers (which users appear to be fine with).

Can you link to this example?

For example, that it's possible to do this currently with hand written configs is an unintended consequence of the design decision that the snmp_exporter implementation be dumb and that all the smarts live in the generator.

Unintended or not, if that feature went away, many would fork; us amongst them.

[brian-brazil]

That is not the argument I was making even if the semantics are the same; the argument I was making is specific to MIB tables and the snmp_exporter.

Decisions don't happen in isolation, and I consider consistency across the ecosystem very important. From my standpoint the broad argument is the one you need to make, otherwise this is special pleading.

Every argument you've made can be translated to version numbers, and we've previously rejected those arguments.

Can you link to this example?

I don't have one offhand, I've seen several users with version numbers in their dashboards.

[RichiH]

Result of yesterday's dev summit

• Denormalizing is OK in some cases
• Multiple label lookups will be added to the generator
• Add docs section on when/how to denormalize data

[RichiH]

@brian-brazil Any update on this? #212 makes me want to upgrade to a more current version of snmp_exporter and ideally I would want to migrate to generator at that time.

[brian-brazil]

I've not had time to work on this. Offhand the snmp_exporter should be backwards compatible for older configs.

[RichiH]

One of the upgrades within the Go version forced me to change snmp.yml; mostly via regex, but still: as long as we stick to manual versions, I always have at least the risk and mental overhead.

How many Brian-hours would this take to implement?

[brian-brazil]

I'm not sure offhand, there's some cleanup work I want to do first to make this sane.

[brian-brazil]

This was resolved.