diff --git a/.changelog/5650.fixed b/.changelog/5650.fixed new file mode 100644 index 0000000000..085e6f1b66 --- /dev/null +++ b/.changelog/5650.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: add validation for export_timeout_millis <= 0 in BatchSpanProcessor diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/export/__init__.py index ea02d8e1fe..f8c466ddf5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/export/__init__.py @@ -284,7 +284,12 @@ def __init__( if export_timeout_millis is None: export_timeout_millis = BatchLogRecordProcessor._default_export_timeout_millis() - BatchLogRecordProcessor._validate_arguments(max_queue_size, schedule_delay_millis, max_export_batch_size) + BatchLogRecordProcessor._validate_arguments( + max_queue_size, + schedule_delay_millis, + max_export_batch_size, + export_timeout_millis, + ) # Initializes BatchProcessor self._batch_processor = BatchProcessor( exporter, @@ -379,7 +384,12 @@ def _default_export_timeout_millis(): return _DEFAULT_EXPORT_TIMEOUT_MILLIS @staticmethod - def _validate_arguments(max_queue_size, schedule_delay_millis, max_export_batch_size): + def _validate_arguments( + max_queue_size, + schedule_delay_millis, + max_export_batch_size, + export_timeout_millis=None, + ): if max_queue_size <= 0: raise ValueError("max_queue_size must be a positive integer.") @@ -391,3 +401,6 @@ def _validate_arguments(max_queue_size, schedule_delay_millis, max_export_batch_ if max_export_batch_size > max_queue_size: raise ValueError("max_export_batch_size must be less than or equal to max_queue_size.") + + if export_timeout_millis is not None and export_timeout_millis <= 0: + raise ValueError("export_timeout_millis must be positive.") diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py index 19a6902bfa..8aad6bbfee 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py @@ -180,7 +180,12 @@ def __init__( if export_timeout_millis is None: export_timeout_millis = BatchSpanProcessor._default_export_timeout_millis() - BatchSpanProcessor._validate_arguments(max_queue_size, schedule_delay_millis, max_export_batch_size) + BatchSpanProcessor._validate_arguments( + max_queue_size, + schedule_delay_millis, + max_export_batch_size, + export_timeout_millis, + ) self._batch_processor = BatchProcessor( span_exporter, @@ -274,7 +279,12 @@ def _default_export_timeout_millis(): return _DEFAULT_EXPORT_TIMEOUT_MILLIS @staticmethod - def _validate_arguments(max_queue_size, schedule_delay_millis, max_export_batch_size): + def _validate_arguments( + max_queue_size, + schedule_delay_millis, + max_export_batch_size, + export_timeout_millis=None, + ): if max_queue_size <= 0: raise ValueError("max_queue_size must be a positive integer.") @@ -287,6 +297,9 @@ def _validate_arguments(max_queue_size, schedule_delay_millis, max_export_batch_ if max_export_batch_size > max_queue_size: raise ValueError("max_export_batch_size must be less than or equal to max_queue_size.") + if export_timeout_millis is not None and export_timeout_millis <= 0: + raise ValueError("export_timeout_millis must be positive.") + class ConsoleSpanExporter(SpanExporter): """Implementation of :class:`SpanExporter` that prints spans to the diff --git a/opentelemetry-sdk/tests/logs/test_export.py b/opentelemetry-sdk/tests/logs/test_export.py index 3cfa825d34..f39a8606f9 100644 --- a/opentelemetry-sdk/tests/logs/test_export.py +++ b/opentelemetry-sdk/tests/logs/test_export.py @@ -473,6 +473,12 @@ def test_metrics_already_shutdown(self): # before the end of the test, otherwise the worker thread will continue # to run after the end of the test. class TestBatchLogRecordProcessor(unittest.TestCase): + def test_invalid_export_timeout_millis(self): + with self.assertRaises(ValueError): + BatchLogRecordProcessor(InMemoryLogRecordExporter(), export_timeout_millis=0) + with self.assertRaises(ValueError): + BatchLogRecordProcessor(InMemoryLogRecordExporter(), export_timeout_millis=-500) + def test_emit_call_log_record(self): exporter = InMemoryLogRecordExporter() log_record_processor = Mock(wraps=BatchLogRecordProcessor(exporter)) diff --git a/opentelemetry-sdk/tests/trace/export/test_export.py b/opentelemetry-sdk/tests/trace/export/test_export.py index c5623dc8e5..e796facd33 100644 --- a/opentelemetry-sdk/tests/trace/export/test_export.py +++ b/opentelemetry-sdk/tests/trace/export/test_export.py @@ -241,6 +241,12 @@ def test_args_env_var(self): self.assertEqual(batch_span_processor._batch_processor._export_timeout_millis, 4) batch_span_processor.shutdown() + def test_invalid_export_timeout_millis(self): + with self.assertRaises(ValueError): + export.BatchSpanProcessor(MySpanExporter(destination=[]), export_timeout_millis=0) + with self.assertRaises(ValueError): + export.BatchSpanProcessor(MySpanExporter(destination=[]), export_timeout_millis=-500) + def test_args_env_var_defaults(self): batch_span_processor = export.BatchSpanProcessor(MySpanExporter(destination=[]))