fix(otel): harden partial-install detection + lazy-import (0.8.4)#35
Merged
Conversation
Pairs find_spec('opentelemetry.trace') with try/except ImportError around
the lazy `from opentelemetry import trace`. Closes the partial-install
hole where opentelemetry/ namespace directory exists but api package is
missing or broken — currently crashes a live request mid-cycle.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each fix is TDD: failing test first, then production change. T1 pins the find_spec target via source check (the live probe returns non-None either way under --all-extras). T2 simulates the crash with sys.modules[ 'opentelemetry'] = _BrokenOpenTelemetry() that raises ImportError on attribute access. T3 = release notes + PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`opentelemetry` is a PEP 420 native namespace package — any
`opentelemetry-instrumentation-*` package creates the directory even when
`opentelemetry-api` is absent. `find_spec("opentelemetry")` then returns
non-None and `is_otel_installed` becomes True; the lazy
`from opentelemetry import trace` in `_emit_event` subsequently raises
ImportError mid-request.
`find_spec("opentelemetry.trace")` would fix the false-positive but causes
CPython to load the opentelemetry namespace package into sys.modules as a
side-effect of resolving the submodule, breaking the transitive-import
isolation guarantee enforced by test_optional_extras_isolation.py.
Use `importlib.metadata.distribution("opentelemetry-api")` instead — it
probes the package registry directly with no sys.modules side-effects, and
raises PackageNotFoundError when the api package is absent. The check is
now cheap and correct under every install permutation we can construct.
The new test source-checks the probe target so a future revert to
find_spec trips the regression guard, even though live distribution() calls
succeed either way under --all-extras.
Closes audit Low finding (import_checker.py:8) from
planning/audit/2026-06-07-deep-audit.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
_emit_event gated the OTel path on `if import_checker.is_otel_installed` but ran `from opentelemetry import trace` unguarded. If is_otel_installed was True yet the import failed (PEP 420 namespace false-positive in 0.8.3 and earlier; or any future partial install / broken api package), the ImportError escaped _emit_event and crashed the middleware calling it (AsyncRetry, Retry, AsyncBulkhead, Bulkhead) mid-request. Wrap the lazy import in `try/except ImportError`. On failure, return — the structured log record on the line above has already fired, so emission degrades to log-only. Catch ImportError specifically, not bare Exception: misconfigured-tracer crashes (RuntimeError, AttributeError) should still surface; only the install-gate-is-wrong case is in scope. _emit_event's docstring grows one sentence describing the soft-fallback. Closes audit Low finding (observability.py:40) from planning/audit/2026-06-07-deep-audit.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two paired defensive fixes (importlib.metadata.distribution probe +
try/except ImportError wrap) close the chunk-2 partial-install audit
findings. No API change; the only observable behavior change is "no
longer crashes" on partial installs.
Note: the spec called for find_spec("opentelemetry.trace") as the new
probe; release notes reflect what actually shipped (distribution probe,
chosen to avoid a sys.modules side-effect that breaks the existing
isolation test).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI failed across all Python versions because the new try/except branch in import_checker.py (`except PackageNotFoundError: is_otel_installed = False`) is unreachable under `--all-extras` — opentelemetry-api IS installed, so the False branch never runs at module load time. Same gap for the new test's `else` cleanup branch. Two fixes that satisfy the project's 100% coverage rule: - Extract the probe into `_is_distribution_installed(name)` helper. Two new unit tests cover both branches: monkeypatch distribution() to raise PackageNotFoundError (False path); call against opentelemetry-api which IS installed (True path). - Rewrite the existing partial-install test to use `monkeypatch.setitem` instead of manual save/restore — no else branch needed. The source-pinning assertion (regression guard against revert to find_spec on the namespace) split into two simple `in` checks to satisfy PT018. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes 2 of the remaining audit findings — the OpenTelemetry partial-install hazard from chunk 2. See
planning/specs/2026-06-08-otel-partial-install-design.mdfor the design andplanning/releases/0.8.4.mdfor the user-facing release notes.The hazard
find_spec("opentelemetry")returns truthy becauseopentelemetry/is a PEP 420 namespace package — created by anyopentelemetry-instrumentation-*install.is_otel_installedbecomes True even whenopentelemetry-apiis absent.from opentelemetry import tracein_emit_eventwas unguarded — raised ImportError mid-request, crashing the middleware.The fixes (paired per the audit's recommendation)
import_checker.is_otel_installedprobesimportlib.metadata.distribution("opentelemetry-api")— closes the false-positive at detection time without thesys.modulesside-effect thatfind_spec("opentelemetry.trace")would have introduced._emit_eventwraps the lazy import intry/except ImportError— closes the runtime crash, degrades to log-only emission.Audit findings closed
_internal/import_checker.py:8_internal/observability.py:40Spec deviation
The spec called for
find_spec("opentelemetry.trace")as the new probe. During implementation, this was found to break the existingtest_importing_httpware_does_not_import_opentelemetryisolation test — CPython resolves submodule probes by importing the parent namespace package, populatingsys.modulesas a side-effect.importlib.metadata.distribution("opentelemetry-api")probes the package registry directly with nosys.modulesside effects, and is arguably a tighter match for the spec's intent ("require the api package").Test plan
distribution("opentelemetry-api")must appear in import_checker.py).sys.modules['opentelemetry'] = _BrokenOpenTelemetry()and verifies_emit_eventreturns without raising while still emitting the structured log record.just lint-ci+ full retry + bulkhead + observability + isolation suites green after each commit.Release
Tag
0.8.4from the merge SHA after this PR lands.🤖 Generated with Claude Code