Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
winklemad wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −3
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@


from collections.abc import Callable
from fnmatch import fnmatch
from fnmatch import fnmatchcase
from logging import getLogger

from opentelemetry.metrics import Instrument
Expand Down Expand Up @@ -161,11 +161,17 @@ def _match(self, instrument: _Instrument) -> bool:
return False

if self._instrument_name is not None:
if not fnmatch(instrument.name, self._instrument_name):
# Instrument names are treated case-insensitively and are stored
# lower-cased by the SDK, so the pattern is lower-cased to match.
# fnmatchcase is used instead of fnmatch so it does not rely
# on the host platform's filename case sensitivity (normcase).
if not fnmatchcase(instrument.name, self._instrument_name.lower()):
return False

if self._instrument_unit is not None:
if not fnmatch(instrument.unit, self._instrument_unit):
# Units are case-sensitive; fnmatchcase keeps matching consistent
# across platforms.
if not fnmatchcase(instrument.unit, self._instrument_unit):
return False

if self._meter_name is not None:
Expand Down
26 changes: 26 additions & 0 deletions opentelemetry-sdk/tests/metrics/test_view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,23 @@ def test_instrument_name(self):
View(instrument_name="instrument_name")._match(mock_instrument)
)

def test_instrument_name_case_insensitive(self):
# The SDK stores instrument names lower-cased, so a view pattern that
# reuses the instrument's original name must still match regardless of
# case (and regardless of the host platform).
mock_instrument = Mock()
mock_instrument.configure_mock(**{"name": "instrument_name"})

self.assertTrue(
View(instrument_name="Instrument_Name")._match(mock_instrument)
)
self.assertTrue(
View(instrument_name="INSTRUMENT_*")._match(mock_instrument)
)
self.assertFalse(
View(instrument_name="other_name")._match(mock_instrument)
)

def test_instrument_unit(self):
mock_instrument = Mock()
mock_instrument.configure_mock(**{"unit": "instrument_unit"})
Expand All @@ -33,6 +50,15 @@ def test_instrument_unit(self):
View(instrument_unit="instrument_unit")._match(mock_instrument)
)

def test_instrument_unit_case_sensitive(self):
# Units are case-sensitive and matching must not depend on the host
# platform's filename case sensitivity.
mock_instrument = Mock()
mock_instrument.configure_mock(**{"unit": "By"})

self.assertTrue(View(instrument_unit="By")._match(mock_instrument))
self.assertFalse(View(instrument_unit="by")._match(mock_instrument))

def test_meter_name(self):
self.assertTrue(
View(meter_name="meter_name")._match(
Expand Down