[ENHANCEMENT] Alerts: remove metrics for removed Alertmanagers #13909
Conversation
So they don't continue to report stale values. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
|
For the general context (see also #6864): The traditional orthodox view would be that a program should avoid changing the metrics it exposes over its runtime. However, for programs with higher complexity, we have routinely deregistered or deleted metrics that aren't used anymore. If you partition metrics by backend, and your backends have high churn, you can accumulate a lot of "dead" metrics. I think in the current state of affairs, it is fine to delete those metrics. What do you all think? The code changed here is ancient, BTW. Let's page old-hands like @juliusv and @roidelapluie , but anyone else, please chime in if you have an opinion. |
|
I see. Thank you for the updated PR. I guess it will work now. But adding a test would be better, of course. |
There was a problem hiding this comment.
Plus we already do the same in other managers: https://github.com/search?q=repo%3Aprometheus%2Fprometheus%20DeleteLabelValues&type=code
Maybe we can add some checks using client_golang/prometheus/testutil in existing reload tests.
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
|
I think this one can be merged. |
|
@bboreham Gonna leave it up to you to do the final merge, just because it's been sitting here for a while and I want to make sure you're still happy with it all as well. |
So they don't continue to report stale values.
Fixes #6864