Fix start_time_unix nano for delta collection temporality for Explici…

…tBucketHistogramAggregation (open-telemetry#4009) Fixes open-telemetry#4008
ocelotl · Jul 2, 2024 · 6c3b703 · 6c3b703
1 parent c6fb299
commit 6c3b703
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,8 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
-- Fix `start_time_unix_nano` for delta collection for sum aggregation
+- Fix `start_time_unix_nano` for delta collection for explicit bucket histogram aggregation
   ([#4009](https://github.com/open-telemetry/opentelemetry-python/pull/4009))
+- Fix `start_time_unix_nano` for delta collection for sum aggregation
+  ([#4011](https://github.com/open-telemetry/opentelemetry-python/pull/4011))
 - Do not execute Flask Tests in debug mode
   ([#3956](https://github.com/open-telemetry/opentelemetry-python/pull/3956))
 - When encountering an error encoding metric attributes in the OTLP exporter, log the key that had an error.

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py
@@ -471,16 +471,16 @@ def collect(
                     is AggregationTemporality.DELTA
                 ):
 
-                    if current_value is None:
-                        return None
-
                     previous_collection_start_nano = (
                         self._previous_collection_start_nano
                     )
                     self._previous_collection_start_nano = (
                         collection_start_nano
                     )
 
+                    if current_value is None:
+                        return None
+
                     return HistogramDataPoint(
                         attributes=self._attributes,
                         start_time_unix_nano=previous_collection_start_nano,

diff --git a/...elemetry-sdk/tests/metrics/integration_test/test_explicit_bucket_histogram_aggregation.py b/...elemetry-sdk/tests/metrics/integration_test/test_explicit_bucket_histogram_aggregation.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 from platform import system
+from time import sleep
 from unittest import TestCase
 
 from pytest import mark
@@ -123,11 +124,43 @@ def test_synchronous_delta_temporality(self):
 
             results.append(reader.get_metrics_data())
 
-        provider.shutdown()
-
         for metrics_data in results:
             self.assertIsNone(metrics_data)
 
+        results = []
+
+        histogram.record(1)
+        results.append(reader.get_metrics_data())
+
+        sleep(0.1)
+        results.append(reader.get_metrics_data())
+
+        histogram.record(2)
+        results.append(reader.get_metrics_data())
+
+        metric_data_0 = (
+            results[0]
+            .resource_metrics[0]
+            .scope_metrics[0]
+            .metrics[0]
+            .data.data_points[0]
+        )
+        metric_data_2 = (
+            results[2]
+            .resource_metrics[0]
+            .scope_metrics[0]
+            .metrics[0]
+            .data.data_points[0]
+        )
+
+        self.assertIsNone(results[1])
+
+        self.assertGreater(
+            metric_data_2.start_time_unix_nano, metric_data_0.time_unix_nano
+        )
+
+        provider.shutdown()
+
     @mark.skipif(
         system() != "Linux",
         reason=(