feat(telemetry): add hierarchical session tracing spans#4071
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “session tracing” layer in packages/core telemetry to build a hierarchical OpenTelemetry span tree (interaction → LLM request → tool → tool.execution) and wires it into the client, tool scheduler, and content generator paths.
Changes:
- Add new
telemetry/session-tracing.tsmodule to manage span lifecycles (including concurrency-safe tool spans). - Integrate interaction spans into
core/client.ts, tool spans intocore/coreToolScheduler.ts, and LLM request spans intocore/loggingContentGenerator.ts. - Add telemetry shutdown handling and re-export the new tracing utilities from
telemetry/index.ts, plus new span-name constants.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/telemetry/session-tracing.ts | Adds hierarchical span creation/ending utilities with ALS + registry-based tracking. |
| packages/core/src/telemetry/session-tracing.test.ts | Adds Vitest coverage for interaction/tool/LLM spans, including concurrency and noop behavior. |
| packages/core/src/telemetry/constants.ts | Adds constants for the new span names. |
| packages/core/src/telemetry/index.ts | Re-exports the new session tracing APIs. |
| packages/core/src/telemetry/sdk.ts | Attempts to end an active interaction span during telemetry shutdown. |
| packages/core/src/core/client.ts | Starts/ends interaction spans around sendMessageStream execution paths. |
| packages/core/src/core/coreToolScheduler.ts | Starts/ends tool spans and tool execution sub-spans around tool execution. |
| packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts | Starts/ends LLM request spans around generateContent (streaming and non-streaming). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add OpenTelemetry-based session tracing that creates hierarchical spans for user interactions. Interaction spans wrap the full sendMessageStream lifecycle and are properly ended at all exit points (ok, error, cancelled). - New session-tracing.ts module with concurrent-safe span management - Interaction spans started for UserQuery/Cron/Notification message types - Shutdown safety net ends active interaction span before SDK shutdown - Memory-safe span storage using WeakRef + strongRefs + 30-min TTL cleanup - NOOP_SPAN sentinel avoids leaked spans when SDK is not initialized
3ed7ca7 to
b2545d2
Compare
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
- TTL cleanup now sets ctx.ended=true before span.end() to prevent double-end - Interaction spans stored in strongSpans for GC-safe TTL cleanup - cancelled status uses SpanStatusCode.OK per codebase convention - endLLMRequestSpan treats missing metadata as OK, not ERROR - Module-level lastInteractionCtx fallback for shutdown outside ALS context - try/finally safety net in sendMessageStream for uncaught exceptions
- Fix next-speaker continuation span leak by overriding options.type to Hook - Add setStatus call to endToolExecutionSpan for consistent span status - Sanitize API error messages in interaction span status
wenshao
reviewed
May 12, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
wenshao
approved these changes
May 12, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Additional findings from second-opinion review (DeepSeek/deepseek-v4-pro):
[Suggestion] TTL cleanup logic has zero test coverage: ensureCleanupInterval (setInterval-driven 30-min TTL cleanup in session-tracing.ts:81-89) has no vitest fake-timer tests — zero vi.useFakeTimers() calls in session-tracing.test.ts. Long-running sessions could leak spans or prematurely end them without CI catching it.
[Suggestion] client.ts span integration has zero test coverage: sendMessageStream has 14 startInteractionSpan/endInteractionSpan call sites but client.test.ts has zero references to any span function. The try/finally safety net and status-setting on early-return paths (max turns, loop detection, cancellation, hook continuation) are not end-to-end verified.
[Suggestion] Significant code duplication across four end*Span functions: endInteractionSpan, endLLMRequestSpan, endToolSpan, and endToolExecutionSpan (lines 146-376) share the same ~30-line pattern of WeakRef lookup → ended guard → status/attribute build → span.end() → dual-map delete. Consider extracting a shared internal helper.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
doudouOUC
commented
May 12, 2026
doudouOUC
left a comment
Collaborator
Author
There was a problem hiding this comment.
Thanks for the second-opinion findings. Responses below:
TTL cleanup test coverage: Agreed this would be nice to have, but the TTL cleanup is a simple defensive mechanism (check startTime < cutoff → end span). The core ended flag fix is already covered by unit tests. Will consider adding fake-timer tests as a follow-up, not blocking this PR.
client.ts span integration tests: Testing sendMessageStream span integration requires mocking GeminiChat, Turn, loopDetector, and many other dependencies — this is integration test territory. The session-tracing module itself has 20 unit tests covering API correctness. The client.ts calls are straightforward start/end pass-throughs with no complex logic.
Code duplication across end*Span functions: Each function has meaningful differences that make extraction counterproductive:
endInteractionSpan: ALS fallback vialastInteractionCtx+enterWith(undefined)endLLMRequestSpan:metadata === undefined→ OK, token attributesendToolSpan:success !== falsethree-value logicendToolExecutionSpan: minimal version, no status setting
Extracting a shared helper would add conditional branches and parameter plumbing that reduce readability. Each function is ~20 lines — direct reading is clearer than indirection.
None adopted in this PR. Could you take another look at the current state?
Open
53 tasks
5 tasks
wenshao
pushed a commit
that referenced
this pull request
May 17, 2026
* feat(telemetry): add hierarchical session tracing with interaction spans Add OpenTelemetry-based session tracing that creates hierarchical spans for user interactions. Interaction spans wrap the full sendMessageStream lifecycle and are properly ended at all exit points (ok, error, cancelled). - New session-tracing.ts module with concurrent-safe span management - Interaction spans started for UserQuery/Cron/Notification message types - Shutdown safety net ends active interaction span before SDK shutdown - Memory-safe span storage using WeakRef + strongRefs + 30-min TTL cleanup - NOOP_SPAN sentinel avoids leaked spans when SDK is not initialized * fix(telemetry): address review comments on session-tracing - TTL cleanup now sets ctx.ended=true before span.end() to prevent double-end - Interaction spans stored in strongSpans for GC-safe TTL cleanup - cancelled status uses SpanStatusCode.OK per codebase convention - endLLMRequestSpan treats missing metadata as OK, not ERROR - Module-level lastInteractionCtx fallback for shutdown outside ALS context - try/finally safety net in sendMessageStream for uncaught exceptions * fix: apply auto-fixes from /review - Fix next-speaker continuation span leak by overriding options.type to Hook - Add setStatus call to endToolExecutionSpan for consistent span status - Sanitize API error messages in interaction span status --------- Co-authored-by: jinye.djy <jinye.djy@alibaba-inc.com>
Merged
3 tasks
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.
Summary
Add OpenTelemetry hierarchical session tracing with interaction-level spans that wrap the full
sendMessageStreamlifecycle.session-tracing.tsmodule — concurrent-safe span management using explicit span references (no shared ALS for tool/LLM spans),WeakRef+strongRefs+ 30-min TTL cleanup,NOOP_SPANsentinel when SDK is uninitializedclient.ts— started for UserQuery/Cron/Notification message types, properly ended at all exit points (ok/error/cancelled), including recursive yield* paths, with try/finally safety net for uncaught exceptionssdk.ts— ends active interaction span before SDK shutdown to prevent orphan spans on Ctrl+CSPAN_INTERACTION,SPAN_LLM_REQUEST,SPAN_TOOL,SPAN_TOOL_EXECUTIONin constants.tsReview fixes (second commit)
ctx.ended = truebeforespan.end()to prevent double-endstrongSpansfor GC-safe TTL cleanupcancelledstatus usesSpanStatusCode.OKper codebase convention (log-to-span-processor.ts) —qwen-code.turn_statusattribute provides classificationendLLMRequestSpantreats missing metadata as OK, not ERRORlastInteractionCtxfallback ensuresshutdownTelemetry()can end the active span even outside ALS contextsendMessageStreambody wrapped in try/finally — idempotent safety net for span cleanup on uncaught exceptionsTest plan
人工验证
符合预期
🤖 Generated with Qwen Code