From 85db79f8732d0546c0719b59a7365ab312bcea14 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 19 Jan 2022 18:39:12 -0800 Subject: [PATCH 1/2] instr --- .../opentelemetry/sdk/_metrics/instrument.py | 17 ++++++-- .../tests/metrics/test_instrument.py | 42 +++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 3b2db188354..98d9e195cc7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -14,6 +14,7 @@ # pylint: disable=too-many-ancestors +import logging from typing import Dict, Generator, Iterable, Union from opentelemetry._metrics.instrument import CallbackT @@ -33,6 +34,7 @@ from opentelemetry.sdk._metrics.measurement_consumer import MeasurementConsumer from opentelemetry.sdk.util.instrumentation import InstrumentationInfo +logger = logging.getLogger(__name__) class _Synchronous: def __init__( @@ -87,8 +89,9 @@ def add( self, amount: Union[int, float], attributes: Dict[str, str] = None ): if amount < 0: - raise Exception("amount must be non negative") - + logger.warning( + "Add amount must be non-negative on Counter %s.", self.name) + return self._measurement_consumer.consume_measurement( Measurement(amount, attributes) ) @@ -112,7 +115,15 @@ class ObservableUpDownCounter(_Asynchronous, APIObservableUpDownCounter): class Histogram(_Synchronous, APIHistogram): - def record(self, amount, attributes=None): + def record( + self, + amount: Union[int, float], + attributes: Dict[str, str] = None + ): + if amount < 0: + logger.warning( + "Record amount must be non-negative on Histogram %s.", self.name) + return self._measurement_consumer.consume_measurement( Measurement(amount, attributes) ) diff --git a/opentelemetry-sdk/tests/metrics/test_instrument.py b/opentelemetry-sdk/tests/metrics/test_instrument.py index 64d20d68628..e7281f79d9d 100644 --- a/opentelemetry-sdk/tests/metrics/test_instrument.py +++ b/opentelemetry-sdk/tests/metrics/test_instrument.py @@ -16,12 +16,41 @@ from unittest.mock import Mock from opentelemetry.sdk._metrics.instrument import ( + Counter, + Histogram, ObservableCounter, ObservableGauge, ObservableUpDownCounter, + UpDownCounter, ) +class TestCounter(TestCase): + def test_add(self): + mc = Mock() + counter = Counter("name", Mock(), mc) + counter.add(1.0) + mc.consume_measurement.assert_called_once() + + def test_add_monotonic(self): + mc = Mock() + counter = Counter("name", Mock(), mc) + counter.add(-1.0) + mc.consume_measurement.assert_not_called() + +class TestUpDownCounter(TestCase): + def test_add(self): + mc = Mock() + counter = UpDownCounter("name", Mock(), mc) + counter.add(1.0) + mc.consume_measurement.assert_called_once() + + def test_add_non_monotonic(self): + mc = Mock() + counter = UpDownCounter("name", Mock(), mc) + counter.add(-1.0) + mc.consume_measurement.assert_called_once() + class TestObservableGauge(TestCase): def test_callable_callback(self): def callback(): @@ -82,3 +111,16 @@ def callback(): ) self.assertEqual(observable_up_down_counter.callback(), [1, 2, 3]) + +class TestHistogram(TestCase): + def test_record(self): + mc = Mock() + hist = Histogram("name", Mock(), mc) + hist.record(1.0) + mc.consume_measurement.assert_called_once() + + def test_add_monotonic(self): + mc = Mock() + hist = Histogram("name", Mock(), mc) + hist.record(-1.0) + mc.consume_measurement.assert_not_called() From 087bb1d8b0f1f352af76f371758e4f410c5d18b0 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 20 Jan 2022 08:53:50 -0800 Subject: [PATCH 2/2] lint --- .../opentelemetry/sdk/_metrics/instrument.py | 18 ++++++++++-------- .../tests/metrics/test_instrument.py | 7 +++++-- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 98d9e195cc7..d9da9a067a0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -34,7 +34,8 @@ from opentelemetry.sdk._metrics.measurement_consumer import MeasurementConsumer from opentelemetry.sdk.util.instrumentation import InstrumentationInfo -logger = logging.getLogger(__name__) +_logger = logging.getLogger(__name__) + class _Synchronous: def __init__( @@ -89,8 +90,9 @@ def add( self, amount: Union[int, float], attributes: Dict[str, str] = None ): if amount < 0: - logger.warning( - "Add amount must be non-negative on Counter %s.", self.name) + _logger.warning( + "Add amount must be non-negative on Counter %s.", self.name + ) return self._measurement_consumer.consume_measurement( Measurement(amount, attributes) @@ -116,13 +118,13 @@ class ObservableUpDownCounter(_Asynchronous, APIObservableUpDownCounter): class Histogram(_Synchronous, APIHistogram): def record( - self, - amount: Union[int, float], - attributes: Dict[str, str] = None + self, amount: Union[int, float], attributes: Dict[str, str] = None ): if amount < 0: - logger.warning( - "Record amount must be non-negative on Histogram %s.", self.name) + _logger.warning( + "Record amount must be non-negative on Histogram %s.", + self.name, + ) return self._measurement_consumer.consume_measurement( Measurement(amount, attributes) diff --git a/opentelemetry-sdk/tests/metrics/test_instrument.py b/opentelemetry-sdk/tests/metrics/test_instrument.py index e7281f79d9d..5504fdb686b 100644 --- a/opentelemetry-sdk/tests/metrics/test_instrument.py +++ b/opentelemetry-sdk/tests/metrics/test_instrument.py @@ -32,12 +32,13 @@ def test_add(self): counter.add(1.0) mc.consume_measurement.assert_called_once() - def test_add_monotonic(self): + def test_add_non_monotonic(self): mc = Mock() counter = Counter("name", Mock(), mc) counter.add(-1.0) mc.consume_measurement.assert_not_called() + class TestUpDownCounter(TestCase): def test_add(self): mc = Mock() @@ -51,6 +52,7 @@ def test_add_non_monotonic(self): counter.add(-1.0) mc.consume_measurement.assert_called_once() + class TestObservableGauge(TestCase): def test_callable_callback(self): def callback(): @@ -112,6 +114,7 @@ def callback(): self.assertEqual(observable_up_down_counter.callback(), [1, 2, 3]) + class TestHistogram(TestCase): def test_record(self): mc = Mock() @@ -119,7 +122,7 @@ def test_record(self): hist.record(1.0) mc.consume_measurement.assert_called_once() - def test_add_monotonic(self): + def test_record_non_monotonic(self): mc = Mock() hist = Histogram("name", Mock(), mc) hist.record(-1.0)