Allow to delete a given set of labels from metrics

[kamaradclimber]

This will allow to expose transient metrics. Their existence might
become irrelevant through the application lifecycle.

[kamaradclimber]

@grobie

[coveralls]

Coverage increased (+0.7%) to 98.681% when pulling 14928a5 on kamaradclimber:delete_labels into 7265125 on prometheus:master.

[kamaradclimber]

@grobie any feedback on the PR?

[kazegusuri]

+1 for this feature.
In fluent-plugin-prometheus, which is a fluent plugin to collect metrics from messages, a server collects metrics contain many contexts not only the server self. Sometimes metrics will be stale because of other server stopped, data changed, and etc. So we want to remove stale metrics.
ref fluent/fluent-plugin-prometheus#20

[kamaradclimber]

@grobie I'm willing to update the PR the fix the conflicts if you give me some feedback first. Would you be interested by this change?

[grobie]

@kamaradclimber Sorry for the late response. The problem is, that label sets should be stable for an application in general. The exception are usually exporters which proxy metrics from a different system, exactly as @kazegusuri described.

The proper design for that would be use constant metrics like in the golang client. The ruby client doesn't support that concept at all, and a rewrite has been discussed for a long time. I feel a bit unsure about adding something to the client we generally consider to be an anti-pattern. On the other hand, the whole client will probably change a lot with the future rewrite anyway.

[brian-brazil]

There are valid use cases for this, but they are extremely rare (as in I've came across 2 ever) outside of tests.

[kamaradclimber]

My current example:
We export network related metrics about containers running on a host. Contziner identifier is a guid and is part of the label set.
Containers disappear after being terminated by our cluster manager and we'd like the related metrics to disappear as well.
In this example prometheus is used as an aggregation step in a path towards graphite where we store metrics long term.

We could let label set exist but it would have a non bounded memory footprint on the exporter.

Feel free to advise better pattern if there is any

[brian-brazil]

That's a case where you want to use const metrics. I don't believe offhand that the ruby client supports them, but Go/Python/Java do. See https://www.robustperception.io/setting-a-prometheus-counter/

[uberjay]

I've got a similar situation -- I'm translating a set of dynamically configured data sources into some prometheus metrics, and it'd be really fantastic to be able to unset a particularly labelled value. I've resorted to unregistering & reregistering the metric, and essentially replaying the values which should remain.

It's pretty gross, so having an alternative (whether it's const metrics, or merging this PR) would be lovely!

[brian-brazil]

It's pretty gross, so having an alternative (whether it's const metrics, or merging this PR) would be lovely!

As it stands you should switch to the Go/Python/Java client which support the relevant features. This PR doesn't help you with that, though the Ruby client will likely support those features at some point.

[uberjay]

Unfortunately, reimplementing my application using another language isn't currently an option... I'm not sure why you say this PR wouldn't help -- it seems like it's exactly what I need!

[brian-brazil]

An exporter should not maintain state across scrapes, and this PR only helps you if you're maintaining state across scrapes.

[kamaradclimber]

@brian-brazil , thanks for your comments. Could you explain your statement about exporter that should not maintain state.
From my view, this is required for some use case (at least mine!).

[brian-brazil]

That's getting into developer questions, which are better suited for the prometheus-developer mailing list rather than going off-topic here.

[uberjay]

My exporter doesn't maintain state across scrapes -- it's the prometheus-client Registry that is the problem. In other words, I could destroy and re-create all of my metrics on every scrape to work around this, but that seems pretty inefficient.

[dmagliola]

@brian-brazil I'm going through all the PR's in this repo to make a decision on what to do about each of these issues.
On this one, there's something I'm clearly not understanding fully, and I was wondering if you could help me interpret the Client Libraries Best Practices.

Looking at the Labels section, there's this:

The general way to provide access to labeled dimension of a metric is via a labels() method (...)
Metrics with labels SHOULD support a remove() method with the same signature as labels() that will remove a Child from the metric no longer exporting it

My interpretation of that line about remove() sounds like exactly what this PR is providing (although with a slightly different interface). That is, stopping the export of a certain set of labels.

Is this something that has changed in the best practices since the time this PR was opened? Or am I misunderstanding the Best Practices?

[brian-brazil]

That bit of the best practices likely needs some reconsidering. There are use cases for this, but they're very very rare. Mostly the people looking for this actually want const metrics.

[dmagliola]

@kamaradclimber @uberjay I don't have a very strong opinion on this change.
The best practices state that it should exist, and according to @brian-brazil re should revise those.
I'm not sure what's best.

That said, this code will definitely not work on the current version of master, with the "data stores" implementation separated from the metrics.
Adapting it will also not be trivial. In particular, the store that we expect to be used the most is file-backed (to get around the Unicorn Multi-Process problem), and the way it works with files, removing entries from them would be somewhat complicated / messy, and would add quite a bit of complexity to that code, which could get a bit risky.

I think the best things to do here is close this PR, open an issue about this, and move the conversation there. Will do that next.