Skip to content
1 change: 1 addition & 0 deletions .changelog/5704.fixed
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you calling two times environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT ? One here (line below) and one from timeout_from_env()?

self.timeout = timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10))

def export(self, spans: Sequence[Span]) -> SpanExportResult:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
Loading