-
Notifications
You must be signed in to change notification settings - Fork 453
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
Update documentation for Metrics API #2280
base: main
Are you sure you want to change the base?
Update documentation for Metrics API #2280
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2280 +/- ##
=====================================
Coverage 76.8% 76.8%
=====================================
Files 122 122
Lines 21823 21823
=====================================
Hits 16772 16772
Misses 5051 5051 ☔ View full report in Codecov by Sentry. |
@@ -11,6 +11,8 @@ static GLOBAL_METER_PROVIDER: Lazy<RwLock<GlobalMeterProvider>> = | |||
|
|||
/// Sets the given [`MeterProvider`] instance as the current global meter | |||
/// provider. | |||
/// | |||
/// **NOTE:** This function should be called before getting [`Meter`] instances via [`meter()`] or [`meter_with_scope()`]. Otherwise, you could get no-op [`Meter`] instances. |
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.
can we mention something like - do this at the earliest stage of your application/similar? Or is this sufficient?
I am also wondering what happens if a library statically creates Meter - is that going to be no-ops always, as we won't have an opportunity to set the global provider beforehand?
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.
Library author should just get meter from provider without the need to know the whether the provider is set or not. Application owner is responsible to instrument the provider before collectiong metrics.
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.
Yes! This API is for application owners only (this is the set one), so we should include doc comment targeting app owners. In fact, I think we should also mention that "if you are library author, do not use this API.
Changes