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

[receiver/prometheusreceiver]: trim type and unit suffixes from metric name #16071

Conversation

paivagustavo
Copy link
Member

@paivagustavo paivagustavo commented Nov 3, 2022

Description: Trim type's and unit's suffixes from metric name as per otel specs.

There was a lot of changes required to pass down a featuregate.Registry in order to be testable. The important bit in the receiver code path is the metricFamily.appendMetric which trims the suffixes when creating an OTLP metric.

Units suffixes will only take effect when using the OpenMetrics format, since the prometheus parser doesn't support units.

This is being guarded by the gate: --feature-gates=pkg.translator.prometheus.NormalizeName

Link to tracking Issue: #8950

Testing: Units tests were added for the translator package, and prometheus receiver tests updated.

@dashpole
Copy link
Contributor

dashpole commented Nov 4, 2022

Can you put this behind the --feature-gates=pkg.translator.prometheus.NormalizeName feature gate?

@paivagustavo paivagustavo changed the title prometheusreceiver: trim type and unit suffixes from metric name [receiver/prometheusreceiver]: trim type and unit suffixes from metric name Nov 4, 2022
@paivagustavo paivagustavo force-pushed the gustavo/promreceiver/normalize_metric_suffixes branch from ea6b8fd to d8ca297 Compare November 4, 2022 20:16
@paivagustavo
Copy link
Member Author

@dashpole I've guarded this with the featuregate as asked, and added some specific tests for this on the receiver side as well.

@dashpole
Copy link
Contributor

dashpole commented Nov 7, 2022

One of the tests had an invalid case, exposing both foo and a foo_total metric

Note that this case is valid in prometheus format, just not in OpenMetrics format.

@paivagustavo paivagustavo force-pushed the gustavo/promreceiver/normalize_metric_suffixes branch 3 times, most recently from feb607f to 9d23af1 Compare November 7, 2022 20:30
@paivagustavo
Copy link
Member Author

Note that this case is valid in prometheus format, just not in OpenMetrics format.

That makes sense, reverted those tests changes :)

@paivagustavo paivagustavo force-pushed the gustavo/promreceiver/normalize_metric_suffixes branch from 9d23af1 to c592716 Compare November 9, 2022 14:50
@paivagustavo
Copy link
Member Author

Seems like TestEnsureRecordedMetrics fromgithub.com/open-telemetry/opentelemetry-collector-contrib/receiver/opencensusreceiver/internal/ocmetrics is flaky. Just failed at another commit on main: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/3464963384/jobs/5787150358.

@dashpole do you mind retrying please?

@paivagustavo paivagustavo force-pushed the gustavo/promreceiver/normalize_metric_suffixes branch from fdd1e62 to 5550604 Compare November 15, 2022 15:47
@codeboten codeboten merged commit 27c9f42 into open-telemetry:main Nov 18, 2022
@codeboten codeboten deleted the gustavo/promreceiver/normalize_metric_suffixes branch November 18, 2022 18:17
JaredTan95 pushed a commit to openinsight-proj/opentelemetry-collector-contrib that referenced this pull request Nov 21, 2022
…c name (open-telemetry#16071)

 Trim type's and unit's suffixes from metric name as per otel specs.

There was a lot of changes required to pass down a featuregate.Registry in order to be testable. The important bit in the receiver code path is the metricFamily.appendMetric which trims the suffixes when creating an OTLP metric.

Units suffixes will only take effect when using the OpenMetrics format, since the prometheus parser doesn't support units.

This is being guarded by the gate: --feature-gates=pkg.translator.prometheus.NormalizeName
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2022
…c name (open-telemetry#16071)

 Trim type's and unit's suffixes from metric name as per otel specs.

There was a lot of changes required to pass down a featuregate.Registry in order to be testable. The important bit in the receiver code path is the metricFamily.appendMetric which trims the suffixes when creating an OTLP metric.

Units suffixes will only take effect when using the OpenMetrics format, since the prometheus parser doesn't support units.

This is being guarded by the gate: --feature-gates=pkg.translator.prometheus.NormalizeName
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants