Repository navigation
Conversation
…pen-telemetry#5664) The OpenTelemetry Metrics Specification specifies that if name is provided, the View MUST match at most one instrument, and if instrument_name contains wildcard characters (*, ?, [seq], [!seq]), name MUST NOT be provided. Previously, View.__init__ only checked for '*' and '?' in instrument_name, allowing character sequence wildcards such as '[' and ']' (e.g. 'http_[0-9]') to bypass validation when name is set. Update View.__init__ to detect character pattern wildcards ('[', ']') in addition to '*' and '?', and add test coverage for character sequence wildcards. Signed-off-by: Soumyajit Ghosh <jobsoumyajit6124@gmail.com>
Pull request dashboard statusClosed · refreshed 2026-09-25 15:26 UTC Status above doesn't look right?
|
|
/dashboard route:reviewers |
|
@somuai, this pull request was routed to reviewers. The handoff remains active across pushes until newer actionable human feedback arrives. Top-level feedback through this request will not return; unresolved review threads remain open. |
herin049
left a comment
There was a problem hiding this comment.
There appears to be some unrelated formatting changes being applied here.
Signed-off-by: Soumyajit Ghosh <jobsoumyajit6124@gmail.com>
Signed-off-by: Soumyajit Ghosh <jobsoumyajit6124@gmail.com>
|
Going to close this since it's a duplicate. |
|
Thank you for reviewing and maintaining the repository. I wanted to share a quick point of clarification regarding the timeline and technical scope of this PR relative to #5674. PR #5673 was submitted on 2026-09-18 at 15:55:19 UTC, approximately 2.5 hours prior to PR #5674 (submitted at 18:18:06 UTC). In addition, this PR includes a couple of technical details that may be relevant to the review comments left on #5674:
If you would like to proceed with this PR, I would be happy to have it reopened. Alternatively, if you prefer to keep #5674 since it has already received initial review, please feel free to use the test cases and bracket check from here in that PR. Whichever approach works best for the maintainers works for me. Thank you again for your time and guidance! |
Description
Fixes #5664
According to the OpenTelemetry Metrics Specification for Views (https://opentelemetry.io/docs/specs/otel/metrics/sdk/#view):
Previously,
View.__init__checked for wildcard characters ininstrument_nameonly by inspecting whether"*" in instrument_name or "?" in instrument_name. However, instrument matching inView._matchis executed viafnmatch.fnmatchcase(), which supports character sequence wildcards such as[seq]and[!seq](for example,instrument_name="http_[0-9]"). Because brackets were not checked, specifying a character set wildcard alongside a customnamebypassed the validation check, permitting multiple instruments to match a single renamed View stream in violation of the specification constraint.This change updates
View.__init__to detect character pattern wildcards ("[","]") ininstrument_namein addition to"*"and"?", raising an exception whenevernameis provided with any wildcard pattern.Changes
opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py:View.__init__wildcard check to verifyany(c in instrument_name for c in "*?[]").opentelemetry-sdk/tests/metrics/test_view.py:test_view_namewith subtests covering*,?,[0-9],[!a-z],[abc],[, and], verifying they raise an exception whennameis provided.instrument_namesucceeds withname..changelog/5664.fixed.Verification
PYTHONPATH=opentelemetry-api/src:opentelemetry-sdk/src:opentelemetry-semantic-conventions/src:tests/opentelemetry-test-utils/src pytest opentelemetry-sdk/tests/metrics/test_view.py opentelemetry-sdk/tests/metrics/test_view_instrument_match.py -vResult: 27 passed, 8 subtests passed (100% green).
Signed-off-by: Soumyajit Ghosh <jobsoumyajit6124@gmail.com>.