Skip to content

Conversation

@codeboten
Copy link
Contributor

Fixes #12458

@codeboten codeboten requested a review from a team as a code owner February 26, 2025 16:19
@codeboten codeboten requested a review from dmitryax February 26, 2025 16:19
@codeboten codeboten added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 26, 2025
@codecov
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.00%. Comparing base (58f3efb) to head (3131675).
Report is 23 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12500      +/-   ##
==========================================
- Coverage   92.16%   92.00%   -0.17%     
==========================================
  Files         465      469       +4     
  Lines       25201    25355     +154     
==========================================
+ Hits        23226    23327     +101     
- Misses       1576     1619      +43     
- Partials      399      409      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


The change to update the internal telemetry to use [otel-go config](https://pkg.go.dev/go.opentelemetry.io/contrib/config) can cause unexpected behaviour
for end users. This change is caused by the default values in `config` being different from what the Collector has used in previous versions. The
following changes can occur when users configure their `service::telemetry::metrics::readers`:
Copy link
Member

Choose a reason for hiding this comment

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

QQ: does it mean as long as you don't explicitly configure readers you won't run into this? e.g. if I just have

service:
  telemetry:
    metrics:
      level: detailed

the behavior of this should be the same in all versions, right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think it worth mentioning that the breaking change only affects user how customized the prometheus reader in any way. The default behavior is the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to add a note at the beginning to capture this, ptal @dmitryax @songy23

Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

LGTM

@bogdandrutu bogdandrutu added this pull request to the merge queue Feb 28, 2025
Merged via the queue into open-telemetry:main with commit 6a26bdc Feb 28, 2025
42 checks passed
@codeboten codeboten deleted the codeboten/fix-12458 branch July 16, 2025 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry Skip Contrib Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[service] Internal counter metrics now have a _total suffix in Prometheus

5 participants