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
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 changed the title prometheus exporters: Add add_type_and_unit_suffixes configuration option prometheus exporters: Add add_metric_suffixes configuration option Jul 14, 2023
@dmitryax
Copy link
Member

@dashpole PTAL at the linter failures

…hich can disable the addition of type and unit suffixes.
@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
88 of 89 checks passed
@github-actions github-actions bot added this to the next release milestone Jul 18, 2023
michaelyeager-wf pushed a commit to michaelyeager-wf/opentelemetry-collector-contrib that referenced this pull request Jan 25, 2024
…pen-telemetry#24260)

**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
open-telemetry#21743
Part of
open-telemetry#8950

---------

Co-authored-by: Dmitrii Anoshin <[email protected]>
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