Repository navigation
fix(runtime): bound the tracer flush wait on streaming responses - #730
Closed
aidandaly24 wants to merge 1 commit into
Closed
aidandaly24 wants to merge 1 commit into
aidandaly24 wants to merge 1 commit into
Conversation
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:09 — with
GitHub Actions
Failure
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #730 +/- ##
=======================================
Coverage ? 89.68%
=======================================
Files ? 130
Lines ? 11365
Branches ? 1763
=======================================
Hits ? 10193
Misses ? 768
Partials ? 404
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Since 1.25.2, sync-generator streams (including async-generator entrypoints, which are bridged to a sync generator) end with an uncapped _flush_tracer_provider(). OTel SDK 1.45's BatchProcessor.force_flush ignores timeout_millis, and AgentCore Runtime's OTEL_EXPORTER_OTLP_TIMEOUT=5000 is read as seconds, so with no route to the OTLP endpoint the response streams every event and then never closes. Run force_flush on a daemon thread and stop waiting after the cap, lowered from 30 s to 5 s. An overrunning flush keeps _flush_lock until it returns, so later calls skip instead of stacking threads. This also frees the async path's executor thread at the cap, so a stuck flush no longer blocks event loop shutdown. Streaming responses no longer flush in _handle_invocation before they are returned: the stream wrapper flushes when the stream ends, and the extra flush could only hold back the response headers.
aidandaly24
force-pushed
the
fix/bounded-sync-trace-flush
branch
from
November 3, 2026 16:36
defdc20 to
daf319c
Compare
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
aidandaly24
had a problem deploying
to
auto-approve
November 3, 2026 16:36 — with
GitHub Actions
Failure
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
|
Closing this PR in favor of reverting the problematic PR. |
This branch had an error being 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.
Issue #, if available: Regression from #721 (follow-up to #629).
Description of changes:
What
_flush_tracer_provider(), which runs at the end of every streamed/invocationsresponse. The flush now runs on a daemon thread and the caller waits for it withjoin(timeout)instead of relying onforce_flush(timeout_millis=...)._handle_invocationran before returning a streaming response. It only delayed the response headers: the stream wrapper flushes again when the stream ends. Non-streaming responses still flush before they return.Why
Since 1.25.2 (#721), sync-generator streams end with
finally: _flush_tracer_provider(), and that call had no wait cap. Async-generator entrypoints are bridged to a sync generator (_async_gen_to_sync_gen), so they take the same path. Only the async helper had a cap, viaasyncio.wait_for.The timeout passed to
force_flushdoes not bound the wait:BatchProcessor.force_flushignorestimeout_millisand exports until the queue is empty.OTEL_EXPORTER_OTLP_TIMEOUT=5000, which the OTel Python exporter reads as seconds.So on a runtime whose network has no route to the OTLP endpoint (for example VPC mode with no NAT and no X-Ray interface endpoint), a streamed response sends every event and then never closes:
ReadTimeoutError60 s after the last event._flush_lockmakes later invocations skip the flush while the first one is stuck.The pre-stream flush had a smaller version of the same problem. If spans were already queued when a streaming request arrived, it held the headers for up to the cap, and the end-of-stream flush was then skipped because the lock was taken.
How
_flush_tracer_providertakes_flush_lockwithout blocking, starts a daemon thread that callsforce_flushand releases the lock when it returns, and waits at mosttimeout_millis.Lock semantics. A flush that overruns keeps holding
_flush_lockuntil it really returns, so later calls skip it instead of piling up threads behind it. If the thread can't be started, the lock is released at once.Spans are not dropped. Spans the flush misses stay queued for the batch processor's background exporter.
Why 5 s.
OTEL_EXPORTER_OTLP_TIMEOUT=5000ms).Async path.
_flush_tracer_provider_asyncalready bounded the response wait withwait_for(to_thread(...)), but the stuck flush kept a default-executor thread busy, andasyncio.runwaits for that thread when the loop shuts down. Because the inner call is now bounded, the executor thread returns at the cap.Measured on Python 3.10, 3.11, 3.12 and 3.13 with a provider whose
force_flushignores the timeout and blocks for 20 s:asyncio.runreturns afterWhat this does not change. Process exit can still wait for a stuck export, because OTel's own
atexitprovider shutdown waits for it. With opentelemetry-sdk 1.45.1, aBatchSpanProcessorand an exporter blocked for 15 s:Bounding that needs a correct exporter timeout, for example a platform value that OTel Python reads correctly. It is outside this SDK.
Testing
Unit:
uv run pytest tests/ --cov=src --cov-branch: 3773 passed, 10 skipped, 4 xpassed. Coverage is 91.20% in total, 96.01% forruntime/app.pyand 90.0% forruntime/tracing.py.New and updated tests:
test_stream_closes_when_force_flush_ignores_timeout[sync_generator|async_generator](test_app.py). It uses a tracer provider whoseforce_flushignorestimeout_millisand blocks once spans are pending. Before this change the/invocationsstream does not finish (the test fails after 5 s); after it, the stream closes.test_stream_headers_not_delayed_by_flush_of_earlier_spans(test_app.py). Spans are already queued when the request arrives, and the response must not wait on a pre-stream flush.test_returned_async_generator_flushes_after_stream_finishes(test_app.py) covers the async stream path.test_flush_wait_is_bounded_when_provider_ignores_timeout(test_tracing.py). It checks the bounded wait and the lock behaviour: a concurrent flush is skipped while the abandoned one runs, and the next flush runs after it finishes.test_lock_is_released_when_flush_thread_cannot_start(test_tracing.py).Each of these fails without the corresponding source change and passes with it. The runtime test package also passes on Python 3.13.
Lint:
uv run pre-commit run --all-filespasses (ruff, ruff-format and the other hooks).End to end on AgentCore Runtime. Setup:
xray.<region>.amazonaws.com.OTEL_EXPORTER_OTLP_TIMEOUT=5000.InvokeAgentRuntime, reading the full stream with a 60 s read timeout.metadataarrive at about 1 s, then the stream never closes: 5/5ReadTimeoutErrorat about 61 s, in each of 3 roundsmetadata, with headers at 0.4–0.5 s. The second invoke in each session closes 2–4 ms aftermetadata(flush skipped). OneTracer provider flush did not finish within 5000 mswarning per sessionmetadata. 10 traces (50 spans) exported, no export errorsKnown trade-off and follow-up
In a network with no route to the OTLP endpoint, the first response of each session still closes up to 5 s late, where 1.25.1 had no delay. Removing that needs the flush to move off the response path: close the response at once, and keep the session from being frozen or reclaimed while a bounded flush finishes. That depends on how AgentCore Runtime treats a session after the response (for example whether
HealthyBusyfrom/pingdefers a freeze), so it is left to a follow-up. This change stops the hang and is compatible with that design.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.