Skip to content

Conversation

@anuraaga
Copy link
Contributor

Description

Adds a parameter to get_sorted_metrics to filter by scope name to allow the SDK to generate its own metrics and not affect instrumentation. By default, the SDK metrics are filtered out, while instrumentation can instead provide their own scope name to fetch metrics by scope, which can be used like in ASGI to reduce heavy for looping to assert scopes

As it's test-util only I figure it can be skip-changelog

This has been extracted from #4880

@xrmx to help review

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Unit tests

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@anuraaga anuraaga requested a review from a team as a code owner January 23, 2026 04:18
Comment on lines +161 to +164
if scope is None:
if scope_metrics.scope.name == "opentelemetry-sdk":
continue
elif scope_metrics.scope.name != scope:
Copy link
Contributor

@xrmx xrmx Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if scope is None:
if scope_metrics.scope.name == "opentelemetry-sdk":
continue
elif scope_metrics.scope.name != scope:
if scope is not None and scope_metrics.scope.name != scope:

I think we should try with a less opinionated default of returning all metrics first, or we can add a exclude_scopes=Optional[Sequence[str]] but I would like to keep the ability to get all the metrics

Copy link
Contributor Author

@anuraaga anuraaga Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible it would be nice if the default can preserve current behavior which is to not return SDK metrics and what I have here. Otherwise we can't make contrib changes forwards compatible and would need to wait for releases before implementing SDK metrics. Can we try to loosen it after releasing, updating contrib to pass a scope, then update here? Currently there's no use case for returning all the metrics as a default (all current tests want only the instrumentation's metrics) so I think that could be reasonable

Copy link
Contributor

@xrmx xrmx Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anuraaga the opentelemetry-util-http version that instrumentations depends on is fixed, so once this change lands in main here we're fine to use it in -contrib.

If we really want to skip opentelemetry-sdk by default let's use exclude_scopes: [Sequence[str]] = ("opentelemetry-sdk",) then so that if one wants to have all metrics is able to do so.

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