Implement direct calling convention of metric instruments (#224)

Author: lzchen

docs/conf.py

@@ -52,7 +52,10 @@
 # http://www.sphinx-doc.org/en/master/config.html#confval-nitpicky
 # Sphinx will warn about all references where the target cannot be found.
 nitpicky = True
-nitpick_ignore = []
+# Sphinx does not recognize generic type TypeVars
+# Container supposedly were fixed, but does not work
+# https://github.com/sphinx-doc/sphinx/pull/3744
+nitpick_ignore = [("py:class", "ValueT"), ("py:class", "typing.Tuple")]
 
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ["_templates"]

examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py

@@ -31,8 +31,15 @@
 )
 
 label_values = ("staging",)
+
+# Direct metric usage
+counter.add(label_values, 25)
+
+# Handle usage
 counter_handle = counter.get_handle(label_values)
 counter_handle.add(100)
+
+# Record batch usage
 meter.record_batch(label_values, [(counter, 50)])
 print(counter_handle.data)
 

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

@@ -26,7 +26,6 @@
 
 
 """
-from abc import ABC, abstractmethod
 from typing import Callable, Optional, Sequence, Tuple, Type, TypeVar
 
 from opentelemetry.util import loader
@@ -43,27 +42,38 @@ class DefaultMetricHandle:
 
 class CounterHandle:
     def add(self, value: ValueT) -> None:
-        """Increases the value of the handle by ``value``"""
+        """Increases the value of the handle by ``value``.
+
+        Args:
+            value: The value to record to the handle.
+        """
 
 
 class GaugeHandle:
     def set(self, value: ValueT) -> None:
-        """Sets the current value of the handle to ``value``."""
+        """Sets the current value of the handle to ``value``.
+
+        Args:
+            value: The value to record to the handle.
+        """
 
 
 class MeasureHandle:
     def record(self, value: ValueT) -> None:
-        """Records the given ``value`` to this handle."""
+        """Records the given ``value`` to this handle.
+
+        Args:
+            value: The value to record to the handle.
+        """
 
 
-class Metric(ABC):
+class Metric:
     """Base class for various types of metrics.
 
     Metric class that inherit from this class are specialized with the type of
     handle that the metric holds.
     """
 
-    @abstractmethod
     def get_handle(self, label_values: Sequence[str]) -> "object":
         """Gets a handle, used for repeated-use of metrics instruments.
 
@@ -83,6 +93,11 @@ class DefaultMetric(Metric):
     """The default Metric used when no Metric implementation is available."""
 
     def get_handle(self, label_values: Sequence[str]) -> "DefaultMetricHandle":
+        """Gets a `DefaultMetricHandle`.
+
+        Args:
+            label_values: The label values associated with the handle.
+        """
         return DefaultMetricHandle()
 
 
@@ -93,6 +108,14 @@ def get_handle(self, label_values: Sequence[str]) -> "CounterHandle":
         """Gets a `CounterHandle`."""
         return CounterHandle()
 
+    def add(self, label_values: Sequence[str], value: ValueT) -> None:
+        """Increases the value of the counter by ``value``.
+
+        Args:
+            label_values: The label values associated with the metric.
+            value: The value to add to the counter metric.
+        """
+
 
 class Gauge(Metric):
     """A gauge type metric that expresses a pre-calculated value.
@@ -107,6 +130,14 @@ def get_handle(self, label_values: Sequence[str]) -> "GaugeHandle":
         """Gets a `GaugeHandle`."""
         return GaugeHandle()
 
+    def set(self, label_values: Sequence[str], value: ValueT) -> None:
+        """Sets the value of the gauge to ``value``.
+
+        Args:
+            label_values: The label values associated with the metric.
+            value: The value to set the gauge metric to.
+        """
+
 
 class Measure(Metric):
     """A measure type metric that represent raw stats that are recorded.
@@ -120,6 +151,14 @@ def get_handle(self, label_values: Sequence[str]) -> "MeasureHandle":
         """Gets a `MeasureHandle` with a float value."""
         return MeasureHandle()
 
+    def record(self, label_values: Sequence[str], value: ValueT) -> None:
+        """Records the ``value`` to the measure.
+
+        Args:
+            label_values: The label values associated with the metric.
+            value: The value to record to this measure metric.
+        """
+
 
 MetricT = TypeVar("MetricT", Counter, Gauge, Measure)
 
