diff --git a/.changelog/5656.fixed b/.changelog/5656.fixed new file mode 100644 index 00000000000..70b570c6563 --- /dev/null +++ b/.changelog/5656.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..19013d7acbe 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,16 @@ 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,11 +484,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." - ) 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..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,22 +195,38 @@ 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() + 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() + 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):