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 exporter: do not append _total if the metric already ends in _total #4373

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

dashpole
Copy link
Contributor

Fixes #4371

It accomplishes this by trimming _total suffix from counter metrics if we are going to re-add the suffix later.

@dashpole dashpole force-pushed the duplicate_total_fix branch 2 times, most recently from ca06bbe to fe54953 Compare July 26, 2023 20:47
@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #4373 (cea96be) into main (9452b93) will decrease coverage by 0.2%.
The diff coverage is 91.3%.

❗ Current head cea96be differs from pull request most recent head 12554e4. Consider uploading reports for the commit 12554e4 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4373     +/-   ##
=======================================
- Coverage   83.5%   83.4%   -0.2%     
=======================================
  Files        229     184     -45     
  Lines      18647   14389   -4258     
=======================================
- Hits       15578   12004   -3574     
+ Misses      2751    2157    -594     
+ Partials     318     228     -90     
Files Changed Coverage Δ
exporters/prometheus/exporter.go 83.1% <91.3%> (+0.3%) ⬆️

... and 67 files with indirect coverage changes

@dashpole dashpole marked this pull request as ready for review July 26, 2023 20:58
CHANGELOG.md Show resolved Hide resolved
exporters/prometheus/exporter.go Show resolved Hide resolved
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the changelog needs to be refined 😉

CHANGELOG.md Outdated Show resolved Hide resolved
@MrAlias
Copy link
Contributor

MrAlias commented Aug 7, 2023

@dashpole this looks ready to merge. Can you resolve the conflicts and sync with main?

@dashpole dashpole force-pushed the duplicate_total_fix branch from b833918 to 12554e4 Compare August 7, 2023 17:05
@dashpole
Copy link
Contributor Author

dashpole commented Aug 7, 2023

@MrAlias rebased

@MrAlias MrAlias added the pkg:exporter:prometheus Related to the Prometheus exporter package label Aug 7, 2023
@MrAlias MrAlias added this to the v1.17.0/v0.40.0 milestone Aug 7, 2023
@MrAlias MrAlias merged commit e1e6b96 into open-telemetry:main Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:exporter:prometheus Related to the Prometheus exporter package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

prometheus: _total is appended when one already exists
4 participants