From 1c6d53978bc4deee36defd4a8282a6b3931bf50b Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 17 Feb 2025 11:06:47 +0100 Subject: [PATCH 1/3] opentelemetry-sdk: fix OTLP exporting of histogram explicit buckets advisory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OTLP exporters don't rely on the default aggregation for histogram instead they do it explicitly. So instead of setting boundaries at init time of ExplicitBucketHistogramAggregation delay it in _create_aggregation when we have the Histogram at hand. Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- .../sdk/metrics/_internal/aggregation.py | 10 +++--- ...est_histogram_advisory_explicit_buckets.py | 31 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 3e17b6d3f64..8443d9516cf 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -1367,10 +1367,6 @@ def __init__( boundaries: Optional[Sequence[float]] = None, record_min_max: bool = True, ) -> None: - if boundaries is None: - boundaries = ( - _DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES - ) self._boundaries = boundaries self._record_min_max = record_min_max @@ -1391,6 +1387,12 @@ def _create_aggregation( AggregationTemporality.CUMULATIVE ) + if self._boundaries is None: + self._boundaries = ( + instrument._advisory.explicit_bucket_boundaries + or _DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES + ) + return _ExplicitBucketHistogramAggregation( attributes, instrument_aggregation_temporality, diff --git a/opentelemetry-sdk/tests/metrics/integration_test/test_histogram_advisory_explicit_buckets.py b/opentelemetry-sdk/tests/metrics/integration_test/test_histogram_advisory_explicit_buckets.py index 945abf572e2..2f46dca87a6 100644 --- a/opentelemetry-sdk/tests/metrics/integration_test/test_histogram_advisory_explicit_buckets.py +++ b/opentelemetry-sdk/tests/metrics/integration_test/test_histogram_advisory_explicit_buckets.py @@ -15,6 +15,7 @@ from unittest import TestCase from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics._internal.instrument import Histogram from opentelemetry.sdk.metrics.export import InMemoryMetricReader from opentelemetry.sdk.metrics.view import ( ExplicitBucketHistogramAggregation, @@ -133,3 +134,33 @@ def test_view_overrides_buckets(self): self.assertEqual( metric.data.data_points[0].explicit_bounds, (10.0, 100.0, 1000.0) ) + + def test_explicit_aggregation(self): + reader = InMemoryMetricReader( + preferred_aggregation={ + Histogram: ExplicitBucketHistogramAggregation() + } + ) + meter_provider = MeterProvider( + metric_readers=[reader], + ) + meter = meter_provider.get_meter("testmeter") + histogram = meter.create_histogram( + "testhistogram", + explicit_bucket_boundaries_advisory=[1.0, 2.0, 3.0], + ) + histogram.record(1, {"label": "value"}) + histogram.record(2, {"label": "value"}) + histogram.record(3, {"label": "value"}) + + metrics = reader.get_metrics_data() + self.assertEqual(len(metrics.resource_metrics), 1) + self.assertEqual(len(metrics.resource_metrics[0].scope_metrics), 1) + self.assertEqual( + len(metrics.resource_metrics[0].scope_metrics[0].metrics), 1 + ) + metric = metrics.resource_metrics[0].scope_metrics[0].metrics[0] + self.assertEqual(metric.name, "testhistogram") + self.assertEqual( + metric.data.data_points[0].explicit_bounds, (1.0, 2.0, 3.0) + ) From 54f65be4fba7559b56cd52399cb7d91e30fa5290 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 17 Feb 2025 11:14:34 +0100 Subject: [PATCH 2/3] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5f6e59c346..5fecfa3aeb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Tolerates exceptions when loading resource detectors via `OTEL_EXPERIMENTAL_RESOURCE_DETECTORS` ([#4373](https://github.com/open-telemetry/opentelemetry-python/pull/4373)) +- opentelemetry-sdk: fix OTLP exporting of Histograms withs explicit buckets advisory + ([#4434](https://github.com/open-telemetry/opentelemetry-python/pull/4434)) ## Version 1.30.0/0.51b0 (2025-02-03) From da67b5da1704955b31784e75fdeb43dee32a64c5 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 17 Feb 2025 14:08:04 +0100 Subject: [PATCH 3/3] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fecfa3aeb1..1d46e1f3129 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Tolerates exceptions when loading resource detectors via `OTEL_EXPERIMENTAL_RESOURCE_DETECTORS` ([#4373](https://github.com/open-telemetry/opentelemetry-python/pull/4373)) -- opentelemetry-sdk: fix OTLP exporting of Histograms withs explicit buckets advisory +- opentelemetry-sdk: fix OTLP exporting of Histograms with explicit buckets advisory ([#4434](https://github.com/open-telemetry/opentelemetry-python/pull/4434)) ## Version 1.30.0/0.51b0 (2025-02-03)