Skip to content

prevent conflict metric description #3469

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

Conversation

fatsheep9146
Copy link
Contributor

@fatsheep9146 fatsheep9146 commented Nov 15, 2022

@fatsheep9146 fatsheep9146 force-pushed the prevent-conflict-metric-description branch from a1a652a to 4cb3b8f Compare November 21, 2022 04:51
@fatsheep9146 fatsheep9146 marked this pull request as ready for review November 21, 2022 04:52
@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Attention: Patch coverage is 83.87097% with 10 lines in your changes missing coverage. Please review.

Project coverage is 78.1%. Comparing base (e97704c) to head (cb2627a).
Report is 1818 commits behind head on main.

Files with missing lines Patch % Lines
exporters/prometheus/exporter.go 83.8% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #3469   +/-   ##
=====================================
  Coverage   78.0%   78.1%           
=====================================
  Files        165     165           
  Lines      11755   11809   +54     
=====================================
+ Hits        9179    9228   +49     
- Misses      2380    2385    +5     
  Partials     196     196           
Files with missing lines Coverage Δ
exporters/prometheus/exporter.go 83.4% <83.8%> (+0.9%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fatsheep9146
Copy link
Contributor Author

@MrAlias @dashpole the pr is ready to be reviews, could you help review that?

@fatsheep9146 fatsheep9146 force-pushed the prevent-conflict-metric-description branch 2 times, most recently from df1967a to 3c84770 Compare November 25, 2022 05:36
@fatsheep9146
Copy link
Contributor Author

@dashpole @MrAlias all comments are fixed, could you help review this again?

@fatsheep9146 fatsheep9146 force-pushed the prevent-conflict-metric-description branch from 97f5e40 to efd6eb0 Compare November 25, 2022 23:43
@MrAlias MrAlias added this to the Metric v0.34.0 milestone Nov 29, 2022
@fatsheep9146 fatsheep9146 force-pushed the prevent-conflict-metric-description branch from a4d8c05 to c68bee6 Compare December 1, 2022 04:51
@fatsheep9146 fatsheep9146 requested a review from MrAlias December 1, 2022 05:10
@fatsheep9146 fatsheep9146 force-pushed the prevent-conflict-metric-description branch from 69f5348 to 7c5e3d8 Compare December 2, 2022 14:45
@fatsheep9146
Copy link
Contributor Author

I add more test cases to cover all possible conflict cases,

  • conflict help for 3 different metric type Counter, UpDownCounter and Histogram
  • conflict unit for 3 different metric type Counter, UpDownCounter and Histogram
  • conflict type for 2 different combinations
    • histogram and updowncounter
    • counter and updowncounter

@Aneurysm9 @dashpole @MrAlias

@MadVikingGod MadVikingGod merged commit 2243431 into open-telemetry:main Dec 5, 2022
codeboten added a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Nov 18, 2024
#36356)


#### Description
Fixes bug where exporting fails due to different help messages for the
same metric. With this solution, the exporter will always export metrics
of the same name with the first description it receives. This also
rejects metrics whose types have changed. These changes follow the
[spec](https://opentelemetry.io/docs/specs/otel/compatibility/prometheus_and_openmetrics/#metric-metadata-1):
>Exporters MUST drop entire metrics to prevent conflicting TYPE
comments, but SHOULD NOT drop metric points as a result of conflicting
UNIT or HELP comments. Instead, all but one of the conflicting UNIT and
HELP comments (but not metric points) SHOULD be dropped. If dropping a
comment or metric points, the exporter SHOULD warn the user through
error logging.

Based on open-telemetry/opentelemetry-go#3469

#### Link to tracking issue
Fixes
#28617

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit test cases added.

---------

Co-authored-by: David Ashpole <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
rshiyani pushed a commit to rshiyani/opentelemetry-collector-contrib that referenced this pull request Dec 9, 2024
open-telemetry#36356)


#### Description
Fixes bug where exporting fails due to different help messages for the
same metric. With this solution, the exporter will always export metrics
of the same name with the first description it receives. This also
rejects metrics whose types have changed. These changes follow the
[spec](https://opentelemetry.io/docs/specs/otel/compatibility/prometheus_and_openmetrics/#metric-metadata-1):
>Exporters MUST drop entire metrics to prevent conflicting TYPE
comments, but SHOULD NOT drop metric points as a result of conflicting
UNIT or HELP comments. Instead, all but one of the conflicting UNIT and
HELP comments (but not metric points) SHOULD be dropped. If dropping a
comment or metric points, the exporter SHOULD warn the user through
error logging.

Based on open-telemetry/opentelemetry-go#3469

#### Link to tracking issue
Fixes
open-telemetry#28617

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit test cases added.

---------

Co-authored-by: David Ashpole <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
open-telemetry#36356)


#### Description
Fixes bug where exporting fails due to different help messages for the
same metric. With this solution, the exporter will always export metrics
of the same name with the first description it receives. This also
rejects metrics whose types have changed. These changes follow the
[spec](https://opentelemetry.io/docs/specs/otel/compatibility/prometheus_and_openmetrics/#metric-metadata-1):
>Exporters MUST drop entire metrics to prevent conflicting TYPE
comments, but SHOULD NOT drop metric points as a result of conflicting
UNIT or HELP comments. Instead, all but one of the conflicting UNIT and
HELP comments (but not metric points) SHOULD be dropped. If dropping a
comment or metric points, the exporter SHOULD warn the user through
error logging.

Based on open-telemetry/opentelemetry-go#3469

#### Link to tracking issue
Fixes
open-telemetry#28617

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Unit test cases added.

---------

Co-authored-by: David Ashpole <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
@MrAlias MrAlias added the area:metrics Part of OpenTelemetry Metrics label Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Impossible to create Instrument with the same name from the different MeterProvider
6 participants