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

Prometheus: Allow changing metric names by default when translating from Prometheus to OpenTelemetry #3679

Merged
merged 2 commits into from
Sep 5, 2023

Conversation

dashpole
Copy link
Contributor

Changes

The current Prometheus -> OTel spec was written based on the needs of the Prometheus receiver. Prometheus/client_java recently released OTLP support by following the Prometheus -> OTel spec in the java prometheus client.

When the collector tried to trim suffixes by default in the Prometheus receiver (open-telemetry/opentelemetry-collector-contrib#21743), it broke a number of users, which is why the current language requires not changing metric names. However, in cases where OTLP export is "new", such as the java prometheus client, it may be acceptable to remove suffixes by default.

This changes the requirement to not alter metric names by default from a MUST to a SHOULD to make it a recommendation.

@dashpole dashpole requested review from a team August 30, 2023 18:15
@dashpole dashpole force-pushed the allow_changing_name branch from 9f9cdfe to 5ebe65b Compare August 30, 2023 18:16
@dashpole dashpole force-pushed the allow_changing_name branch from 5ebe65b to 8e75b83 Compare August 30, 2023 18:18
@dashpole
Copy link
Contributor Author

@open-telemetry/wg-prometheus @fstab

@fstab
Copy link
Member

fstab commented Aug 30, 2023

Thanks a lot @dashpole.

Copy link
Member

@fstab fstab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I have the rights to approve this, but LGTM :)

@pirgeo
Copy link
Member

pirgeo commented Sep 5, 2023

Would this also open the path for allowing a refactoring of metric names (e.g. implementing a NameTranslator or similar) that would replace underscores with dots or whatever else the user might want? Thinking about semantic conventions for example: if they are already collected in a "compatible" format in the Prom client, could they be exported directly in the dot-notation when exporting to an OTLP consumer? This is probably the wrong issue to discuss this, but I wonder if that is something that would be interesting to folks.

Edit: I see that the wording still explicitly calls out that this is mainly about trimming suffixes. I still wonder if there would be an interest to have metrics that map 1:1 onto OTel semantic conventions translated.

@dashpole
Copy link
Contributor Author

dashpole commented Sep 5, 2023

@pirgeo I think whatever mechanism we land on for mapping prometheus names/labels to OTel names/labels, it should live outside of the prometheus exporters/receivers themselves. I could see schemas potentially solving that problem, as they define a mapping from one naming convention to another.

@reyang reyang merged commit 722a61a into open-telemetry:main Sep 5, 2023
6 checks passed
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
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.

7 participants