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

OM 2.0: Decision around __type__ and __unit__ labels in Text format #280

Open
bwplotka opened this issue Nov 28, 2024 · 3 comments
Open

Comments

@bwplotka
Copy link
Member

bwplotka commented Nov 28, 2024

There is a proposal that makes it possible for the unit and type of the metric being part of the metric/metric family identification. This issue is about considering this an option and tracking the proposal for the spec changes.

There are a few key discussion points, if we would like to accept: prometheus/proposals#39

  • Should we flatten the format (use __type__ and __unit__ labels instead of TYPE and UNIT); mix approach or not allow those labels?
  • How to solve the problem of the OM 1.0 not allowing multiple metric families (essentially names)?

Sounds like this generally requires redefinition of metric family (to include type and unit).

@ArthurSens
Copy link
Member

I feel like it's a mistake to add units and type as labels in the exposition format. Since this is repeated for the whole metric family, there's no need for it.

I do see the problem that Prometheus can scrape two different targets with same metric family name but different type/unit, but this is a Prometheus problem, not the exposition 🤔

@bwplotka
Copy link
Member Author

bwplotka commented Dec 1, 2024

I do see the problem that Prometheus can scrape two different targets with same metric family name but different type/unit, but this is a Prometheus problem, not the exposition 🤔

This is not the biggest problem as usually target is identifiable, so those are two different series. The problem is with ONE target with e.g.

TYPE metric1 counter
UNIT metric1 seconds
HELP metric1 ...
metric1{...}
TYPE metric1 gauge
UNIT metric1 milliseconds
HELP metric1 ...
metric1{...}

It feels we could consider allowing that from 2.0, which requires storage to handle things a bit differently (e.g Prometheus putting those into label). Thanks of major version change, it might a quite clean transition. One thing to check would be: what storage/scraper won't handle it for some reasons (e.g. the internal model does not allow this). Is this fine as those storages could simply ignore second series as the worst case or is there a bigger consequence of this?

@dashpole
Copy link

dashpole commented Dec 2, 2024

I do think the TYPE and UNIT comments are more readable than __type__ and __unit__ in all labels, which is important for the text format. If we ever considered adding other metadata (e.g. alias, or something similar in concept to OTel's instrumentation scope), adding all of them as labels would make the text format less useable over time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

3 participants