Test min, max and sum

Author: ocelotl

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py

@@ -414,6 +414,9 @@ def __init__(
 
         self._previous_collection_start_nano = self._start_time_unix_nano
         self._previous_cumulative_value = self._get_empty_bucket_counts()
+        self._previous_min = inf
+        self._previous_max = -inf
+        self._previous_sum = 0
 
     def _get_empty_bucket_counts(self) -> List[int]:
         return [0] * (len(self._boundaries) + 1)
@@ -496,71 +499,22 @@ def collect(
                         previous_cumulative_value_element,
                     ) in zip(current_value, self._previous_cumulative_value)
                 ]
+                self._previous_min = min(min_, self._previous_min)
+                self._previous_max = max(max_, self._previous_max)
+                self._previous_sum = sum_ + self._previous_sum
 
                 return HistogramDataPoint(
                     attributes=self._attributes,
                     start_time_unix_nano=self._start_time_unix_nano,
                     time_unix_nano=collection_start_nano,
                     count=sum(self._previous_cumulative_value),
-                    sum=sum_,
+                    sum=self._previous_sum,
                     bucket_counts=tuple(self._previous_cumulative_value),
                     explicit_bounds=self._boundaries,
-                    min=min_,
-                    max=max_,
+                    min=self._previous_min,
+                    max=self._previous_max,
                 )
 
-            # This happens when the corresponding instrument for this
-            # aggregation is asynchronous.
-
-            if current_value is None:
-                # This happens when the corresponding instrument callback
-                # does not produce measurements.
-                return None
-
-            if (
-                collection_aggregation_temporality
-                is AggregationTemporality.DELTA
-            ):
-
-                result_value = [
-                    current_value_element - previous_cumulative_value_element
-                    for (
-                        current_value_element,
-                        previous_cumulative_value_element,
-                    ) in zip(current_value, self._previous_cumulative_value)
-                ]
-
-                self._previous_cumulative_value = current_value
-
-                previous_collection_start_nano = (
-                    self._previous_collection_start_nano
-                )
-                self._previous_collection_start_nano = collection_start_nano
-
-                return HistogramDataPoint(
-                    attributes=self._attributes,
-                    start_time_unix_nano=previous_collection_start_nano,
-                    time_unix_nano=collection_start_nano,
-                    count=sum(result_value),
-                    sum=sum_,
-                    bucket_counts=tuple(result_value),
-                    explicit_bounds=self._boundaries,
-                    min=min_,
-                    max=max_,
-                )
-
-            return HistogramDataPoint(
-                attributes=self._attributes,
-                start_time_unix_nano=self._start_time_unix_nano,
-                time_unix_nano=collection_start_nano,
-                count=sum(current_value),
-                sum=sum_,
-                bucket_counts=tuple(current_value),
-                explicit_bounds=self._boundaries,
-                min=min_,
-                max=max_,
-            )
-
 
 # pylint: disable=protected-access
 class _ExponentialBucketHistogramAggregation(_Aggregation[HistogramPoint]):

opentelemetry-sdk/tests/metrics/integration_test/test_explicit_bucket_histogram_aggregation.py

@@ -27,8 +27,7 @@
 
 class TestExplicitBucketHistogramAggregation(TestCase):
 
-    def setUp(self):
-        self.test_values = iter([1, 6, 11, 26, 51, 76, 101, 251, 501, 751])
+    test_values = [1, 6, 11, 26, 51, 76, 101, 251, 501, 751]
 
     @mark.skipif(
         system() != "Linux",
@@ -67,38 +66,29 @@ def test_synchronous_delta_temporality(self):
             histogram.record(test_value)
             results.append(reader.get_metrics_data())
 
-        previous_time_unix_nano = (
+        metric_data = (
             results[0]
             .resource_metrics[0]
             .scope_metrics[0]
             .metrics[0]
-            .data.data_points[0]
-            .time_unix_nano
+            .data.
+            data_points[0]
         )
 
+        previous_time_unix_nano = metric_data.time_unix_nano
+
         self.assertEqual(
-            (
-                results[0]
-                .resource_metrics[0]
-                .scope_metrics[0]
-                .metrics[0]
-                .data.data_points[0]
-                .bucket_counts
-            ),
+            metric_data.bucket_counts,
             (0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
         )
 
         self.assertLess(
-            (
-                results[0]
-                .resource_metrics[0]
-                .scope_metrics[0]
-                .metrics[0]
-                .data.data_points[0]
-                .start_time_unix_nano
-            ),
+            metric_data.start_time_unix_nano,
             previous_time_unix_nano,
         )
+        self.assertEqual(metric_data.min, self.test_values[0])
+        self.assertEqual(metric_data.max, self.test_values[0])
+        self.assertEqual(metric_data.sum, self.test_values[0])
 
         for index, metrics_data in enumerate(results[1:]):
             metric_data = (
@@ -125,6 +115,9 @@ def test_synchronous_delta_temporality(self):
             self.assertLess(
                 metric_data.start_time_unix_nano, metric_data.time_unix_nano
             )
+            self.assertEqual(metric_data.min, self.test_values[index + 1])
+            self.assertEqual(metric_data.max, self.test_values[index + 1])
+            self.assertEqual(metric_data.sum, self.test_values[index + 1])
 
         results = []
 
@@ -205,6 +198,11 @@ def test_synchronous_cumulative_temporality(self):
                     ]
                 )
             )
+            self.assertEqual(metric_data.min, self.test_values[0])
+            self.assertEqual(metric_data.max, self.test_values[index])
+            self.assertEqual(
+                metric_data.sum, sum(self.test_values[:index + 1])
+            )
 
         results = []
 
@@ -239,3 +237,8 @@ def test_synchronous_cumulative_temporality(self):
                 metric_data.bucket_counts,
                 (0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0)
             )
+            self.assertEqual(metric_data.min, self.test_values[0])
+            self.assertEqual(metric_data.max, self.test_values[-1])
+            self.assertEqual(
+                metric_data.sum, sum(self.test_values)
+            )

opentelemetry-sdk/tests/metrics/test_aggregation.py

@@ -270,7 +270,7 @@ def test_aggregate(self):
         explicit_bucket_histogram_aggregation = (
             _ExplicitBucketHistogramAggregation(
                 Mock(),
-                AggregationTemporality.CUMULATIVE,
+                AggregationTemporality.DELTA,
                 0,
                 boundaries=[0, 2, 4],
             )
@@ -356,7 +356,7 @@ def test_collect(self):
         explicit_bucket_histogram_aggregation = (
             _ExplicitBucketHistogramAggregation(
                 Mock(),
-                AggregationTemporality.CUMULATIVE,
+                AggregationTemporality.DELTA,
                 0,
                 boundaries=[0, 1, 2],
             )
@@ -375,8 +375,6 @@ def test_collect(self):
         # CI fails the last assertion without this
         sleep(0.1)
 
-        from ipdb import set_trace
-        set_trace()
         explicit_bucket_histogram_aggregation.aggregate(measurement(1))
         # 2 is used here directly to simulate the instant the second
         # collection process starts.