Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for meters with same names but different tag keys in prometheus registry #2653

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

angry-cellophane
Copy link

I have an idea how to resolve this issue #877
and raising this PR to discuss it.
The PR has a basic implementation + some simple tests to verify it works and makes sense in general.
I wanted to make sure that the solution makes sense first before going the full path and spending hours or days on making it bullet-proof and efficient. So any input is welcome.

So what this fix does:

  1. Current version of micrometer-prometheus creates a collector for each pair of <metric name, list of tag keys>. Collector stores key values dynamically in a map.
  2. Prometheus doesn't allow to register two collectors with the same metric name, but it doesn't limit the collector to one particular set of label names (or tag keys in micrometer). The main job of the collector is to return metric samples, which is a list of <tag key, tag value, value> basically. Prometheus supports metrics samples with the same metric name but different tag keys.
  3. So if the MicrometerCollector is changed to store tag keys dynamically like it does for tag values now, it can return tag keys and values in samples without any issue and prometheus will be able to parse it correctly.

What I did:

  1. Added a new holder for tags to keep a list of keys and values separately for convenience.
  2. Changed the map in MicrometerCollector to store the holders instead of tag values.
  3. Changed some other dependent methods in MicrometerCollector to get tag keys from the holders.

…keys dynamically like it already does it for tag values
@angry-cellophane angry-cellophane changed the title Issue 877: Add support for meters with the same name but different tag keys in prometheus registry Issue 877: Add support for meters with same names but different tag keys in prometheus registry Jun 17, 2021
christosarvanitis added a commit to armory-plugins/armory-observability-plugin that referenced this pull request Jul 6, 2021
@nhartner
Copy link

nhartner commented Sep 7, 2021

It would be great to see this merged. Is there any reason it hasn't been? Not sure who to tag on this but how about it @jonatan-ivanov?

@shakuzen shakuzen changed the title Issue 877: Add support for meters with same names but different tag keys in prometheus registry Support for meters with same names but different tag keys in prometheus registry Sep 15, 2021
@crolopez
Copy link

Any updates? The solution seems simple and would provide great value.

@jonatan-ivanov
Copy link
Member

Please see this issue for more details: prometheus/client_java#696
TL;DR: it possible but strongly discouraged.

@zorba128
Copy link

zorba128 commented Dec 5, 2023

Please take a look at prometheus/client_java#901
While I might agree that having two metrics with different sets of labels is somehow problematic, I think this all is deeper problem that its disallowed to have same metrics published by two different collectors (same set of labels, just different label values).
And this is core problem that makes new api really hard to use anywhere but on simple code as on code snippets...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants