-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[receiver/collectd] remove opencensus #28657
Conversation
82eee6c
to
bf6e9a5
Compare
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.
LGTM
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.
Requesting change to ensure the metric naming isn't broken. If the metric naming changes, the breaking change procedure should be followed :)
Breaking it all the way - no more metrics, let's use obsrecv instead. |
6cb648d
to
2840d32
Compare
…for component metrics
2840d32
to
fadd68d
Compare
fadd68d
to
3f0c463
Compare
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.
I think this is ok since the component is still marked as alpha, for beta components i would expect this change would require a feature gate 👍🏻
Indeed. Note I have opened an issue to move this component to beta here: #28658 and am doing this change now because of it, partially. |
use telemetry settings, don't rely on opencensus for component metrics Fixes open-telemetry#25148
use telemetry settings, don't rely on opencensus for component metrics Fixes open-telemetry#25148
Description:
use telemetry settings, don't rely on opencensus for component metrics
Link to tracking Issue:
Fixes #25148