Throw an error when multiple instruments are registered by the same name

.gitignore

@@ -16,7 +16,9 @@ var
 sdist
 develop-eggs
 .installed.cfg
+pyvenv.cfg
 lib
+share/
 lib64
 __pycache__
 venv*/

exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py

@@ -91,13 +91,13 @@ def test_get_collector_metric_type(self):
     def test_get_collector_point(self):
         aggregator = aggregate.SumAggregator()
         int_counter = self._meter.create_counter(
-            "testName", "testDescription", "unit", int,
+            "testNameIntCounter", "testDescription", "unit", int,
         )
         float_counter = self._meter.create_counter(
-            "testName", "testDescription", "unit", float,
+            "testNameFloatCounter", "testDescription", "unit", float,
         )
         valuerecorder = self._meter.create_valuerecorder(
-            "testName", "testDescription", "unit", float,
+            "testNameValueRecorder", "testDescription", "unit", float,
         )
         result = metrics_exporter.get_collector_point(
             ExportRecord(
@@ -168,7 +168,7 @@ def test_export(self):
 
     def test_translate_to_collector(self):
         test_metric = self._meter.create_counter(
-            "testname", "testdesc", "unit", int, self._labels.keys()
+            "testcollector", "testdesc", "unit", int, self._labels.keys()
         )
         aggregator = aggregate.SumAggregator()
         aggregator.update(123)
@@ -185,7 +185,9 @@ def test_translate_to_collector(self):
         )
         self.assertEqual(len(output_metrics), 1)
         self.assertIsInstance(output_metrics[0], metrics_pb2.Metric)
-        self.assertEqual(output_metrics[0].metric_descriptor.name, "testname")
+        self.assertEqual(
+            output_metrics[0].metric_descriptor.name, "testcollector"
+        )
         self.assertEqual(
             output_metrics[0].metric_descriptor.description, "testdesc"
         )

opentelemetry-instrumentation/tests/test_metric.py

@@ -72,7 +72,7 @@ def test_ctor(self):
             "measures the duration of the outbound HTTP request",
         )
 
-    def test_ctor_types(self):
+    def test_ctor_type_client(self):
         meter = metrics_api.get_meter(__name__)
         recorder = HTTPMetricRecorder(meter, HTTPMetricType.CLIENT)
         self.assertEqual(recorder._http_type, HTTPMetricType.CLIENT)
@@ -81,13 +81,17 @@ def test_ctor_types(self):
         )
         self.assertIsNone(recorder._server_duration)
 
+    def test_ctor_type_server(self):
+        meter = metrics_api.get_meter(__name__)
         recorder = HTTPMetricRecorder(meter, HTTPMetricType.SERVER)
         self.assertEqual(recorder._http_type, HTTPMetricType.SERVER)
         self.assertTrue(
             isinstance(recorder._server_duration, metrics.ValueRecorder)
         )
         self.assertIsNone(recorder._client_duration)
 
