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

prevent conflict metric description #3469

Conversation

fatsheep9146
Copy link
Contributor

@fatsheep9146 fatsheep9146 commented Nov 15, 2022

@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Merging #3469 (cb2627a) into main (e97704c) will increase coverage by 0.0%.
The diff coverage is 83.8%.

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           
Impacted Files Coverage Δ
exporters/prometheus/exporter.go 83.4% <83.8%> (+0.9%) ⬆️
sdk/trace/batch_span_processor.go 81.9% <0.0%> (+0.8%) ⬆️

@fatsheep9146
Copy link
Contributor Author

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

exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Show resolved Hide resolved
@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
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter_test.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter_test.go Show resolved Hide resolved
@fatsheep9146 fatsheep9146 force-pushed the prevent-conflict-metric-description branch from a4d8c05 to c68bee6 Compare December 1, 2022 04:51
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
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