Remove monotonic and absolute metrics intrument options (open-telemetry#410)

mauriciovasquezbernal · web-flow · commit 96943b242fc5 · 2020-02-14T11:07:32.000-08:00
Following opentelemetry-specification/430.
diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py
@@ -208,9 +208,7 @@ def set(self, value: ValueT, label_set: LabelSet) -> None:
 class Measure(Metric):
     """A measure type metric that represent raw stats that are recorded.
 
-    Measure metrics represent raw statistics that are recorded. By
-    default, measure metrics can accept both positive and negatives.
-    Negative inputs will be discarded when monotonic is True.
+    Measure metrics represent raw statistics that are recorded.
     """
 
     def get_handle(self, label_set: LabelSet) -> "MeasureHandle":
@@ -268,8 +266,6 @@ def create_metric(
         metric_type: Type[MetricT],
         label_keys: Sequence[str] = (),
         enabled: bool = True,
-        monotonic: bool = False,
-        absolute: bool = True,
     ) -> "Metric":
         """Creates a ``metric_kind`` metric with type ``value_type``.
 
@@ -281,10 +277,6 @@ def create_metric(
             metric_type: The type of metric being created.
             label_keys: The keys for the labels with dynamic values.
             enabled: Whether to report the metric by default.
-            monotonic: Configure a counter or gauge that accepts only
-                monotonic/non-monotonic updates.
-            absolute: Configure a measure that does or does not accept negative
-                updates.
         Returns: A new ``metric_type`` metric with values of ``value_type``.
         """
 
@@ -318,8 +310,6 @@ def create_metric(
         metric_type: Type[MetricT],
         label_keys: Sequence[str] = (),
         enabled: bool = True,
-        monotonic: bool = False,
-        absolute: bool = True,
     ) -> "Metric":
         # pylint: disable=no-self-use
         return DefaultMetric()
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py
@@ -55,9 +55,6 @@ class BaseHandle:
     Args:
         value_type: The type of values this handle holds (int, float).
         enabled: True if the originating instrument is enabled.
-        monotonic: Indicates acceptance of only monotonic/non-monotonic values
-            for updating counter and gauge handles.
-        absolute: Indicates acceptance of negative updates to measure handles.
         aggregator: The aggregator for this handle. Will handle aggregation
             upon updates and checkpointing of values for exporting.
     """
@@ -66,14 +63,10 @@ def __init__(
         self,
         value_type: Type[metrics_api.ValueT],
         enabled: bool,
-        monotonic: bool,
-        absolute: bool,
         aggregator: Aggregator,
     ):
         self.value_type = value_type
         self.enabled = enabled
-        self.monotonic = monotonic
-        self.absolute = absolute
         self.aggregator = aggregator
         self.last_update_timestamp = time_ns()
 
@@ -103,29 +96,20 @@ class CounterHandle(metrics_api.CounterHandle, BaseHandle):
     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.update(value)
 
 
 class GaugeHandle(metrics_api.GaugeHandle, BaseHandle):
     def set(self, value: metrics_api.ValueT) -> None:
         """See `opentelemetry.metrics.GaugeHandle.set`."""
         if self._validate_update(value):
-            if self.monotonic and value < self.aggregator.current:
-                logger.warning("Monotonic gauge cannot descend.")
-                return
             self.update(value)
 
 
 class MeasureHandle(metrics_api.MeasureHandle, BaseHandle):
     def record(self, value: metrics_api.ValueT) -> None:
         """See `opentelemetry.metrics.MeasureHandle.record`."""
         if self._validate_update(value):
-            if self.absolute and value < 0:
-                logger.warning("Absolute measure cannot accept negatives.")
-                return
             self.update(value)
 
 
@@ -149,8 +133,6 @@ def __init__(
         meter: "Meter",
         label_keys: Sequence[str] = (),
         enabled: bool = True,
-        monotonic: bool = False,
-        absolute: bool = True,
     ):
         self.name = name
         self.description = description
@@ -159,8 +141,6 @@ def __init__(
         self.meter = meter
         self.label_keys = label_keys
         self.enabled = enabled
-        self.monotonic = monotonic
-        self.absolute = absolute
         self.handles = {}
 
     def get_handle(self, label_set: LabelSet) -> BaseHandle:
@@ -170,8 +150,6 @@ def get_handle(self, label_set: LabelSet) -> BaseHandle:
             handle = self.HANDLE_TYPE(
                 self.value_type,
                 self.enabled,
-                self.monotonic,
-                self.absolute,
                 # Aggregator will be created based off type of metric
                 self.meter.batcher.aggregator_for(self.__class__),
             )
@@ -188,10 +166,6 @@ def __repr__(self):
 
 class Counter(Metric, metrics_api.Counter):
     """See `opentelemetry.metrics.Counter`.
-
-    By default, counter values can only go up (monotonic). Negative inputs
-    will be rejected for monotonic counter metrics. Counter metrics that have a
-    monotonic option set to False allows negative inputs.
     """
 
     HANDLE_TYPE = CounterHandle
@@ -205,8 +179,6 @@ def __init__(
         meter: "Meter",
         label_keys: Sequence[str] = (),
         enabled: bool = True,
-        monotonic: bool = True,
-        absolute: bool = False,
     ):
         super().__init__(
             name,
@@ -216,8 +188,6 @@ def __init__(
             meter,
             label_keys=label_keys,
             enabled=enabled,
-            monotonic=monotonic,
-            absolute=absolute,
         )
 
     def add(self, value: metrics_api.ValueT, label_set: LabelSet) -> None:
@@ -229,9 +199,6 @@ def add(self, value: metrics_api.ValueT, label_set: LabelSet) -> None:
 
 class Gauge(Metric, metrics_api.Gauge):
     """See `opentelemetry.metrics.Gauge`.
-
-    By default, gauge values can go both up and down (non-monotonic).
-    Negative inputs will be rejected for monotonic gauge metrics.
     """
 
     HANDLE_TYPE = GaugeHandle
@@ -245,8 +212,6 @@ def __init__(
         meter: "Meter",
         label_keys: Sequence[str] = (),
         enabled: bool = True,
-        monotonic: bool = False,
-        absolute: bool = False,
     ):
         super().__init__(
             name,
@@ -256,8 +221,6 @@ def __init__(
             meter,
             label_keys=label_keys,
             enabled=enabled,
-            monotonic=monotonic,
-            absolute=absolute,
         )
 
     def set(self, value: metrics_api.ValueT, label_set: LabelSet) -> None:
@@ -339,8 +302,6 @@ def create_metric(
         metric_type: Type[metrics_api.MetricT],
         label_keys: Sequence[str] = (),
         enabled: bool = True,
-        monotonic: bool = False,
-        absolute: bool = True,
     ) -> metrics_api.MetricT:
         """See `opentelemetry.metrics.Meter.create_metric`."""
         # Ignore type b/c of mypy bug in addition to missing annotations
@@ -352,8 +313,6 @@ def create_metric(
             self,
             label_keys=label_keys,
             enabled=enabled,
-            monotonic=monotonic,
-            absolute=absolute,
         )
         self.metrics.add(metric)
         return metric
diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py
@@ -203,36 +203,28 @@ def test_record(self):
 class TestCounterHandle(unittest.TestCase):
     def test_add(self):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.CounterHandle(int, True, False, False, aggregator)
+        handle = metrics.CounterHandle(int, True, aggregator)
         handle.add(3)
         self.assertEqual(handle.aggregator.current, 3)
 
     def test_add_disabled(self):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.CounterHandle(int, False, False, False, aggregator)
+        handle = metrics.CounterHandle(int, False, aggregator)
         handle.add(3)
         self.assertEqual(handle.aggregator.current, 0)
 
-    @mock.patch("opentelemetry.sdk.metrics.logger")
-    def test_add_monotonic(self, logger_mock):
-        aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.CounterHandle(int, True, True, False, aggregator)
-        handle.add(-3)
-        self.assertEqual(handle.aggregator.current, 0)
-        self.assertTrue(logger_mock.warning.called)
-
     @mock.patch("opentelemetry.sdk.metrics.logger")
     def test_add_incorrect_type(self, logger_mock):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.CounterHandle(int, True, False, False, aggregator)
+        handle = metrics.CounterHandle(int, True, aggregator)
         handle.add(3.0)
         self.assertEqual(handle.aggregator.current, 0)
         self.assertTrue(logger_mock.warning.called)
 
     @mock.patch("opentelemetry.sdk.metrics.time_ns")
     def test_update(self, time_mock):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.CounterHandle(int, True, False, False, aggregator)
+        handle = metrics.CounterHandle(int, True, aggregator)
         time_mock.return_value = 123
         handle.update(4.0)
         self.assertEqual(handle.last_update_timestamp, 123)
@@ -243,36 +235,28 @@ def test_update(self, time_mock):
 class TestGaugeHandle(unittest.TestCase):
     def test_set(self):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.GaugeHandle(int, True, False, False, aggregator)
+        handle = metrics.GaugeHandle(int, True, aggregator)
         handle.set(3)
         self.assertEqual(handle.aggregator.current, 3)
 
     def test_set_disabled(self):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.GaugeHandle(int, False, False, False, aggregator)
+        handle = metrics.GaugeHandle(int, False, aggregator)
         handle.set(3)
         self.assertEqual(handle.aggregator.current, 0)
 
-    @mock.patch("opentelemetry.sdk.metrics.logger")
-    def test_set_monotonic(self, logger_mock):
-        aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.GaugeHandle(int, True, True, False, aggregator)
-        handle.set(-3)
-        self.assertEqual(handle.aggregator.current, 0)
-        self.assertTrue(logger_mock.warning.called)
-
     @mock.patch("opentelemetry.sdk.metrics.logger")
     def test_set_incorrect_type(self, logger_mock):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.GaugeHandle(int, True, False, False, aggregator)
+        handle = metrics.GaugeHandle(int, True, aggregator)
         handle.set(3.0)
         self.assertEqual(handle.aggregator.current, 0)
         self.assertTrue(logger_mock.warning.called)
 
     @mock.patch("opentelemetry.sdk.metrics.time_ns")
     def test_update(self, time_mock):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.GaugeHandle(int, True, False, False, aggregator)
+        handle = metrics.GaugeHandle(int, True, aggregator)
         time_mock.return_value = 123
         handle.update(4.0)
         self.assertEqual(handle.last_update_timestamp, 123)
@@ -283,36 +267,28 @@ def test_update(self, time_mock):
 class TestMeasureHandle(unittest.TestCase):
     def test_record(self):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.MeasureHandle(int, False, False, False, aggregator)
+        handle = metrics.MeasureHandle(int, False, aggregator)
         handle.record(3)
         self.assertEqual(handle.aggregator.current, 0)
 
     def test_record_disabled(self):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.MeasureHandle(int, False, False, False, aggregator)
+        handle = metrics.MeasureHandle(int, False, aggregator)
         handle.record(3)
         self.assertEqual(handle.aggregator.current, 0)
 
-    @mock.patch("opentelemetry.sdk.metrics.logger")
-    def test_record_monotonic(self, logger_mock):
-        aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.MeasureHandle(int, True, False, True, aggregator)
-        handle.record(-3)
-        self.assertEqual(handle.aggregator.current, 0)
-        self.assertTrue(logger_mock.warning.called)
-
     @mock.patch("opentelemetry.sdk.metrics.logger")
     def test_record_incorrect_type(self, logger_mock):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.MeasureHandle(int, True, False, False, aggregator)
+        handle = metrics.MeasureHandle(int, True, aggregator)
         handle.record(3.0)
         self.assertEqual(handle.aggregator.current, 0)
         self.assertTrue(logger_mock.warning.called)
 
     @mock.patch("opentelemetry.sdk.metrics.time_ns")
     def test_update(self, time_mock):
         aggregator = export.aggregate.CounterAggregator()
-        handle = metrics.MeasureHandle(int, True, False, False, aggregator)
+        handle = metrics.MeasureHandle(int, True, aggregator)
         time_mock.return_value = 123
         handle.update(4.0)
         self.assertEqual(handle.last_update_timestamp, 123)
