Skip to content

fix(runtime): flush tracing spans after invocations - #721

Merged
jariy17 merged 1 commit into
mainfrom
fix/runtime-flush-tracer-provider
Oct 27, 2026
Merged

jariy17 merged 1 commit into
mainfrom
fix/runtime-flush-tracer-provider

Conversation

@jariy17

@jariy17 jariy17 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What

  • Force-flush the active OpenTelemetry tracer provider before a non-streaming invocation returns.
  • Flush again after synchronous or asynchronous streaming responses finish.
  • Keep tracing optional and prevent exporter failures from changing invocation results.
  • Add regression coverage for non-streaming and both streaming paths.

How

_flush_tracer_provider calls force_flush defensively on the active provider. _handle_invocation invokes 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 BatchSpanProcessor may not export pending spans before that freeze, causing request traces to be lost.

Fixes #629

def _flush_tracer_provider(timeout_millis: int = 30000) -> None:
"""Flush buffered spans before AgentCore Runtime freezes the microVM."""
try:
from opentelemetry import trace

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import should be on top

@github-actions github-actions Bot added the size/s PR size: S label Oct 27, 2026
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 16:53 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

✅ No Breaking Changes Detected

No public API breaking changes found in this PR.

@github-actions github-actions Bot added size/s PR size: S and removed size/s PR size: S labels Oct 27, 2026
@jariy17
jariy17 marked this pull request as ready for review October 27, 2026 17:04
@jariy17
jariy17 requested a review from a team October 27, 2026 17:04
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Oct 27, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Oct 27, 2026

@nborges-aws nborges-aws left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current commit after the branch refresh is e15e250 (superseding the hash in my previous reply).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jariy17
jariy17 force-pushed the fix/runtime-flush-tracer-provider branch from 18967c2 to 1924ef7 Compare October 27, 2026 19:05
@jariy17
jariy17 force-pushed the fix/runtime-flush-tracer-provider branch from 1924ef7 to e15e250 Compare October 27, 2026 19:05
@github-actions github-actions Bot added size/m PR size: M and removed size/s PR size: S size/m PR size: M labels Oct 27, 2026
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@jariy17
jariy17 deployed to auto-approve October 27, 2026 20:04 — with GitHub Actions Active
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Oct 27, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Oct 27, 2026
@jariy17
jariy17 merged commit 343da26 into main Oct 27, 2026
47 of 48 checks passed

This branch was successfully deployed

1 active deployment
auto-approve — e15e250f Deployed Oct 27, 2026 by jariy17 via Test (identity) #1672
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spans are lost because _handle_invocation never flushes the TracerProvider before the microVM freezes

2 participants