Need to be able to collect metrics without adding an instance (or job or exporter_instance or exporter_job) label

[brian-brazil]

Service-level stats are exported by things like the push gateway and cloudwatch exporter that don't have an instance label. Currently they'll end up with the instance, such as that of the target.

We need a way to signal that for some metrics, that the injection logic isn't to add in an instance or exporter_instance label. Maybe a flag in the config file for targets where this applies, and an empty instance label in the metric protobuf?

[beorn7]

Then we also have to change the behavior of pushgateway, which guarantees that an instance label is added. (We could do {instance="n/a"} as a work-around, but then people have to think of it. The old rationale was that only people who know what they are doing would set an instance label different from the host ip...)
Filed prometheus/pushgateway#18

[brian-brazil]

There's use cases beyond pushgateway. The main use case is where you want no instance label at all, and there's also use cases where you want an instance label without exporter_instance being added.

[beorn7]

Without having a whole lot about it, I'd say having a "no-instance" target config option would be easy to add and pretty risk-free (as users have to explicitly enable it).

The specific pushgateway case has a few ramifications, see prometheus/pushgateway#18

[brian-brazil]

Maybe "honor-instance", as sometimes you do want the instance label going through (aggregators).

Do we need to worry more generally about when target labels clash with scraped labels?

[beorn7]

IIRC label crashes are currently always resolved by appending exporter_.

[beorn7]

As recently discussed in prometheus/pushgateway#18 , it looks we should have a directive honor-client-labels, in which case the server will not add exporter_... labels in case of clashes, but just not add the server provided labels. In addition, if a client wants to prevent an instance (or even job) label altogether, it has to set the respective label value to "" (empty string), in which case the server will not add its own version of the label (provided honor-client-labels is set, otherwise it will add an exporter_instance label), but after that, it will effectively disregard the empty instance label, as empty labels are treated equivalent to labels not set by the expression language (see #494).

[beorn7]

New idea here:

exporter is arguably a prefix that is sometimes confusing. Imagine a label is attached as a target label (or global label or as a result of SD-driven relabeling). As an example, use the infamous monitor="codelab-monitor global label from the example config. If a scraped metric has a label monitor="foobar", the resulting metric would look like {monitor="foobar",exporter_monitor="codelab-monitor"}.

Suggestion now is to not change the default prefix, but add a single scrape config option, which is collision_prefix. Default is "exporter". But in situation where you don't like it, you could change it.

That one additional option would, as a byproduct, allow us to switch off collision handling by setting the prefix to "". In that case, the colliding labels would simply be discarded. It's a minimal change that would serve two different purposes and have a pretty stringent semantics.

[brian-brazil]

One thing I want to change about this is which label gets the collision prefix, as it should be the scraped label rather than the target label that gets changed.

The idea here is that in normal usage, you don't want a rouge job to be able to generate labels for other jobs that may break their alerts/consoles.

This wouldn't allow for what @beorn7 is proposing, but I don't think wanting to change the collision prefix is a major requirement as it's more about having a prefix than it's exact value.

[beorn7]

We could implement the preference (what is renamed (or disappear in the case of an empty prefix), what stays) as another option. But then you want to change the prefix even more dearly because 'exporter' becomes really confusing.

We could just change the fixed prefix in the code, but that's a breaking change. So I think we should better make it configurable (and at the same time solve the problem this issue is about). Two birds with one stone.

[brian-brazil]

exported would be one option.

Aside from PGW, I haven't come across other servers using job or instance so I think a breaking change is okay here (especially as the default semantics need change anyway for safety).

[beorn7]

With the given growing userbase, I'd rather not assume too much. If we provide the option to change the collision prefix, at least you can keep things running even if the default changes.

[beorn7]

I agree that we should change the default precedence for collision handling. The question is: Should we flip it for good or making it configurable? People who depend on the old behavior could keep it if they want.

The configurable prefix (for both cases) makes sense IMHO because it solves the pushgateway/federation need, too (by setting it to empty).

[brian-brazil]

Should we flip it for good or making it configurable?

For good. I've got a change in the works that'll as a side effect let it be undone if you really want to.

The configurable prefix (for both cases) makes sense IMHO because it solves the pushgateway/federation need, too (by setting it to empty).

It solves it with the current precedence for collision handling, but not with the new precedence as the target labels would win.

[beorn7]

Which is a good reason to make the precedence configurable. :)

[beorn7]

But yeah, two things to flip to set up pushgateway or federation is kind of meh...

[brian-brazil]

Yeah, a single setting should be sufficient for the normal use cases.

#490 would allow for someone who wanted the precedence to keep the current behaviour, but I'm having difficulty coming up with a use case that wouldn't be better served by the honor/federate behaviour.

[brian-brazil]

This is resolved with the honor_labels setting.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.