-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add metric level registry for components #14338
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
base: main
Are you sure you want to change the base?
Conversation
…metry levels for their metrics Signed-off-by: Israel Blancas <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14338 +/- ##
=======================================
Coverage 92.13% 92.13%
=======================================
Files 668 673 +5
Lines 41529 41574 +45
=======================================
+ Hits 38263 38305 +42
- Misses 2230 2231 +1
- Partials 1036 1038 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // a given meter or instrument. Components can register these declarations so | ||
| // the service can derive default views that drop metrics when the level is | ||
| // below the configured threshold. | ||
| type MetricLevelConfig struct { |
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.
Should we allow components to register views instead?
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.
Maybe. I'm not against that. I was thinking about keeping this because maybe want to do something based on the stability metric level? Something we can add later. But I'm ok with the change. Let me know what do you think and I will be more than happy to change this.
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 haven't reviewed the rest of this PR but I think this should be on an xcomponent package instead of here since this would (at the beginning at least) an experimental package (there's also spec work about this, see open-telemetry/opentelemetry-specification/issues/4391)
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'm ok with that. Will implement the change.
Description
Created a registry, similar to the feature gates, for the component metrics.
Link to tracking issue
Fixes #11754