Skip to content

Conversation

@ongardie-atomix
Copy link
Contributor

@ongardie-atomix ongardie-atomix commented Nov 14, 2025

Note: This change is in conflict with #2985 by @cijothomas:

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.


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

  • Adds a getter for the description field of metrics::Instrument in the SDK.
  • Updates docs based on the field definitions, which seem more descriptive, and cleans up some others.
  • Does this need a CHANGELOG.md entry?

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

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.
@ongardie-atomix ongardie-atomix requested a review from a team as a code owner November 14, 2025 05:43
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 14, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.8%. Comparing base (95af815) to head (971db88).

Files with missing lines Patch % Lines
opentelemetry-sdk/src/metrics/instrument.rs 0.0% 3 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cijothomas
Copy link
Member

Can you elaborate the use case more, about fixing descriptions.?

@ongardie-atomix
Copy link
Contributor Author

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.

@ongardie-atomix
Copy link
Contributor Author

I'll elaborate about my real motivation in another issue soon. I'm just trying to understand things a little better first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants