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

Define OTLP -> Prometheus metric name conversion rule #2437

Closed
legendecas opened this issue Mar 24, 2022 · 5 comments · Fixed by #2440
Closed

Define OTLP -> Prometheus metric name conversion rule #2437

legendecas opened this issue Mar 24, 2022 · 5 comments · Fixed by #2440
Assignees
Labels
spec:metrics Related to the specification/metrics directory

Comments

@legendecas
Copy link
Member

legendecas commented Mar 24, 2022

What are you trying to achieve?

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/datamodel.md#otlp-metric-points-to-prometheus defined OTLP -> Prometheus metric conversion on instrument type, attributes, exemplars and resource attributes. But the conversion rule for instrument name is missing.

Like, Prometheus defines that instrument units should be concatenated to the metric name. Should this apply in the conversion of OTLP -> Prometheus?

Additional context.

Originally posted by @aabmass open-telemetry/opentelemetry-js#2824 (review)

@aabmass
Copy link
Member

aabmass commented Mar 24, 2022

The official python prometheus client library actually accepts the unit and concatenates it into the metric name.

@dashpole
Copy link
Contributor

dashpole commented Mar 24, 2022

Good catch. I think we are required to do this to be compatible with the OpenMetrics spec: Unit specifies MetricFamily units. If non-empty, it MUST be a suffix of the MetricFamily name separated by an underscore. Be aware that further generation rules might make it an infix in the text format..

It does mean the OTLP -> Prom -> OTLP round-trip is even further from the original name in OTLP, but I think we just have to accept that consequence. Or, it might be possible to trim known unit suffixes in the prometheus receiver.

@aabmass
Copy link
Member

aabmass commented Mar 24, 2022

@dashpole I noticed the python prometheus client also adds _total suffix for CounterMetricFamily. See OM spec here.

Looks like there was some previous OTel discussion about this for the receiver open-telemetry/opentelemetry-collector#3603, but I don't see anything about this in the datamodel spec right now.

@dashpole
Copy link
Contributor

We should also note that in the datamodel spec.

@dashpole
Copy link
Contributor

I opened a PR to clarify those aspects. LMK if I missed anything else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants