-
Notifications
You must be signed in to change notification settings - Fork 593
feat: Add description getter to metrics::Instrument in SDK
#3245
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?
feat: Add description getter to metrics::Instrument in SDK
#3245
Conversation
This change is in conflict with <open-telemetry#2985>: > Instrument currently allows users to select an instrument based on > description, which is not allowed in the spec. Only name, kind, unit, scope > should be exposed. The spec at <https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#instrument-selection-criteria> says: > The SDK MAY accept additional criteria. So it seems like the description MAY be exposed. This commit also updates docs based on the field definitions, which seem more descriptive, and cleans up some others.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3245 +/- ##
=====================================
Coverage 80.8% 80.8%
=====================================
Files 129 129
Lines 23199 23202 +3
=====================================
+ Hits 18745 18749 +4
+ Misses 4454 4453 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Can you elaborate the use case more, about fixing descriptions.? |
Sure. With this PR, someone could do something like this to make sure all their descriptions end with punctuation: let meter_provider = meter_provider_builder
.with_view(|instrument| {
let description = instrument.description();
if description.is_empty() || description.ends_with(['.', '!', '?']) {
None
} else {
Stream::builder()
.with_description(format!("{description}."))
.build()
.ok()
}
})
.build();I think this is similar to other use cases for views. |
|
I'll elaborate about my real motivation in another issue soon. I'm just trying to understand things a little better first. |
Note: This change is in conflict with #2985 by @cijothomas:
The spec at https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#instrument-selection-criteria says:
So it seems like the description MAY be exposed.
I could imagine accessing the description in a View for valid reasons, like wanting to append a "." (or remove a ".") from the end of the descriptions. I admit my motivation for doing this PR is part of a larger hack, however. I want my application to be able to list all of its metrics with descriptions, even for metrics that have been registered but not used. I think the only way I can get access to those currently is with a View, but I know that's not its intended use.
Changes
metrics::Instrumentin the SDK.CHANGELOG.mdentry?Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes