Skip to content

Add support for excluding attributes in metrics View - #5332

Open
saurabh-saraswat wants to merge 74 commits into
open-telemetry:mainfrom
saurabh-saraswat:exclude_attribute
Open

saurabh-saraswat wants to merge 74 commits into
open-telemetry:mainfrom
saurabh-saraswat:exclude_attribute

Conversation

@saurabh-saraswat

@saurabh-saraswat saurabh-saraswat commented Jun 19, 2026 •

Copy link
Copy Markdown

Description

Added support for parameter exclude_attribute_keys in View. During consume_measurement removed the attributes which were in exclude_attribute_keys

Fixes #5331

Type of change

Please delete options that are not relevant.

  • [✔] Bug fix (non-breaking change which fixes an issue)

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

  • [ ✔ ] Use exclude parameter while creating view like 👍
  view = View(
            instrument_name="api_counter",
            exclude_attribute_keys={"email", "phone"},
        )

Does This PR Require a Contrib Repo Change?

No

Checklist:

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

@saurabh-saraswat
saurabh-saraswat requested a review from a team as a code owner June 19, 2026 09:53
@xrmx xrmx changed the title Exclude attribute Add support for excluding attributes in metrics view Jun 22, 2026
@xrmx xrmx changed the title Add support for excluding attributes in metrics view Add support for excluding attributes in metrics View Jun 22, 2026
Comment thread .changelog/5332.fixed Resolved
@xrmx xrmx moved this to Ready for review in Python PR digest Jun 30, 2026

@xrmx xrmx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like you haven't run the tests locally

@github-project-automation github-project-automation Bot moved this from Ready for review to Reviewed PRs that need fixes in Python PR digest Jul 1, 2026
@saurabh-saraswat

saurabh-saraswat commented Jul 2, 2026 •

Copy link
Copy Markdown
Author

@xrmx . I have updated the pull request for the comments provided. Local tests was passing.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds View-level support for excluding specific attribute keys from metric stream identity, aligning the Python SDK View API with the Metrics SDK spec’s exclude-list behavior (issue #5331).

Changes:

  • Introduces exclude_attribute_keys on View and applies it during measurement consumption to remove matching keys before stream identification.
  • Updates declarative meter provider configuration to map attribute_keys.excluded into View(exclude_attribute_keys=...).
  • Adds/updates unit tests and changelog entry for the new behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py Adds exclude_attribute_keys to the View API and stores it for later use.
opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py Applies the exclude-list while building the attribute set used for aggregation keying.
opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_meter_provider.py Wires declarative config’s excluded attribute keys into the SDK View.
opentelemetry-sdk/tests/metrics/test_view_instrument_match.py Adds tests validating exclude-list behavior and include-then-exclude precedence.
opentelemetry-sdk/tests/_configuration/test_meter_provider.py Updates config test to assert excluded attribute keys are applied to View.
.changelog/5332.added Documents the added View(exclude_attribute_keys=...) support.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@saurabh-saraswat saurabh-saraswat left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated for github review comments

@saurabh-saraswat saurabh-saraswat left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have incorporated comments, let me know if something is pending from my side

@xrmx xrmx moved this from Reviewed PRs that need fixes to Ready for review in Python PR digest Jul 10, 2026
@xrmx
xrmx self-requested a review July 10, 2026 10:09
@xrmx
xrmx requested a review from Copilot July 14, 2026 08:04
@saurabh-saraswat

Copy link
Copy Markdown
Author

@herin049 thanks for comments, I have incorporated the changes. Kindly check and help in closure of pull request.

@opentelemetry-pr-dashboard

This comment has been minimized.

@saurabh-saraswat

Copy link
Copy Markdown
Author

@herin049, @xrmx I have resolved all the comments. Kindly go through it and see if this pull request can be merged now.

@herin049

Copy link
Copy Markdown
Contributor

There are several issues here identified by Codex:

[P1] Preserve attributes when filtering is unconfigured.
In the final filtering branch, attributes = {} discards the copied attributes when neither list is configured. This also affects default streams with no registered views.

Reproduction: recording two counter measurements with region="us" and region="eu" produces one attribute-free point with value 2, instead of two distinct points. An explicitly empty exclusion list has the same problem.

The review should request retaining the copied attributes when no filter applies.

[P2] Pattern-string matching does not establish that patterns are disjoint.
The constructor’s overlap check tests whether either pattern string matches the other. That misses patterns whose matching sets intersect.

Reproduction: inclusion http.* and exclusion *.status pass construction, although both match http.status. Likewise, a? and ?b both match ab. Because measurement filtering uses elif, accepted configurations silently ignore the exclusion whenever inclusion is configured.

The review should distinguish this from valid exact-overlap rejection: failing fast is allowed, but this check cannot guarantee disjointness. Recommend accepting intersections and applying exclusions after inclusions, avoiding a glob-intersection algorithm.

[P2] Direct fnmatchcase calls violate literal and declarative matching behavior.
The attribute matching expressions interpret bracket expressions as character classes.

Reproduction with both header[0] and header0 present:

  • Including header[0] retains header0 and drops the identical literal key.
  • Excluding header[0] removes header0 and retains the identical literal key.

This breaks existing literal-key inclusion behavior and the declarative schema’s matching contract. The constructor also falsely rejects inclusion header0 plus exclusion header[0].

Request exact matching plus wildcard handling limited to * and ?, treating brackets and backslashes literally.

[P2] Declarative configurations cannot express the specified exclusion precedence.
The configuration adapter forwards both lists to the rejecting constructor.

Reproduction: included=["region"], excluded=["region"] raises ValueError, rather than constructing a stream that excludes region. Inclusion http.* plus exclusion http.status also raises.

SDK-level permission to reject conflicts does not implement the declarative schema’s explicit exclusion-priority behavior. Recommend applying exclusion precedence consistently in both interfaces

This branch has not been deployed

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

Labels

None yet

Projects

Status: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

Support for exclude list is missing in attributes

5 participants