From f7be70405db857c3024d6d8d94aa23e322529ede Mon Sep 17 00:00:00 2001 From: RichardoMu <44485717+RichardoMrMu@users.noreply.github.com> Date: Fri, 16 Oct 2026 20:43:58 +0800 Subject: [PATCH 1/4] fix(zipkin): tolerate invalid OTEL_EXPORTER_ZIPKIN_TIMEOUT env var Both Zipkin exporters parsed OTEL_EXPORTER_ZIPKIN_TIMEOUT with a bare int() in __init__, so an invalid value (e.g. "10s") raised ValueError during exporter construction and took SDK startup down, unlike the OTLP HTTP/gRPC exporters which warn and fall back to the default. Add _timeout_from_env() in each exporter that skips unset/empty values silently, logs a warning for non-integer values, and falls back to the 10 second default. Explicit timeout argument precedence is unchanged. Assisted-by: Doubao --- .changelog/5704.fixed | 1 + .../exporter/zipkin/json/__init__.py | 20 +++++++- .../tests/test_zipkin_exporter.py | 47 +++++++++++++++++++ .../exporter/zipkin/proto/http/__init__.py | 20 +++++++- .../tests/test_zipkin_exporter.py | 47 +++++++++++++++++++ 5 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 .changelog/5704.fixed diff --git a/.changelog/5704.fixed b/.changelog/5704.fixed new file mode 100644 index 00000000000..aef00dd8750 --- /dev/null +++ b/.changelog/5704.fixed @@ -0,0 +1 @@ +`opentelemetry-exporter-zipkin-json`: warn and fall back to the default instead of raising `ValueError` when `OTEL_EXPORTER_ZIPKIN_TIMEOUT` holds a non-integer value; the same fix is applied to `opentelemetry-exporter-zipkin-proto-http`. diff --git a/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py b/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py index e976a07b387..95427e08916 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py @@ -81,10 +81,27 @@ DEFAULT_ENDPOINT = "http://localhost:9411/api/v2/spans" REQUESTS_SUCCESS_STATUS_CODES = (200, 202) +_DEFAULT_TIMEOUT = 10 logger = logging.getLogger(__name__) +def _timeout_from_env() -> int: + value = environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) + if value is None or not value.strip(): + return _DEFAULT_TIMEOUT + try: + return int(value) + except ValueError: + logger.warning( + "Invalid value %r for %s, using default of %s seconds", + value, + OTEL_EXPORTER_ZIPKIN_TIMEOUT, + _DEFAULT_TIMEOUT, + ) + return _DEFAULT_TIMEOUT + + class ZipkinExporter(SpanExporter): def __init__( self, @@ -128,7 +145,8 @@ def __init__( self.session = session or requests.Session() self.session.headers.update({"Content-Type": self.encoder.content_type()}) self._closed = False - self.timeout = timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) + timeout = timeout or _timeout_from_env() + self.timeout = timeout def export(self, spans: Sequence[Span]) -> SpanExportResult: # After the call to Shutdown subsequent calls to Export are diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py index ced0bd27d81..55e58c87959 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py @@ -63,6 +63,53 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.local_node.port, None) self.assertEqual(exporter.timeout, 15) + def test_constructor_invalid_timeout_env_var_falls_back(self): + os.environ[OTEL_EXPORTER_ZIPKIN_TIMEOUT] = "10s" + + with self.assertLogs("opentelemetry.exporter.zipkin.json", level="WARNING") as cm: + exporter = ZipkinExporter() + + self.assertEqual(exporter.timeout, 10) + self.assertEqual(len(cm.records), 1) + self.assertTrue( + any("Invalid value" in message for message in cm.output), + cm.output, + ) + + def test_constructor_empty_timeout_env_var_falls_back(self): + for value in ("", " ", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_unset_timeout_env_var_uses_default(self): + with patch.dict(os.environ): + os.environ.pop(OTEL_EXPORTER_ZIPKIN_TIMEOUT, None) + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_numeric_timeout_env_var_preserves_values(self): + for value, expected in (("15", 15), (" 15 ", 15), ("0", 0), ("-1", -1)): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, expected) + + def test_constructor_explicit_timeout_ignores_invalid_env_var(self): + for value in ("10s", "", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + exporter = ZipkinExporter(timeout=7) + self.assertEqual(exporter.timeout, 7) + + def test_constructor_explicit_timeout_preserves_truthiness(self): + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "15"}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.json", level="WARNING"): + self.assertEqual(ZipkinExporter(timeout=0).timeout, 15) + self.assertEqual(ZipkinExporter(timeout=-1).timeout, -1) + def test_constructor_protocol_endpoint(self): """Test the constructor for the common usage of providing the protocol and endpoint arguments.""" diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py b/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py index e664f20624b..358691d3001 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py @@ -78,10 +78,27 @@ DEFAULT_ENDPOINT = "http://localhost:9411/api/v2/spans" REQUESTS_SUCCESS_STATUS_CODES = (200, 202) +_DEFAULT_TIMEOUT = 10 logger = logging.getLogger(__name__) +def _timeout_from_env() -> int: + value = environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) + if value is None or not value.strip(): + return _DEFAULT_TIMEOUT + try: + return int(value) + except ValueError: + logger.warning( + "Invalid value %r for %s, using default of %s seconds", + value, + OTEL_EXPORTER_ZIPKIN_TIMEOUT, + _DEFAULT_TIMEOUT, + ) + return _DEFAULT_TIMEOUT + + class ZipkinExporter(SpanExporter): def __init__( self, @@ -121,7 +138,8 @@ def __init__( self.session = session or requests.Session() self.session.headers.update({"Content-Type": self.encoder.content_type()}) self._closed = False - self.timeout = timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) + timeout = timeout or _timeout_from_env() + self.timeout = timeout def export(self, spans: Sequence[Span]) -> SpanExportResult: # After the call to Shutdown subsequent calls to Export are diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py index 78e30b0b310..978878790b6 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py @@ -65,6 +65,53 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.local_node.port, None) self.assertEqual(exporter.timeout, 15) + def test_constructor_invalid_timeout_env_var_falls_back(self): + os.environ[OTEL_EXPORTER_ZIPKIN_TIMEOUT] = "10s" + + with self.assertLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING") as cm: + exporter = ZipkinExporter() + + self.assertEqual(exporter.timeout, 10) + self.assertEqual(len(cm.records), 1) + self.assertTrue( + any("Invalid value" in message for message in cm.output), + cm.output, + ) + + def test_constructor_empty_timeout_env_var_falls_back(self): + for value in ("", " ", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_unset_timeout_env_var_uses_default(self): + with patch.dict(os.environ): + os.environ.pop(OTEL_EXPORTER_ZIPKIN_TIMEOUT, None) + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, 10) + + def test_constructor_numeric_timeout_env_var_preserves_values(self): + for value, expected in (("15", 15), (" 15 ", 15), ("0", 0), ("-1", -1)): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter() + self.assertEqual(exporter.timeout, expected) + + def test_constructor_explicit_timeout_ignores_invalid_env_var(self): + for value in ("10s", "", "\t\n"): + with self.subTest(value=value), patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: value}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + exporter = ZipkinExporter(timeout=7) + self.assertEqual(exporter.timeout, 7) + + def test_constructor_explicit_timeout_preserves_truthiness(self): + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "15"}): + with self.assertNoLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING"): + self.assertEqual(ZipkinExporter(timeout=0).timeout, 15) + self.assertEqual(ZipkinExporter(timeout=-1).timeout, -1) + def test_constructor_protocol_endpoint(self): """Test the constructor for the common usage of providing the protocol and endpoint arguments.""" From 7a72e8a712863ec683f3075d06c09384a4a1636a Mon Sep 17 00:00:00 2001 From: RichardoMu <44485717+RichardoMrMu@users.noreply.github.com> Date: Sat, 17 Oct 2026 13:36:32 +0800 Subject: [PATCH 2/4] style(zipkin): inline timeout assignment in both exporters Collapse the two-line timeout resolution into a single assignment per review nit; no behavior change. Assisted-by: Doubao --- .../src/opentelemetry/exporter/zipkin/json/__init__.py | 3 +-- .../src/opentelemetry/exporter/zipkin/proto/http/__init__.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py b/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py index 95427e08916..426230615bc 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py @@ -145,8 +145,7 @@ def __init__( self.session = session or requests.Session() self.session.headers.update({"Content-Type": self.encoder.content_type()}) self._closed = False - timeout = timeout or _timeout_from_env() - self.timeout = timeout + self.timeout = timeout or _timeout_from_env() def export(self, spans: Sequence[Span]) -> SpanExportResult: # After the call to Shutdown subsequent calls to Export are diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py b/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py index 358691d3001..28dc8f2c85c 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py @@ -138,8 +138,7 @@ def __init__( self.session = session or requests.Session() self.session.headers.update({"Content-Type": self.encoder.content_type()}) self._closed = False - timeout = timeout or _timeout_from_env() - self.timeout = timeout + self.timeout = timeout or _timeout_from_env() def export(self, spans: Sequence[Span]) -> SpanExportResult: # After the call to Shutdown subsequent calls to Export are From b00b09ac29bfb4171c71824d5e0b9b61509396da Mon Sep 17 00:00:00 2001 From: RichardoMu <44485717+RichardoMrMu@users.noreply.github.com> Date: Wed, 21 Oct 2026 13:41:35 +0800 Subject: [PATCH 3/4] test(zipkin): patch os.environ in invalid-timeout fallback test Use patch.dict to set OTEL_EXPORTER_ZIPKIN_TIMEOUT instead of assigning os.environ directly, restoring the process environment after the test; applied symmetrically to both exporters. No behavior change. Assisted-by: Doubao --- .../tests/test_zipkin_exporter.py | 22 ++++++++++--------- .../tests/test_zipkin_exporter.py | 22 ++++++++++--------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py index 55e58c87959..0e08c0d9246 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py @@ -64,17 +64,19 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.timeout, 15) def test_constructor_invalid_timeout_env_var_falls_back(self): - os.environ[OTEL_EXPORTER_ZIPKIN_TIMEOUT] = "10s" - - with self.assertLogs("opentelemetry.exporter.zipkin.json", level="WARNING") as cm: - exporter = ZipkinExporter() + previous = os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "10s"}): + with self.assertLogs("opentelemetry.exporter.zipkin.json", level="WARNING") as cm: + exporter = ZipkinExporter() - self.assertEqual(exporter.timeout, 10) - self.assertEqual(len(cm.records), 1) - self.assertTrue( - any("Invalid value" in message for message in cm.output), - cm.output, - ) + self.assertEqual(exporter.timeout, 10) + self.assertEqual(len(cm.records), 1) + self.assertTrue( + any("Invalid value" in message for message in cm.output), + cm.output, + ) + # patch.dict restores the process environment; no leak between tests. + self.assertEqual(os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT), previous) def test_constructor_empty_timeout_env_var_falls_back(self): for value in ("", " ", "\t\n"): diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py index 978878790b6..178f66b567c 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py @@ -66,17 +66,19 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.timeout, 15) def test_constructor_invalid_timeout_env_var_falls_back(self): - os.environ[OTEL_EXPORTER_ZIPKIN_TIMEOUT] = "10s" - - with self.assertLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING") as cm: - exporter = ZipkinExporter() + previous = os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) + with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "10s"}): + with self.assertLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING") as cm: + exporter = ZipkinExporter() - self.assertEqual(exporter.timeout, 10) - self.assertEqual(len(cm.records), 1) - self.assertTrue( - any("Invalid value" in message for message in cm.output), - cm.output, - ) + self.assertEqual(exporter.timeout, 10) + self.assertEqual(len(cm.records), 1) + self.assertTrue( + any("Invalid value" in message for message in cm.output), + cm.output, + ) + # patch.dict restores the process environment; no leak between tests. + self.assertEqual(os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT), previous) def test_constructor_empty_timeout_env_var_falls_back(self): for value in ("", " ", "\t\n"): From ea8dc2ee5491ee45be3dc391388d27f6df104b13 Mon Sep 17 00:00:00 2001 From: RichardoMu <44485717+RichardoMrMu@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:09:35 +0800 Subject: [PATCH 4/4] test(zipkin): drop redundant environment restore assertion patch.dict already restores the process environment, so asserting it in the invalid-timeout fallback test adds nothing; remove it from both exporters. No behavior change. Assisted-by: Doubao --- .../tests/test_zipkin_exporter.py | 3 --- .../tests/test_zipkin_exporter.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py index 0e08c0d9246..0e49d1d81f5 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py @@ -64,7 +64,6 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.timeout, 15) def test_constructor_invalid_timeout_env_var_falls_back(self): - previous = os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "10s"}): with self.assertLogs("opentelemetry.exporter.zipkin.json", level="WARNING") as cm: exporter = ZipkinExporter() @@ -75,8 +74,6 @@ def test_constructor_invalid_timeout_env_var_falls_back(self): any("Invalid value" in message for message in cm.output), cm.output, ) - # patch.dict restores the process environment; no leak between tests. - self.assertEqual(os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT), previous) def test_constructor_empty_timeout_env_var_falls_back(self): for value in ("", " ", "\t\n"): diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py index 178f66b567c..522d4fb43cb 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py @@ -66,7 +66,6 @@ def test_constructor_env_vars(self): self.assertEqual(exporter.timeout, 15) def test_constructor_invalid_timeout_env_var_falls_back(self): - previous = os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT) with patch.dict(os.environ, {OTEL_EXPORTER_ZIPKIN_TIMEOUT: "10s"}): with self.assertLogs("opentelemetry.exporter.zipkin.proto.http", level="WARNING") as cm: exporter = ZipkinExporter() @@ -77,8 +76,6 @@ def test_constructor_invalid_timeout_env_var_falls_back(self): any("Invalid value" in message for message in cm.output), cm.output, ) - # patch.dict restores the process environment; no leak between tests. - self.assertEqual(os.environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT), previous) def test_constructor_empty_timeout_env_var_falls_back(self): for value in ("", " ", "\t\n"):