feat(telemetry): per-prompt traceId for bounded, renderable traces by doudouOUC · Pull Request #4661 · QwenLM/qwen-code

doudouOUC · 2026-05-31T17:07:11Z

Summary

Each interaction/prompt now gets its own fresh traceId (trace root) instead of sharing one session-level traceId derived from SHA-256(sessionId)
Adds SessionIdSpanProcessor to stamp session.id attribute on all exported spans (including auto-instrumented HTTP spans), enabling cross-prompt correlation via attribute query
withInteractionSpan defaults to ROOT_CONTEXT when no parentContext is provided (daemon passes its HTTP span as parent)
Simplifies parent context resolution (resolveParentContext / getParentContext) by removing the now-unnecessary session root fallback
Debug logger falls back to deriveTraceId(sessionId) for log-line grep correlation

Motivation

Long sessions produce thousands of spans under a single traceId — ARMS and Jaeger cannot render such traces. Daemon mode is worse: the entire process lifetime shares one traceId. This was identified as a remaining sub-item of #4554.

What doesn't change

session.id attribute (already present on interaction spans) — now stamped on ALL spans
LogToSpanProcessor fallback — orphaned logs still use deriveTraceId(sessionId) as catch-all
createSessionRootContext — retained as @deprecated for backward compatibility
Interactive/TUI behavior — no user-visible change
resolveSessionId — daemon multi-session attribution unchanged

Migration

ARMS queries: traceId = SHA256(sessionId)[:32] → attribute.session.id = <sessionId>
Coexistence: old sessions (single traceId) and new sessions (per-prompt traceId) naturally coexist

Test plan

209 telemetry tests pass (tracer, session-tracing, sdk, log-to-span-processor, trace-id-utils, debugLogger)
Two rounds of undirected code audit + two rounds of Codex review
Manual: OTEL_TRACES_EXPORTER=console TELEMETRY_ENABLED=true npx qwen --print "hello" — verify distinct traceIds per interaction

🤖 Generated with Qwen Code

github-actions · 2026-05-31T17:08:18Z

📋 Review Summary

