Repository navigation
fix(runtime): flush tracing spans after invocations - #721
Conversation
| def _flush_tracer_provider(timeout_millis: int = 30000) -> None: | ||
| """Flush buffered spans before AgentCore Runtime freezes the microVM.""" | ||
| try: | ||
| from opentelemetry import trace |
There was a problem hiding this comment.
import should be on top
✅ No Breaking Changes DetectedNo public API breaking changes found in this PR. |
|
Claude Security Review: no high-confidence findings. (run) |
nborges-aws
left a comment
There was a problem hiding this comment.
One comment, otherwise LGTM
| duration = time.time() - start_time | ||
| self.logger.exception("Invocation failed (%.3fs)", duration) | ||
| return JSONResponse({"error": str(e)}, status_code=500) | ||
| finally: |
There was a problem hiding this comment.
calls force_flush() from the async invocation handler; the streaming wrapper does the same at L#899. If an exporter stalls, /ping and other requests cant run while the flush waits. OpenTelemetry’s processor can block during export, and its current implementation does not enforce the timeout. Run the flush off the event loop and bound how long a response waits for it
There was a problem hiding this comment.
Addressed in 1924ef7. Async response paths now run the flush with asyncio.to_thread() and bound the await with asyncio.wait_for(), so the event loop remains available for /ping and other requests even if an exporter stalls. A non-blocking flush lock also coalesces overlapping flushes so a stuck exporter cannot consume additional worker threads. Added coverage for worker-thread execution, bounded waiting, and duplicate-flush handling; all runtime tests pass.
There was a problem hiding this comment.
Current commit after the branch refresh is e15e250 (superseding the hash in my previous reply).
There was a problem hiding this comment.
Also validated this against a hosted AgentCore Runtime using the PR commit. A test span processor deliberately blocked force_flush() for 45 seconds while ignoring its timeout. During that blocked flush, a same-process /ping probe returned Healthy in 9 ms. The invocation stopped waiting at the 30-second bound, and a second invocation on the same runtime session/process completed in 2.36 seconds while the original flush was still running, confirming overlapping flushes are coalesced. The temporary runtime and stack were deleted after the test.
18967c2 to
1924ef7
Compare
1924ef7 to
e15e250
Compare
|
Claude Security Review: no high-confidence findings. (run) |
What
How
_flush_tracer_providercallsforce_flushdefensively on the active provider._handle_invocationinvokes it before returning, and both streaming wrappers invoke it again after their generators finish.Why
AgentCore Runtime can freeze the microVM immediately after the invocation response completes. Buffered processors such as
BatchSpanProcessormay not export pending spans before that freeze, causing request traces to be lost.Fixes #629