From ba80a97488b7f15c287b28949d97813d9c3e6d5a Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 25 Nov 2021 17:15:10 -0600 Subject: [PATCH 01/35] Refactor aggregations Fixes #2305 --- opentelemetry-sdk/setup.cfg | 1 + .../opentelemetry/sdk/_metrics/aggregation.py | 160 +++++++++++++----- .../opentelemetry/sdk/_metrics/measurement.py | 21 ++- .../src/opentelemetry/sdk/_metrics/point.py | 49 ++++++ .../tests/metrics/test_aggregation.py | 68 ++++---- 5 files changed, 219 insertions(+), 80 deletions(-) create mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py diff --git a/opentelemetry-sdk/setup.cfg b/opentelemetry-sdk/setup.cfg index 6b4281cc2e7..bdc34ccfa02 100644 --- a/opentelemetry-sdk/setup.cfg +++ b/opentelemetry-sdk/setup.cfg @@ -46,6 +46,7 @@ install_requires = opentelemetry-api == 1.8.0 opentelemetry-semantic-conventions == 0.27b0 setuptools >= 16.0 + dataclasses == 0.8; python_version < '3.7' [options.packages.find] where = src diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 456e4471621..525a46e6dd0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -16,82 +16,138 @@ from collections import OrderedDict from logging import getLogger from math import inf +from threading import Lock +from typing import Generic, Optional, Sequence, TypeVar -from opentelemetry._metrics.instrument import _Monotonic +from opentelemetry.sdk._metrics.measurement import Measurement +from opentelemetry.sdk._metrics.point import Gauge, Histogram, PointT, Sum from opentelemetry.util._time import _time_ns +# FIXME this is being copied directly from +# opentelemetry.proto.metrics.v1.metrics_pb2. The only reason for doing so is +# to avoid havinv protobuf as a indirect dependency in the SDK. This +# duplication of code is not ideal. + +AGGREGATION_TEMPORALITY_UNSPECIFIED = 0 +AGGREGATION_TEMPORALITY_DELTA = 1 +AGGREGATION_TEMPORALITY_CUMULATIVE = 2 + +_PointVarT = TypeVar("_PointVarT", bound=PointT) + _logger = getLogger(__name__) -class Aggregation(ABC): +class Aggregation(ABC, Generic[_PointVarT]): + def __init__(self, is_monotonic: bool): + self._value = None + self._is_monotonic = is_monotonic + self._lock = Lock() + + @property + def is_monotonic(self): + return self._is_monotonic + @property def value(self): - return self._value # pylint: disable=no-member + return self._value @abstractmethod - def aggregate(self, value): + def aggregate(self, measurement: Measurement) -> None: pass @abstractmethod - def make_point_and_reset(self): - """ - Atomically return a point for the current value of the metric and reset the internal state. - """ - + def _collect(self) -> Optional[_PointVarT]: + pass -class SumAggregation(Aggregation): - """ - This aggregation collects data for the SDK sum metric point. - """ - def __init__(self, instrument): +class SynchronousSumAggregation(Aggregation[Sum]): + def __init__(self, is_monotonic: bool): + super().__init__(is_monotonic) self._value = 0 + self._start_time_unix_nano = _time_ns() - def aggregate(self, value): - self._value = self._value + value + def aggregate(self, measurement: Measurement) -> None: + with self._lock: + self._value = self._value + measurement.value - def make_point_and_reset(self): - pass + def _collect(self): + now = _time_ns() + with self._lock: + self._value = 0 + self._start_time_unix_nano = now + 1 -class LastValueAggregation(Aggregation): + return Sum( + aggregation_temporality=AGGREGATION_TEMPORALITY_DELTA, + is_monotonic=self._is_monotonic, + start_time_unix_nano=self._start_time_unix_nano, + time_unix_nano=now, + value=self._value, + ) - """ - This aggregation collects data for the SDK sum metric point. - """ - def __init__(self, instrument): - self._value = None - self._timestamp = _time_ns() +class AsynchronousSumAggregation(Aggregation[Sum]): + def __init__(self, is_monotonic: bool): + super().__init__(is_monotonic) + self._start_time_unix_nano = _time_ns() - def aggregate(self, value): - self._value = value - self._timestamp = _time_ns() + def aggregate(self, measurement: Measurement) -> None: + with self._lock: + self._value = measurement.value - def make_point_and_reset(self): - pass + def _collect(self): + if self._value is None: + return None + + return Sum( + start_time_unix_nano=self._start_time_unix_nano, + time_unix_nano=_time_ns(), + value=self._value, + aggregation_temporality=AGGREGATION_TEMPORALITY_CUMULATIVE, + is_monotonic=self._is_monotonic, + ) -class ExplicitBucketHistogramAggregation(Aggregation): +class LastValueAggregation(Aggregation[Gauge]): + def aggregate(self, measurement: Measurement): + with self._lock: + self._value = measurement.value + + def _collect(self): + if self._value is None: + return None + + return Gauge( + time_unix_nano=_time_ns(), + value=self._value, + ) - """ - This aggregation collects data for the SDK sum metric point. - """ +class ExplicitBucketHistogramAggregation(Aggregation[Histogram]): def __init__( self, - instrument, - *args, - boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000), - record_min_max=True, + is_monotonic: bool, + boundaries: Sequence[int] = ( + 0, + 5, + 10, + 25, + 50, + 75, + 100, + 250, + 500, + 1000, + ), + record_min_max: bool = True, ): - super().__init__() + super().__init__(is_monotonic) self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) self._min = inf self._max = -inf self._sum = 0 - self._instrument = instrument self._record_min_max = record_min_max + self._start_time_unix_nano = _time_ns() @property def min(self): @@ -109,7 +165,7 @@ def max(self): @property def sum(self): - if isinstance(self._instrument, _Monotonic): + if self._is_monotonic: return self._sum _logger.warning( @@ -118,12 +174,15 @@ def sum(self): ) return None - def aggregate(self, value): + def aggregate(self, measurement: Measurement): + + value = measurement.value + if self._record_min_max: self._min = min(self._min, value) self._max = max(self._max, value) - if isinstance(self._instrument, _Monotonic): + if self._is_monotonic: self._sum += value for key in self._value.keys(): @@ -133,5 +192,16 @@ def aggregate(self, value): break - def make_point_and_reset(self): - pass + def _collect(self): + now = _time_ns() + + with self._lock: + self._value = 0 + self._start_time_unix_nano = now + 1 + + return Histogram( + start_time_unix_nano=self._start_time_unix_nano, + time_unix_nano=now, + value=self._value, + aggregation_temporality=AGGREGATION_TEMPORALITY_CUMULATIVE, + ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py index fbaae02c786..13fb5b30abf 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py @@ -12,6 +12,25 @@ # See the License for the specific language governing permissions and # limitations under the License. +from dataclasses import dataclass +from typing import Optional, Union +from opentelemetry.util.types import Attributes + + +@dataclass(frozen=True) class Measurement: - pass + value: Union[int, float] + attributes: Optional[Attributes] = None + + +# Can't make Exemplar a subclass of Measurement because that makes +# time_unix_nano to be defined after attribute, a keyword argument +@dataclass(frozen=True) +class Exemplar: + value: Union[int, float] + time_unix_nano: int + span_id: int + trace_id: int + attributes: Optional[Attributes] = None + filtered_attributes: Optional[Attributes] = None diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py new file mode 100644 index 00000000000..96d935a0753 --- /dev/null +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py @@ -0,0 +1,49 @@ +# 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 dataclasses import dataclass +from typing import Union + + +@dataclass(frozen=True) +class Sum: + # NumberDataPoint attributes + start_time_unix_nano: int + time_unix_nano: int + value: Union[int, float] + # Sum attributes + aggregation_temporality: int + is_monotonic: bool + + +@dataclass(frozen=True) +class Gauge: + # NumberDataPoint attributes + # start_time_unix_nano is not added here because the proto says it is to + # be ignored. + time_unix_nano: int + value: Union[int, float] + + +@dataclass(frozen=True) +class Histogram: + # NumberDataPoint attributes + start_time_unix_nano: int + time_unix_nano: int + value: Union[int, float] + # Histogram attributes + aggregation_temporality: int + + +PointT = Union[Sum, Gauge, Histogram] diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 1c4fa1420e6..91be5514ddb 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -16,34 +16,34 @@ from logging import WARNING from math import inf from unittest import TestCase -from unittest.mock import Mock from opentelemetry.sdk._metrics.aggregation import ( ExplicitBucketHistogramAggregation, LastValueAggregation, - SumAggregation, + SynchronousSumAggregation, ) +from opentelemetry.sdk._metrics.measurement import Measurement -class TestSumAggregation(TestCase): +class TestSynchronousSumAggregation(TestCase): def test_aggregate(self): """ - `SumAggregation` collects data for sum metric points + `SynchronousSumAggregation` collects data for sum metric points """ - sum_aggregation = SumAggregation(Mock()) + sum_aggregation = SynchronousSumAggregation(True) - sum_aggregation.aggregate(1) - sum_aggregation.aggregate(2) - sum_aggregation.aggregate(3) + sum_aggregation.aggregate(Measurement(1)) + sum_aggregation.aggregate(Measurement(2)) + sum_aggregation.aggregate(Measurement(3)) self.assertEqual(sum_aggregation.value, 6) - sum_aggregation = SumAggregation(Mock()) + sum_aggregation = SynchronousSumAggregation(True) - sum_aggregation.aggregate(1) - sum_aggregation.aggregate(-2) - sum_aggregation.aggregate(3) + sum_aggregation.aggregate(Measurement(1)) + sum_aggregation.aggregate(Measurement(-2)) + sum_aggregation.aggregate(Measurement(3)) self.assertEqual(sum_aggregation.value, 2) @@ -55,15 +55,15 @@ def test_aggregate(self): temporality """ - last_value_aggregation = LastValueAggregation(Mock()) + last_value_aggregation = LastValueAggregation(True) - last_value_aggregation.aggregate(1) + last_value_aggregation.aggregate(Measurement(1)) self.assertEqual(last_value_aggregation.value, 1) - last_value_aggregation.aggregate(2) + last_value_aggregation.aggregate(Measurement(2)) self.assertEqual(last_value_aggregation.value, 2) - last_value_aggregation.aggregate(3) + last_value_aggregation.aggregate(Measurement(3)) self.assertEqual(last_value_aggregation.value, 3) @@ -74,14 +74,14 @@ def test_aggregate(self): """ explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(Mock()) + ExplicitBucketHistogramAggregation(True) ) - explicit_bucket_histogram_aggregation.aggregate(-1) - explicit_bucket_histogram_aggregation.aggregate(2) - explicit_bucket_histogram_aggregation.aggregate(7) - explicit_bucket_histogram_aggregation.aggregate(8) - explicit_bucket_histogram_aggregation.aggregate(9999) + explicit_bucket_histogram_aggregation.aggregate(Measurement(-1)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(2)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(7)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(8)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(9999)) self.assertEqual(explicit_bucket_histogram_aggregation.value[0], -1) self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 2) @@ -97,27 +97,27 @@ def test_min_max(self): """ explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(Mock()) + ExplicitBucketHistogramAggregation(True) ) - explicit_bucket_histogram_aggregation.aggregate(-1) - explicit_bucket_histogram_aggregation.aggregate(2) - explicit_bucket_histogram_aggregation.aggregate(7) - explicit_bucket_histogram_aggregation.aggregate(8) - explicit_bucket_histogram_aggregation.aggregate(9999) + explicit_bucket_histogram_aggregation.aggregate(Measurement(-1)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(2)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(7)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(8)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(9999)) self.assertEqual(explicit_bucket_histogram_aggregation.min, -1) self.assertEqual(explicit_bucket_histogram_aggregation.max, 9999) explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(Mock(), record_min_max=False) + ExplicitBucketHistogramAggregation(True, record_min_max=False) ) - explicit_bucket_histogram_aggregation.aggregate(-1) - explicit_bucket_histogram_aggregation.aggregate(2) - explicit_bucket_histogram_aggregation.aggregate(7) - explicit_bucket_histogram_aggregation.aggregate(8) - explicit_bucket_histogram_aggregation.aggregate(9999) + explicit_bucket_histogram_aggregation.aggregate(Measurement(-1)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(2)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(7)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(8)) + explicit_bucket_histogram_aggregation.aggregate(Measurement(9999)) with self.assertLogs(level=WARNING): self.assertEqual(explicit_bucket_histogram_aggregation.min, inf) From bbe7097f097188ff72bda3fde11d835f934b6b61 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 13:23:45 -0600 Subject: [PATCH 02/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py Co-authored-by: Aaron Abbott --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py index 13fb5b30abf..424a5b544cd 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py @@ -21,7 +21,7 @@ @dataclass(frozen=True) class Measurement: value: Union[int, float] - attributes: Optional[Attributes] = None + attributes: Attributes = None # Can't make Exemplar a subclass of Measurement because that makes From fa2f9ce16329b839d41e0e4621b471d4a522b4cd Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 13:46:49 -0600 Subject: [PATCH 03/35] Removing is_monotonic property --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 525a46e6dd0..279f7753f22 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -43,10 +43,6 @@ def __init__(self, is_monotonic: bool): self._is_monotonic = is_monotonic self._lock = Lock() - @property - def is_monotonic(self): - return self._is_monotonic - @property def value(self): return self._value From 85965d3d2503384dd6a7eb3ae8548c9b905af626 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 13:52:09 -0600 Subject: [PATCH 04/35] Make collect public --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 279f7753f22..125d3ddebb4 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -52,7 +52,7 @@ def aggregate(self, measurement: Measurement) -> None: pass @abstractmethod - def _collect(self) -> Optional[_PointVarT]: + def collect(self) -> Optional[_PointVarT]: pass @@ -66,7 +66,7 @@ def aggregate(self, measurement: Measurement) -> None: with self._lock: self._value = self._value + measurement.value - def _collect(self): + def collect(self) -> Optional[_PointVarT]: now = _time_ns() with self._lock: @@ -91,7 +91,7 @@ def aggregate(self, measurement: Measurement) -> None: with self._lock: self._value = measurement.value - def _collect(self): + def collect(self) -> Optional[_PointVarT]: if self._value is None: return None @@ -109,7 +109,7 @@ def aggregate(self, measurement: Measurement): with self._lock: self._value = measurement.value - def _collect(self): + def collect(self) -> Optional[_PointVarT]: if self._value is None: return None @@ -170,7 +170,7 @@ def sum(self): ) return None - def aggregate(self, measurement: Measurement): + def aggregate(self, measurement: Measurement) -> None: value = measurement.value @@ -188,7 +188,7 @@ def aggregate(self, measurement: Measurement): break - def _collect(self): + def collect(self) -> Optional[_PointVarT]: now = _time_ns() with self._lock: From 7f185b40e910eeb5fbd828a605ba74cdc464fc65 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 14:58:37 -0600 Subject: [PATCH 05/35] Remove warning --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 125d3ddebb4..9e5e48564d3 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -164,10 +164,6 @@ def sum(self): if self._is_monotonic: return self._sum - _logger.warning( - "Sum is not filled out when the associated " - "instrument is not monotonic" - ) return None def aggregate(self, measurement: Measurement) -> None: From 4803336830372a5896e55b39fe0c9d746bb646d0 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 15:05:59 -0600 Subject: [PATCH 06/35] Add docstring --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 9e5e48564d3..ff36a9f9db8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -67,6 +67,10 @@ def aggregate(self, measurement: Measurement) -> None: self._value = self._value + measurement.value def collect(self) -> Optional[_PointVarT]: + """ + Atomically return a point for the current value of the metric and + reset the aggregation value. + """ now = _time_ns() with self._lock: @@ -92,6 +96,9 @@ def aggregate(self, measurement: Measurement) -> None: self._value = measurement.value def collect(self) -> Optional[_PointVarT]: + """ + Atomically return a point for the current value of the metric. + """ if self._value is None: return None @@ -110,6 +117,9 @@ def aggregate(self, measurement: Measurement): self._value = measurement.value def collect(self) -> Optional[_PointVarT]: + """ + Atomically return a point for the current value of the metric. + """ if self._value is None: return None @@ -185,6 +195,9 @@ def aggregate(self, measurement: Measurement) -> None: break def collect(self) -> Optional[_PointVarT]: + """ + Atomically return a point for the current value of the metric. + """ now = _time_ns() with self._lock: From eba1d8258215923ccebea48973838700f2719655 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 15:19:17 -0600 Subject: [PATCH 07/35] Removing properties --- .../opentelemetry/sdk/_metrics/aggregation.py | 21 ------------------- .../tests/metrics/test_aggregation.py | 12 ++++------- 2 files changed, 4 insertions(+), 29 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index ff36a9f9db8..2ac1075f202 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -155,27 +155,6 @@ def __init__( self._record_min_max = record_min_max self._start_time_unix_nano = _time_ns() - @property - def min(self): - if not self._record_min_max: - _logger.warning("Min is not being recorded") - - return self._min - - @property - def max(self): - if not self._record_min_max: - _logger.warning("Max is not being recorded") - - return self._max - - @property - def sum(self): - if self._is_monotonic: - return self._sum - - return None - def aggregate(self, measurement: Measurement) -> None: value = measurement.value diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 91be5514ddb..9ea756ad6f0 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -13,7 +13,6 @@ # limitations under the License. -from logging import WARNING from math import inf from unittest import TestCase @@ -106,8 +105,8 @@ def test_min_max(self): explicit_bucket_histogram_aggregation.aggregate(Measurement(8)) explicit_bucket_histogram_aggregation.aggregate(Measurement(9999)) - self.assertEqual(explicit_bucket_histogram_aggregation.min, -1) - self.assertEqual(explicit_bucket_histogram_aggregation.max, 9999) + self.assertEqual(explicit_bucket_histogram_aggregation._min, -1) + self.assertEqual(explicit_bucket_histogram_aggregation._max, 9999) explicit_bucket_histogram_aggregation = ( ExplicitBucketHistogramAggregation(True, record_min_max=False) @@ -119,8 +118,5 @@ def test_min_max(self): explicit_bucket_histogram_aggregation.aggregate(Measurement(8)) explicit_bucket_histogram_aggregation.aggregate(Measurement(9999)) - with self.assertLogs(level=WARNING): - self.assertEqual(explicit_bucket_histogram_aggregation.min, inf) - - with self.assertLogs(level=WARNING): - self.assertEqual(explicit_bucket_histogram_aggregation.max, -inf) + self.assertEqual(explicit_bucket_histogram_aggregation._min, inf) + self.assertEqual(explicit_bucket_histogram_aggregation._max, -inf) From 63fb67b3c53c7db8a7b7f1d3deef9a5bd41636c6 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 15:23:44 -0600 Subject: [PATCH 08/35] Change to delta --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 2ac1075f202..a24bf9a945f 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -187,5 +187,5 @@ def collect(self) -> Optional[_PointVarT]: start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=now, value=self._value, - aggregation_temporality=AGGREGATION_TEMPORALITY_CUMULATIVE, + aggregation_temporality=AGGREGATION_TEMPORALITY_DELTA, ) From 3a12e3f1efb0d35f9a41b410aa343fda6e02e99a Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 15:24:29 -0600 Subject: [PATCH 09/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py Co-authored-by: Aaron Abbott --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py | 1 - 1 file changed, 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py index 96d935a0753..0d4f466b96e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py @@ -38,7 +38,6 @@ class Gauge: @dataclass(frozen=True) class Histogram: - # NumberDataPoint attributes start_time_unix_nano: int time_unix_nano: int value: Union[int, float] From 6452eaecd63e87e1ac68f613026d169689fbdfbf Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 15:27:11 -0600 Subject: [PATCH 10/35] Remove comments --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py index 0d4f466b96e..a63f6a4bc07 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py @@ -18,20 +18,15 @@ @dataclass(frozen=True) class Sum: - # NumberDataPoint attributes start_time_unix_nano: int time_unix_nano: int value: Union[int, float] - # Sum attributes aggregation_temporality: int is_monotonic: bool @dataclass(frozen=True) class Gauge: - # NumberDataPoint attributes - # start_time_unix_nano is not added here because the proto says it is to - # be ignored. time_unix_nano: int value: Union[int, float] @@ -41,7 +36,6 @@ class Histogram: start_time_unix_nano: int time_unix_nano: int value: Union[int, float] - # Histogram attributes aggregation_temporality: int From 322e531c7e155df5b897ad67401a8cfe63b523f2 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 15:30:59 -0600 Subject: [PATCH 11/35] Remove Exemplars --- .../src/opentelemetry/sdk/_metrics/measurement.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py index 424a5b544cd..437eed5c73b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py @@ -22,15 +22,3 @@ class Measurement: value: Union[int, float] attributes: Attributes = None - - -# Can't make Exemplar a subclass of Measurement because that makes -# time_unix_nano to be defined after attribute, a keyword argument -@dataclass(frozen=True) -class Exemplar: - value: Union[int, float] - time_unix_nano: int - span_id: int - trace_id: int - attributes: Optional[Attributes] = None - filtered_attributes: Optional[Attributes] = None From 040f11913da81a0f834ac0375a4bb5f259cfc31c Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 15:32:59 -0600 Subject: [PATCH 12/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py Co-authored-by: Aaron Abbott --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py index a63f6a4bc07..c19d34c882d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py @@ -35,7 +35,8 @@ class Gauge: class Histogram: start_time_unix_nano: int time_unix_nano: int - value: Union[int, float] + bucket_counts: Sequence[int] + explicit_bounds: Sequence[float] aggregation_temporality: int From 628a067f32eab0b97e59f6c066bd7a641a7bf2c3 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 16:05:09 -0600 Subject: [PATCH 13/35] Adding AggregationTemporality enum --- .../opentelemetry/sdk/_metrics/aggregation.py | 25 +++++++++++-------- .../src/opentelemetry/sdk/_metrics/point.py | 2 +- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index a24bf9a945f..4e41c7a3759 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -18,19 +18,18 @@ from math import inf from threading import Lock from typing import Generic, Optional, Sequence, TypeVar +from enum import IntEnum from opentelemetry.sdk._metrics.measurement import Measurement from opentelemetry.sdk._metrics.point import Gauge, Histogram, PointT, Sum from opentelemetry.util._time import _time_ns -# FIXME this is being copied directly from -# opentelemetry.proto.metrics.v1.metrics_pb2. The only reason for doing so is -# to avoid havinv protobuf as a indirect dependency in the SDK. This -# duplication of code is not ideal. -AGGREGATION_TEMPORALITY_UNSPECIFIED = 0 -AGGREGATION_TEMPORALITY_DELTA = 1 -AGGREGATION_TEMPORALITY_CUMULATIVE = 2 +class AggregationTemporality(IntEnum): + AGGREGATION_TEMPORALITY_UNSPECIFIED = 0 + AGGREGATION_TEMPORALITY_DELTA = 1 + AGGREGATION_TEMPORALITY_CUMULATIVE = 2 + _PointVarT = TypeVar("_PointVarT", bound=PointT) @@ -78,7 +77,9 @@ def collect(self) -> Optional[_PointVarT]: self._start_time_unix_nano = now + 1 return Sum( - aggregation_temporality=AGGREGATION_TEMPORALITY_DELTA, + aggregation_temporality=( + AggregationTemporality.AGGREGATION_TEMPORALITY_DELTA + ), is_monotonic=self._is_monotonic, start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=now, @@ -106,7 +107,9 @@ def collect(self) -> Optional[_PointVarT]: start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=_time_ns(), value=self._value, - aggregation_temporality=AGGREGATION_TEMPORALITY_CUMULATIVE, + aggregation_temporality=( + AggregationTemporality.AGGREGATION_TEMPORALITY_CUMULATIVE + ), is_monotonic=self._is_monotonic, ) @@ -187,5 +190,7 @@ def collect(self) -> Optional[_PointVarT]: start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=now, value=self._value, - aggregation_temporality=AGGREGATION_TEMPORALITY_DELTA, + aggregation_temporality=( + AggregationTemporality.AGGREGATION_TEMPORALITY_DELTA + ), ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py index c19d34c882d..61049ea133b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py @@ -13,7 +13,7 @@ # limitations under the License. from dataclasses import dataclass -from typing import Union +from typing import Union, Sequence @dataclass(frozen=True) From d0787db1a40823641604c4d86af05719c2fe59fb Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 16:13:56 -0600 Subject: [PATCH 14/35] Removing Generic --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 4e41c7a3759..7586c07c104 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -17,7 +17,7 @@ from logging import getLogger from math import inf from threading import Lock -from typing import Generic, Optional, Sequence, TypeVar +from typing import Optional, Sequence, TypeVar from enum import IntEnum from opentelemetry.sdk._metrics.measurement import Measurement @@ -36,7 +36,7 @@ class AggregationTemporality(IntEnum): _logger = getLogger(__name__) -class Aggregation(ABC, Generic[_PointVarT]): +class Aggregation(ABC): def __init__(self, is_monotonic: bool): self._value = None self._is_monotonic = is_monotonic From 6965cad92f172b68438279dce2048c2be13d238f Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 17:00:55 -0600 Subject: [PATCH 15/35] Remove types from aggregations --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 7586c07c104..3afc7eb7223 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -55,7 +55,7 @@ def collect(self) -> Optional[_PointVarT]: pass -class SynchronousSumAggregation(Aggregation[Sum]): +class SynchronousSumAggregation(Aggregation): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) self._value = 0 @@ -87,7 +87,7 @@ def collect(self) -> Optional[_PointVarT]: ) -class AsynchronousSumAggregation(Aggregation[Sum]): +class AsynchronousSumAggregation(Aggregation): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) self._start_time_unix_nano = _time_ns() @@ -114,7 +114,7 @@ def collect(self) -> Optional[_PointVarT]: ) -class LastValueAggregation(Aggregation[Gauge]): +class LastValueAggregation(Aggregation): def aggregate(self, measurement: Measurement): with self._lock: self._value = measurement.value @@ -132,7 +132,7 @@ def collect(self) -> Optional[_PointVarT]: ) -class ExplicitBucketHistogramAggregation(Aggregation[Histogram]): +class ExplicitBucketHistogramAggregation(Aggregation): def __init__( self, is_monotonic: bool, From b1fd1fa85a81324a186a48e911dacfad269c7038 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 17:17:03 -0600 Subject: [PATCH 16/35] Fix lint --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 14 +++++++++----- .../src/opentelemetry/sdk/_metrics/measurement.py | 2 +- .../src/opentelemetry/sdk/_metrics/point.py | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 3afc7eb7223..a2a62792c7a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -14,11 +14,11 @@ from abc import ABC, abstractmethod from collections import OrderedDict +from enum import IntEnum from logging import getLogger from math import inf from threading import Lock from typing import Optional, Sequence, TypeVar -from enum import IntEnum from opentelemetry.sdk._metrics.measurement import Measurement from opentelemetry.sdk._metrics.point import Gauge, Histogram, PointT, Sum @@ -151,12 +151,15 @@ def __init__( record_min_max: bool = True, ): super().__init__(is_monotonic) - self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) + self._bucket_counts = OrderedDict( + [(key, 0) for key in (*boundaries, inf)] + ) self._min = inf self._max = -inf self._sum = 0 self._record_min_max = record_min_max self._start_time_unix_nano = _time_ns() + self._boundaries = boundaries def aggregate(self, measurement: Measurement) -> None: @@ -169,10 +172,10 @@ def aggregate(self, measurement: Measurement) -> None: if self._is_monotonic: self._sum += value - for key in self._value.keys(): + for key in self._bucket_counts.keys(): if value < key: - self._value[key] = self._value[key] + value + self._bucket_counts[key] = self._value[key] + 1 break @@ -189,7 +192,8 @@ def collect(self) -> Optional[_PointVarT]: return Histogram( start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=now, - value=self._value, + bucket_counts=self._bucket_counts, + explicit_bounds=self._boundaries, aggregation_temporality=( AggregationTemporality.AGGREGATION_TEMPORALITY_DELTA ), diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py index 437eed5c73b..18110b07430 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/measurement.py @@ -13,7 +13,7 @@ # limitations under the License. from dataclasses import dataclass -from typing import Optional, Union +from typing import Union from opentelemetry.util.types import Attributes diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py index 61049ea133b..2503bb43e3b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py @@ -13,7 +13,7 @@ # limitations under the License. from dataclasses import dataclass -from typing import Union, Sequence +from typing import Sequence, Union @dataclass(frozen=True) From e2c4badabc30e06c4fefb50973d1bbc6203d1ba5 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 17:28:11 -0600 Subject: [PATCH 17/35] Fix histogram test cases --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 10 ++++------ opentelemetry-sdk/tests/metrics/test_aggregation.py | 10 ++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index a2a62792c7a..354cc4df336 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -151,9 +151,7 @@ def __init__( record_min_max: bool = True, ): super().__init__(is_monotonic) - self._bucket_counts = OrderedDict( - [(key, 0) for key in (*boundaries, inf)] - ) + self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) self._min = inf self._max = -inf self._sum = 0 @@ -172,10 +170,10 @@ def aggregate(self, measurement: Measurement) -> None: if self._is_monotonic: self._sum += value - for key in self._bucket_counts.keys(): + for key in self._value.keys(): if value < key: - self._bucket_counts[key] = self._value[key] + 1 + self._value[key] = self._value[key] + 1 break @@ -192,7 +190,7 @@ def collect(self) -> Optional[_PointVarT]: return Histogram( start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=now, - bucket_counts=self._bucket_counts, + bucket_counts=tuple(self._value.values()), explicit_bounds=self._boundaries, aggregation_temporality=( AggregationTemporality.AGGREGATION_TEMPORALITY_DELTA diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 9ea756ad6f0..63bdad002d4 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -82,12 +82,10 @@ def test_aggregate(self): explicit_bucket_histogram_aggregation.aggregate(Measurement(8)) explicit_bucket_histogram_aggregation.aggregate(Measurement(9999)) - self.assertEqual(explicit_bucket_histogram_aggregation.value[0], -1) - self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 2) - self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 15) - self.assertEqual( - explicit_bucket_histogram_aggregation.value[inf], 9999 - ) + self.assertEqual(explicit_bucket_histogram_aggregation.value[0], 1) + self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 1) + self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 2) + self.assertEqual(explicit_bucket_histogram_aggregation.value[inf], 1) def test_min_max(self): """ From c313eb4c0e5d2c1e935a06278dc504a14c569ac8 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 17:40:13 -0600 Subject: [PATCH 18/35] Revert "Removing Generic" This reverts commit 132a0f7e93144b71c37968110139c9111fb7a969. --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 354cc4df336..facb7a8d030 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -18,7 +18,7 @@ from logging import getLogger from math import inf from threading import Lock -from typing import Optional, Sequence, TypeVar +from typing import Generic, Optional, Sequence, TypeVar from opentelemetry.sdk._metrics.measurement import Measurement from opentelemetry.sdk._metrics.point import Gauge, Histogram, PointT, Sum @@ -36,7 +36,7 @@ class AggregationTemporality(IntEnum): _logger = getLogger(__name__) -class Aggregation(ABC): +class Aggregation(ABC, Generic[_PointVarT]): def __init__(self, is_monotonic: bool): self._value = None self._is_monotonic = is_monotonic From 3712d38760d5b0f7f9e663b88204b237fb820130 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 17:42:23 -0600 Subject: [PATCH 19/35] Revert "Remove types from aggregations" This reverts commit 49b3d2a6e4ebabe45e643b41602bdfd51cbad4f5. --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index facb7a8d030..18f3f0468c0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -55,7 +55,7 @@ def collect(self) -> Optional[_PointVarT]: pass -class SynchronousSumAggregation(Aggregation): +class SynchronousSumAggregation(Aggregation[Sum]): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) self._value = 0 @@ -87,7 +87,7 @@ def collect(self) -> Optional[_PointVarT]: ) -class AsynchronousSumAggregation(Aggregation): +class AsynchronousSumAggregation(Aggregation[Sum]): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) self._start_time_unix_nano = _time_ns() @@ -114,7 +114,7 @@ def collect(self) -> Optional[_PointVarT]: ) -class LastValueAggregation(Aggregation): +class LastValueAggregation(Aggregation[Gauge]): def aggregate(self, measurement: Measurement): with self._lock: self._value = measurement.value @@ -132,7 +132,7 @@ def collect(self) -> Optional[_PointVarT]: ) -class ExplicitBucketHistogramAggregation(Aggregation): +class ExplicitBucketHistogramAggregation(Aggregation[Histogram]): def __init__( self, is_monotonic: bool, From c0d7f56d492e93d73c7657f912cefa6969e585e0 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 15 Dec 2021 17:49:38 -0600 Subject: [PATCH 20/35] Fix self._value --- .../opentelemetry/sdk/_metrics/aggregation.py | 10 +++++----- .../tests/metrics/test_aggregation.py | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 18f3f0468c0..1ec7a92c86d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -38,14 +38,9 @@ class AggregationTemporality(IntEnum): class Aggregation(ABC, Generic[_PointVarT]): def __init__(self, is_monotonic: bool): - self._value = None self._is_monotonic = is_monotonic self._lock = Lock() - @property - def value(self): - return self._value - @abstractmethod def aggregate(self, measurement: Measurement) -> None: pass @@ -90,6 +85,7 @@ def collect(self) -> Optional[_PointVarT]: class AsynchronousSumAggregation(Aggregation[Sum]): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) + self._value = 0 self._start_time_unix_nano = _time_ns() def aggregate(self, measurement: Measurement) -> None: @@ -115,6 +111,10 @@ def collect(self) -> Optional[_PointVarT]: class LastValueAggregation(Aggregation[Gauge]): + def __init__(self, is_monotonic: bool): + super().__init__(is_monotonic) + self._value = None + def aggregate(self, measurement: Measurement): with self._lock: self._value = measurement.value diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 63bdad002d4..5a6d4724280 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -36,7 +36,7 @@ def test_aggregate(self): sum_aggregation.aggregate(Measurement(2)) sum_aggregation.aggregate(Measurement(3)) - self.assertEqual(sum_aggregation.value, 6) + self.assertEqual(sum_aggregation._value, 6) sum_aggregation = SynchronousSumAggregation(True) @@ -44,7 +44,7 @@ def test_aggregate(self): sum_aggregation.aggregate(Measurement(-2)) sum_aggregation.aggregate(Measurement(3)) - self.assertEqual(sum_aggregation.value, 2) + self.assertEqual(sum_aggregation._value, 2) class TestLastValueAggregation(TestCase): @@ -57,13 +57,13 @@ def test_aggregate(self): last_value_aggregation = LastValueAggregation(True) last_value_aggregation.aggregate(Measurement(1)) - self.assertEqual(last_value_aggregation.value, 1) + self.assertEqual(last_value_aggregation._value, 1) last_value_aggregation.aggregate(Measurement(2)) - self.assertEqual(last_value_aggregation.value, 2) + self.assertEqual(last_value_aggregation._value, 2) last_value_aggregation.aggregate(Measurement(3)) - self.assertEqual(last_value_aggregation.value, 3) + self.assertEqual(last_value_aggregation._value, 3) class TestExplicitBucketHistogramAggregation(TestCase): @@ -82,10 +82,10 @@ def test_aggregate(self): explicit_bucket_histogram_aggregation.aggregate(Measurement(8)) explicit_bucket_histogram_aggregation.aggregate(Measurement(9999)) - self.assertEqual(explicit_bucket_histogram_aggregation.value[0], 1) - self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 1) - self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 2) - self.assertEqual(explicit_bucket_histogram_aggregation.value[inf], 1) + self.assertEqual(explicit_bucket_histogram_aggregation._value[0], 1) + self.assertEqual(explicit_bucket_histogram_aggregation._value[5], 1) + self.assertEqual(explicit_bucket_histogram_aggregation._value[10], 2) + self.assertEqual(explicit_bucket_histogram_aggregation._value[inf], 1) def test_min_max(self): """ From 74d6c8da347dea4334db17a589f64cfb9911d073 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 10:13:43 -0600 Subject: [PATCH 21/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Co-authored-by: Aaron Abbott --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 1ec7a92c86d..ef495509f12 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -177,7 +177,7 @@ def aggregate(self, measurement: Measurement) -> None: break - def collect(self) -> Optional[_PointVarT]: + def collect(self) -> Optional[Histogram]: """ Atomically return a point for the current value of the metric. """ From 59bd4aacf4979a300c07f9ea1d76aaa3304612cd Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 10:13:51 -0600 Subject: [PATCH 22/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Co-authored-by: Aaron Abbott --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index ef495509f12..9da9b5fef22 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -119,7 +119,7 @@ def aggregate(self, measurement: Measurement): with self._lock: self._value = measurement.value - def collect(self) -> Optional[_PointVarT]: + def collect(self) -> Optional[Gauge]: """ Atomically return a point for the current value of the metric. """ From e0a78dbd9990dbcd799cce5f8954bf47ea919be4 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 10:13:58 -0600 Subject: [PATCH 23/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Co-authored-by: Aaron Abbott --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 9da9b5fef22..7eaac4e8302 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -92,7 +92,7 @@ def aggregate(self, measurement: Measurement) -> None: with self._lock: self._value = measurement.value - def collect(self) -> Optional[_PointVarT]: + def collect(self) -> Optional[Sum]: """ Atomically return a point for the current value of the metric. """ From a347bb485af8524d087712bb9ffee2d39236b752 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 10:14:31 -0600 Subject: [PATCH 24/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Co-authored-by: Aaron Abbott --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 7eaac4e8302..1393b6618c7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -60,7 +60,7 @@ def aggregate(self, measurement: Measurement) -> None: with self._lock: self._value = self._value + measurement.value - def collect(self) -> Optional[_PointVarT]: + def collect(self) -> Optional[Sum]: """ Atomically return a point for the current value of the metric and reset the aggregation value. From 8595ad8e70758bb94953723f4f827e28f67e4577 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 10:15:31 -0600 Subject: [PATCH 25/35] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Co-authored-by: Srikanth Chekuri --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 1393b6618c7..fce27a037d9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -184,7 +184,7 @@ def collect(self) -> Optional[Histogram]: now = _time_ns() with self._lock: - self._value = 0 + self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) self._start_time_unix_nano = now + 1 return Histogram( From eb67095c0380a8b9a89ae592c91daf4a50e3ae8c Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 10:17:15 -0600 Subject: [PATCH 26/35] Use self._boundaries --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index fce27a037d9..4879fc3e881 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -184,7 +184,9 @@ def collect(self) -> Optional[Histogram]: now = _time_ns() with self._lock: - self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) + self._value = OrderedDict( + [(key, 0) for key in (*self._boundaries, inf)] + ) self._start_time_unix_nano = now + 1 return Histogram( From 38ebe4a08cb791161b7851023146dcd40c767bf7 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 10:26:55 -0600 Subject: [PATCH 27/35] Remove aggregation temporality prefix --- .../opentelemetry/sdk/_metrics/aggregation.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 4879fc3e881..4d2cbfc5a30 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -26,9 +26,9 @@ class AggregationTemporality(IntEnum): - AGGREGATION_TEMPORALITY_UNSPECIFIED = 0 - AGGREGATION_TEMPORALITY_DELTA = 1 - AGGREGATION_TEMPORALITY_CUMULATIVE = 2 + UNSPECIFIED = 0 + DELTA = 1 + CUMULATIVE = 2 _PointVarT = TypeVar("_PointVarT", bound=PointT) @@ -72,9 +72,7 @@ def collect(self) -> Optional[Sum]: self._start_time_unix_nano = now + 1 return Sum( - aggregation_temporality=( - AggregationTemporality.AGGREGATION_TEMPORALITY_DELTA - ), + aggregation_temporality=(AggregationTemporality.DELTA), is_monotonic=self._is_monotonic, start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=now, @@ -103,9 +101,7 @@ def collect(self) -> Optional[Sum]: start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=_time_ns(), value=self._value, - aggregation_temporality=( - AggregationTemporality.AGGREGATION_TEMPORALITY_CUMULATIVE - ), + aggregation_temporality=(AggregationTemporality.CUMULATIVE), is_monotonic=self._is_monotonic, ) @@ -194,7 +190,5 @@ def collect(self) -> Optional[Histogram]: time_unix_nano=now, bucket_counts=tuple(self._value.values()), explicit_bounds=self._boundaries, - aggregation_temporality=( - AggregationTemporality.AGGREGATION_TEMPORALITY_DELTA - ), + aggregation_temporality=(AggregationTemporality.DELTA), ) From 66ce90893f76dec469175d069a71cea43b1c75c3 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 18:59:13 -0600 Subject: [PATCH 28/35] Add MonotonicitySensitiveAggregation --- .../opentelemetry/sdk/_metrics/aggregation.py | 23 ++++++--- .../tests/metrics/test_aggregation.py | 47 +++++++++++++++++-- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 4d2cbfc5a30..b6fc63f2637 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -36,9 +36,14 @@ class AggregationTemporality(IntEnum): _logger = getLogger(__name__) -class Aggregation(ABC, Generic[_PointVarT]): +class _MonotonicitySensitiveAggregation: def __init__(self, is_monotonic: bool): self._is_monotonic = is_monotonic + super().__init__() + + +class Aggregation(ABC, Generic[_PointVarT]): + def __init__(self): self._lock = Lock() @abstractmethod @@ -50,7 +55,9 @@ def collect(self) -> Optional[_PointVarT]: pass -class SynchronousSumAggregation(Aggregation[Sum]): +class SynchronousSumAggregation( + _MonotonicitySensitiveAggregation, Aggregation[Sum] +): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) self._value = 0 @@ -80,7 +87,9 @@ def collect(self) -> Optional[Sum]: ) -class AsynchronousSumAggregation(Aggregation[Sum]): +class AsynchronousSumAggregation( + _MonotonicitySensitiveAggregation, Aggregation[Sum] +): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) self._value = 0 @@ -107,8 +116,8 @@ def collect(self) -> Optional[Sum]: class LastValueAggregation(Aggregation[Gauge]): - def __init__(self, is_monotonic: bool): - super().__init__(is_monotonic) + def __init__(self): + super().__init__() self._value = None def aggregate(self, measurement: Measurement): @@ -128,7 +137,9 @@ def collect(self) -> Optional[Gauge]: ) -class ExplicitBucketHistogramAggregation(Aggregation[Histogram]): +class ExplicitBucketHistogramAggregation( + _MonotonicitySensitiveAggregation, Aggregation[Histogram] +): def __init__( self, is_monotonic: bool, diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 5a6d4724280..1287c9edede 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -15,22 +15,38 @@ from math import inf from unittest import TestCase +from unittest.mock import Mock from opentelemetry.sdk._metrics.aggregation import ( ExplicitBucketHistogramAggregation, LastValueAggregation, SynchronousSumAggregation, + _MonotonicitySensitiveAggregation, ) from opentelemetry.sdk._metrics.measurement import Measurement class TestSynchronousSumAggregation(TestCase): + def test_monotonicity_sensitivity(self): + """ + `SynchronousSumAggregation` is sensitive to instrument monotonicity + """ + + sum_aggregation = SynchronousSumAggregation(True) + self.assertIsInstance( + sum_aggregation, _MonotonicitySensitiveAggregation + ) + self.assertTrue(sum_aggregation._is_monotonic) + + sum_aggregation = SynchronousSumAggregation(False) + self.assertFalse(sum_aggregation._is_monotonic) + def test_aggregate(self): """ `SynchronousSumAggregation` collects data for sum metric points """ - sum_aggregation = SynchronousSumAggregation(True) + sum_aggregation = SynchronousSumAggregation(Mock()) sum_aggregation.aggregate(Measurement(1)) sum_aggregation.aggregate(Measurement(2)) @@ -38,7 +54,7 @@ def test_aggregate(self): self.assertEqual(sum_aggregation._value, 6) - sum_aggregation = SynchronousSumAggregation(True) + sum_aggregation = SynchronousSumAggregation(Mock()) sum_aggregation.aggregate(Measurement(1)) sum_aggregation.aggregate(Measurement(-2)) @@ -48,13 +64,23 @@ def test_aggregate(self): class TestLastValueAggregation(TestCase): + def test_monotonicity_sensitivity(self): + """ + `LastValueAggregation` is not sensitive to instrument monotonicity + """ + + sum_aggregation = LastValueAggregation() + self.assertNotIsInstance( + sum_aggregation, _MonotonicitySensitiveAggregation + ) + def test_aggregate(self): """ `LastValueAggregation` collects data for gauge metric points with delta temporality """ - last_value_aggregation = LastValueAggregation(True) + last_value_aggregation = LastValueAggregation() last_value_aggregation.aggregate(Measurement(1)) self.assertEqual(last_value_aggregation._value, 1) @@ -67,6 +93,21 @@ def test_aggregate(self): class TestExplicitBucketHistogramAggregation(TestCase): + def test_monotonicity_sensitivity(self): + """ + `ExplicitBucketHistogramAggregation` is sensitive to instrument + monotonicity + """ + + sum_aggregation = ExplicitBucketHistogramAggregation(True) + self.assertIsInstance( + sum_aggregation, _MonotonicitySensitiveAggregation + ) + self.assertTrue(sum_aggregation._is_monotonic) + + sum_aggregation = ExplicitBucketHistogramAggregation(False) + self.assertFalse(sum_aggregation._is_monotonic) + def test_aggregate(self): """ `ExplicitBucketHistogramAggregation` collects data for explicit_bucket_histogram metric points From 6c84c66b012be27035e87f094d2a59833110f4ac Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 20:21:36 -0600 Subject: [PATCH 29/35] Add collect test cases --- .../opentelemetry/sdk/_metrics/aggregation.py | 22 ++- .../tests/metrics/test_aggregation.py | 175 ++++++++++++++++-- 2 files changed, 172 insertions(+), 25 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index b6fc63f2637..42f4d00ef2e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -75,15 +75,18 @@ def collect(self) -> Optional[Sum]: now = _time_ns() with self._lock: + value = self._value + start_time_unix_nano = self._start_time_unix_nano + self._value = 0 self._start_time_unix_nano = now + 1 return Sum( - aggregation_temporality=(AggregationTemporality.DELTA), + aggregation_temporality=AggregationTemporality.DELTA, is_monotonic=self._is_monotonic, - start_time_unix_nano=self._start_time_unix_nano, + start_time_unix_nano=start_time_unix_nano, time_unix_nano=now, - value=self._value, + value=value, ) @@ -92,7 +95,7 @@ class AsynchronousSumAggregation( ): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) - self._value = 0 + self._value = None self._start_time_unix_nano = _time_ns() def aggregate(self, measurement: Measurement) -> None: @@ -110,7 +113,7 @@ def collect(self) -> Optional[Sum]: start_time_unix_nano=self._start_time_unix_nano, time_unix_nano=_time_ns(), value=self._value, - aggregation_temporality=(AggregationTemporality.CUMULATIVE), + aggregation_temporality=AggregationTemporality.CUMULATIVE, is_monotonic=self._is_monotonic, ) @@ -191,15 +194,18 @@ def collect(self) -> Optional[Histogram]: now = _time_ns() with self._lock: + value = self._value + start_time_unix_nano = self._start_time_unix_nano + self._value = OrderedDict( [(key, 0) for key in (*self._boundaries, inf)] ) self._start_time_unix_nano = now + 1 return Histogram( - start_time_unix_nano=self._start_time_unix_nano, + start_time_unix_nano=start_time_unix_nano, time_unix_nano=now, - bucket_counts=tuple(self._value.values()), + bucket_counts=tuple(value.values()), explicit_bounds=self._boundaries, - aggregation_temporality=(AggregationTemporality.DELTA), + aggregation_temporality=AggregationTemporality.DELTA, ) diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 1287c9edede..67e44ffc2d6 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -15,9 +15,9 @@ from math import inf from unittest import TestCase -from unittest.mock import Mock from opentelemetry.sdk._metrics.aggregation import ( + AsynchronousSumAggregation, ExplicitBucketHistogramAggregation, LastValueAggregation, SynchronousSumAggregation, @@ -32,35 +32,126 @@ def test_monotonicity_sensitivity(self): `SynchronousSumAggregation` is sensitive to instrument monotonicity """ - sum_aggregation = SynchronousSumAggregation(True) + synchronous_sum_aggregation = SynchronousSumAggregation(True) self.assertIsInstance( - sum_aggregation, _MonotonicitySensitiveAggregation + synchronous_sum_aggregation, _MonotonicitySensitiveAggregation ) - self.assertTrue(sum_aggregation._is_monotonic) + self.assertTrue(synchronous_sum_aggregation._is_monotonic) - sum_aggregation = SynchronousSumAggregation(False) - self.assertFalse(sum_aggregation._is_monotonic) + synchronous_sum_aggregation = SynchronousSumAggregation(False) + self.assertFalse(synchronous_sum_aggregation._is_monotonic) def test_aggregate(self): """ - `SynchronousSumAggregation` collects data for sum metric points + `SynchronousSumAggregation` aggregates data for sum metric points + """ + + synchronous_sum_aggregation = SynchronousSumAggregation(True) + + synchronous_sum_aggregation.aggregate(Measurement(1)) + synchronous_sum_aggregation.aggregate(Measurement(2)) + synchronous_sum_aggregation.aggregate(Measurement(3)) + + self.assertEqual(synchronous_sum_aggregation._value, 6) + + synchronous_sum_aggregation = SynchronousSumAggregation(True) + + synchronous_sum_aggregation.aggregate(Measurement(1)) + synchronous_sum_aggregation.aggregate(Measurement(-2)) + synchronous_sum_aggregation.aggregate(Measurement(3)) + + self.assertEqual(synchronous_sum_aggregation._value, 2) + + def test_collect(self): + """ + `SynchronousSumAggregation` collects sum metric points + """ + + synchronous_sum_aggregation = SynchronousSumAggregation(True) + + synchronous_sum_aggregation.aggregate(Measurement(1)) + first_sum = synchronous_sum_aggregation.collect() + + self.assertEquals(first_sum.value, 1) + self.assertTrue(first_sum.is_monotonic) + + synchronous_sum_aggregation.aggregate(Measurement(1)) + second_sum = synchronous_sum_aggregation.collect() + + self.assertEquals(second_sum.value, 1) + self.assertTrue(second_sum.is_monotonic) + + self.assertGreater( + second_sum.start_time_unix_nano, first_sum.start_time_unix_nano + ) + + +class TestAsynchronousSumAggregation(TestCase): + def test_monotonicity_sensitivity(self): + """ + `AsynchronousSumAggregation` is sensitive to instrument monotonicity + """ + + asynchronous_sum_aggregation = AsynchronousSumAggregation(True) + self.assertIsInstance( + asynchronous_sum_aggregation, _MonotonicitySensitiveAggregation + ) + self.assertTrue(asynchronous_sum_aggregation._is_monotonic) + + asynchronous_sum_aggregation = AsynchronousSumAggregation(False) + self.assertFalse(asynchronous_sum_aggregation._is_monotonic) + + def test_aggregate(self): + """ + `AsynchronousSumAggregation` aggregates data for sum metric points + """ + + asynchronous_sum_aggregation = AsynchronousSumAggregation(True) + + asynchronous_sum_aggregation.aggregate(Measurement(1)) + self.assertEqual(asynchronous_sum_aggregation._value, 1) + + asynchronous_sum_aggregation.aggregate(Measurement(2)) + self.assertEqual(asynchronous_sum_aggregation._value, 2) + + asynchronous_sum_aggregation.aggregate(Measurement(3)) + self.assertEqual(asynchronous_sum_aggregation._value, 3) + + asynchronous_sum_aggregation = AsynchronousSumAggregation(True) + + asynchronous_sum_aggregation.aggregate(Measurement(1)) + self.assertEqual(asynchronous_sum_aggregation._value, 1) + + asynchronous_sum_aggregation.aggregate(Measurement(-2)) + self.assertEqual(asynchronous_sum_aggregation._value, -2) + + asynchronous_sum_aggregation.aggregate(Measurement(3)) + self.assertEqual(asynchronous_sum_aggregation._value, 3) + + def test_collect(self): + """ + `AsynchronousSumAggregation` collects sum metric points """ - sum_aggregation = SynchronousSumAggregation(Mock()) + asynchronous_sum_aggregation = AsynchronousSumAggregation(True) + + self.assertIsNone(asynchronous_sum_aggregation.collect()) - sum_aggregation.aggregate(Measurement(1)) - sum_aggregation.aggregate(Measurement(2)) - sum_aggregation.aggregate(Measurement(3)) + asynchronous_sum_aggregation.aggregate(Measurement(1)) + first_sum = asynchronous_sum_aggregation.collect() - self.assertEqual(sum_aggregation._value, 6) + self.assertEquals(first_sum.value, 1) + self.assertTrue(first_sum.is_monotonic) - sum_aggregation = SynchronousSumAggregation(Mock()) + asynchronous_sum_aggregation.aggregate(Measurement(1)) + second_sum = asynchronous_sum_aggregation.collect() - sum_aggregation.aggregate(Measurement(1)) - sum_aggregation.aggregate(Measurement(-2)) - sum_aggregation.aggregate(Measurement(3)) + self.assertEquals(second_sum.value, 1) + self.assertTrue(second_sum.is_monotonic) - self.assertEqual(sum_aggregation._value, 2) + self.assertEqual( + second_sum.start_time_unix_nano, first_sum.start_time_unix_nano + ) class TestLastValueAggregation(TestCase): @@ -91,6 +182,29 @@ def test_aggregate(self): last_value_aggregation.aggregate(Measurement(3)) self.assertEqual(last_value_aggregation._value, 3) + def test_collect(self): + """ + `LastValueAggregation` collects sum metric points + """ + + last_value_aggregation = LastValueAggregation() + + self.assertIsNone(last_value_aggregation.collect()) + + last_value_aggregation.aggregate(Measurement(1)) + first_gauge = last_value_aggregation.collect() + + self.assertEquals(first_gauge.value, 1) + + last_value_aggregation.aggregate(Measurement(1)) + second_gauge = last_value_aggregation.collect() + + self.assertEquals(second_gauge.value, 1) + + self.assertGreater( + second_gauge.time_unix_nano, first_gauge.time_unix_nano + ) + class TestExplicitBucketHistogramAggregation(TestCase): def test_monotonicity_sensitivity(self): @@ -159,3 +273,30 @@ def test_min_max(self): self.assertEqual(explicit_bucket_histogram_aggregation._min, inf) self.assertEqual(explicit_bucket_histogram_aggregation._max, -inf) + + def test_collect(self): + """ + `ExplicitBucketHistogramAggregation` collects sum metric points + """ + + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation(True) + ) + + explicit_bucket_histogram_aggregation.aggregate(Measurement(1)) + first_histogram = explicit_bucket_histogram_aggregation.collect() + + self.assertEquals( + first_histogram.bucket_counts, (0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0) + ) + + explicit_bucket_histogram_aggregation.aggregate(Measurement(1)) + second_histogram = explicit_bucket_histogram_aggregation.collect() + + self.assertEquals( + second_histogram.bucket_counts, (0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0) + ) + + self.assertGreater( + second_histogram.time_unix_nano, first_histogram.time_unix_nano + ) From 8ab680dbda05929da8905af89e72fa20dccdf300 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 20:30:04 -0600 Subject: [PATCH 30/35] Use assertEqual instead --- .../tests/metrics/test_aggregation.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 67e44ffc2d6..54db8355a9a 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -72,13 +72,13 @@ def test_collect(self): synchronous_sum_aggregation.aggregate(Measurement(1)) first_sum = synchronous_sum_aggregation.collect() - self.assertEquals(first_sum.value, 1) + self.assertEqual(first_sum.value, 1) self.assertTrue(first_sum.is_monotonic) synchronous_sum_aggregation.aggregate(Measurement(1)) second_sum = synchronous_sum_aggregation.collect() - self.assertEquals(second_sum.value, 1) + self.assertEqual(second_sum.value, 1) self.assertTrue(second_sum.is_monotonic) self.assertGreater( @@ -140,13 +140,13 @@ def test_collect(self): asynchronous_sum_aggregation.aggregate(Measurement(1)) first_sum = asynchronous_sum_aggregation.collect() - self.assertEquals(first_sum.value, 1) + self.assertEqual(first_sum.value, 1) self.assertTrue(first_sum.is_monotonic) asynchronous_sum_aggregation.aggregate(Measurement(1)) second_sum = asynchronous_sum_aggregation.collect() - self.assertEquals(second_sum.value, 1) + self.assertEqual(second_sum.value, 1) self.assertTrue(second_sum.is_monotonic) self.assertEqual( @@ -194,12 +194,12 @@ def test_collect(self): last_value_aggregation.aggregate(Measurement(1)) first_gauge = last_value_aggregation.collect() - self.assertEquals(first_gauge.value, 1) + self.assertEqual(first_gauge.value, 1) last_value_aggregation.aggregate(Measurement(1)) second_gauge = last_value_aggregation.collect() - self.assertEquals(second_gauge.value, 1) + self.assertEqual(second_gauge.value, 1) self.assertGreater( second_gauge.time_unix_nano, first_gauge.time_unix_nano @@ -286,14 +286,14 @@ def test_collect(self): explicit_bucket_histogram_aggregation.aggregate(Measurement(1)) first_histogram = explicit_bucket_histogram_aggregation.collect() - self.assertEquals( + self.assertEqual( first_histogram.bucket_counts, (0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0) ) explicit_bucket_histogram_aggregation.aggregate(Measurement(1)) second_histogram = explicit_bucket_histogram_aggregation.collect() - self.assertEquals( + self.assertEqual( second_histogram.bucket_counts, (0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0) ) From 0ee91c426fa867be210a3a865b7916e46a0c5b01 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 20:42:16 -0600 Subject: [PATCH 31/35] Add delay --- opentelemetry-sdk/tests/metrics/test_aggregation.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 54db8355a9a..e0303730415 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -14,6 +14,7 @@ from math import inf +from time import sleep from unittest import TestCase from opentelemetry.sdk._metrics.aggregation import ( @@ -197,6 +198,10 @@ def test_collect(self): self.assertEqual(first_gauge.value, 1) last_value_aggregation.aggregate(Measurement(1)) + + # CI fails the last assertion without this + sleep(0.1) + second_gauge = last_value_aggregation.collect() self.assertEqual(second_gauge.value, 1) @@ -290,6 +295,9 @@ def test_collect(self): first_histogram.bucket_counts, (0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0) ) + # CI fails the last assertion without this + sleep(0.1) + explicit_bucket_histogram_aggregation.aggregate(Measurement(1)) second_histogram = explicit_bucket_histogram_aggregation.collect() From 3c68beb67d8745389cc3ad6643618147dcc32892 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 16 Dec 2021 20:57:07 -0600 Subject: [PATCH 32/35] Use Aware instead of Sensitive --- .../opentelemetry/sdk/_metrics/aggregation.py | 8 ++--- .../tests/metrics/test_aggregation.py | 31 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 42f4d00ef2e..1a7a088fe90 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -36,7 +36,7 @@ class AggregationTemporality(IntEnum): _logger = getLogger(__name__) -class _MonotonicitySensitiveAggregation: +class _MonotonicityAwareAggregation: def __init__(self, is_monotonic: bool): self._is_monotonic = is_monotonic super().__init__() @@ -56,7 +56,7 @@ def collect(self) -> Optional[_PointVarT]: class SynchronousSumAggregation( - _MonotonicitySensitiveAggregation, Aggregation[Sum] + _MonotonicityAwareAggregation, Aggregation[Sum] ): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) @@ -91,7 +91,7 @@ def collect(self) -> Optional[Sum]: class AsynchronousSumAggregation( - _MonotonicitySensitiveAggregation, Aggregation[Sum] + _MonotonicityAwareAggregation, Aggregation[Sum] ): def __init__(self, is_monotonic: bool): super().__init__(is_monotonic) @@ -141,7 +141,7 @@ def collect(self) -> Optional[Gauge]: class ExplicitBucketHistogramAggregation( - _MonotonicitySensitiveAggregation, Aggregation[Histogram] + _MonotonicityAwareAggregation, Aggregation[Histogram] ): def __init__( self, diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index e0303730415..486b6ca7c63 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -22,20 +22,20 @@ ExplicitBucketHistogramAggregation, LastValueAggregation, SynchronousSumAggregation, - _MonotonicitySensitiveAggregation, + _MonotonicityAwareAggregation, ) from opentelemetry.sdk._metrics.measurement import Measurement class TestSynchronousSumAggregation(TestCase): - def test_monotonicity_sensitivity(self): + def test_monotonicity_awareness(self): """ - `SynchronousSumAggregation` is sensitive to instrument monotonicity + `SynchronousSumAggregation` is aware of the instrument monotonicity """ synchronous_sum_aggregation = SynchronousSumAggregation(True) self.assertIsInstance( - synchronous_sum_aggregation, _MonotonicitySensitiveAggregation + synchronous_sum_aggregation, _MonotonicityAwareAggregation ) self.assertTrue(synchronous_sum_aggregation._is_monotonic) @@ -88,14 +88,14 @@ def test_collect(self): class TestAsynchronousSumAggregation(TestCase): - def test_monotonicity_sensitivity(self): + def test_monotonicity_awareness(self): """ - `AsynchronousSumAggregation` is sensitive to instrument monotonicity + `AsynchronousSumAggregation` is aware of the instrument monotonicity """ asynchronous_sum_aggregation = AsynchronousSumAggregation(True) self.assertIsInstance( - asynchronous_sum_aggregation, _MonotonicitySensitiveAggregation + asynchronous_sum_aggregation, _MonotonicityAwareAggregation ) self.assertTrue(asynchronous_sum_aggregation._is_monotonic) @@ -156,14 +156,14 @@ def test_collect(self): class TestLastValueAggregation(TestCase): - def test_monotonicity_sensitivity(self): + def test_monotonicity_awareness(self): """ - `LastValueAggregation` is not sensitive to instrument monotonicity + `LastValueAggregation` is not aware of the instrument monotonicity """ sum_aggregation = LastValueAggregation() self.assertNotIsInstance( - sum_aggregation, _MonotonicitySensitiveAggregation + sum_aggregation, _MonotonicityAwareAggregation ) def test_aggregate(self): @@ -212,16 +212,14 @@ def test_collect(self): class TestExplicitBucketHistogramAggregation(TestCase): - def test_monotonicity_sensitivity(self): + def test_monotonicity_awareness(self): """ - `ExplicitBucketHistogramAggregation` is sensitive to instrument + `ExplicitBucketHistogramAggregation` is aware of the instrument monotonicity """ sum_aggregation = ExplicitBucketHistogramAggregation(True) - self.assertIsInstance( - sum_aggregation, _MonotonicitySensitiveAggregation - ) + self.assertIsInstance(sum_aggregation, _MonotonicityAwareAggregation) self.assertTrue(sum_aggregation._is_monotonic) sum_aggregation = ExplicitBucketHistogramAggregation(False) @@ -229,7 +227,8 @@ def test_monotonicity_sensitivity(self): def test_aggregate(self): """ - `ExplicitBucketHistogramAggregation` collects data for explicit_bucket_histogram metric points + `ExplicitBucketHistogramAggregation` collects data for + explicit bucket histogram metric points """ explicit_bucket_histogram_aggregation = ( From c40bfd639ca9b9b7c89c5095312cb6b72e8025ad Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 20 Dec 2021 18:33:18 -0600 Subject: [PATCH 33/35] Rename is_monotonic --- .../opentelemetry/sdk/_metrics/aggregation.py | 30 ++++++++-------- .../tests/metrics/test_aggregation.py | 34 +++++++++++-------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 1a7a088fe90..94cd1edf032 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -36,9 +36,9 @@ class AggregationTemporality(IntEnum): _logger = getLogger(__name__) -class _MonotonicityAwareAggregation: - def __init__(self, is_monotonic: bool): - self._is_monotonic = is_monotonic +class _InstrumentMonotonicityAwareAggregation: + def __init__(self, instrument_is_monotonic: bool): + self._instrument_is_monotonic = instrument_is_monotonic super().__init__() @@ -56,10 +56,10 @@ def collect(self) -> Optional[_PointVarT]: class SynchronousSumAggregation( - _MonotonicityAwareAggregation, Aggregation[Sum] + _InstrumentMonotonicityAwareAggregation, Aggregation[Sum] ): - def __init__(self, is_monotonic: bool): - super().__init__(is_monotonic) + def __init__(self, instrument_is_monotonic: bool): + super().__init__(instrument_is_monotonic) self._value = 0 self._start_time_unix_nano = _time_ns() @@ -83,7 +83,7 @@ def collect(self) -> Optional[Sum]: return Sum( aggregation_temporality=AggregationTemporality.DELTA, - is_monotonic=self._is_monotonic, + is_monotonic=self._instrument_is_monotonic, start_time_unix_nano=start_time_unix_nano, time_unix_nano=now, value=value, @@ -91,10 +91,10 @@ def collect(self) -> Optional[Sum]: class AsynchronousSumAggregation( - _MonotonicityAwareAggregation, Aggregation[Sum] + _InstrumentMonotonicityAwareAggregation, Aggregation[Sum] ): - def __init__(self, is_monotonic: bool): - super().__init__(is_monotonic) + def __init__(self, instrument_is_monotonic: bool): + super().__init__(instrument_is_monotonic) self._value = None self._start_time_unix_nano = _time_ns() @@ -114,7 +114,7 @@ def collect(self) -> Optional[Sum]: time_unix_nano=_time_ns(), value=self._value, aggregation_temporality=AggregationTemporality.CUMULATIVE, - is_monotonic=self._is_monotonic, + is_monotonic=self._instrument_is_monotonic, ) @@ -141,11 +141,11 @@ def collect(self) -> Optional[Gauge]: class ExplicitBucketHistogramAggregation( - _MonotonicityAwareAggregation, Aggregation[Histogram] + _InstrumentMonotonicityAwareAggregation, Aggregation[Histogram] ): def __init__( self, - is_monotonic: bool, + instrument_is_monotonic: bool, boundaries: Sequence[int] = ( 0, 5, @@ -160,7 +160,7 @@ def __init__( ), record_min_max: bool = True, ): - super().__init__(is_monotonic) + super().__init__(instrument_is_monotonic) self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) self._min = inf self._max = -inf @@ -177,7 +177,7 @@ def aggregate(self, measurement: Measurement) -> None: self._min = min(self._min, value) self._max = max(self._max, value) - if self._is_monotonic: + if self._instrument_is_monotonic: self._sum += value for key in self._value.keys(): diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 486b6ca7c63..6da9b28a28e 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -22,25 +22,26 @@ ExplicitBucketHistogramAggregation, LastValueAggregation, SynchronousSumAggregation, - _MonotonicityAwareAggregation, + _InstrumentMonotonicityAwareAggregation, ) from opentelemetry.sdk._metrics.measurement import Measurement class TestSynchronousSumAggregation(TestCase): - def test_monotonicity_awareness(self): + def test_instrument_monotonicity_awareness(self): """ `SynchronousSumAggregation` is aware of the instrument monotonicity """ synchronous_sum_aggregation = SynchronousSumAggregation(True) self.assertIsInstance( - synchronous_sum_aggregation, _MonotonicityAwareAggregation + synchronous_sum_aggregation, + _InstrumentMonotonicityAwareAggregation, ) - self.assertTrue(synchronous_sum_aggregation._is_monotonic) + self.assertTrue(synchronous_sum_aggregation._instrument_is_monotonic) synchronous_sum_aggregation = SynchronousSumAggregation(False) - self.assertFalse(synchronous_sum_aggregation._is_monotonic) + self.assertFalse(synchronous_sum_aggregation._instrument_is_monotonic) def test_aggregate(self): """ @@ -88,19 +89,20 @@ def test_collect(self): class TestAsynchronousSumAggregation(TestCase): - def test_monotonicity_awareness(self): + def test_instrument_monotonicity_awareness(self): """ `AsynchronousSumAggregation` is aware of the instrument monotonicity """ asynchronous_sum_aggregation = AsynchronousSumAggregation(True) self.assertIsInstance( - asynchronous_sum_aggregation, _MonotonicityAwareAggregation + asynchronous_sum_aggregation, + _InstrumentMonotonicityAwareAggregation, ) - self.assertTrue(asynchronous_sum_aggregation._is_monotonic) + self.assertTrue(asynchronous_sum_aggregation._instrument_is_monotonic) asynchronous_sum_aggregation = AsynchronousSumAggregation(False) - self.assertFalse(asynchronous_sum_aggregation._is_monotonic) + self.assertFalse(asynchronous_sum_aggregation._instrument_is_monotonic) def test_aggregate(self): """ @@ -156,14 +158,14 @@ def test_collect(self): class TestLastValueAggregation(TestCase): - def test_monotonicity_awareness(self): + def test_instrument_monotonicity_awareness(self): """ `LastValueAggregation` is not aware of the instrument monotonicity """ sum_aggregation = LastValueAggregation() self.assertNotIsInstance( - sum_aggregation, _MonotonicityAwareAggregation + sum_aggregation, _InstrumentMonotonicityAwareAggregation ) def test_aggregate(self): @@ -212,18 +214,20 @@ def test_collect(self): class TestExplicitBucketHistogramAggregation(TestCase): - def test_monotonicity_awareness(self): + def test_instrument_monotonicity_awareness(self): """ `ExplicitBucketHistogramAggregation` is aware of the instrument monotonicity """ sum_aggregation = ExplicitBucketHistogramAggregation(True) - self.assertIsInstance(sum_aggregation, _MonotonicityAwareAggregation) - self.assertTrue(sum_aggregation._is_monotonic) + self.assertIsInstance( + sum_aggregation, _InstrumentMonotonicityAwareAggregation + ) + self.assertTrue(sum_aggregation._instrument_is_monotonic) sum_aggregation = ExplicitBucketHistogramAggregation(False) - self.assertFalse(sum_aggregation._is_monotonic) + self.assertFalse(sum_aggregation._instrument_is_monotonic) def test_aggregate(self): """ From e511c962eaafe3e89e4feb33a35219d502948bcd Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 11 Jan 2022 11:22:00 -0600 Subject: [PATCH 34/35] Make Histogram non monotonicity aware --- .../opentelemetry/sdk/_metrics/aggregation.py | 10 +++----- .../tests/metrics/test_aggregation.py | 23 ++++--------------- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 94cd1edf032..03a2debfe0b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -140,12 +140,9 @@ def collect(self) -> Optional[Gauge]: ) -class ExplicitBucketHistogramAggregation( - _InstrumentMonotonicityAwareAggregation, Aggregation[Histogram] -): +class ExplicitBucketHistogramAggregation(Aggregation[Histogram]): def __init__( self, - instrument_is_monotonic: bool, boundaries: Sequence[int] = ( 0, 5, @@ -160,7 +157,7 @@ def __init__( ), record_min_max: bool = True, ): - super().__init__(instrument_is_monotonic) + super().__init__() self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) self._min = inf self._max = -inf @@ -177,8 +174,7 @@ def aggregate(self, measurement: Measurement) -> None: self._min = min(self._min, value) self._max = max(self._max, value) - if self._instrument_is_monotonic: - self._sum += value + self._sum += value for key in self._value.keys(): diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 6da9b28a28e..c8064b9d1dc 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -214,21 +214,6 @@ def test_collect(self): class TestExplicitBucketHistogramAggregation(TestCase): - def test_instrument_monotonicity_awareness(self): - """ - `ExplicitBucketHistogramAggregation` is aware of the instrument - monotonicity - """ - - sum_aggregation = ExplicitBucketHistogramAggregation(True) - self.assertIsInstance( - sum_aggregation, _InstrumentMonotonicityAwareAggregation - ) - self.assertTrue(sum_aggregation._instrument_is_monotonic) - - sum_aggregation = ExplicitBucketHistogramAggregation(False) - self.assertFalse(sum_aggregation._instrument_is_monotonic) - def test_aggregate(self): """ `ExplicitBucketHistogramAggregation` collects data for @@ -236,7 +221,7 @@ def test_aggregate(self): """ explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(True) + ExplicitBucketHistogramAggregation() ) explicit_bucket_histogram_aggregation.aggregate(Measurement(-1)) @@ -257,7 +242,7 @@ def test_min_max(self): """ explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(True) + ExplicitBucketHistogramAggregation() ) explicit_bucket_histogram_aggregation.aggregate(Measurement(-1)) @@ -270,7 +255,7 @@ def test_min_max(self): self.assertEqual(explicit_bucket_histogram_aggregation._max, 9999) explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(True, record_min_max=False) + ExplicitBucketHistogramAggregation(record_min_max=False) ) explicit_bucket_histogram_aggregation.aggregate(Measurement(-1)) @@ -288,7 +273,7 @@ def test_collect(self): """ explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(True) + ExplicitBucketHistogramAggregation() ) explicit_bucket_histogram_aggregation.aggregate(Measurement(1)) From 0ebac64adac8d76f125bb1fb4ac43d0d901d88af Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 11 Jan 2022 13:03:14 -0600 Subject: [PATCH 35/35] Update SHA --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 50076d83bcd..7c8b1b1d0a3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,7 +10,7 @@ env: # Otherwise, set variable to the commit of your branch on # opentelemetry-python-contrib which is compatible with these Core repo # changes. - CONTRIB_REPO_SHA: 30d0c2ea90dffa7d958a28a699a9021ecb04aa71 + CONTRIB_REPO_SHA: 23394ccd80878a91534f8421b82a7410eb775e65 # This is needed because we do not clone the core repo in contrib builds anymore. # When running contrib builds as part of core builds, we use actions/checkout@v2 which # does not set an environment variable (simply just runs tox), which is different when