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

Revert public API changes in #10931, preserve logic #11415

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

bogdandrutu
Copy link
Member

This PR removes APIs that expose configtelemetry.Level. Internal functionality still uses the Level, but will be soon changed to use the new Instrument.Enabled.

@bogdandrutu bogdandrutu requested a review from a team as a code owner October 11, 2024 19:24
@bogdandrutu bogdandrutu requested a review from songy23 October 11, 2024 19:24
@bogdandrutu bogdandrutu force-pushed the revert-10931 branch 2 times, most recently from b0a127f to 3f595d9 Compare October 11, 2024 19:32
@bogdandrutu bogdandrutu changed the title Rever public API changes in #10931, preserve logic Revert public API changes in #10931, preserve logic Oct 11, 2024
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.

@songy23 songy23 requested a review from codeboten October 15, 2024 13:22
@bogdandrutu bogdandrutu force-pushed the revert-10931 branch 2 times, most recently from 3544610 to fa6ca74 Compare October 21, 2024 17:17
Copy link
Contributor

github-actions bot commented Nov 5, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 5, 2024
@bogdandrutu bogdandrutu requested a review from songy23 November 14, 2024 16:24
@bogdandrutu bogdandrutu removed the Stale label Nov 14, 2024
@bogdandrutu bogdandrutu force-pushed the revert-10931 branch 6 times, most recently from dd0b7b1 to de2d84f Compare November 14, 2024 16:49
Copy link

codecov bot commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 17 lines in your changes missing coverage. Please review.

Project coverage is 91.54%. Comparing base (1d87709) to head (829446c).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
component/componenttest/obsreporttest.go 0.00% 14 Missing ⚠️
config/confighttp/confighttp.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11415      +/-   ##
==========================================
- Coverage   91.64%   91.54%   -0.10%     
==========================================
  Files         442      442              
  Lines       23746    23792      +46     
==========================================
+ Hits        21761    21780      +19     
- Misses       1616     1641      +25     
- Partials      369      371       +2     

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

@bogdandrutu
Copy link
Member Author

@songy23 can you please review

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.

I assume the levels in metrics will be deprecated / replaced in follow-up PRs right?

Level configtelemetry.Level `mapstructure:"level"`

@songy23
Copy link
Member

songy23 commented Nov 14, 2024

Also it looks like this breaks the generated telemetry test in filter processor in contrib: https://github.com/open-telemetry/opentelemetry-collector/actions/runs/11841970416/job/33009752455?pr=11415

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 pending the contrib test fix

or if you prefer feel free to track ^ separately in contrib

Signed-off-by: Bogdan Drutu <[email protected]>
@bogdandrutu
Copy link
Member Author

@bogdandrutu bogdandrutu merged commit 8e522ad into open-telemetry:main Nov 16, 2024
47 of 49 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 16, 2024
@bogdandrutu bogdandrutu deleted the revert-10931 branch November 16, 2024 00:50
djaglowski pushed a commit to djaglowski/opentelemetry-collector that referenced this pull request Nov 21, 2024
…en-telemetry#11415)

This PR removes APIs that expose `configtelemetry.Level`. Internal
functionality still uses the Level, but will be soon changed to use the
new Instrument.Enabled.

Signed-off-by: Bogdan Drutu <[email protected]>
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this pull request Dec 19, 2024
…en-telemetry#11415)

This PR removes APIs that expose `configtelemetry.Level`. Internal
functionality still uses the Level, but will be soon changed to use the
new Instrument.Enabled.

Signed-off-by: Bogdan Drutu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants