-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[consumer] Mark module as stable #11492
base: main
Are you sure you want to change the base?
Conversation
cc @open-telemetry/collector-approvers |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11492 +/- ##
=======================================
Coverage 91.63% 91.63%
=======================================
Files 442 442
Lines 23776 23776
=======================================
Hits 21787 21787
Misses 1618 1618
Partials 371 371 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
I would merge this after the release to not rush it. Are we ok with that? |
Definitely, and regardless of the release let's at least wait for a few days at least in case somebody has an objection |
It seems that the current API only permits passing signal-agnostic |
Fromt he 2024-10-28 Collector stability meeting, @bogdandrutu will take a look and see if we should do #11492 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will merge after today's release if no one has objections
About #11492 (comment) publicly: we discussed @jade-guiton-dd's comment offline. The summary is: we can still do this by detecting whether an option is signal specific at construction time and returning an error. Jade's suggestion would still improve the API in that it would move a construction time error into a compile time error, but after trying this out it would be quite complex to implement, so we think we can leave the API as is. TL;DR: we are good to go :) |
Description
Mark
consumer
module as stable. This is now possible after #11491, sinceconsumererror
is a separate module now.Link to tracking issue
Fixes #9046