opentelemetry-sdk: preserve metric instrument name casing

.changelog/5881.fixed

@@ -0,0 +1 @@
+`opentelemetry-sdk`: Preserve the original casing of metric instrument names.

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py

@@ -116,6 +116,16 @@ def _is_enabled(self) -> bool:
     def _set_meter_config(self, meter_config: _MeterConfig) -> None:
         self._meter_config.update(meter_config)
 
+    @staticmethod
+    def _log_instrument_name_conflict(name: str, registered_name: str) -> None:
+        """Report a different spelling of an already registered instrument name."""
+        if name != registered_name:
+            _logger.warning(
+                "An instrument with name %s has been created already using the name %s and will be used instead.",
+                name,
+                registered_name,
+            )
+
     def create_counter(self, name, unit="", description="") -> APICounter:
         with self._instrument_registration_lock:
             status = self._register_instrument(name, _Counter, unit, description)
@@ -130,6 +140,8 @@ def create_counter(self, name, unit="", description="") -> APICounter:
                 )
             instrument = self._instrument_id_instrument[status.instrument_id]
 
+        self._log_instrument_name_conflict(name, instrument.name)
+
         if status.conflict:
             # FIXME #2558 go through all views here and check if this
             # instrument registration conflict can be fixed. If it can be, do
@@ -157,6 +169,8 @@ def create_up_down_counter(self, name, unit="", description="") -> APIUpDownCoun
                 )
             instrument = self._instrument_id_instrument[status.instrument_id]
 
+        self._log_instrument_name_conflict(name, instrument.name)
+
         if status.conflict:
             # FIXME #2558 go through all views here and check if this
             # instrument registration conflict can be fixed. If it can be, do
@@ -194,6 +208,8 @@ def create_observable_counter(
         if not status.already_registered:
             self._measurement_consumer.register_asynchronous_instrument(instrument)
 
+        self._log_instrument_name_conflict(name, instrument.name)
+
         if status.conflict:
             # FIXME #2558 go through all views here and check if this
             # instrument registration conflict can be fixed. If it can be, do
@@ -251,6 +267,8 @@ def create_histogram(
                 )
             instrument = self._instrument_id_instrument[status.instrument_id]
 
+        self._log_instrument_name_conflict(name, instrument.name)
+
         if status.conflict:
             # FIXME #2558 go through all views here and check if this
             # instrument registration conflict can be fixed. If it can be, do
@@ -278,6 +296,8 @@ def create_gauge(self, name, unit="", description="") -> APIGauge:
                 )
             instrument = self._instrument_id_instrument[status.instrument_id]
 
+        self._log_instrument_name_conflict(name, instrument.name)
+
         if status.conflict:
             # FIXME #2558 go through all views here and check if this
             # instrument registration conflict can be fixed. If it can be, do
@@ -309,6 +329,8 @@ def create_observable_gauge(self, name, callbacks=None, unit="", description="")
         if not status.already_registered:
             self._measurement_consumer.register_asynchronous_instrument(instrument)
 
+        self._log_instrument_name_conflict(name, instrument.name)
+
         if status.conflict:
             # FIXME #2558 go through all views here and check if this
             # instrument registration conflict can be fixed. If it can be, do
