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..fe92f43bfa0 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,6 +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 or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) def export(self, spans: Sequence[Span]) -> SpanExportResult: 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..422bd407bba 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,6 +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 or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) def export(self, spans: Sequence[Span]) -> SpanExportResult: 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."""