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 exporters: Add add_metric_suffixes configuration option #24260

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Jul 13, 2023

Description:
Add add_metric_suffixes configuration option, which can disable the addition of type and unit suffixes.
This is backwards-compatible, since the default is true.

This is recommended by the specification for sum suffixes in https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/prometheus_and_openmetrics.md#sums and allowed in metadata
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/prometheus_and_openmetrics.md#metric-metadata-1:

Exporters SHOULD provide a configuration option to disable the addition of _total suffixes.

The resulting unit SHOULD be added to the metric as OpenMetrics UNIT metadata and as a suffix to the metric name unless the metric name already contains the unit, or the unit MUST be omitted

This deprecates the BuildPromCompliantName function in-favor of BuildCompliantName, which includes the additional argument for configuring suffixes.

Link to tracking Issue:

Fixes #21743
Part of #8950

Testing:

Updated tests

Documentation:

Added Documentation.

@dashpole
Copy link
Contributor Author

Looks like I need to update the GMP library before this can move forward.

@dashpole dashpole force-pushed the prw_exporter_normalization branch 2 times, most recently from 486df2f to 788f619 Compare July 13, 2023 18:27
@dashpole
Copy link
Contributor Author

I'll need to follow the deprecation process for BuildPromCompliantName

@dashpole dashpole force-pushed the prw_exporter_normalization branch from 788f619 to f36b1a0 Compare July 13, 2023 18:39
@dashpole
Copy link
Contributor Author

cc @dmitryax
Since this is the exporter portion of #24256

@dashpole
Copy link
Contributor Author

I think it makes sense to keep the feature gate in the exporter since it will be enabled by default, and that change will be breaking for most users

@dashpole dashpole force-pushed the prw_exporter_normalization branch from 734e086 to bf81bcf Compare July 14, 2023 17:18
@dashpole dashpole changed the title prometheus exporters: Add add_type_and_unit_suffixes configuration option prometheus exporters: Add add_metric_suffixes configuration option Jul 14, 2023
@dashpole dashpole force-pushed the prw_exporter_normalization branch from 2ff5490 to a844b83 Compare July 14, 2023 17:20
@dashpole dashpole force-pushed the prw_exporter_normalization branch from f97c8d8 to d68ba60 Compare July 17, 2023 16:57
@dmitryax
Copy link
Member

@dashpole PTAL at the linter failures

…hich can disable the addition of type and unit suffixes.
@dashpole dashpole force-pushed the prw_exporter_normalization branch from d68ba60 to fb3d9c7 Compare July 18, 2023 13:38
@dashpole
Copy link
Contributor Author

@dmitryax fixed

@dashpole dashpole added the ready to merge Code review completed; ready to merge by maintainers label Jul 18, 2023
@dmitryax dmitryax merged commit 2bc9904 into open-telemetry:main Jul 18, 2023
@github-actions github-actions bot added this to the next release milestone Jul 18, 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.

Should prometheus metric name normalization be exposed as a configuration option?
3 participants