-
Notifications
You must be signed in to change notification settings - Fork 897
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
[Prometheus compatibility] Do not require trimming suffixes by default #3580
[Prometheus compatibility] Do not require trimming suffixes by default #3580
Conversation
5227b8b
to
9fe783e
Compare
Maybe I am missing something, but what is the use case for round-tripping through a Prometheus endpoint when I am already using an OTel SDK and an OTel collector? Maybe I am just misunderstanding the meaning of "round-trip" here. |
Good question. Some users may prefer prometheus' pull model for ease of local development, for the |
Gotcha, so this would allow folks to basically use the collector in an all-prometheus deployment.
The idea is that the data produced in the app and scraped by the local collector is the same as the data showing up in the backend, even if the data format was OTLP in between, is that right? |
Your example demonstrates the Prometheus -> OTLP -> Prometheus round-trip. The simpler example of this is a single collector with a Prometheus receiver and a Prometheus exporter, since the internal representation in the collector is OTLP. That round-trip works well today, and is not impacted by this PR. The other round-trip, which is impacted by this PR, is the OTLP -> Prometheus -> OTLP round trip. This would occur when someone uses an OTel SDK (which has an OTel representation of metric data) with a prometheus exporter that is scraped by an OTel collector's prometheus receiver, and then pushed via OTLP somewhere. |
I see, thanks for clarifying that. |
9fe783e
to
237893b
Compare
237893b
to
29fc3c2
Compare
Part of open-telemetry/prometheus-interoperability-spec#72
Changes
We currently require Prometheus -> OpenTelemetry translation to remove type and unit suffixes in metric names. For example, the Prometheus metric
http_request_duration_seconds_total
would becomehttp_request_duration
in the resulting OTel metric. When this change was made, it received a large amount of user pushback, as it broke many existing metric names.This PR makes trimming suffixes from names disabled by default, and encourages implementations to provide a config knob to enable the suffix trimming.
The original motivation for trimming suffixes by default was so that metrics could be round-tripped between an OTel SDK and the collector through the prometheus endpoint. For example, the OTel metric
process.network.io
becomesprocess_network_io_bytes_total
with type and unit suffixes. When converting that back to OpenTelemetry, trimming suffixes would result inprocess_network_io
, which is close to the original OpenTelemetry metric name, but isn't the same as the original.We would like to make the removal of suffixes disabled by default because:
.
are substituted for_
.cc @open-telemetry/wg-prometheus @jmacd @gouthamve