+    def test_ctor_type_both(self):
+        meter = metrics_api.get_meter(__name__)
         recorder = HTTPMetricRecorder(meter, HTTPMetricType.BOTH)
         self.assertEqual(recorder._http_type, HTTPMetricType.BOTH)
         self.assertTrue(

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

@@ -356,12 +356,34 @@ def __init__(
     ):
         self.instrumentation_info = instrumentation_info
         self.processor = Processor(source.stateful, source.resource)
-        self.metrics = set()
-        self.observers = set()
+        self.metrics = {}
+        self.observers = {}
         self.metrics_lock = threading.Lock()
         self.observers_lock = threading.Lock()
         self.view_manager = ViewManager()
 
+    def _register_metric(self, instrument: metrics_api.Metric):
+        name = instrument.name.strip().lower()
+        with self.metrics_lock:
+            if name in self.metrics:
+                raise ValueError(
+                    "Multiple instruments can't be registered by the same name: ({})".format(
+                        name
+                    )
+                )
+            self.metrics[name] = instrument
+
+    def _register_observer(self, instrument: metrics_api.Observer):
+        name = instrument.name.strip().lower()
+        with self.observers_lock:
+            if name in self.observers:
+                raise ValueError(
+                    "Multiple instruments can't be registered by the same name: ({})".format(
+                        name
+                    )
+                )
+            self.observers[name] = instrument
+
     def collect(self) -> None:
         """Collects all the metrics created with this `Meter` for export.
 
@@ -374,7 +396,7 @@ def collect(self) -> None:
         self._collect_observers()
 
     def _collect_metrics(self) -> None:
-        for metric in self.metrics:
+        for metric in self.metrics.values():
             if not metric.enabled:
                 continue
             to_remove = []
@@ -398,7 +420,7 @@ def _collect_metrics(self) -> None:
 
     def _collect_observers(self) -> None:
         with self.observers_lock:
-            for observer in self.observers:
+            for observer in self.observers.values():
                 if not observer.enabled:
                     continue
 
@@ -432,8 +454,7 @@ def create_counter(
         counter = Counter(
             name, description, unit, value_type, self, enabled=enabled
         )
-        with self.metrics_lock:
-            self.metrics.add(counter)
+        self._register_metric(counter)
         return counter
 
     def create_updowncounter(
@@ -448,8 +469,7 @@ def create_updowncounter(
         counter = UpDownCounter(
             name, description, unit, value_type, self, enabled=enabled
         )
-        with self.metrics_lock:
-            self.metrics.add(counter)
+        self._register_metric(counter)
         return counter
 
     def create_valuerecorder(
@@ -464,8 +484,7 @@ def create_valuerecorder(
         recorder = ValueRecorder(
             name, description, unit, value_type, self, enabled=enabled
         )
-        with self.metrics_lock:
-            self.metrics.add(recorder)
+        self._register_metric(recorder)
         return recorder
 
     def register_sumobserver(
@@ -488,8 +507,7 @@ def register_sumobserver(
             label_keys,
             enabled,
         )
-        with self.observers_lock:
-            self.observers.add(ob)
+        self._register_observer(ob)
         return ob
 
     def register_updownsumobserver(
@@ -512,8 +530,7 @@ def register_updownsumobserver(
             label_keys,
             enabled,
         )
-        with self.observers_lock:
-            self.observers.add(ob)
+        self._register_observer(ob)
         return ob
 
     def register_valueobserver(
@@ -536,13 +553,13 @@ def register_valueobserver(
             label_keys,
             enabled,
         )
-        with self.observers_lock:
-            self.observers.add(ob)
+        self._register_observer(ob)
         return ob
 
     def unregister_observer(self, observer: metrics_api.Observer) -> None:
+        name = observer.name.strip().lower()
         with self.observers_lock:
-            self.observers.remove(observer)
+            self.observers.pop(name)
 
     def register_view(self, view):
         self.view_manager.register_view(view)

opentelemetry-sdk/tests/metrics/test_metrics.py

@@ -118,11 +118,9 @@ def callback(observer):
             self.assertIsInstance(observer, metrics_api.Observer)
             observer.observe(45, {})
 
-        observer = metrics.ValueObserver(
+        meter.register_valueobserver(
             callback, "name", "desc", "unit", int, (), True
         )
-
-        meter.observers.add(observer)
         meter.collect()
         self.assertTrue(processor_mock.process.called)
 
@@ -164,6 +162,23 @@ def test_create_counter(self):
         self.assertIs(meter_provider.resource, resource)
         self.assertEqual(counter.meter, meter)
 
+    def test_instrument_same_name_error(self):
+        resource = Mock(spec=resources.Resource)
+        meter_provider = metrics.MeterProvider(resource=resource)
+        meter = meter_provider.get_meter(__name__)
+        counter = meter.create_counter("name", "desc", "unit", int,)
+        self.assertIsInstance(counter, metrics.Counter)
+        self.assertEqual(counter.value_type, int)
+        self.assertEqual(counter.name, "name")
+        self.assertIs(meter_provider.resource, resource)
+        self.assertEqual(counter.meter, meter)
+        with self.assertRaises(ValueError) as ctx:
+            _ = meter.create_counter("naME", "desc", "unit", int,)
+        self.assertTrue(
+            "Multiple instruments can't be registered by the same name: (name)"
+            in str(ctx.exception)
+        )
+
     def test_create_updowncounter(self):
         meter = metrics.MeterProvider().get_meter(__name__)
         updowncounter = meter.create_updowncounter(
@@ -255,6 +270,26 @@ def test_unregister_observer(self):
         meter.unregister_observer(observer)
         self.assertEqual(len(meter.observers), 0)
 
+    def test_unregister_and_reregister_observer(self):
+        meter = metrics.MeterProvider().get_meter(__name__)
+
+        callback = Mock()
+
+        observer = meter.register_valueobserver(
+            callback,
+            "nameCaSEinSENsitive",
+            "desc",
+            "unit",
+            int,
+            metrics.ValueObserver,
+        )
+
+        meter.unregister_observer(observer)
+        self.assertEqual(len(meter.observers), 0)
+        observer = meter.register_valueobserver(
+            callback, "name", "desc", "unit", int, metrics.ValueObserver
+        )
+
 
 class TestMetric(unittest.TestCase):
     def test_bind(self):