@@ -342,6 +364,8 @@ def create_observable_up_down_counter(
         if not status.already_registered:
             self._measurement_consumer.register_asynchronous_instrument(instrument)
 
+        self._log_instrument_name_conflict(name, instrument.name)
+
         if status.conflict:
             # FIXME #2558 go through all views here and check if this
             # instrument registration conflict can be fixed. If it can be, do

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py

@@ -111,7 +111,7 @@ def conflicts(self, other: "_ViewInstrumentMatch") -> bool:
         # pylint: disable=protected-access
 
         result = (
-            self._name == other._name
+            self._name.lower() == other._name.lower()
             and self._instrument.unit == other._instrument.unit
             # The aggregation class is being used here instead of data point
             # type since they are functionally equivalent.

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py

@@ -85,7 +85,7 @@ def __init__(
         unit = result["unit"]
         description = cast(str, result["description"])
 
-        self.name = name.lower()
+        self.name = name
         self.unit = unit
         self.description = description
         self.instrumentation_scope = instrumentation_scope
@@ -124,7 +124,7 @@ def __init__(
         unit = result["unit"]
         description = cast(str, result["description"])
 
-        self.name = name.lower()
+        self.name = name
         self.unit = unit
         self.description = description
         self.instrumentation_scope = instrumentation_scope

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py

@@ -174,11 +174,11 @@ def _match(self, instrument: _Instrument) -> bool:
                 return False
 
         if self._instrument_name is not None:
-            # Instrument names are treated case-insensitively and are stored
-            # lower-cased by the SDK, so the pattern is lower-cased to match.
+            # Instrument names are treated case-insensitively while preserving
+            # their original casing, so both operands are 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()):
+            if not fnmatchcase(instrument.name.lower(), self._instrument_name.lower()):
                 return False
 
         if self._instrument_unit is not None:

opentelemetry-sdk/tests/metrics/integration_test/test_instrument_name.py

@@ -0,0 +1,86 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+from unittest import TestCase
+
+from opentelemetry.metrics import Observation
+from opentelemetry.sdk.metrics import MeterProvider
+from opentelemetry.sdk.metrics.export import InMemoryMetricReader
+from opentelemetry.sdk.metrics.view import View
+
+
+class TestInstrumentNameCase(TestCase):
+    _INSTRUMENTS = (
+        ("counter", "add"),
+        ("up_down_counter", "add"),
+        ("histogram", "record"),
+        ("gauge", "set"),
+        ("observable_counter", None),
+        ("observable_up_down_counter", None),
+        ("observable_gauge", None),
+    )
+
+    def _setup_meter(self, views=()):
+        reader = InMemoryMetricReader()
+        provider = MeterProvider(metric_readers=[reader], views=views)
+        self.addCleanup(provider.shutdown)
+        return provider.get_meter("testmeter"), reader
+
+    @staticmethod
+    def _create_instrument(meter, kind, name):
+        factory = getattr(meter, f"create_{kind}")
+        if kind.startswith("observable_"):
+            return factory(name, callbacks=[lambda options: [Observation(7)]])
+        return factory(name)
+
+    def test_collected_metric_names_preserve_case(self):
+        for kind, record in self._INSTRUMENTS:
+            with self.subTest(kind=kind):
+                meter, reader = self._setup_meter()
+                instrument = self._create_instrument(meter, kind, "RequestCount")
+                if record is not None:
+                    getattr(instrument, record)(7)
+                metrics = reader.get_metrics_data().resource_metrics[0].scope_metrics[0].metrics
+                self.assertEqual(len(metrics), 1)
+                self.assertEqual(metrics[0].name, "RequestCount")
+
+    def test_repeated_name_keeps_first_case(self):
+        for kind, record in self._INSTRUMENTS:
+            with self.subTest(kind=kind):
+                meter, reader = self._setup_meter()
+                first = self._create_instrument(meter, kind, "RequestCount")
+                with self.assertNoLogs("opentelemetry.sdk.metrics._internal", level="WARNING"):
+                    self.assertIs(self._create_instrument(meter, kind, "RequestCount"), first)
+                with self.assertLogs("opentelemetry.sdk.metrics._internal", level="WARNING") as logs:
+                    duplicate = self._create_instrument(meter, kind, "requestcount")
+                self.assertIs(duplicate, first)
+                if record is not None:
+                    getattr(duplicate, record)(7)
+                metric = reader.get_metrics_data().resource_metrics[0].scope_metrics[0].metrics[0]
+                self.assertEqual(metric.name, "RequestCount")
+                self.assertIn("RequestCount", logs.output[0])
+                self.assertIn("requestcount", logs.output[0])
+
+    def test_view_matches_instrument_name_case_insensitively(self):
+        for kind, record in (("counter", "add"), ("observable_gauge", None)):
+            for pattern in ("requestcount", "REQUESTCOUNT", "request*", "REQUEST*"):
+                with self.subTest(kind=kind, pattern=pattern):
+                    meter, reader = self._setup_meter(views=[View(instrument_name=pattern, description="matched")])
+                    instrument = self._create_instrument(meter, kind, "RequestCount")
+                    if record is not None:
+                        getattr(instrument, record)(7)
+                    metric = reader.get_metrics_data().resource_metrics[0].scope_metrics[0].metrics[0]
+                    self.assertEqual(metric.description, "matched")
+                    self.assertEqual(metric.name, "RequestCount")
+
+    def test_view_rename_preserves_configured_case(self):
+        meter, reader = self._setup_meter(views=[View(instrument_name="requestcount", name="ExportedCount")])
+        meter.create_counter("RequestCount").add(7)
+        metric = reader.get_metrics_data().resource_metrics[0].scope_metrics[0].metrics[0]
+        self.assertEqual(metric.name, "ExportedCount")
+
+    def test_conflict_warning_remains_case_insensitive(self):
+        meter, _ = self._setup_meter(views=[View(instrument_name="*")])
+        meter.create_counter("RequestCount", description="first").add(1)
+        with self.assertLogs("opentelemetry.sdk.metrics._internal.metric_reader_storage", level="WARNING"):
+            meter.create_counter("requestcount", description="second").add(1)

opentelemetry-sdk/tests/metrics/test_instrument.py

@@ -36,7 +36,7 @@
 class TestCounter(TestCase):
     def testname(self):
         self.assertEqual(_Counter("name", Mock(), Mock()).name, "name")
-        self.assertEqual(_Counter("Name", Mock(), Mock()).name, "name")
+        self.assertEqual(_Counter("Name", Mock(), Mock()).name, "Name")
 
     def test_add(self):
         mc = Mock()
@@ -154,7 +154,7 @@ def generator_callback_1():
 class TestObservableGauge(TestCase):
     def testname(self):
         self.assertEqual(_ObservableGauge("name", Mock(), Mock()).name, "name")
-        self.assertEqual(_ObservableGauge("Name", Mock(), Mock()).name, "name")
+        self.assertEqual(_ObservableGauge("Name", Mock(), Mock()).name, "Name")
 
     def test_callable_callback_0(self):
         observable_gauge = _ObservableGauge("name", Mock(), Mock(), [callable_callback_0])
@@ -428,7 +428,7 @@ def test_disallow_direct_observable_counter_creation(self):
 class TestGauge(TestCase):
     def testname(self):
         self.assertEqual(_Gauge("name", Mock(), Mock()).name, "name")
-        self.assertEqual(_Gauge("Name", Mock(), Mock()).name, "name")
+        self.assertEqual(_Gauge("Name", Mock(), Mock()).name, "Name")
 
     def test_set(self):
         mc = Mock()

opentelemetry-sdk/tests/metrics/test_view.py

@@ -35,12 +35,12 @@ def test_instrument_name(self):
         self.assertTrue(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).
+        # A view must match regardless of the instrument name's preserved case
+        # and regardless of the host platform.
         mock_instrument = Mock()
-        mock_instrument.configure_mock(name="instrument_name")
+        mock_instrument.configure_mock(name="Instrument_Name")
 
+        self.assertTrue(View(instrument_name="instrument_name")._match(mock_instrument))
         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))