From 594f0d08e4593f81f325ac1749f9450163ce5b14 Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Mon, 22 Jun 2026 19:22:13 +0530 Subject: [PATCH 01/11] fix: raise ValueError for unsorted ExplicitBucketHistogramAggregation boundaries --- .../sdk/metrics/_internal/aggregation.py | 5 ++++ .../tests/metrics/test_aggregation.py | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index dea213f4282..6fc032c7c9c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -475,6 +475,11 @@ def __init__( instrument_aggregation_temporality ) self._start_time_unix_nano = start_time_unix_nano + for i in range(1, len(boundaries)): + if boundaries[i - 1] >= boundaries[i]: + raise ValueError( + f"boundaries must be in increasing order, got {list(boundaries)}" + ) self._boundaries = tuple(boundaries) self._record_min_max = record_min_max diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 1f11393cee0..948ee1e1661 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -477,6 +477,30 @@ def test_create_aggregation_on_instrument_without_boundaries(self): ) self.assertIsInstance(result, _ExplicitBucketHistogramAggregation) + def test_unsorted_boundaries_raise(self): + with self.assertRaises(ValueError): + _ExplicitBucketHistogramAggregation( + Mock(), + AggregationTemporality.DELTA, + 0, + _default_reservoir_factory( + _ExplicitBucketHistogramAggregation + ), + boundaries=[100, 10, 50], + ) + + def test_duplicate_boundaries_raise(self): + with self.assertRaises(ValueError): + _ExplicitBucketHistogramAggregation( + Mock(), + AggregationTemporality.DELTA, + 0, + _default_reservoir_factory( + _ExplicitBucketHistogramAggregation + ), + boundaries=[10, 50, 50, 100], + ) + class TestAggregationFactory(TestCase): def test_sum_factory(self): From 6afd07749f3b1198e7ec21809ce331b3a77e8fef Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Mon, 22 Jun 2026 19:23:30 +0530 Subject: [PATCH 02/11] docs: add changelog fragment for #5340 --- .changelog/5340.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/5340.fixed diff --git a/.changelog/5340.fixed b/.changelog/5340.fixed new file mode 100644 index 00000000000..5921973ce66 --- /dev/null +++ b/.changelog/5340.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: raise `ValueError` when `ExplicitBucketHistogramAggregation` boundaries are not strictly increasing From d93c40f012ca17badf2d9b1132f786f81435ad8f Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Mon, 22 Jun 2026 19:54:56 +0530 Subject: [PATCH 03/11] fix: also reject NaN and infinite boundary values --- .../sdk/metrics/_internal/aggregation.py | 7 +++- .../tests/metrics/test_aggregation.py | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 6fc032c7c9c..cf5dbfd5693 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -9,7 +9,7 @@ from enum import IntEnum from functools import partial from logging import getLogger -from math import inf +from math import inf, isfinite from threading import Lock from typing import ( Generic, @@ -475,6 +475,11 @@ def __init__( instrument_aggregation_temporality ) self._start_time_unix_nano = start_time_unix_nano + for b in boundaries: + if not isfinite(b): + raise ValueError( + f"boundaries must be finite, got {b}" + ) for i in range(1, len(boundaries)): if boundaries[i - 1] >= boundaries[i]: raise ValueError( diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 948ee1e1661..12a42b31771 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -501,6 +501,42 @@ def test_duplicate_boundaries_raise(self): boundaries=[10, 50, 50, 100], ) + def test_nan_boundary_raises(self): + with self.assertRaises(ValueError): + _ExplicitBucketHistogramAggregation( + Mock(), + AggregationTemporality.DELTA, + 0, + _default_reservoir_factory( + _ExplicitBucketHistogramAggregation + ), + boundaries=[10, float("nan"), 100], + ) + + def test_inf_boundary_raises(self): + with self.assertRaises(ValueError): + _ExplicitBucketHistogramAggregation( + Mock(), + AggregationTemporality.DELTA, + 0, + _default_reservoir_factory( + _ExplicitBucketHistogramAggregation + ), + boundaries=[10, 50, float("inf")], + ) + + def test_negative_inf_boundary_raises(self): + with self.assertRaises(ValueError): + _ExplicitBucketHistogramAggregation( + Mock(), + AggregationTemporality.DELTA, + 0, + _default_reservoir_factory( + _ExplicitBucketHistogramAggregation + ), + boundaries=[float("-inf"), 50, 100], + ) + class TestAggregationFactory(TestCase): def test_sum_factory(self): From aa3be145331ef8404a62efda53ac0c3c6e53ac03 Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Tue, 23 Jun 2026 15:14:40 +0530 Subject: [PATCH 04/11] Update .changelog/5340.fixed Co-authored-by: Riccardo Magliocchetti --- .changelog/5340.fixed | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/5340.fixed b/.changelog/5340.fixed index 5921973ce66..eb1ba3b8a0e 100644 --- a/.changelog/5340.fixed +++ b/.changelog/5340.fixed @@ -1 +1 @@ -`opentelemetry-sdk`: raise `ValueError` when `ExplicitBucketHistogramAggregation` boundaries are not strictly increasing +`opentelemetry-sdk`: raise `ValueError` when `ExplicitBucketHistogramAggregation` boundaries are not strictly increasing or finite From c06aa1893956b140b65b35c327e97d1147f1c12f Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Tue, 23 Jun 2026 15:24:32 +0530 Subject: [PATCH 05/11] fix: single-pass boundary validation with clearer error messages --- .../sdk/metrics/_internal/aggregation.py | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index cf5dbfd5693..b63c3fd3523 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -9,7 +9,7 @@ from enum import IntEnum from functools import partial from logging import getLogger -from math import inf, isfinite +from math import inf, isnan from threading import Lock from typing import ( Generic, @@ -464,6 +464,21 @@ def __init__( boundaries = ( _DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES ) + if boundaries: + if isnan(boundaries[0]): + raise ValueError("invalid boundary: NaN") + if boundaries[0] == -inf: + raise ValueError("invalid boundary: -Inf") + for i in range(1, len(boundaries)): + if isnan(boundaries[i]): + raise ValueError("invalid boundary: NaN") + if boundaries[i - 1] >= boundaries[i]: + raise ValueError( + f"boundaries must be strictly increasing:" + f" {boundaries[i - 1]} >= {boundaries[i]}" + ) + if boundaries[-1] == inf: + raise ValueError("invalid boundary: +Inf") super().__init__( attributes, reservoir_builder=partial( @@ -475,16 +490,6 @@ def __init__( instrument_aggregation_temporality ) self._start_time_unix_nano = start_time_unix_nano - for b in boundaries: - if not isfinite(b): - raise ValueError( - f"boundaries must be finite, got {b}" - ) - for i in range(1, len(boundaries)): - if boundaries[i - 1] >= boundaries[i]: - raise ValueError( - f"boundaries must be in increasing order, got {list(boundaries)}" - ) self._boundaries = tuple(boundaries) self._record_min_max = record_min_max From 1daf99f6b05cdac8b0af8785b5cd0976ee4cd658 Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Wed, 24 Jun 2026 21:51:14 +0530 Subject: [PATCH 06/11] Update aggregation.py Co-authored-by: Lukas Hering <40302054+herin049@users.noreply.github.com> --- .../sdk/metrics/_internal/aggregation.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index b63c3fd3523..9c12e1a3cdf 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -465,20 +465,11 @@ def __init__( _DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES ) if boundaries: - if isnan(boundaries[0]): - raise ValueError("invalid boundary: NaN") - if boundaries[0] == -inf: - raise ValueError("invalid boundary: -Inf") - for i in range(1, len(boundaries)): - if isnan(boundaries[i]): - raise ValueError("invalid boundary: NaN") - if boundaries[i - 1] >= boundaries[i]: - raise ValueError( - f"boundaries must be strictly increasing:" - f" {boundaries[i - 1]} >= {boundaries[i]}" - ) - if boundaries[-1] == inf: - raise ValueError("invalid boundary: +Inf") + for i, x in enumerate(boundaries): + if not math.isfinite(x): + raise ValueError(f"invalid boundary: {x!r}") + if i and boundaries[i - 1] >= x: + raise ValueError("boundaries must be strictly increasing") super().__init__( attributes, reservoir_builder=partial( From 0fca08072ecd6ca5c8be4b157e701d113c564f99 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 25 Jun 2026 14:30:44 +0200 Subject: [PATCH 07/11] Apply suggestion from @xrmx --- .../opentelemetry/sdk/metrics/_internal/aggregation.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 9c12e1a3cdf..2c04ffdad0b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -9,7 +9,13 @@ from enum import IntEnum from functools import partial from logging import getLogger -from math import inf, isnan +import math +from abc import ABC, abstractmethod +from bisect import bisect_left +from collections.abc import Callable, Sequence +from enum import IntEnum +from functools import partial +from logging import getLogger from threading import Lock from typing import ( Generic, From b70a0fbfc10172ef639a61bbf7e2d75a0324c626 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 25 Jun 2026 14:33:25 +0200 Subject: [PATCH 08/11] Update aggregation.py --- .../src/opentelemetry/sdk/metrics/_internal/aggregation.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 2c04ffdad0b..e749be04be2 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -3,12 +3,6 @@ # pylint: disable=too-many-lines -from abc import ABC, abstractmethod -from bisect import bisect_left -from collections.abc import Callable, Sequence -from enum import IntEnum -from functools import partial -from logging import getLogger import math from abc import ABC, abstractmethod from bisect import bisect_left From 9488069466226c485548ebdc243d9722feb64509 Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Fri, 26 Jun 2026 00:19:16 +0530 Subject: [PATCH 09/11] fix: rename single-char vars and use math.inf instead of bare inf --- .../sdk/metrics/_internal/aggregation.py | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index e749be04be2..4696c5d366c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -465,10 +465,10 @@ def __init__( _DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES ) if boundaries: - for i, x in enumerate(boundaries): - if not math.isfinite(x): - raise ValueError(f"invalid boundary: {x!r}") - if i and boundaries[i - 1] >= x: + for idx, bound in enumerate(boundaries): + if not math.isfinite(bound): + raise ValueError(f"invalid boundary: {bound!r}") + if idx and boundaries[idx - 1] >= bound: raise ValueError("boundaries must be strictly increasing") super().__init__( attributes, @@ -485,13 +485,13 @@ def __init__( self._record_min_max = record_min_max self._value = None - self._min = inf - self._max = -inf + self._min = math.inf + self._max = -math.inf self._sum = 0 self._previous_value = None - self._previous_min = inf - self._previous_max = -inf + self._previous_min = math.inf + self._previous_max = -math.inf self._previous_sum = 0 self._previous_collection_start_nano = self._start_time_unix_nano @@ -535,8 +535,8 @@ def collect( self._value = None self._sum = 0 - self._min = inf - self._max = -inf + self._min = math.inf + self._max = -math.inf if ( self._instrument_aggregation_temporality @@ -680,8 +680,8 @@ def __init__( self._value_positive = None self._value_negative = None - self._min = inf - self._max = -inf + self._min = math.inf + self._max = -math.inf self._sum = 0 self._count = 0 self._zero_count = 0 @@ -689,8 +689,8 @@ def __init__( self._previous_value_positive = None self._previous_value_negative = None - self._previous_min = inf - self._previous_max = -inf + self._previous_min = math.inf + self._previous_max = -math.inf self._previous_sum = 0 self._previous_count = 0 self._previous_zero_count = 0 @@ -836,8 +836,8 @@ def collect( self._value_positive = None self._value_negative = None self._sum = 0 - self._min = inf - self._max = -inf + self._min = math.inf + self._max = -math.inf self._count = 0 self._zero_count = 0 self._scale = None @@ -1349,7 +1349,7 @@ def _create_aggregation( class ExplicitBucketHistogramAggregation(Aggregation): - """This aggregation informs the SDK to collect: + """This aggregation math.informs the SDK to collect: - Count of Measurement values falling within explicit bucket boundaries. - Arithmetic sum of Measurement values in population. This SHOULD NOT be collected when used with instruments that record negative measurements, e.g. UpDownCounter or ObservableGauge. @@ -1409,7 +1409,7 @@ def _create_aggregation( class SumAggregation(Aggregation): - """This aggregation informs the SDK to collect: + """This aggregation math.informs the SDK to collect: - The arithmetic sum of Measurement values. """ @@ -1442,7 +1442,7 @@ def _create_aggregation( class LastValueAggregation(Aggregation): """ - This aggregation informs the SDK to collect: + This aggregation math.informs the SDK to collect: - The last Measurement. - The timestamp of the last Measurement. From 76cf4e3f5877a50b4b629afcd7520da669c4a6bb Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Fri, 26 Jun 2026 11:19:36 +0530 Subject: [PATCH 10/11] Update opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py accidentally changed this because i used inf without importing math previously so i replaced all instances of 'inf' with math.inf Co-authored-by: Lukas Hering <40302054+herin049@users.noreply.github.com> --- .../src/opentelemetry/sdk/metrics/_internal/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 4696c5d366c..619f954c4a6 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -1349,7 +1349,7 @@ def _create_aggregation( class ExplicitBucketHistogramAggregation(Aggregation): - """This aggregation math.informs the SDK to collect: + """This aggregation informs the SDK to collect: - Count of Measurement values falling within explicit bucket boundaries. - Arithmetic sum of Measurement values in population. This SHOULD NOT be collected when used with instruments that record negative measurements, e.g. UpDownCounter or ObservableGauge. From 295a35cbb2c00898e9d5842b55d00c8a9708d12d Mon Sep 17 00:00:00 2001 From: Rudra Dudhat Date: Fri, 26 Jun 2026 11:20:01 +0530 Subject: [PATCH 11/11] fix: restore informs in docstrings corrupted by overeager inf replacement --- .../src/opentelemetry/sdk/metrics/_internal/aggregation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 619f954c4a6..ec1da4bcd6d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -1409,7 +1409,7 @@ def _create_aggregation( class SumAggregation(Aggregation): - """This aggregation math.informs the SDK to collect: + """This aggregation informs the SDK to collect: - The arithmetic sum of Measurement values. """ @@ -1442,7 +1442,7 @@ def _create_aggregation( class LastValueAggregation(Aggregation): """ - This aggregation math.informs the SDK to collect: + This aggregation informs the SDK to collect: - The last Measurement. - The timestamp of the last Measurement.