@@ -145,8 +184,9 @@ def record_batch(
         match the key-value pairs in the label tuples.
 
         Args:
-            label_values: The values that will be matched against to record for
-                the handles under each metric that has those labels.
+            label_values: The label values associated with all measurements in
+                the batch. A measurement is a tuple, representing the `Metric`
+                being recorded and the corresponding value to record.
             record_tuples: A sequence of pairs of `Metric` s and the
                 corresponding value to record for that metric.
         """

opentelemetry-api/tests/metrics/test_metrics.py

@@ -42,16 +42,28 @@ def test_counter(self):
         handle = counter.get_handle(("test", "test1"))
         self.assertIsInstance(handle, metrics.CounterHandle)
 
+    def test_counter_add(self):
+        counter = metrics.Counter()
+        counter.add(("value",), 1)
+
     def test_gauge(self):
         gauge = metrics.Gauge()
         handle = gauge.get_handle(("test", "test1"))
         self.assertIsInstance(handle, metrics.GaugeHandle)
 
+    def test_gauge_set(self):
+        gauge = metrics.Gauge()
+        gauge.set(("value",), 1)
+
     def test_measure(self):
         measure = metrics.Measure()
         handle = measure.get_handle(("test", "test1"))
         self.assertIsInstance(handle, metrics.MeasureHandle)
 
+    def test_measure_record(self):
+        measure = metrics.Measure()
+        measure.record(("value",), 1)
+
     def test_default_handle(self):
         metrics.DefaultMetricHandle()
 

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

@@ -44,43 +44,34 @@ def _validate_update(self, value: metrics_api.ValueT) -> bool:
 
 
 class CounterHandle(metrics_api.CounterHandle, BaseHandle):
-    def update(self, value: metrics_api.ValueT) -> None:
+    def add(self, value: metrics_api.ValueT) -> None:
+        """See `opentelemetry.metrics.CounterHandle.add`."""
         if self._validate_update(value):
             if self.monotonic and value < 0:
                 logger.warning("Monotonic counter cannot descend.")
                 return
             self.data += value
 
-    def add(self, value: metrics_api.ValueT) -> None:
-        """See `opentelemetry.metrics.CounterHandle._add`."""
-        self.update(value)
-
 
 class GaugeHandle(metrics_api.GaugeHandle, BaseHandle):
-    def update(self, value: metrics_api.ValueT) -> None:
+    def set(self, value: metrics_api.ValueT) -> None:
+        """See `opentelemetry.metrics.GaugeHandle.set`."""
         if self._validate_update(value):
             if self.monotonic and value < self.data:
                 logger.warning("Monotonic gauge cannot descend.")
                 return
             self.data = value
 
-    def set(self, value: metrics_api.ValueT) -> None:
-        """See `opentelemetry.metrics.GaugeHandle._set`."""
-        self.update(value)
-
 
 class MeasureHandle(metrics_api.MeasureHandle, BaseHandle):
-    def update(self, value: metrics_api.ValueT) -> None:
+    def record(self, value: metrics_api.ValueT) -> None:
+        """See `opentelemetry.metrics.MeasureHandle.record`."""
         if self._validate_update(value):
             if self.monotonic and value < 0:
                 logger.warning("Monotonic measure cannot accept negatives.")
                 return
             # TODO: record
 
-    def record(self, value: metrics_api.ValueT) -> None:
-        """See `opentelemetry.metrics.MeasureHandle._record`."""
-        self.update(value)
-
 
 class Metric(metrics_api.Metric):
     """See `opentelemetry.metrics.Metric`."""
@@ -116,8 +107,10 @@ def get_handle(self, label_values: Sequence[str]) -> BaseHandle:
         self.handles[label_values] = handle
         return handle
 
+    UPDATE_FUNCTION = lambda x, y: None  # noqa: E731
+
 
-class Counter(Metric):
+class Counter(Metric, metrics_api.Counter):
     """See `opentelemetry.metrics.Counter`.
 
     By default, counter values can only go up (monotonic). Negative inputs
@@ -147,8 +140,16 @@ def __init__(
             monotonic=monotonic,
         )
 
+    def add(
+        self, label_values: Sequence[str], value: metrics_api.ValueT
+    ) -> None:
+        """See `opentelemetry.metrics.Counter.add`."""
+        self.get_handle(label_values).add(value)
+
+    UPDATE_FUNCTION = add
+
 
-class Gauge(Metric):
+class Gauge(Metric, metrics_api.Gauge):
     """See `opentelemetry.metrics.Gauge`.
 
     By default, gauge values can go both up and down (non-monotonic).
@@ -177,8 +178,16 @@ def __init__(
             monotonic=monotonic,
         )
 
+    def set(
+        self, label_values: Sequence[str], value: metrics_api.ValueT
+    ) -> None:
+        """See `opentelemetry.metrics.Gauge.set`."""
+        self.get_handle(label_values).set(value)
+
+    UPDATE_FUNCTION = set
 
-class Measure(Metric):
+
+class Measure(Metric, metrics_api.Measure):
     """See `opentelemetry.metrics.Measure`.
 
     By default, measure metrics can accept both positive and negatives.
@@ -207,6 +216,14 @@ def __init__(
             monotonic=monotonic,
         )
 
+    def record(
+        self, label_values: Sequence[str], value: metrics_api.ValueT
+    ) -> None:
+        """See `opentelemetry.metrics.Measure.record`."""
+        self.get_handle(label_values).record(value)
+
+    UPDATE_FUNCTION = record
+
 
 class Meter(metrics_api.Meter):
     """See `opentelemetry.metrics.Meter`."""
@@ -218,7 +235,7 @@ def record_batch(
     ) -> None:
         """See `opentelemetry.metrics.Meter.record_batch`."""
         for metric, value in record_tuples:
-            metric.get_handle(label_values).update(value)
+            metric.UPDATE_FUNCTION(label_values, value)
 
     def create_metric(
         self,

opentelemetry-sdk/tests/metrics/test_metrics.py

@@ -51,8 +51,8 @@ def test_record_batch_exists(self):
         label_keys = ("key1",)
         label_values = ("value1",)
         counter = metrics.Counter("name", "desc", "unit", float, label_keys)
+        counter.add(label_values, 1.0)
         handle = counter.get_handle(label_values)
-        handle.update(1.0)
         record_tuples = [(counter, 1.0)]
         meter.record_batch(label_values, record_tuples)
         self.assertEqual(counter.get_handle(label_values), handle)
@@ -96,12 +96,35 @@ def test_get_handle(self):
             self.assertEqual(metric.handles.get(label_values), handle)
 
 
-class TestCounterHandle(unittest.TestCase):
-    def test_update(self):
-        handle = metrics.CounterHandle(float, True, False)
-        handle.update(2.0)
-        self.assertEqual(handle.data, 2.0)
+class TestCounter(unittest.TestCase):
+    def test_add(self):
+        metric = metrics.Counter("name", "desc", "unit", int, ("key",))
+        handle = metric.get_handle(("value",))
+        metric.add(("value",), 3)
+        metric.add(("value",), 2)
+        self.assertEqual(handle.data, 5)
 
+
+class TestGauge(unittest.TestCase):
+    def test_set(self):
+        metric = metrics.Gauge("name", "desc", "unit", int, ("key",))
+        handle = metric.get_handle(("value",))
+        metric.set(("value",), 3)
+        self.assertEqual(handle.data, 3)
+        metric.set(("value",), 2)
+        self.assertEqual(handle.data, 2)
+
+
+class TestMeasure(unittest.TestCase):
+    def test_record(self):
+        metric = metrics.Measure("name", "desc", "unit", int, ("key",))
+        handle = metric.get_handle(("value",))
+        metric.record(("value",), 3)
+        # Record not implemented yet
+        self.assertEqual(handle.data, 0)
+
+
+class TestCounterHandle(unittest.TestCase):
     def test_add(self):
         handle = metrics.CounterHandle(int, True, False)
         handle.add(3)
@@ -128,11 +151,6 @@ def test_add_incorrect_type(self, logger_mock):
 
 
 class TestGaugeHandle(unittest.TestCase):
-    def test_update(self):
-        handle = metrics.GaugeHandle(float, True, False)
-        handle.update(2.0)
-        self.assertEqual(handle.data, 2.0)
-
     def test_set(self):
         handle = metrics.GaugeHandle(int, True, False)
         handle.set(3)
@@ -159,14 +177,10 @@ def test_set_incorrect_type(self, logger_mock):
 
 
 class TestMeasureHandle(unittest.TestCase):
-    def test_update(self):
-        handle = metrics.MeasureHandle(float, False, False)
-        handle.update(2.0)
-        self.assertEqual(handle.data, 0)
-
     def test_record(self):
         handle = metrics.MeasureHandle(int, False, False)
         handle.record(3)
+        # Record not implemented yet
         self.assertEqual(handle.data, 0)
 
     def test_record_disabled(self):