fix(telemetry): re-land the OTel shutdown/SDK fix that missed main - #9
Merged
Conversation
…tdown
Two defects made every CLI invocation print a traceback while silently
dropping remote product telemetry:
1. `emit_product_log` imported `LogRecord` from `opentelemetry.sdk._logs`,
which 1.43 turned into an internal ABC (the public names are now
ReadableLogRecord / ReadWriteLogRecord, and the fields moved onto
`emit()`). pyproject floats the SDK at >=1.27, so both call shapes have
to keep working: fall back to `emit(body=..., attributes=...)` on
ImportError. That path reads the span context from the active context --
the same span `get_current_span()` returned before.
2. The telemetry drain thread is a daemon thread, so it can still be
draining after the interpreter starts finalizing. `Resource.create()`
submits to a ThreadPoolExecutor and the SDK registers atexit hooks
during construction, both of which raise then ("cannot schedule new
futures after interpreter shutdown", "can't register atexit after
shutdown"). Guard the three entry points with `sys.is_finalizing()`;
the event is still recorded locally with exported=False.
Also contain pipeline construction (Resource / LoggerProvider / processor)
in the same try block as the imports, and route the handlers through the
module logger at debug level. Root-level `logging.exception` printed a full
traceback onto the user's terminal on every command, which defeats the
purpose of a module whose job is to keep OTel quiet (see `_apply_silence`).
Measured with the provider stubbed, on opentelemetry-sdk 1.43.0:
before: returns False, 0 events emitted, 6 lines on stderr
after: returns True, 1 event emitted, 0 lines on stderr
Co-Authored-By: lemoncrow <302591943+lemoncrow-agent[bot]@users.noreply.github.com>
`pyproject.toml` has carried `portable = ["zstandard>=0.22"]` since the Phase B portable-sidecar work, but the lock was never regenerated, so `uv.lock` disagreed with the manifest on both the extra and its `provides-extras` list. Co-Authored-By: lemoncrow <302591943+lemoncrow-agent[bot]@users.noreply.github.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.
Re-lands PR #6, which merged but never reached
main.What happened
#6 was opened with base
merge-upstream-v0.6.8(stacked, so itsuv.lockchange sat on the 0.6.8 version line). #5 then merged commit3c90e844of that branch intomain. #6 merged afterwards, into the branch — producing9d5f2ae6, a commitmainnever saw. Deleting the merged branch during cleanup left those two commits unreachable from any ref.GitHub retargets a stacked PR when its base branch is deleted, not when the base branch is merged, so nothing flagged the gap. Both PRs read as merged and the work was gone from
main.This PR
The same two commits, cherry-picked onto current
main(c600f277), unchanged in content:fix(telemetry)— the opentelemetry-sdk >=1.43LogRecordfallback and the threesys.is_finalizing()guardschore(lock)— theportable = ["zstandard>=0.22"]extra inuv.lockBoth applied without conflict. Verified on this branch:
is_finalizingpresent inotel.py(2 occurrences),portablepresent inuv.lock.Verification
The failure is
test_async_emit_stays_off_hot_path_and_persists_after_flush, pre-existing and order-dependent — it passes when run alone and fails under this selection with or without these commits (verified against the parent in the original PR).Note for next time
Merge a stacked PR before its base, or retarget it to
mainonce the base lands. The parent-branch cleanup is what made this recoverable only from local objects.