diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 3b2db188354..d9da9a067a0 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,8 @@ 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 +90,10 @@ 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 +117,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..5504fdb686b 100644 --- a/opentelemetry-sdk/tests/metrics/test_instrument.py +++ b/opentelemetry-sdk/tests/metrics/test_instrument.py @@ -16,12 +16,43 @@ 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_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() + 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 +113,17 @@ 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_record_non_monotonic(self): + mc = Mock() + hist = Histogram("name", Mock(), mc) + hist.record(-1.0) + mc.consume_measurement.assert_not_called()