Repository navigation
fix(sdk): flush concurrent span/log processors inline when executor shut down at exit - #5835
Open
RichardoMrMu wants to merge 27 commits into
Open
RichardoMrMu wants to merge 27 commits into
RichardoMrMu wants to merge 27 commits into
Conversation
Pull request dashboard statusWaiting on reviewers · refreshed 2026-10-26 21:38 UTC Review the latest changes. Status above doesn't look right?
|
…uble-invoke on submit RuntimeError Adopt the contain+log error-handling contract for ConcurrentMultiSpanProcessor and ConcurrentMultiLogRecordProcessor: - Add a per-dispatch _CallbackAttempt that shares a lock-guarded 'started' claim between the queue worker and the inline fallback taken when executor.submit raises RuntimeError. This prevents the callback from running twice when Thread.start fails after the work item was already enqueued (the previous code ran it inline AND the queued worker ran it). - Contain callback exceptions per processor (logger.exception) so a faulty processor does not block healthy siblings, leak out of shutdown(), or replace the caller's own exception. - Wait for claimed attempts to finish before returning. Tests: - Replace the propagation assertion with containment + sibling-continues + application-exception-preserved assertions. - Add a Thread.start-failure no-double-invoke test for trace and logs. - Real subprocess atexit exit verified: a faulty sibling no longer blocks the healthy Batch processor's EXPORT/SHUTDOWN at interpreter exit. Known limitations: no retry/delivery guarantee, no global exactly-once, no bounded shutdown wait; force_flush path unchanged; logs self-diagnostics recursion not separately exercised.
Remove protected _executor access in test teardown (pylint W0212) by shutting the concurrent processors down through their public shutdown() method, and collapse a double blank line flagged by ruff-format. No production behavior change; the callback containment, at-most-once dispatch and logging reentry isolation are already covered by the behavior tests.
… to avoid handler-lock deadlock A real logging handler holds its own lock across emit(). When ConcurrentMultiLogRecordProcessor.on_emit dispatches to its executor and then blocks waiting on the futures, a worker that re-enters logging through the same LoggingHandler deadlocks on the handler lock the dispatching thread still holds: the dispatcher waits for the worker, the worker waits for the lock. Mark the bridge boundary in LoggingHandler.emit with a thread-local and extend the existing reentry guard so the multi-processor runs callbacks inline whenever it is dispatched from inside that bridge, matching the callback reentry path. Add regression tests using the real LoggingHandler bridge (not a synthetic handler) for reentry inline dispatch and handler-raise isolation, asserting business errors are contained and healthy siblings still receive records.
…idge reentry test
…; no code change)
Wrap the executor-shutdown DEBUG diagnostic and the resolve/submit/future exception logs in safe wrappers so that a raising user logging handler cannot abort the inline fallback or starve healthy siblings. The DEBUG record and business exception traceback are still emitted (via stderr fallback if a handler raises); no log level is raised or diagnostic dropped.
Add a handler that raises outward (no self-catch, DEBUG level, executor pre-shutdown) to prove the executor-shutdown diagnostic and per-callback exception logs no longer abort dispatch. Fails on 8b7e960, passes on the safe-log wrapper fix.
Replace the hollow _BoomHandler (try/except pass) with a handler that raises outward, and add a regression test for the callback-resolution diagnostic path. Covers resolve/submit/future/executor-shutdown-debug under a truly raising user handler.
Address precommit lint: ruff format both test files and add # pylint: disable=protected-access where tests intentionally shut down the internal executor to exercise the atexit inline path.
Patch executor.submit to enqueue via original then raise RuntimeError, with an outward-raising DEBUG handler. Asserts at-most-once dispatch (healthy/faulty each once), 2 enqueues, no escape.
Wait on enqueued futures before asserting, then executor.shutdown(wait=True), and assert stderr contains the diagnostic fallback record.
…essor-atexit-inline-flush
This branch has not been deployed
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
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.
Fixes #5568
Problem
TracerProvider.shutdown()andLoggerProvider.shutdown()are registered viaatexit. CPython runsthreading._shutdown()(which shuts downThreadPoolExecutors) before the atexit queue. By the time the provider shutdown handlers run, theThreadPoolExecutorowned byConcurrentMultiSpanProcessor/ConcurrentMultiLogRecordProcessoris already closed, soexecutor.submit()raisesRuntimeError: cannot schedule new futures after shutdown. The underlyingBatchSpanProcessor/BatchLogRecordProcessornever getshutdown()called, and buffered telemetry is dropped at interpreter exit.Fix
In
_submit_and_await(trace) and_submit_and_wait(logs), wrap only theexecutor.submit()call intry/except RuntimeError. When the executor is already shut down/broken, run the callback inline so shutdown/on_end/on_emit still reach the underlying processors. ARuntimeErrorraised by the callback itself (while the executor is alive) is not caught and still propagates viafuture.result().force_flushtimeout behavior is intentionally left unchanged.Tests
test_shutdown_runs_inline_after_executor_shutdown: executor shut down first, thenshutdown()still calls each underlying processor.test_on_end_runs_inline_after_executor_shutdown(trace):on_endruns inline after executor shutdown.test_callback_runtime_error_propagates_not_swallowed(trace + logs): a callback raisingRuntimeErrorstill propagates out ofshutdown().