Make measurement a concrete class (open-telemetry#2153)

* Make Measurement a concrete class

* comments

* update changelog

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2182](https://github.com/open-telemetry/opentelemetry-python/pull/2182))
 - Automatically load OTEL environment variables as options for `opentelemetry-instrument`
   ([#1969](https://github.com/open-telemetry/opentelemetry-python/pull/1969))
+- Make Measurement a concrete class
+  ([#2153](https://github.com/open-telemetry/opentelemetry-python/pull/2153))
 - Add metrics API
   ([#1887](https://github.com/open-telemetry/opentelemetry-python/pull/1887))
 - `opentelemetry-semantic-conventions` Update to semantic conventions v1.6.1

opentelemetry-api/src/opentelemetry/metrics/measurement.py

@@ -12,28 +12,41 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# pylint: disable=too-many-ancestors
-# type:ignore
+from typing import Union
 
+from opentelemetry.util.types import Attributes
 
-from abc import ABC, abstractmethod
 
+class Measurement:
+    """A measurement observed in an asynchronous instrument
+
+    Return/yield instances of this class from asynchronous instrument callbacks.
+
+    Args:
+        value: The float or int measured value
+        attributes: The measurement's attributes
+    """
+
+    def __init__(
+        self, value: Union[int, float], attributes: Attributes = None
+    ) -> None:
+        self._value = value
+        self._attributes = attributes
 
-class Measurement(ABC):
     @property
-    def value(self):
+    def value(self) -> Union[float, int]:
         return self._value
 
     @property
-    def attributes(self):
+    def attributes(self) -> Attributes:
         return self._attributes
 
-    @abstractmethod
-    def __init__(self, value, attributes=None):
-        self._value = value
-        self._attributes = attributes
-
+    def __eq__(self, other: object) -> bool:
+        return (
+            isinstance(other, Measurement)
+            and self.value == other.value
+            and self.attributes == other.attributes
+        )
 
-class DefaultMeasurement(Measurement):
-    def __init__(self, value, attributes=None):
-        super().__init__(value, attributes=attributes)
+    def __repr__(self) -> str:
+        return f"Measurement(value={self.value}, attributes={self.attributes})"

opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py

@@ -23,14 +23,6 @@
 # FIXME Test that the instrument methods can be called concurrently safely.
 
 
-class ChildMeasurement(Measurement):
-    def __init__(self, value, attributes=None):
-        super().__init__(value, attributes=attributes)
-
-    def __eq__(self, o: Measurement) -> bool:
-        return self.value == o.value and self.attributes == o.attributes
-
-
 class TestCpuTimeIntegration(TestCase):
     """Integration test of scraping CPU time from proc stat with an observable
     counter"""
@@ -48,24 +40,24 @@ class TestCpuTimeIntegration(TestCase):
 softirq 1644603067 0 166540056 208 309152755 8936439 0 1354908 935642970 13 222975718\n"""
 
     measurements_expected = [
-        ChildMeasurement(6150, {"cpu": "cpu0", "state": "user"}),
-        ChildMeasurement(3177, {"cpu": "cpu0", "state": "nice"}),
-        ChildMeasurement(5946, {"cpu": "cpu0", "state": "system"}),
-        ChildMeasurement(891264, {"cpu": "cpu0", "state": "idle"}),
-        ChildMeasurement(1296, {"cpu": "cpu0", "state": "iowait"}),
-        ChildMeasurement(0, {"cpu": "cpu0", "state": "irq"}),
-        ChildMeasurement(8343, {"cpu": "cpu0", "state": "softirq"}),
-        ChildMeasurement(421, {"cpu": "cpu0", "state": "guest"}),
-        ChildMeasurement(0, {"cpu": "cpu0", "state": "guest_nice"}),
-        ChildMeasurement(5882, {"cpu": "cpu1", "state": "user"}),
-        ChildMeasurement(3491, {"cpu": "cpu1", "state": "nice"}),
-        ChildMeasurement(6404, {"cpu": "cpu1", "state": "system"}),
-        ChildMeasurement(891564, {"cpu": "cpu1", "state": "idle"}),
-        ChildMeasurement(1244, {"cpu": "cpu1", "state": "iowait"}),
-        ChildMeasurement(0, {"cpu": "cpu1", "state": "irq"}),
-        ChildMeasurement(2410, {"cpu": "cpu1", "state": "softirq"}),
-        ChildMeasurement(418, {"cpu": "cpu1", "state": "guest"}),
-        ChildMeasurement(0, {"cpu": "cpu1", "state": "guest_nice"}),
+        Measurement(6150, {"cpu": "cpu0", "state": "user"}),
+        Measurement(3177, {"cpu": "cpu0", "state": "nice"}),
+        Measurement(5946, {"cpu": "cpu0", "state": "system"}),
+        Measurement(891264, {"cpu": "cpu0", "state": "idle"}),
+        Measurement(1296, {"cpu": "cpu0", "state": "iowait"}),
+        Measurement(0, {"cpu": "cpu0", "state": "irq"}),
+        Measurement(8343, {"cpu": "cpu0", "state": "softirq"}),
+        Measurement(421, {"cpu": "cpu0", "state": "guest"}),
+        Measurement(0, {"cpu": "cpu0", "state": "guest_nice"}),
+        Measurement(5882, {"cpu": "cpu1", "state": "user"}),
+        Measurement(3491, {"cpu": "cpu1", "state": "nice"}),
+        Measurement(6404, {"cpu": "cpu1", "state": "system"}),
+        Measurement(891564, {"cpu": "cpu1", "state": "idle"}),
+        Measurement(1244, {"cpu": "cpu1", "state": "iowait"}),
+        Measurement(0, {"cpu": "cpu1", "state": "irq"}),
+        Measurement(2410, {"cpu": "cpu1", "state": "softirq"}),
+        Measurement(418, {"cpu": "cpu1", "state": "guest"}),
+        Measurement(0, {"cpu": "cpu1", "state": "guest_nice"}),
     ]
 
     def test_cpu_time_callback(self):
@@ -78,31 +70,31 @@ def cpu_time_callback() -> Iterable[Measurement]:
                 if not line.startswith("cpu"):
                     break
                 cpu, *states = line.split()
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[0]) // 100, {"cpu": cpu, "state": "user"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[1]) // 100, {"cpu": cpu, "state": "nice"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[2]) // 100, {"cpu": cpu, "state": "system"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[3]) // 100, {"cpu": cpu, "state": "idle"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[4]) // 100, {"cpu": cpu, "state": "iowait"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[5]) // 100, {"cpu": cpu, "state": "irq"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[6]) // 100, {"cpu": cpu, "state": "softirq"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[7]) // 100, {"cpu": cpu, "state": "guest"}
                 )
-                yield ChildMeasurement(
+                yield Measurement(
                     int(states[8]) // 100, {"cpu": cpu, "state": "guest_nice"}
                 )
 
@@ -130,54 +122,54 @@ def cpu_time_generator() -> Generator[
                         break
                     cpu, *states = line.split()
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[0]) // 100,
                             {"cpu": cpu, "state": "user"},
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[1]) // 100,
                             {"cpu": cpu, "state": "nice"},
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[2]) // 100,
                             {"cpu": cpu, "state": "system"},
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[3]) // 100,
                             {"cpu": cpu, "state": "idle"},
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[4]) // 100,
                             {"cpu": cpu, "state": "iowait"},
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[5]) // 100, {"cpu": cpu, "state": "irq"}
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[6]) // 100,
                             {"cpu": cpu, "state": "softirq"},
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[7]) // 100,
                             {"cpu": cpu, "state": "guest"},
                         )
                     )
                     measurements.append(
-                        ChildMeasurement(
+                        Measurement(
                             int(states[8]) // 100,
                             {"cpu": cpu, "state": "guest_nice"},
                         )

opentelemetry-api/tests/metrics/test_instruments.py

@@ -45,11 +45,6 @@ def __init__(self, name, *args, unit="", description="", **kwargs):
         )
 
 
-class ChildMeasurement(Measurement):
-    def __init__(self, value, attributes=None):
-        super().__init__(value, attributes=attributes)
-
-
 class TestInstrument(TestCase):
     def test_instrument_has_name(self):
         """
@@ -341,8 +336,8 @@ def callback():
             list(observable_counter.callback())
 
         def callback():
-            yield [ChildMeasurement(1), ChildMeasurement(2)]
-            yield [ChildMeasurement(-1)]
+            yield [Measurement(1), Measurement(2)]
+            yield [Measurement(-1)]
 
         observable_counter = DefaultObservableCounter("name", callback())
 
@@ -382,7 +377,7 @@ def callback_invalid_return():
             list(observable_counter.callback())
 
         def callback_valid():
-            return [ChildMeasurement(1), ChildMeasurement(2)]
+            return [Measurement(1), Measurement(2)]
 
         observable_counter = DefaultObservableCounter("name", callback_valid)
 
@@ -391,7 +386,7 @@ def callback_valid():
                 list(observable_counter.callback())
 
         def callback_one_invalid():
-            return [ChildMeasurement(1), ChildMeasurement(-2)]
+            return [Measurement(1), Measurement(-2)]
 
         observable_counter = DefaultObservableCounter(
             "name", callback_one_invalid
@@ -578,7 +573,7 @@ def callback():
             list(observable_gauge.callback())
 
         def callback():
-            yield [ChildMeasurement(1), ChildMeasurement(-1)]
+            yield [Measurement(1), Measurement(-1)]
 
         observable_gauge = DefaultObservableGauge("name", callback())
         with self.assertRaises(AssertionError):
@@ -786,8 +781,8 @@ def test_observable_up_down_counter_callback(self):
         )
 
         def callback():
-            yield ChildMeasurement(1)
-            yield ChildMeasurement(-1)
+            yield Measurement(1)
+            yield Measurement(-1)
 
         with self.assertRaises(AssertionError):
             with self.assertLogs(level=ERROR):

opentelemetry-api/tests/metrics/test_measurement.py

@@ -0,0 +1,46 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from unittest import TestCase
+
+from opentelemetry.metrics.measurement import Measurement
+
+
+class TestMeasurement(TestCase):
+    def test_measurement_init(self):
+        try:
+            # int
+            Measurement(321, {"hello": "world"})
+
+            # float
+            Measurement(321.321, {"hello": "world"})
+        except Exception:  # pylint: disable=broad-except
+            self.fail(
+                "Unexpected exception raised when instantiating Measurement"
+            )
+
+    def test_measurement_equality(self):
+        self.assertEqual(
+            Measurement(321, {"hello": "world"}),
+            Measurement(321, {"hello": "world"}),
+        )
+
+        self.assertNotEqual(
+            Measurement(321, {"hello": "world"}),
+            Measurement(321.321, {"hello": "world"}),
+        )
+        self.assertNotEqual(
+            Measurement(321, {"baz": "world"}),
+            Measurement(321, {"hello": "world"}),
+        )