Skip to content

Fix View instrument-name matching to be case-insensitive and platform-independent#5430

Open
winklemad wants to merge 1 commit into
open-telemetry:mainfrom
winklemad:fix-view-instrument-name-case
Open

Fix View instrument-name matching to be case-insensitive and platform-independent#5430
winklemad wants to merge 1 commit into
open-telemetry:mainfrom
winklemad:fix-view-instrument-name-case

Conversation

@winklemad

Copy link
Copy Markdown

Description

A View built with an instrument's real, documented name silently fails to match, so its aggregation / drop / rename / attribute filter is never applied.

Instrument names are valid mixed-case (the API name regex is [a-zA-Z][-_./a-zA-Z0-9]{0,254}), but the SDK lower-cases them on construction (_Synchronous/_Asynchronous set self.name = name.lower()). View._match compared the user's instrument_name pattern against that lower-cased name with fnmatch without lower-casing the pattern:

from opentelemetry.sdk.metrics import MeterProvider
from opentelemetry.sdk.metrics.view import View, DropAggregation
from opentelemetry.sdk.metrics.export import InMemoryMetricReader

reader = InMemoryMetricReader()
mp = MeterProvider(metric_readers=[reader],
    views=[View(instrument_name="MyLatency", aggregation=DropAggregation())])
mp.get_meter("m").create_counter("MyLatency").add(5)
# before: the counter is still exported (view did NOT match "mylatency")
# after:  the counter is dropped (view matches)

There's a second problem at the same spot: fnmatch applies os.path.normcase, which lower-cases on Windows but is identity on POSIX — so the same View matches on Windows and not on Linux/macOS.

Fix

Lower-case the name pattern (matching the SDK's own lower-casing, so names match case-insensitively as the duplicate-instrument-name check at #2409 already assumes), and use fnmatchcase for both the name and unit sites so matching no longer depends on the host platform. Units stay case-sensitive (per UCUM).

Test

Added test_instrument_name_case_insensitive and test_instrument_unit_case_sensitive to test_view.py; they fail on the previous code and pass with the fix. test_view.py → 11 passed.

Disclosure: this change was written with AI assistance (noted via an Assisted-by: commit trailer, per the project's guidance); I've reviewed and verified every line and the reproduction.

@winklemad
winklemad requested a review from a team as a code owner July 17, 2026 16:14
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 17, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: winklemad / name: winklemad (64d11a0)

…istic

Instrument names are valid mixed-case (the API regex allows [a-zA-Z]...),
but the SDK lower-cases them on construction (_Synchronous/_Asynchronous
set self.name = name.lower()). View._match compared the user's
instrument_name pattern against that lower-cased name with fnmatch,
without lower-casing the pattern, so a View built with the instrument's
real name (e.g. View(instrument_name="MyLatency")) silently failed to
match and its aggregation/drop/rename/attribute filter was ignored.

fnmatch also applies os.path.normcase, which lower-cases on Windows but
is identity on POSIX, so the same View matched on Windows but not on
Linux/macOS.

Lower-case the name pattern (matching the SDK's own lower-casing) and use
fnmatchcase for both name and unit so matching no longer depends on the
host platform's filename case sensitivity. Units stay case-sensitive, per
UCUM.

Assisted-by: Claude (Anthropic)
@winklemad
winklemad force-pushed the fix-view-instrument-name-case branch from 4814fa8 to 64d11a0 Compare July 17, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant