Force-flush trace spans before the A2A response completes for substrate#2176
Force-flush trace spans before the A2A response completes for substrate#2176iplay88keys wants to merge 4 commits into
Conversation
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
There was a problem hiding this comment.
Pull request overview
This PR ensures OpenTelemetry spans are force-flushed before an A2A response completes on Agent Substrate (to avoid buffered spans being checkpoint-frozen and exported only on the next resume), and updates Helm helper templates to optionally include an image.name segment when constructing image references.
Changes:
- Add Go
telemetry.ForceFlush(ctx)and invoke it after the root invocation span ends in the Go A2A executor. - Add Python
kagent.core.tracing.force_flush()and invoke it at the end of the Python ADK executor viaasyncio.to_threadto avoid blocking the event loop. - Update
grafana-mcpandquerydocHelm image helpers to include an optionalimage.namesegment in the image path.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| python/packages/kagent-core/tests/test_tracing_configure.py | Adds unit tests for the new Python tracing force-flush helper behavior. |
| python/packages/kagent-core/src/kagent/core/tracing/_utils.py | Introduces force_flush() to flush buffered spans with a bounded timeout and swallow exporter errors. |
| python/packages/kagent-core/src/kagent/core/tracing/init.py | Exports force_flush from the tracing package public API. |
| python/packages/kagent-adk/src/kagent/adk/_agent_executor.py | Calls tracing force-flush in execute() cleanup using asyncio.to_thread. |
| helm/tools/querydoc/templates/_helpers.tpl | Extends image reference construction to include optional image.name. |
| helm/tools/grafana-mcp/templates/_helpers.tpl | Extends image reference construction to include optional image.name. |
| go/adk/pkg/telemetry/tracing.go | Adds ForceFlush helper that detaches from request cancellation and flushes spans with a 3s timeout. |
| go/adk/pkg/telemetry/tracing_test.go | Adds test ensuring buffered spans are exported on flush and that no-op providers don’t panic. |
| go/adk/pkg/a2a/executor.go | Flushes spans after ending the invocation span in deferred executor cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
…-updates Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
| # The flush is best-effort; clear the cancellation and move on. | ||
| current_task = asyncio.current_task() | ||
| if current_task is not None: | ||
| while current_task.uncancel() > 0: |
There was a problem hiding this comment.
Task.uncancel() was added in python 3.11 but pyproject.toml declares requires-python = '>=3.10'
| ctx, invocationSpan := telemetry.StartInvocationSpan(ctx) | ||
| defer invocationSpan.End() | ||
| defer func() { | ||
| invocationSpan.End() |
There was a problem hiding this comment.
Hm, I don't think invocation is the root span. Not sure if we can flush after the middleware span though in case of substrate. Can you please try it?
| if !ok { | ||
| return | ||
| } | ||
| flushCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 3*time.Second) |
There was a problem hiding this comment.
Maybe we should make this configurable, 3s seems a lot in some cases, especially if users expect "user-visible chat latency".
Description
On Agent Substrate, the session actor is gVisor-checkpointed as soon as the A2A response body closes. Spans still buffered in the SDK's
BatchSpanProcessor(default ~5s flush interval) freeze into the snapshot and only export when the session is next resumed — so a session's last message never produces a trace, and every other message's trace is delayed until the next chat. Both ADK executors now force-flush the tracer provider before the response completes.Go ADK
telemetry.ForceFlush(ctx): flushes the global tracer provider's batch processor with a detached 3s timeout (the request context is already canceled when deferred cleanup runs); no-op for providers withoutForceFlush(tracing disabled).a2a/executor.gocalls it in the execute defer, after the rootinvocationspan ends so the root span is included in the flush.Python ADK
kagent.core.tracing.force_flush(timeout_millis=3000): no-op when the provider has noforce_flush; exporter errors are logged, never raised._agent_executor.pycalls it in afinallyonexecute()viaasyncio.to_thread, so the blocking gRPC export cannot stall the event loop.A collector outage costs at most one bounded 3s flush per invocation, after response events are already published — it never adds user-visible chat latency.
Also included: the
grafana-mcpandquerydochelm image helpers now append the optionalimage.namesegment when set (compactdrops it when absent, so charts using the default two-segmentregistry/repositoryvalues render unchanged).