prometheusexporter: Automatic normalization of metrics from Otel to Prometheus naming convention #10028

bertysentry · 2022-05-15T21:49:07Z

Description: In prometheusexporter and prometheusremotewriteexporter, automatic normalization of metrics from OpenTelemetry semantic convention to Prometheus naming convention

The renaming is in alpha stage and thus disabled by default (behind a feature gate). Enable with --feature-gates=pkg.translator.prometheus.NormalizeName option.

Shared code between prometheusexporter and prometheusremotewriteexporter has been moved to a new module: pkg/translator/prometheus. This module provides 2 main functions:

func NormalizeLabel(label string) string for labels normalization
func BuildPromCompliantName(metric pmetric.Metric, namespace string) string for metric names normalization

All normalization operations are detailed in pkg/translator/prometheus/README.md.

Note
It is recommended to import pkg/translator/prometheus as prometheustranslator to avoid conflict with the "real" Prometheus module, from Prometheus.

Link to tracking Issue: #8950

Testing: See unit tests in the pkg/translator/prometheus module

Documentation: Updated README.md in prometheusexporter and prometheusremotewriteexporter

Aneurysm9 · 2022-05-17T09:05:32Z

This looks reasonable to me, though I'm still not clear on whether _total should be appended to counter names or not. I think some prometheus client libraries do that and some don't.

What happens if metric names already contain a suffix or go through multiple round trips? Will the suffix be omitted if it already exists?

bertysentry · 2022-05-17T10:19:33Z

@Aneurysm9 Thank you for the comment!

_total must be appended to Counter metrics as per Prometheus naming standard.

Good point about round-trip conversion: units must not be appended when already present in the metric name (this goes for units, _total and _ratio suffixes).

Also, the issue #8950 mentions the need for the Prometheus receiver to perform the reverse "conversion", like in the example below:

system_io_bytes_total ==> system.io Counter with Units By

This should be covered in a separate PR.

dashpole

Whoops, forgot to click send...

exporter/prometheusexporter/README.md

dashpole · 2022-05-24T13:03:40Z

readme LGTM

bertysentry · 2022-05-26T17:01:46Z

@Aneurysm9 @dashpole @pmm-sumo Code pushed, according to our discussion.

Next step: update prometheusremotewriteexporter the same way.

exporter/prometheusexporter/metric_rename.go

bertysentry · 2022-06-01T23:03:59Z

@dashpole Complete rewrite to properly handle all metrics (and their units) in current Otel receivers. Added many unit tests to cover these cases.

Designed to be fast and robust, while covering most reasonable cases (and 100% of current Otel receivers).

bertysentry · 2022-06-01T23:05:03Z

@Aneurysm9 @pmm-sumo Please review! 😊

exporter/prometheusexporter/README.md

exporter/prometheusexporter/metric_rename.go

bertysentry · 2022-06-02T18:44:27Z

Code updated following suggestions, and rebased to solve conflict.

bertysentry · 2022-06-06T13:54:39Z

@dashpole @pmm-sumo @Aneurysm9 If you got some time, this PR needs approvals 😉

bertysentry · 2022-06-08T08:59:36Z

@Aneurysm9 @pmm-sumo I need this PR to be merged so I can start working on prometheusremotewriteexporter to implement the same mechanism. Is there anyone else you would like to review these changes? Thank you!

bertysentry · 2022-06-08T18:10:42Z

Rebased on main to solve recently introduced conflict in CHANGELOG.md

bertysentry · 2022-06-13T11:27:39Z

Restructured the code so that common code for normalizing metric names and labels is now in module /pkg/translator/prometheus. Both prometheusexporter and prometheusremotewriteexporter now leverage these functions, so the behavior is consistent.

@dashpole Your review and comments are appreciated 😊
@Aneurysm9 and @pmm-sumo This PR changes a few things in the Prometheus exporters. Your comments are welcome.

I will look into the Prometheus compliance tests that are now failing.

bertysentry · 2022-06-15T16:39:01Z

@dmitryax @alolita @Aneurysm9 This metric normalization is required as per OpenTelemetry's official specification. Please review (and merge if okay). I'm concerned these changes could cause conflicts with other PRs that update the Prometheus-specific code.

If review is not possible at this time (which I totally understand), could you please redirect me to other potential reviewers? Thank you!

bertysentry · 2022-06-16T22:32:35Z

I am sorry to insist, but this PR fixes an important issue in the Prometheus exporters. How can I help to get this merged?

bertysentry · 2022-06-22T17:09:21Z