This PR refactors telemetry tracing to use per-prompt traceIds instead of a single session-level traceId, solving the ARMS/Jaeger rendering issue for long sessions (#4554). The implementation is clean and well-tested, introducing a SessionIdSpanProcessor for cross-prompt correlation via session.id attribute while allowing each interaction to be its own trace root.

🔍 General Feedback

Architecture: The shift from traceId-based correlation to attribute-based (session.id) is the right approach for scalable observability
Test coverage: Comprehensive test updates across 8 files demonstrate thorough validation of the new behavior
Backward compatibility: Good handling of deprecated APIs and coexistence of old/new session formats
Code quality: Clean removal of session-root fallback logic simplifies parent context resolution significantly
Documentation: PR description clearly explains motivation, migration path, and test plan

🎯 Specific Feedback

🟡 High

File: packages/core/src/telemetry/session-tracing.ts:122-133 - The resolveParentContext function now only checks for explicit parent and falls back to otelContext.active(). The comment mentions "tools-inside-tools cases" but the fallback behavior when there's no active span could use clarification. Consider adding a comment about what happens when a tool executes completely outside any interaction context (does it become a trace root with no session correlation beyond the attribute?).

🟢 Medium

File: packages/core/src/telemetry/sdk.ts:157-167 - The SessionIdSpanProcessor is a clean implementation, but consider whether onEnd should be a no-op. If future requirements need session-based span processing, having the hook already present would be convenient. A brief comment explaining why it's intentionally empty would help.
File: packages/core/src/telemetry/session-tracing.ts:382-395 - The withInteractionSpan function now defaults to ROOT_CONTEXT when no parentContext is provided. This is correct for daemon mode, but the comment could explicitly mention that daemon callers should pass their HTTP span context explicitly to maintain trace continuity for the HTTP request itself.
File: packages/core/src/telemetry/tracer.ts:74-78 - The simplified getParentContext now just returns context.active(). The SYNC comment references session-tracing.ts but that file's resolveParentContext now has more logic (checks for explicit parent first). Consider updating the comment to note they diverge intentionally now.

🔵 Low

File: packages/core/src/telemetry/sdk.ts:356-365 - The refreshSessionContext Javadoc mentions "so that SessionIdSpanProcessor stamps spans with the correct session.id" which is accurate. Consider adding a brief example of when session changes occur (the test mentions /clear, /resume commands) to help future maintainers understand the use cases.
File: packages/core/src/utils/debugLogger.ts:96-106 - The getSessionRootTraceContext now uses deriveTraceId(sessionId) with a synthetic spanId of 16 zeros. This is clever for log correlation, but the magic number '0'.repeat(16) could use a comment explaining it's a synthetic/placeholder spanId for log-to-trace correlation.
File: packages/core/src/telemetry/session-tracing.ts:459-472 - The startLLMRequestSpan comment says "attaches to the tool span instead of becoming a separate trace root" but with the new logic, if there's no active context, it will become a trace root. Consider updating the comment to reflect the actual behavior.

✅ Highlights

Excellent migration planning: The PR description's migration section clearly spells out the ARMS query change from traceId = SHA256(sessionId) to attribute.session.id = sessionId
Clean deprecation strategy: Keeping createSessionRootContext as @deprecated rather than removing it avoids breaking existing code
Thoughtful test updates: Test changes correctly validate the new ROOT_CONTEXT behavior and session attribute stamping
Minimal invasive changes: The core logic changes are surgical—only modifying parent context resolution and adding the attribute processor
Good observability design: Using span attributes for correlation instead of traceId is the industry-standard approach for high-cardinality session tracking

Copilot

Pull request overview

This PR updates the core OpenTelemetry tracing model so each interaction/prompt becomes its own trace root (new traceId per prompt) to keep traces bounded/renderable, while preserving cross-prompt correlation via a session.id span attribute applied broadly across spans.

Changes:

Make interaction spans start from ROOT_CONTEXT (fresh traceId per prompt) and simplify parent context resolution to rely on context.active() rather than a synthetic session root.
Add SessionIdSpanProcessor intended to stamp session.id onto all spans (including auto-instrumented HTTP spans).
Update debug logger trace correlation fallback to derive traceId from sessionId when no active span exists, and align tests with the new behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/core/src/utils/debugLogger.ts	Switch fallback trace correlation from session root span context to `deriveTraceId(sessionId)`.
packages/core/src/utils/debugLogger.test.ts	Update fallback trace context test to match derived traceId behavior.
packages/core/src/telemetry/tracer.ts	Simplify parent context selection to always use `context.active()`; deprecate session-root-based parenting.
packages/core/src/telemetry/tracer.test.ts	Update parent context selection tests for the new `context.active()`-only behavior.
packages/core/src/telemetry/session-tracing.ts	Make interaction spans trace roots (`ROOT_CONTEXT`) and simplify parent resolution logic.
packages/core/src/telemetry/session-tracing.test.ts	Update interaction span parenting assertions and add coverage for session.id correlation semantics.
packages/core/src/telemetry/sdk.ts	Add `SessionIdSpanProcessor` and stop creating/storing a synthetic session root context at init/refresh.
packages/core/src/telemetry/sdk.test.ts	Adjust session context refresh tests to reflect removal of synthetic session root context creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@deprecated

Previously all spans within a session shared one traceId derived from SHA-256(sessionId). Long sessions produced unbounded traces that ARMS and Jaeger could not render. This change makes each interaction span a trace root with a fresh SDK-generated traceId. Cross-prompt correlation uses the session.id span attribute (already present on interaction spans, now stamped on all spans via SessionIdSpanProcessor). Key changes: - startInteractionSpan uses ROOT_CONTEXT instead of session root - withInteractionSpan defaults to ROOT_CONTEXT when no parentContext - SessionIdSpanProcessor stamps session.id on every exported span - resolveParentContext / getParentContext simplified (no session root fallback) - debugLogger falls back to deriveTraceId(sessionId) for log-line grep - LogToSpanProcessor unchanged (naturally adapts) - createSessionRootContext marked @deprecated Closes #4554 (per-prompt traceId sub-item)

wenshao

[Suggestion] startToolBlockedOnUserSpan and startHookSpan don't set session.id explicitly in their span attributes, unlike the other five span creators in this file (startInteractionSpan, withInteractionSpan, startLLMRequestSpan, startToolSpan, startToolExecutionSpan) which all use resolveSessionId(). These two functions rely entirely on SessionIdSpanProcessor reading the process-global getCurrentSessionId(). In daemon mode with concurrent sessions, this could stamp the wrong session ID on these spans. Consider adding resolveSessionId(parentCtx) to both functions for consistency with the established pattern.

— qwen3.7-max via Qwen Code /review

…ry/catch

doudouOUC · 2026-06-01T05:17:52Z

Review response — `6d43586`

Comment	Action
Cache `deriveTraceId` in debugLogger (debugLogger.ts:101)	Fixed — added session-level caching
Remove vestigial try/catch in `refreshSessionContext` (sdk.ts:375)	Fixed — removed dead try/catch
`SessionIdSpanProcessor` unit tests (sdk.ts:157)	Deferred — follow-up PR
Dead `ctx` param in `setSessionContext` (sdk.ts:360)	Deferred — follow-up cleanup PR
Remove `createSessionRootContext` + `shouldForceSampled` (tracer.ts:250)	Deferred — follow-up cleanup PR
`startToolBlockedOnUserSpan`/`startHookSpan` missing `resolveSessionId`	Deferred — pre-existing gap (not introduced by this PR), will address in daemon multi-session follow-up

chiga0

Review: feat(telemetry): per-prompt traceId for bounded, renderable traces

PR: #4661 | Author: doudouOUC | +104/-144 across 8 files, 2 commits

Summary

Each interaction/prompt now gets its own trace root with a fresh SDK-generated traceId, replacing the old model where all spans in a session shared one traceId derived from SHA-256(sessionId). This prevents unbounded traces that ARMS/Jaeger can't render. Cross-prompt correlation uses the session.id span attribute stamped on all exported spans via SessionIdSpanProcessor.

Review Coverage

Reviewed all 8 files: sdk.ts (SessionIdSpanProcessor, init/refresh simplification), session-tracing.ts (ROOT_CONTEXT parenting, simplified resolveParentContext), tracer.ts (simplified getParentContext, deprecated createSessionRootContext), debugLogger.ts (cached deriveTraceId fallback), and all 4 test files.

Findings

0 blocking findings. Clean telemetry architecture change.

Highlights:

SessionIdSpanProcessor correctly checks attributes['session.id'] before writing — doesn't clobber explicitly set values from resolveSessionId()
startInteractionSpan uses ROOT_CONTEXT — each interaction is a trace root with fresh traceId
resolveParentContext simplified from 4-level priority (explicit parent → active span → session root → active context) to 2-level (explicit parent → active context)
withInteractionSpan defaults to ROOT_CONTEXT when no parentContext — correct for daemon cron paths
Debug logger caches deriveTraceId(sessionId) — avoids synchronous SHA-256 on every log call (commit 2 fix)
refreshSessionContext correctly simplified — try/catch removed (no longer needed)
createSessionRootContext properly deprecated, retained for backward compatibility
All test files updated: session-context mocks removed, ROOT_CONTEXT assertions added, session.id stamping tested

wenshao review status: 7 suggestions — cache deriveTraceId + remove vestigial try/catch fixed in commit 2; SessionIdSpanProcessor unit tests, resetDebugLoggingState cache clearing, setAttribute try/catch, dead code cleanup, setSessionContext signature — deferred to follow-up PRs (agreed by author).

LGTM.

— QoderWork automated review

… in resetDebugLoggingState

doudouOUC · 2026-06-01T06:50:44Z

Review response — `0a9a3a1`

Comment	Action
`onStart` needs try/catch (sdk.ts:158)	Fixed — wrapped in try/catch
`resetDebugLoggingState` must clear cache (debugLogger.ts:97)	Fixed — clears cachedSessionId/cachedTraceId

wenshao · 2026-06-01T10:37:39Z

Verification Report — PR #4661

Commit: 0a9a3a134a (fix: guard SessionIdSpanProcessor.onStart with try/catch, clear cache in resetDebugLoggingState)
Base: daemon_mode_b_main
Tester: wenshao
Date: 2026-06-01

Test Results

Check	Result	Details
Unit tests (telemetry + debugLogger)	PASS	6 files, 210 tests passed
TypeScript (`tsc --noEmit`)	PASS	0 errors
ESLint (`--max-warnings 0`)	PASS	0 warnings, 0 errors
Build (`packages/core`)	PASS	Successfully copied files
`git diff --check`	PASS	No whitespace errors

Test File Breakdown

File	Tests	Time
`telemetry/session-tracing.test.ts`	82	21ms
`telemetry/sdk.test.ts`	35	77ms
`telemetry/log-to-span-processor.test.ts`	32	17ms
`telemetry/tracer.test.ts`	31	9ms
`utils/debugLogger.test.ts`	22	17ms
`telemetry/trace-id-utils.test.ts`	8	4ms

Execution Environment

Method: tmux parallel execution (4 windows: tests, typecheck, lint, build)
Node: v22.17.0
Vitest: v3.2.4
Platform: macOS Darwin 25.4.0

Verdict

All 5 checks pass. 210 telemetry-related tests cover the changed files (tracer, session-tracing, sdk, debugLogger). No regressions detected.

wenshao

Clean per-prompt traceId refactor. All R1/R2 feedback addressed. 170 tests pass, CI green. No blocking findings. — qwen3.7-max via Qwen Code /review

chiga0

Re-approve on new HEAD 0a9a3a13 (commit 3: "guard SessionIdSpanProcessor.onStart with try/catch, clear cache in resetDebugLoggingState").

Changes since last approval:

SessionIdSpanProcessor.onStart now wrapped in try/catch — OTel processor errors won't break span creation (addresses wenshao)
resetDebugLoggingState() now clears cachedSessionId/cachedTraceId — test isolation fix (addresses wenshao)

All original review findings still hold — 0 blocking issues. wenshao also approved at this HEAD.

— QoderWork automated review

Copilot AI review requested due to automatic review settings May 31, 2026 17:07

Copilot started reviewing on behalf of doudouOUC May 31, 2026 17:07 View session

doudouOUC mentioned this pull request May 31, 2026

feat(telemetry): cover qwen serve daemon end-to-end with OpenTelemetry #4554

Closed

10 tasks

Copilot AI reviewed May 31, 2026

View reviewed changes

Comment thread packages/core/src/telemetry/sdk.ts

doudouOUC force-pushed the feat/per-prompt-traceid branch from baa5efc to beb62ed Compare May 31, 2026 17:16

doudouOUC requested review from chiga0 and wenshao June 1, 2026 03:50

wenshao reviewed Jun 1, 2026

View reviewed changes

Comment thread packages/core/src/telemetry/sdk.ts

Comment thread packages/core/src/telemetry/sdk.ts

Comment thread packages/core/src/utils/debugLogger.ts Outdated

Comment thread packages/core/src/telemetry/sdk.ts Outdated

Comment thread packages/core/src/telemetry/tracer.ts

fix: address wenshao review — cache deriveTraceId, remove vestigial t…

6d43586

…ry/catch

wenshao reviewed Jun 1, 2026

View reviewed changes

Comment thread packages/core/src/utils/debugLogger.ts

Comment thread packages/core/src/telemetry/sdk.ts

chiga0 previously approved these changes Jun 1, 2026

View reviewed changes

fix: guard SessionIdSpanProcessor.onStart with try/catch, clear cache…

0a9a3a1

… in resetDebugLoggingState

doudouOUC dismissed chiga0’s stale review via 0a9a3a1 June 1, 2026 06:47

doudouOUC requested a review from wenshao June 1, 2026 07:00

wenshao approved these changes Jun 1, 2026

View reviewed changes

chiga0 approved these changes Jun 1, 2026

View reviewed changes

doudouOUC merged commit 4b3d384 into daemon_mode_b_main Jun 1, 2026
6 checks passed

doudouOUC deleted the feat/per-prompt-traceid branch June 1, 2026 11:07

Conversation

doudouOUC commented May 31, 2026

Summary

Motivation

What doesn't change

Migration

Test plan

Uh oh!

github-actions Bot commented May 31, 2026

📋 Review Summary

🔍 General Feedback

🎯 Specific Feedback

🟡 High

🟢 Medium

🔵 Low

✅ Highlights

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doudouOUC commented Jun 1, 2026

Review response — 6d43586

Uh oh!

Uh oh!

Uh oh!

chiga0 left a comment

Choose a reason for hiding this comment

Review: feat(telemetry): per-prompt traceId for bounded, renderable traces

Summary

Review Coverage

Findings

Uh oh!

doudouOUC commented Jun 1, 2026

Review response — 0a9a3a1

Uh oh!

wenshao commented Jun 1, 2026

Verification Report — PR #4661

Test Results

Test File Breakdown

Execution Environment

Verdict

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

chiga0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Review response — `6d43586`

Review response — `0a9a3a1`