fix(zipkin): tolerate invalid OTEL_EXPORTER_ZIPKIN_TIMEOUT env var by RichardoMrMu · Pull Request #5704 · open-telemetry/opentelemetry-python

RichardoMrMu · 2026-09-28T17:03:15Z

Description

Both Zipkin span exporters construct their timeout with a bare int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) in __init__ (opentelemetry-exporter-zipkin-json and opentelemetry-exporter-zipkin-proto-http). A malformed value such as OTEL_EXPORTER_ZIPKIN_TIMEOUT=10s raises ValueError: invalid literal for int() while the exporter is being built, which takes SDK/TracerProvider startup down before any telemetry can flow.

The OTLP exporters already tolerate this: the HTTP transport has _resolve_timeout() (#5389) and the gRPC transport now has _timeout_from_env() (#5448), both of which log a warning and fall back to the default. The Zipkin exporters were the remaining outlier parsing this env var with an unguarded conversion.

This change adds a private _timeout_from_env() in each Zipkin exporter that:

returns the default (10 seconds) silently when the variable is unset or empty/whitespace;
logs a warning and returns the default when the value is not an integer;
otherwise returns the parsed integer.

Precedence of an explicit timeout= constructor argument is unchanged (timeout or _timeout_from_env()).

Type of change

Bug fix (non-breaking change)

How Has This Been Tested?

Pure-Python, no collector or network needed. Added two tests to each exporter's test_zipkin_exporter.py:

test_constructor_invalid_timeout_env_var_falls_back: sets OTEL_EXPORTER_ZIPKIN_TIMEOUT=10s, asserts the exporter builds with timeout == 10 and emits a WARNING.
test_constructor_empty_timeout_env_var_falls_back: sets the variable to "", asserts timeout == 10.

Red (before the fix), run per package:

>       self.timeout = timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10))
E       ValueError: invalid literal for int() with base 10: '10s'
====================== 2 failed, 15 deselected ======================

Green (after the fix):

opentelemetry-exporter-zipkin-json ........ 17 passed in 0.23s
opentelemetry-exporter-zipkin-proto-http ... 17 passed in 0.26s

The pre-existing constructor/env/export tests in both files continue to pass, including test_constructor_all_params_and_env_vars (explicit timeout= precedence) and test_constructor_env_vars (valid numeric env value).

Verification boundary: logic-level unit tests only (constructor configuration); no export round-trip against a live Zipkin instance was exercised, consistent with the existing tests in these modules which mock requests.Session.post.

opentelemetry-pr-dashboard · 2026-09-28T17:04:58Z

Pull request dashboard status

Waiting on maintainers · refreshed 2026-10-17 07:38 UTC

Merge when ready.

Status above doesn't look right?

Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Copilot

Copilot review overview

🟡 Changes recommended

Empty and whitespace-only values incorrectly emit warnings, and tests do not verify silent fallback.

Review effort: Balanced
Findings: 4

Open (4)

Silently handle blank values before conversion · New
Test no warnings for empty and whitespace-only values · New
Silently handle blank values before conversion · New
Test no warnings for empty and whitespace-only values · New

What changed in this PR

Adds resilient Zipkin timeout environment parsing to prevent malformed values from breaking SDK startup.

Changes:

Falls back to the default timeout for invalid values.
Adds warning and fallback tests.
Adds a changelog entry.

File	Description
`.changelog/5704.fixed`	Documents the fix.
`exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py`	Adds timeout fallback handling.
`exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py`	Tests invalid and empty values.
`exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py`	Adds timeout fallback handling.
`exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py`	Tests invalid and empty values.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

RichardoMrMu · 2026-10-12T05:09:51Z

Answering the inline question about the double environ.get.

The second read is still there because I could not remove it without breaking a check. _timeout_from_env() (b59b960) already resolves the variable and implements the fallback, so self.timeout can be a single read -- I tried exactly that and misc / public-symbols-check failed on run 38206018887:

exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py:148:
ZipkinExporter.timeout: Attribute value was changed:
timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) -> timeout or _timeout_from_env()