I'm begging here: I need a review of this PR, as I can't keep resolving conflicts with upstream.
Please remember that changes in this PR are behind a feature gate, so this PR is not going to cause havoc on the project, but it refactors some duplicate code between the exporters for Prometheus (hence the many changes in this PR).
Please @jpkrohling @Aneurysm9 your help is much appreciated. Thanks!

…metrics to follow Prometheus naming convention * Updated **prometheusremotewriteexporter** to normalize metric names * Moved common code to new module /pkg/translator/prometheus * Added documentation

bertysentry · 2022-06-22T18:17:23Z

Also, many tests are now failing after rebasing on main. Failures are completely unrelated to the changes in this PR. What is happening? Is there anyone who can help?

djaglowski · 2022-06-22T21:10:08Z

@bertysentry, CI failed on an unrelated flaky test (#11451). I've restarted the workflow.

bertysentry · 2022-06-23T09:39:27Z

Daily burden of go.mod conflict resolution...

Can anyone give me a hint on how to get these changes approved and merged? It's critical to change the behavior of the Prometheus exporters as soon as possible so they follow Otel's specifications and Prometheus' conventions. The longer we wait, the worse it gets to end users.
Feel free to contact me on CNCF's Slack if you want to discuss the changes. @Aneurysm9 @pmm-sumo @jpkrohling
Thank you!

djaglowski · 2022-06-23T13:36:23Z

Hi @bertysentry, I tried to resolve the conflicts and push but you have not allowed maintainers to push to your branch. If you want to click the box (bottom of the righthand panel), I'll push. Then hopefully we can get this merged today.

bertysentry · 2022-06-23T14:00:04Z

@djaglowski Awesome news! Weirdly, I can't see the option to allow maintainers to push to the branch of my PR:

I gave you direct access to my forked repo, so you should be able to push (you should have received an invite). Thank you again for your help!

bertysentry · 2022-06-23T16:38:17Z

Thank you @djaglowski for your help, much appreciated!

djaglowski · 2022-06-23T17:23:36Z

Glad to help @bertysentry.

I should have noticed this before, but since this PR introduced a new module, we really ought to have a code owner for it. Would you, @bertysentry and/or @dashpole, be willing to be the code owner for pkg/translator/prometheus?

dashpole · 2022-06-23T17:24:36Z

I'm happy to be a codeowner

bertysentry · 2022-06-23T17:25:20Z

Happy to!

djaglowski · 2022-06-23T17:33:10Z

Thank you both! I'll make the PR

bertysentry requested review from a team and Aneurysm9 as code owners May 15, 2022 21:49

project-bot bot added this to In progress in Collector May 15, 2022

github-actions bot assigned jpkrohling May 15, 2022

bertysentry mentioned this pull request May 15, 2022

[prometheusreceiver/prometheusexporter] Handle units in prometheus metric names #8950

Closed

Aneurysm9 requested a review from dashpole May 17, 2022 09:01

dashpole reviewed May 17, 2022

View reviewed changes

bertysentry changed the title ~~Add option for automatic renaming of metrics from Otel to Prometheus naming convention~~ Automatic renaming of metrics from Otel to Prometheus naming convention May 25, 2022

bertysentry changed the title ~~Automatic renaming of metrics from Otel to Prometheus naming convention~~ prometheusexporter: Automatic renaming of metrics from Otel to Prometheus naming convention May 25, 2022

bertysentry force-pushed the feature/issue-8950-handle-units-prometheus branch from aff427b to 43303f7 Compare May 26, 2022 15:54

dashpole reviewed May 26, 2022

View reviewed changes

dashpole reviewed May 28, 2022

View reviewed changes

exporter/prometheusexporter/metric_rename.go Outdated Show resolved Hide resolved

exporter/prometheusexporter/metric_rename.go Outdated Show resolved Hide resolved

exporter/prometheusexporter/metric_rename.go Outdated Show resolved Hide resolved

dashpole reviewed Jun 2, 2022

View reviewed changes

exporter/prometheusexporter/README.md Outdated Show resolved Hide resolved

exporter/prometheusexporter/metric_rename.go Outdated Show resolved Hide resolved

exporter/prometheusexporter/metric_rename.go Outdated Show resolved Hide resolved

bertysentry force-pushed the feature/issue-8950-handle-units-prometheus branch from 5703356 to ffdbb11 Compare June 2, 2022 18:42

dashpole approved these changes Jun 6, 2022

View reviewed changes

Collector automation moved this from In progress to Reviewer approved Jun 6, 2022

bertysentry requested a review from pmm-sumo June 6, 2022 22:42

bertysentry force-pushed the feature/issue-8950-handle-units-prometheus branch from 7baad4d to 3839a11 Compare June 8, 2022 18:09

bertysentry force-pushed the feature/issue-8950-handle-units-prometheus branch from 6b1bb37 to e38cef2 Compare June 13, 2022 11:24

bertysentry mentioned this pull request Jun 14, 2022

[zookeeperreceiver] Some metric units don't follow Otel semantic conventions #10567

Closed

bertysentry force-pushed the feature/issue-8950-handle-units-prometheus branch from 52e4424 to 06f14a8 Compare June 22, 2022 10:44

bertysentry added 2 commits June 22, 2022 19:13

Issue open-telemetry#8950 prometheusexporter: Automatic rename of …

1a1f221

…metrics to follow Prometheus naming convention * Updated **prometheusremotewriteexporter** to normalize metric names * Moved common code to new module /pkg/translator/prometheus * Added documentation

Fixes following GitHub checks failures

d4757be

bertysentry force-pushed the feature/issue-8950-handle-units-prometheus branch from 5daffa8 to d4757be Compare June 22, 2022 17:30

Fixed go.mod again

649cf76

djaglowski approved these changes Jun 23, 2022

View reviewed changes

Merge branch 'main' into feature/issue-8950-handle-units-prometheus

e1698bd

djaglowski added 3 commits June 23, 2022 10:26

update module versions

71020ea

gotidy

c5a0e68

Merge branch 'main' into feature/issue-8950-handle-units-prometheus

d4e4f7f

djaglowski merged commit b828db4 into open-telemetry:main Jun 23, 2022

bertysentry deleted the feature/issue-8950-handle-units-prometheus branch June 23, 2022 16:34

This was referenced Jun 23, 2022

[pkg/translator/prometheus] Add codeowners #11515

Merged

[receiver/prometheus] Flaky Test - TestLabelNameLimitConfig #11516

Closed

dashpole mentioned this pull request Oct 18, 2022

Add unit suffixes to prometheus metric names open-telemetry/opentelemetry-go#3352

Merged

paivagustavo mentioned this pull request Nov 3, 2022

[exporters/prometheus] / [exporters/prometheusremotewrite] do not add _total suffix to Sum metric types #11837

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

prometheusexporter: Automatic normalization of metrics from Otel to Prometheus naming convention #10028

prometheusexporter: Automatic normalization of metrics from Otel to Prometheus naming convention #10028

bertysentry commented May 15, 2022 •

edited

Loading

Aneurysm9 commented May 17, 2022

bertysentry commented May 17, 2022

dashpole left a comment

dashpole commented May 24, 2022

bertysentry commented May 26, 2022

bertysentry commented Jun 1, 2022

bertysentry commented Jun 1, 2022

bertysentry commented Jun 2, 2022

bertysentry commented Jun 6, 2022

bertysentry commented Jun 8, 2022

bertysentry commented Jun 8, 2022

bertysentry commented Jun 13, 2022

bertysentry commented Jun 15, 2022

bertysentry commented Jun 16, 2022

bertysentry commented Jun 22, 2022

bertysentry commented Jun 22, 2022

djaglowski commented Jun 22, 2022

bertysentry commented Jun 23, 2022

djaglowski commented Jun 23, 2022

bertysentry commented Jun 23, 2022

bertysentry commented Jun 23, 2022

djaglowski commented Jun 23, 2022

dashpole commented Jun 23, 2022

bertysentry commented Jun 23, 2022

djaglowski commented Jun 23, 2022

prometheusexporter: Automatic normalization of metrics from Otel to Prometheus naming convention #10028

prometheusexporter: Automatic normalization of metrics from Otel to Prometheus naming convention #10028

Conversation

bertysentry commented May 15, 2022 • edited Loading

Aneurysm9 commented May 17, 2022

bertysentry commented May 17, 2022

dashpole left a comment

Choose a reason for hiding this comment

dashpole commented May 24, 2022

bertysentry commented May 26, 2022

bertysentry commented Jun 1, 2022

bertysentry commented Jun 1, 2022

bertysentry commented Jun 2, 2022

bertysentry commented Jun 6, 2022

bertysentry commented Jun 8, 2022

bertysentry commented Jun 8, 2022

bertysentry commented Jun 13, 2022

bertysentry commented Jun 15, 2022

bertysentry commented Jun 16, 2022

bertysentry commented Jun 22, 2022

bertysentry commented Jun 22, 2022

djaglowski commented Jun 22, 2022

bertysentry commented Jun 23, 2022

djaglowski commented Jun 23, 2022

bertysentry commented Jun 23, 2022

bertysentry commented Jun 23, 2022

djaglowski commented Jun 23, 2022

dashpole commented Jun 23, 2022

bertysentry commented Jun 23, 2022

djaglowski commented Jun 23, 2022

bertysentry commented May 15, 2022 •

edited

Loading