From 3bdc868c315d44fb6aa24734664687c789db3f7a Mon Sep 17 00:00:00 2001 From: Dushyant Acharya Date: Sun, 13 Sep 2026 09:33:42 +0530 Subject: [PATCH 1/5] fix(sdk): add validation for export_timeout_millis in PeriodicExportingMetricReader --- .changelog/5655.fixed | 1 + .../sdk/metrics/_internal/export/__init__.py | 5 +++++ .../test_periodic_exporting_metric_reader.py | 20 +++++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 .changelog/5655.fixed diff --git a/.changelog/5655.fixed b/.changelog/5655.fixed new file mode 100644 index 00000000000..70b570c6563 --- /dev/null +++ b/.changelog/5655.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: add validation for export_timeout_millis <= 0 in PeriodicExportingMetricReader diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py index cb1040537d3..7559689c1e3 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py @@ -480,6 +480,11 @@ def _after_in_child() -> None: f"interval value {self._export_interval_millis} is invalid \ and needs to be larger than zero." ) + if self._export_timeout_millis <= 0: + raise ValueError( + f"timeout value {self._export_timeout_millis} is invalid \ + and needs to be larger than zero." + ) def _at_fork_reinit(self): self._daemon_thread = Thread( diff --git a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py index 76ad37b8dbc..746ad841358 100644 --- a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py +++ b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py @@ -212,6 +212,26 @@ def test_ticker_value_exception_on_negative(self): export_interval_millis=-100, ) + def test_timeout_value_exception_on_zero(self): + exporter = FakeMetricsExporter() + exporter.export = Mock() + self.assertRaises( + ValueError, + PeriodicExportingMetricReader, + exporter, + export_timeout_millis=0, + ) + + def test_timeout_value_exception_on_negative(self): + exporter = FakeMetricsExporter() + exporter.export = Mock() + self.assertRaises( + ValueError, + PeriodicExportingMetricReader, + exporter, + export_timeout_millis=-100, + ) + @pytest.mark.flaky(max_runs=3, min_passes=1) def test_ticker_collects_metrics(self): exporter = FakeMetricsExporter() From f5becb9d57d4a12c999ec09e1c2f5184933dafee Mon Sep 17 00:00:00 2001 From: Dushyant Acharya Date: Sat, 19 Sep 2026 18:09:43 +0530 Subject: [PATCH 2/5] Rename changelog fragment to match PR number (#5656) --- .changelog/{5655.fixed => 5656.fixed} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{5655.fixed => 5656.fixed} (100%) diff --git a/.changelog/5655.fixed b/.changelog/5656.fixed similarity index 100% rename from .changelog/5655.fixed rename to .changelog/5656.fixed From 63890e0c71e63b7bfb11077535cee0debd84fede Mon Sep 17 00:00:00 2001 From: Dushyant Acharya Date: Sat, 19 Sep 2026 20:01:04 +0530 Subject: [PATCH 3/5] Validate interval and timeout before starting daemon thread --- .../sdk/metrics/_internal/export/__init__.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py index 7559689c1e3..f1f6f35e753 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py @@ -460,7 +460,18 @@ def __init__( self._shutdown_event = Event() self._shutdown_once = Once() self._daemon_thread = None - if self._export_interval_millis > 0 and self._export_interval_millis < math.inf: + if self._export_interval_millis <= 0: + raise ValueError( + f"interval value {self._export_interval_millis} is invalid \ + and needs to be larger than zero." + ) + if self._export_timeout_millis <= 0: + raise ValueError( + f"timeout value {self._export_timeout_millis} is invalid \ + and needs to be larger than zero." + ) + + if self._export_interval_millis < math.inf: self._daemon_thread = Thread( name="OtelPeriodicExportingMetricReader", target=self._ticker, @@ -475,16 +486,6 @@ def _after_in_child() -> None: at_fork() os.register_at_fork(after_in_child=_after_in_child) - elif self._export_interval_millis <= 0: - raise ValueError( - f"interval value {self._export_interval_millis} is invalid \ - and needs to be larger than zero." - ) - if self._export_timeout_millis <= 0: - raise ValueError( - f"timeout value {self._export_timeout_millis} is invalid \ - and needs to be larger than zero." - ) def _at_fork_reinit(self): self._daemon_thread = Thread( From c4293e7d9d08c135b1ee6d73e5915fd69e568d76 Mon Sep 17 00:00:00 2001 From: Dushyant Acharya Date: Sun, 20 Sep 2026 09:43:32 +0530 Subject: [PATCH 4/5] Fix line continuation in ValueError and validate error message in tests (#5656) --- .../sdk/metrics/_internal/export/__init__.py | 8 +-- .../test_periodic_exporting_metric_reader.py | 52 ++++++++++--------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py index f1f6f35e753..2836e80b57b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py @@ -462,13 +462,13 @@ def __init__( self._daemon_thread = None if self._export_interval_millis <= 0: raise ValueError( - f"interval value {self._export_interval_millis} is invalid \ - and needs to be larger than zero." + f"interval value {self._export_interval_millis} is invalid " + "and needs to be larger than zero." ) if self._export_timeout_millis <= 0: raise ValueError( - f"timeout value {self._export_timeout_millis} is invalid \ - and needs to be larger than zero." + f"timeout value {self._export_timeout_millis} is invalid " + "and needs to be larger than zero." ) if self._export_interval_millis < math.inf: diff --git a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py index 746ad841358..6bb8bde45f3 100644 --- a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py +++ b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py @@ -195,42 +195,46 @@ def test_ticker_not_called_on_infinity(self): def test_ticker_value_exception_on_zero(self): exporter = FakeMetricsExporter() exporter.export = Mock() - self.assertRaises( - ValueError, - PeriodicExportingMetricReader, - exporter, - export_interval_millis=0, - ) + with self.assertRaisesRegex( + ValueError, r"interval value 0.* is invalid and needs to be larger than zero\." + ): + PeriodicExportingMetricReader( + exporter, + export_interval_millis=0, + ) def test_ticker_value_exception_on_negative(self): exporter = FakeMetricsExporter() exporter.export = Mock() - self.assertRaises( - ValueError, - PeriodicExportingMetricReader, - exporter, - export_interval_millis=-100, - ) + with self.assertRaisesRegex( + ValueError, r"interval value -100.* is invalid and needs to be larger than zero\." + ): + PeriodicExportingMetricReader( + exporter, + export_interval_millis=-100, + ) def test_timeout_value_exception_on_zero(self): exporter = FakeMetricsExporter() exporter.export = Mock() - self.assertRaises( - ValueError, - PeriodicExportingMetricReader, - exporter, - export_timeout_millis=0, - ) + with self.assertRaisesRegex( + ValueError, r"timeout value 0.* is invalid and needs to be larger than zero\." + ): + PeriodicExportingMetricReader( + exporter, + export_timeout_millis=0, + ) def test_timeout_value_exception_on_negative(self): exporter = FakeMetricsExporter() exporter.export = Mock() - self.assertRaises( - ValueError, - PeriodicExportingMetricReader, - exporter, - export_timeout_millis=-100, - ) + with self.assertRaisesRegex( + ValueError, r"timeout value -100.* is invalid and needs to be larger than zero\." + ): + PeriodicExportingMetricReader( + exporter, + export_timeout_millis=-100, + ) @pytest.mark.flaky(max_runs=3, min_passes=1) def test_ticker_collects_metrics(self): From 33a3f5f88f698cb51bca5fcbed88285c8941d20c Mon Sep 17 00:00:00 2001 From: Dushyant Acharya Date: Sun, 20 Sep 2026 13:43:31 +0530 Subject: [PATCH 5/5] Apply ruff formatting fixes (#5656) --- .../sdk/metrics/_internal/export/__init__.py | 6 ++---- .../test_periodic_exporting_metric_reader.py | 16 ++++------------ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py index 2836e80b57b..19013d7acbe 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py @@ -462,13 +462,11 @@ def __init__( self._daemon_thread = None if self._export_interval_millis <= 0: raise ValueError( - f"interval value {self._export_interval_millis} is invalid " - "and needs to be larger than zero." + f"interval value {self._export_interval_millis} is invalid and needs to be larger than zero." ) if self._export_timeout_millis <= 0: raise ValueError( - f"timeout value {self._export_timeout_millis} is invalid " - "and needs to be larger than zero." + f"timeout value {self._export_timeout_millis} is invalid and needs to be larger than zero." ) if self._export_interval_millis < math.inf: diff --git a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py index 6bb8bde45f3..00ec640f635 100644 --- a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py +++ b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py @@ -195,9 +195,7 @@ def test_ticker_not_called_on_infinity(self): def test_ticker_value_exception_on_zero(self): exporter = FakeMetricsExporter() exporter.export = Mock() - with self.assertRaisesRegex( - ValueError, r"interval value 0.* is invalid and needs to be larger than zero\." - ): + with self.assertRaisesRegex(ValueError, r"interval value 0.* is invalid and needs to be larger than zero\."): PeriodicExportingMetricReader( exporter, export_interval_millis=0, @@ -206,9 +204,7 @@ def test_ticker_value_exception_on_zero(self): def test_ticker_value_exception_on_negative(self): exporter = FakeMetricsExporter() exporter.export = Mock() - with self.assertRaisesRegex( - ValueError, r"interval value -100.* is invalid and needs to be larger than zero\." - ): + with self.assertRaisesRegex(ValueError, r"interval value -100.* is invalid and needs to be larger than zero\."): PeriodicExportingMetricReader( exporter, export_interval_millis=-100, @@ -217,9 +213,7 @@ def test_ticker_value_exception_on_negative(self): def test_timeout_value_exception_on_zero(self): exporter = FakeMetricsExporter() exporter.export = Mock() - with self.assertRaisesRegex( - ValueError, r"timeout value 0.* is invalid and needs to be larger than zero\." - ): + with self.assertRaisesRegex(ValueError, r"timeout value 0.* is invalid and needs to be larger than zero\."): PeriodicExportingMetricReader( exporter, export_timeout_millis=0, @@ -228,9 +222,7 @@ def test_timeout_value_exception_on_zero(self): def test_timeout_value_exception_on_negative(self): exporter = FakeMetricsExporter() exporter.export = Mock() - with self.assertRaisesRegex( - ValueError, r"timeout value -100.* is invalid and needs to be larger than zero\." - ): + with self.assertRaisesRegex(ValueError, r"timeout value -100.* is invalid and needs to be larger than zero\."): PeriodicExportingMetricReader( exporter, export_timeout_millis=-100,