scripts/griffe_check.py treats the assigned expression of a public attribute as API surface, so any rewrite of that line is reported as a breaking change.

Behaviourally the second read is dead code rather than a second failure path: timeout = timeout or _timeout_from_env() short-circuits for every non-zero value, and when the first read yields 0 the second int(environ.get(..., 10)) returns 0 again. int("") is unreachable now that empty and whitespace-only values return the default before the int() call.

So the choice is: fold it into one line and accept the public-symbols failure plus the Approve Public API check label, or keep the redundant read as it is. Your call -- I reverted my attempt, so the branch is back at d009679 with 576/576 checks green.

…ar (open-telemetry#5705) * fix(prometheus): tolerate invalid OTEL_EXPORTER_PROMETHEUS_PORT env var _AutoPrometheusMetricReader parsed OTEL_EXPORTER_PROMETHEUS_PORT with a bare int() in __init__, so a non-integer value (e.g. "9464abc") or an empty string raised ValueError while building the auto-configured reader and took opentelemetry-instrument/SDK startup down. This is the same class of startup crash already fixed for the OTLP exporters (HTTP _resolve_timeout in open-telemetry#5389, gRPC _timeout_from_env in open-telemetry#5448) and the Zipkin exporters (open-telemetry#5704). Add a private _port_from_env() that skips unset/empty values silently, logs a warning for non-integer values, and falls back to the 9464 default. Explicit configuration is unaffected (the reader's auto server only reads the env var). Assisted-by: Doubao * changelog: add fragment for PR open-telemetry#5705 * test(prometheus): ruff-format the new entrypoint timeout tests The `misc / precommit` job failed because the two added test signatures were split across lines while ruff (v0.16.0, line-length 120) keeps them on one line. Run `ruff format` so `pre-commit run ruff-format` is clean.

herin049 · 2026-10-12T18:02:58Z

Answering the inline question about the double environ.get.

The second read is still there because I could not remove it without breaking a check. _timeout_from_env() (b59b960) already resolves the variable and implements the fallback, so self.timeout can be a single read -- I tried exactly that and misc / public-symbols-check failed on run 38206018887:
exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py:148:
ZipkinExporter.timeout: Attribute value was changed:
timeout or int(environ.get(OTEL_EXPORTER_ZIPKIN_TIMEOUT, 10)) -> timeout or _timeout_from_env()
scripts/griffe_check.py treats the assigned expression of a public attribute as API surface, so any rewrite of that line is reported as a breaking change.

Behaviourally the second read is dead code rather than a second failure path: timeout = timeout or _timeout_from_env() short-circuits for every non-zero value, and when the first read yields 0 the second int(environ.get(..., 10)) returns 0 again. int("") is unreachable now that empty and whitespace-only values return the default before the int() call.

So the choice is: fold it into one line and accept the public-symbols failure plus the Approve Public API check label, or keep the redundant read as it is. Your call -- I reverted my attempt, so the branch is back at d009679 with 576/576 checks green.

Please remove the double call and we can add a tag to skip the public API check.

RichardoMrMu · 2026-10-13T12:41:49Z

/dashboard route:reviewers

@herin049 the double call is gone. Both exporters now read OTEL_EXPORTER_ZIPKIN_TIMEOUT once: 6f544187 (json) and 51ee5706 (proto-http).

The only red on the current head (407856f3) is the expected one. misc / public-symbols-check, and the check gate that aggregates it, flag just these two lines (job log):

exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py:149: ZipkinExporter.timeout: Attribute value was changed
exporter/opentelemetry-exporter-zipkin-proto-http/src/opentelemetry/exporter/zipkin/proto/http/__init__.py:142: ZipkinExporter.timeout: Attribute value was changed

The other 577 checks pass, so this is ready for the Approve Public API check label. I'm routing it to reviewers because the label is the remaining step.

opentelemetry-pr-dashboard · 2026-10-13T12:43:19Z

@RichardoMrMu, this pull request was routed to reviewers. The handoff remains active across pushes until newer actionable human feedback arrives. Top-level feedback through this request will not return; unresolved review threads remain open.

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

Copilot

🟢 Approval recommended

The focused changes preserve existing precedence, include regression coverage, and have no identified blocking issues.

0 open findings

4 resolved since last review

🧠 Review effort: Balanced

herin049 · 2026-10-16T17:29:24Z

        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()


nit: can we move this to a single line.

Done in 7a72e8a: simplified to a single line.

Collapse the two-line timeout resolution into a single assignment per review nit; no behavior change. Assisted-by: Doubao

RichardoMrMu requested a review from a team as a code owner September 28, 2026 17:03

otelbot-python Bot added this to Python PR digest Sep 28, 2026

opentelemetry-pr-dashboard Bot mentioned this pull request Sep 28, 2026

Pull Request Dashboard #5550

Open

RichardoMrMu added a commit to RichardoMrMu/opentelemetry-python that referenced this pull request Sep 28, 2026

changelog: add fragment for PR open-telemetry#5704

d5e3f76

RichardoMrMu mentioned this pull request Sep 28, 2026

fix(prometheus): tolerate invalid OTEL_EXPORTER_PROMETHEUS_PORT env var #5705

Merged

1 task done

tammy-baylis-swi moved this to Ready for review in Python PR digest Oct 1, 2026

lzchen approved these changes Oct 1, 2026

View reviewed changes

herin049 requested a balanced review from Copilot October 1, 2026 21:08

Copilot started reviewing on behalf of herin049 October 1, 2026 21:09

View session

herin049 approved these changes Oct 1, 2026

View reviewed changes

Copilot AI reviewed Oct 1, 2026

View reviewed changes

carlosalberto reviewed Oct 8, 2026

View reviewed changes

Comment thread exporter/opentelemetry-exporter-zipkin-json/src/opentelemetry/exporter/zipkin/json/__init__.py Outdated

RichardoMrMu force-pushed the fix/zipkin-timeout-env-fallback branch from 035b627 to d009679 Compare October 12, 2026 05:06

herin049 removed the status in Python PR digest Oct 12, 2026

RichardoMrMu force-pushed the fix/zipkin-timeout-env-fallback branch from 407856f to f7be704 Compare October 16, 2026 12:44

Merge branch 'main' into fix/zipkin-timeout-env-fallback

4a2e99a

herin049 requested a balanced review from Copilot October 16, 2026 17:27

herin049 added the Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary label Oct 16, 2026

Copilot started reviewing on behalf of herin049 October 16, 2026 17:27

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

herin049 reviewed Oct 16, 2026

View reviewed changes

style(zipkin): inline timeout assignment in both exporters

7a72e8a

Collapse the two-line timeout resolution into a single assignment per review nit; no behavior change. Assisted-by: Doubao

Navigation Menu

Repository navigation

fix(zipkin): tolerate invalid OTEL_EXPORTER_ZIPKIN_TIMEOUT env var - #5704

fix(zipkin): tolerate invalid OTEL_EXPORTER_ZIPKIN_TIMEOUT env var#5704
RichardoMrMu wants to merge 3 commits into
open-telemetry:mainfrom
RichardoMrMu:fix/zipkin-timeout-env-fallback

RichardoMrMu commented Sep 28, 2026

Uh oh!

opentelemetry-pr-dashboard Bot commented Sep 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RichardoMrMu commented Oct 12, 2026

Uh oh!

herin049 commented Oct 12, 2026

Uh oh!

RichardoMrMu commented Oct 13, 2026

Uh oh!

opentelemetry-pr-dashboard Bot commented Oct 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

herin049 Oct 16, 2026

Uh oh!

RichardoMrMu Oct 17, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

RichardoMrMu commented Sep 28, 2026

Description

Type of change

How Has This Been Tested?

Uh oh!

opentelemetry-pr-dashboard Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request dashboard status

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RichardoMrMu commented Oct 12, 2026

Uh oh!

herin049 commented Oct 12, 2026

Uh oh!

RichardoMrMu commented Oct 13, 2026

Uh oh!

opentelemetry-pr-dashboard Bot commented Oct 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

herin049 Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

RichardoMrMu Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

opentelemetry-pr-dashboard Bot commented Sep 28, 2026 •

edited

Loading