Add check for duplicate instrument names

opentelemetry-api/src/opentelemetry/_metrics/__init__.py

@@ -26,7 +26,7 @@
 from logging import getLogger
 from os import environ
 from threading import Lock
-from typing import List, Optional, cast
+from typing import List, Optional, Set, cast
 
 from opentelemetry._metrics.instrument import (
     Counter,
@@ -118,7 +118,8 @@ def __init__(self, name, version=None, schema_url=None):
         self._name = name
         self._version = version
         self._schema_url = schema_url
-        self._instrument_names = set()
+        self._instrument_ids: Set[str] = set()
+        self._instrument_ids_lock = Lock()
 
     @property
     def name(self):
@@ -132,7 +133,27 @@ def version(self):
     def schema_url(self):
         return self._schema_url
 
-    # FIXME check that the instrument name has not been used already
+    def _check_instrument_id(
+        self, name: str, type_: type, unit: str, description: str
+    ) -> bool:
+        """
+        Check if an instrument with the same name, type, unit and description
+        has been registered already.
+        """
+
+        result = False
+
+        instrument_id = ",".join(
+            [name.strip().lower(), type_.__name__, unit, description]
+        )
+
+        with self._instrument_ids_lock:
+            if instrument_id in self._instrument_ids:
+                result = True
+            else:
+                self._instrument_ids.add(instrument_id)
+
+        return result
 
     @abstractmethod
     def create_counter(self, name, unit="", description="") -> Counter:
@@ -401,6 +422,15 @@ class NoOpMeter(Meter):
     def create_counter(self, name, unit="", description="") -> Counter:
         """Returns a no-op Counter."""
         super().create_counter(name, unit=unit, description=description)
+        if self._check_instrument_id(name, DefaultCounter, unit, description):
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                Counter.__name__,
+                unit,
+                description,
+            )
         return DefaultCounter(name, unit=unit, description=description)
 
     def create_up_down_counter(
@@ -410,6 +440,17 @@ def create_up_down_counter(
         super().create_up_down_counter(
             name, unit=unit, description=description
         )
+        if self._check_instrument_id(
+            name, DefaultUpDownCounter, unit, description
+        ):
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                UpDownCounter.__name__,
+                unit,
+                description,
+            )
         return DefaultUpDownCounter(name, unit=unit, description=description)
 
     def create_observable_counter(
@@ -419,6 +460,17 @@ def create_observable_counter(
         super().create_observable_counter(
             name, callback, unit=unit, description=description
         )
+        if self._check_instrument_id(
+            name, DefaultObservableCounter, unit, description
+        ):
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                ObservableCounter.__name__,
+                unit,
+                description,
+            )
         return DefaultObservableCounter(
             name,
             callback,
@@ -429,6 +481,17 @@ def create_observable_counter(
     def create_histogram(self, name, unit="", description="") -> Histogram:
         """Returns a no-op Histogram."""
         super().create_histogram(name, unit=unit, description=description)
+        if self._check_instrument_id(
+            name, DefaultHistogram, unit, description
+        ):
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                Histogram.__name__,
+                unit,
+                description,
+            )
         return DefaultHistogram(name, unit=unit, description=description)
 
     def create_observable_gauge(
@@ -438,6 +501,17 @@ def create_observable_gauge(
         super().create_observable_gauge(
             name, callback, unit=unit, description=description
         )
+        if self._check_instrument_id(
+            name, DefaultObservableGauge, unit, description
+        ):
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                ObservableGauge.__name__,
+                unit,
+                description,
+            )
         return DefaultObservableGauge(
             name,
             callback,
@@ -452,6 +526,17 @@ def create_observable_up_down_counter(
         super().create_observable_up_down_counter(
             name, callback, unit=unit, description=description
         )
+        if self._check_instrument_id(
+            name, DefaultObservableUpDownCounter, unit, description
+        ):
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                ObservableUpDownCounter.__name__,
+                unit,
+                description,
+            )
         return DefaultObservableUpDownCounter(
             name,
             callback,

opentelemetry-api/tests/metrics/test_meter.py

@@ -13,9 +13,11 @@
 # limitations under the License.
 # type: ignore
 
+from logging import WARNING
 from unittest import TestCase
+from unittest.mock import Mock
 
-from opentelemetry._metrics import Meter
+from opentelemetry._metrics import Meter, NoOpMeter
 
 # FIXME Test that the meter methods can be called concurrently safely.
 
@@ -53,6 +55,42 @@ def create_observable_up_down_counter(
 
 
 class TestMeter(TestCase):
+    def test_repeated_instrument_names(self):
+
+        try:
+            test_meter = NoOpMeter("name")
+
+            test_meter.create_counter("counter")
+            test_meter.create_up_down_counter("up_down_counter")
+            test_meter.create_observable_counter("observable_counter", Mock())
+            test_meter.create_histogram("histogram")
+            test_meter.create_observable_gauge("observable_gauge", Mock())
+            test_meter.create_observable_up_down_counter(
+                "observable_up_down_counter", Mock()
+            )
+        except Exception as error:
+            self.fail(f"Unexpected exception raised {error}")
+
+        for instrument_name in [
+            "counter",
+            "up_down_counter",
+            "histogram",
+        ]:
+            with self.assertLogs(level=WARNING):
+                getattr(test_meter, f"create_{instrument_name}")(
+                    instrument_name
+                )
+
+        for instrument_name in [
+            "observable_counter",
+            "observable_gauge",
+            "observable_up_down_counter",
+        ]:
+            with self.assertLogs(level=WARNING):
+                getattr(test_meter, f"create_{instrument_name}")(
+                    instrument_name, Mock()
+                )
+
     def test_create_counter(self):
         """
         Test that the meter provides a function to create a new Counter

opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py

@@ -64,7 +64,20 @@ def __init__(
         self._instrumentation_info = instrumentation_info
         self._measurement_consumer = measurement_consumer
 
-    def create_counter(self, name, unit=None, description=None) -> APICounter:
+    def create_counter(self, name, unit="", description="") -> APICounter:
+        if self._check_instrument_id(name, Counter, unit, description):
+            # FIXME #2558 go through all views here and check if this
+            # instrument registration conflict can be fixed. If it can be, do
+            # not log the following warning.
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                APICounter.__name__,
+                unit,
+                description,
+            )
+
         return Counter(
             name,
             self._instrumentation_info,
@@ -74,8 +87,21 @@ def create_counter(self, name, unit=None, description=None) -> APICounter:
         )
 
     def create_up_down_counter(
-        self, name, unit=None, description=None
+        self, name, unit="", description=""
     ) -> APIUpDownCounter:
+        if self._check_instrument_id(name, UpDownCounter, unit, description):
+            # FIXME #2558 go through all views here and check if this
+            # instrument registration conflict can be fixed. If it can be, do
+            # not log the following warning.
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                APIUpDownCounter.__name__,
+                unit,
+                description,
+            )
+
         return UpDownCounter(
             name,
             self._instrumentation_info,
@@ -85,9 +111,22 @@ def create_up_down_counter(
         )
 
     def create_observable_counter(
-        self, name, callback, unit=None, description=None
+        self, name, callback, unit="", description=""
     ) -> APIObservableCounter:
-
+        if self._check_instrument_id(
+            name, ObservableCounter, unit, description
+        ):
+            # FIXME #2558 go through all views here and check if this
+            # instrument registration conflict can be fixed. If it can be, do
+            # not log the following warning.
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                APIObservableCounter.__name__,
+                unit,
+                description,
+            )
         instrument = ObservableCounter(
             name,
             self._instrumentation_info,
@@ -101,9 +140,19 @@ def create_observable_counter(
 
         return instrument
 
-    def create_histogram(
-        self, name, unit=None, description=None
-    ) -> APIHistogram:
+    def create_histogram(self, name, unit="", description="") -> APIHistogram:
+        if self._check_instrument_id(name, Histogram, unit, description):
+            # FIXME #2558 go through all views here and check if this
+            # instrument registration conflict can be fixed. If it can be, do
+            # not log the following warning.
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                APIHistogram.__name__,
+                unit,
+                description,
+            )
         return Histogram(
             name,
             self._instrumentation_info,
@@ -113,8 +162,20 @@ def create_histogram(
         )
 
     def create_observable_gauge(
-        self, name, callback, unit=None, description=None
+        self, name, callback, unit="", description=""
     ) -> APIObservableGauge:
+        if self._check_instrument_id(name, ObservableGauge, unit, description):
+            # FIXME #2558 go through all views here and check if this
+            # instrument registration conflict can be fixed. If it can be, do
+            # not log the following warning.
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                APIObservableGauge.__name__,
+                unit,
+                description,
+            )
 
         instrument = ObservableGauge(
             name,
@@ -130,8 +191,22 @@ def create_observable_gauge(
         return instrument
 
     def create_observable_up_down_counter(
-        self, name, callback, unit=None, description=None
+        self, name, callback, unit="", description=""
     ) -> APIObservableUpDownCounter:
+        if self._check_instrument_id(
+            name, ObservableUpDownCounter, unit, description
+        ):
+            # FIXME #2558 go through all views here and check if this
+            # instrument registration conflict can be fixed. If it can be, do
+            # not log the following warning.
+            _logger.warning(
+                "An instrument with name %s, type %s, unit %s and "
+                "description %s has been created already.",
+                name,
+                APIObservableUpDownCounter.__name__,
+                unit,
+                description,
+            )
 
         instrument = ObservableUpDownCounter(
             name,
@@ -280,6 +355,8 @@ def get_meter(
         info = InstrumentationInfo(name, version, schema_url)
         with self._meter_lock:
             if not self._meters.get(info):
+                # FIXME #2558 pass SDKConfig object to meter so that the meter
+                # has access to views.
                 self._meters[info] = Meter(
                     info,
                     self._measurement_consumer,