Skip to content

feat(telemetry): propagate X-Qwen-Code-Session-Id on outbound LLM requests (part 2 of #4384)#4393

Closed
doudouOUC wants to merge 2 commits into
feat/telemetry-traceparent-propagationfrom
feat/telemetry-session-id-header
Closed

feat(telemetry): propagate X-Qwen-Code-Session-Id on outbound LLM requests (part 2 of #4384)#4393
doudouOUC wants to merge 2 commits into
feat/telemetry-traceparent-propagationfrom
feat/telemetry-session-id-header

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

Part 2 of #4384. Stacks on top of #4390 (PR A — traceparent via undici instrumentation). Set the base to PR A's branch so the diff shows only this PR's additions.

Summary

Adds a product-namespaced HTTP header X-Qwen-Code-Session-Id to every outbound LLM request when telemetry is enabled. Pattern matched from claude-code (X-Claude-Code-Session-Id, verified at src/services/api/client.ts:108 in their open-source repo). Server-side ingestion (e.g. a custom DashScope proxy or OTLP-aware API gateway) can use the header to stitch its observation of an LLM request back to the originating qwen-code session without parsing trace context.

Critical design: fetch wrapper, not defaultHeaders

The OpenAI / Anthropic providers use a per-request fetch wrapper rather than the SDK's defaultHeaders option. Why: content-generator SDK clients are constructed ONCE and NOT recreated on /clear-triggered session resets — Config.resetSession() updates this.sessionId but the contentGenerator keeps using the stale header value baked into defaultHeaders. Reading config.getSessionId() from inside the fetch wrapper at request time gives the live value.

This is verified end-to-end by the staleness regression test in llm-correlation-fetch.test.ts.

Known limitation: Gemini

@google/genai's HttpOptions interface does NOT expose a fetch hook (only headers / baseUrl / apiVersion / timeout / extraParams). Gemini therefore uses static httpOptions.headers and shares the staleness bug after /clear. Spans and logs still carry the live session id, so trace/log backends correctly attribute requests to the new session — only the wire-header at the LLM-service boundary is stale. Documented in telemetry.md and the design doc §8.6. Lazy-invalidate fix is a follow-up sub-issue.

Integration sites (4 SDK construction points)

  • packages/core/src/core/openaiContentGenerator/provider/default.ts (base — 5 sibling providers inherit; openrouter calls super.buildHeaders)
  • packages/core/src/core/openaiContentGenerator/provider/dashscope.ts (overrides buildClient; QwenContentGenerator inherits via this)
  • packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts
  • packages/core/src/core/geminiContentGenerator/index.ts (factory, NOT the class — no signature change needed; gcConfig already in scope)

Test plan

  • 11 new unit tests in llm-correlation-fetch.test.ts — including the staleness regression test (emit request 1, mutate sessionId, emit request 2, assert new id on wire)
  • Updated provider tests assert OpenAI / GoogleGenAI constructor receives wrapped fetch / merged headers (telemetry on/off cases)
  • All 150 tests across the 6 touched files pass (sdk.test.ts, llm-correlation-fetch.test.ts, default.test.ts, dashscope.test.ts, gemini index.test.ts, gemini geminiContentGenerator.test.ts)
  • End-to-end verify in tmux (/tmp/verify-pr4384.mjs): local HTTP server captures real outbound requests
    • Two scenarios run sequentially in one process
    • Scenario 1: sess-initial set, fire request → captured header has sess-initial
    • Scenario 2: mutate session id to sess-after-clear, fire request → captured header has new id (regression-guards the staleness fix in real SDK conditions, not just mocks)
    • OTLP collector mock receives uploads but those don't carry traceparent — confirms PR A's ignoreRequestHook working

Captured output:
```
[0] POST /v1/chat/completions
X-Qwen-Code-Session-Id: sess-initial
traceparent: 00-21278f951b68d6a0c412abfce3427078-81f936f76a72909b-01
[1] POST /v1/chat/completions
X-Qwen-Code-Session-Id: sess-after-clear
traceparent: 00-c71d416430a4612ba407b8a7e83baaf9-170601e26252e2e0-01
```

Audited but not modified

  • packages/core/src/qwen/qwenContentGenerator.ts — extends OpenAIContentGenerator using DashScopeOpenAICompatibleProvider; auto-inherits the dashscope change
  • packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts — wrapper, doesn't construct SDK clients

Out of scope (per design §3.2, future sub-issues)

  • Subprocess TRACEPARENT env var inheritance (BashTool)
  • Inbound TRACEPARENT read on startup (--prompt / Agent SDK modes)
  • X-Qwen-Code-Request-Id per-request UUID
  • Gemini staleness lazy-invalidate fix

Design doc: docs/design/telemetry-outbound-propagation-design.md (landed in PR A).

🤖 Generated with Qwen Code

…uests

Part 2 of #4384. Stacks on top of PR #4390 (traceparent via undici).

Adds a product-namespaced HTTP header X-Qwen-Code-Session-Id to every
outbound LLM request when telemetry is enabled, so server-side ingestion
can correlate observed requests with qwen-code session metric/log records.
Pattern matched from claude-code (X-Claude-Code-Session-Id, verified at
src/services/api/client.ts:108 in their open-source repo).

Critical design decision (design doc section 4.3): the OpenAI / Anthropic
providers use a per-request fetch wrapper rather than the SDK defaultHeaders
option, because content-generator SDK clients are constructed once and NOT
recreated on /clear-triggered session resets (Config.resetSession updates
this.sessionId but the contentGenerator keeps using the stale header value).
Reading config.getSessionId() from inside the wrapper at request time gives
the live value.

Gemini provider uses static httpOptions.headers — @google/genai HttpOptions
interface does not expose a fetch hook (only headers, baseUrl, apiVersion,
timeout, extraParams). This is a known limitation: after session reset,
Gemini X-Qwen-Code-Session-Id stays stale until the contentGenerator is
recreated. Documented in telemetry.md and the design doc section 8.6;
spans/logs continue to carry the live session id for trace/log correlation.
Lazy-invalidate fix is a follow-up sub-issue.

Header is omitted when telemetry is disabled OR when getSessionId returns
an empty string (some HTTP middleware rejects empty header values).

Integration sites:
- packages/core/src/core/openaiContentGenerator/provider/default.ts
  (base class — automatically covered by deepseek/minimax/mistral/
  modelscope/openrouter subclasses; openrouter calls super.buildHeaders)
- packages/core/src/core/openaiContentGenerator/provider/dashscope.ts
  (overrides buildClient — must be touched separately; QwenContentGenerator
  inherits via this provider)
- packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts
- packages/core/src/core/geminiContentGenerator/index.ts (factory function,
  not the GeminiContentGenerator class — no signature change)

End-to-end verification (local HTTP server in tmux):
  PASS: traceparent + X-Qwen-Code-Session-Id on every LLM request
  PASS: session id refreshes after simulated /clear (staleness regression
        guarded by llm-correlation-fetch.test.ts)
  PASS: OTLP upload traffic not traced (no feedback loop — PR A
        ignoreRequestHook working)

Robot generated with Qwen Code https://github.com/QwenLM/qwen-code
Copilot AI review requested due to automatic review settings May 21, 2026 09:38
@github-actions

Copy link
Copy Markdown
Contributor

📋 Review Summary

This PR adds a product-namespaced X-Qwen-Code-Session-Id HTTP header to outbound LLM requests for server-side session correlation, following the pattern from claude-code. The implementation is well-designed with comprehensive test coverage (11 new unit tests + end-to-end verification), and correctly uses a fetch wrapper approach to track live session IDs across /clear resets. The documented Gemini limitation is acceptable with clear tracking for follow-up.

🔍 General Feedback

  • Excellent documentation in both the PR body and telemetry.md — the "why" behind the fetch wrapper design is clearly explained
  • Strong test coverage including the critical staleness regression test that guards against the known design pitfall
  • Consistent pattern applied across 4 SDK construction points (OpenAI family, Anthropic, Gemini)
  • Good defensive coding: empty session IDs are not emitted, telemetry-gated behavior
  • The fetch wrapper approach correctly reads config.getSessionId() at request time rather than baking in stale values at SDK construction

🎯 Specific Feedback

🟡 High

  • packages/core/src/core/geminiContentGenerator/index.ts:47 - The known Gemini limitation creates inconsistent behavior where session correlation headers go stale after /clear. While documented, this could cause confusion during debugging. Consider adding a runtime diagnostic log when Gemini provider is initialized with telemetry enabled, warning users that session correlation may be stale until the content generator is recreated. This would help operators understand why they might see mismatched session IDs in their telemetry backend.

🟢 Medium

  • packages/core/src/core/openaiContentGenerator/provider/default.ts:96-104 - The comment about spreading runtimeOptions FIRST then overriding fetch is important but easy to miss. Consider extracting this pattern into a helper function or adding a TypeScript type constraint to enforce the override order at compile time:

    // Explicitly override fetch after spreading runtimeOptions to ensure
    // our correlation wrapper wraps the proxy fetch, not replaces it
    const clientOptions = {
      apiKey,
      baseURL: baseUrl,
      // ... other defaults
      ...(runtimeOptions || {}),
      fetch: wrapFetchWithCorrelation(baseFetch, this.cliConfig),
    } as const;
  • packages/core/src/telemetry/llm-correlation-fetch.ts:52-55 - The defensive check for empty session ID is good, but consider logging a debug-level diagnostic when this happens (via the existing createDebugLogger pattern). Silent skips are hard to troubleshoot if a user expects correlation headers but has a misconfigured session ID.

🔵 Low

  • packages/core/src/telemetry/llm-correlation-fetch.ts:17 - The header name constant SESSION_ID_HEADER is exported but only used internally in tests. Consider whether this should be part of the public API for users who want to verify/filter on this header in their own testing, or keep it internal if it's an implementation detail.

  • docs/developers/development/telemetry.md:319 - The documentation mentions "Empty session ids are not emitted" but doesn't explain when a session ID might be empty. Adding a brief note (e.g., "Session IDs may be empty during early initialization before the first session is established") would help future maintainers understand the edge case.

  • packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts:210-212 - The base fetch extraction pattern is duplicated across all three provider files. While simple enough, this could be extracted to a shared helper in runtimeFetchOptions.ts alongside buildRuntimeFetchOptions if this pattern appears elsewhere or might in the future.

✅ Highlights

  • packages/core/src/telemetry/llm-correlation-fetch.test.ts:93-111 - The staleness regression test is excellently crafted. It explicitly simulates the /clear scenario that would break naive implementations and asserts the fetch wrapper reads fresh values. This is exactly the kind of test that prevents future regressions during refactors.

  • packages/core/src/telemetry/llm-correlation-fetch.ts:27-37 - The JSDoc comment clearly explains the why behind the design decision (fetch wrapper vs defaultHeaders), references the design doc section, and explains the caller's responsibility for choosing the base fetch. This is model documentation for future maintainers.

  • End-to-end verification — The PR includes tmux-based E2E testing with captured output showing the actual wire behavior for both pre- and post-/clear scenarios. This goes beyond unit tests to verify the integration works in realistic conditions.

  • Consistent pattern application — All four provider implementations (OpenAI, DashScope, Anthropic, Gemini) follow the same pattern with appropriate adaptations for each SDK's capabilities. The code review shows good attention to maintaining consistency across the codebase.

Copilot AI 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.

Pull request overview

This PR extends the outbound-LLM telemetry propagation work (stacked on #4390) by adding a product-namespaced session correlation header (X-Qwen-Code-Session-Id) to outbound LLM requests when telemetry is enabled, enabling correlation on the server side without needing to parse trace context.

Changes:

  • Introduces wrapFetchWithCorrelation() / staticCorrelationHeaders() helpers to inject X-Qwen-Code-Session-Id (per-request when possible, static for Gemini).
  • Hooks the per-request fetch wrapper into OpenAI-compatible and Anthropic SDK client construction; injects static headers for Gemini due to SDK limitations.
  • Adds unit tests and updates provider tests, plus developer documentation describing behavior and Gemini’s known limitation.

Reviewed changes

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

Show a summary per file
File Description
packages/core/src/telemetry/llm-correlation-fetch.ts Adds correlation header helpers (per-request fetch wrapper + static headers).
packages/core/src/telemetry/llm-correlation-fetch.test.ts Adds unit tests covering wrapper/header behavior and staleness regression.
packages/core/src/core/openaiContentGenerator/provider/default.ts Installs correlation fetch wrapper on OpenAI SDK client.
packages/core/src/core/openaiContentGenerator/provider/default.test.ts Asserts OpenAI client receives the wrapped fetch.
packages/core/src/core/openaiContentGenerator/provider/dashscope.ts Installs correlation fetch wrapper for DashScope OpenAI-compatible client.
packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts Asserts DashScope OpenAI client receives the wrapped fetch.
packages/core/src/core/geminiContentGenerator/index.ts Merges static correlation headers into Gemini httpOptions.headers.
packages/core/src/core/geminiContentGenerator/index.test.ts Tests header presence/absence based on telemetry enabled flag.
packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts Installs correlation fetch wrapper on Anthropic SDK client.
docs/developers/development/telemetry.md Documents the session correlation header and Gemini limitation.

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

Comment thread packages/core/src/telemetry/llm-correlation-fetch.ts
Review feedback on #4390:

1. CI was failing on npm ci because the lockfile was generated with npm 11
   locally (it sprinkles `peer: true` annotations npm 10 reads differently
   and rejects). Regenerated with npm 10 (matching CI's Node 22.x default),
   so the diff vs main is now 18 lines (the actual instrumentation-undici
   entry) instead of 105 lines of npm-version drift noise.

2. (Copilot inline at sdk.ts:330) `otlpUrlPrefixes` was derived from raw
   Config strings, so a settings.json `"otlpEndpoint": "\"http://...\""`
   (quoted) or trailing `#fragment` would silently miss the prefix match
   and reintroduce the feedback loop the hook exists to prevent. Replaced
   the regex-based suffix trim with a WHATWG URL parser:
   - strips ?query, #fragment, trailing slash
   - trims symmetric ASCII quotes a user may have placed in settings.json
   - falls back to safe suffix trimming if URL parsing fails (misconfigured
     endpoint still gets SOME protection)

3. (CodeQL inline) Replaced the `/\?.*$/` regex in ignoreRequestHook with
   `indexOf('?')`/`indexOf('#')` slicing for ReDoS hygiene. The regex was
   linear in practice but flagged as polynomial — using indexOf removes
   the ambiguity and is arguably simpler.

Added 3 tests in sdk.test.ts covering the new normalizations (#fragment
on incoming path, quoted endpoint, #fragment on configured endpoint).

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Comment thread packages/core/src/telemetry/llm-correlation-fetch.test.ts
Comment thread packages/core/src/core/openaiContentGenerator/provider/default.ts
Comment thread packages/core/src/telemetry/llm-correlation-fetch.ts
Comment thread packages/core/src/telemetry/sdk.ts
Comment thread packages/core/src/telemetry/sdk.ts
Comment thread packages/core/src/core/geminiContentGenerator/index.ts
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Per reviewer request, the session-id propagation work has been merged into #4390 to keep a single PR. The session-id commit (0f5fb5a2b from this branch) was cherry-picked onto #4390 as 89582ac6a. Closing this PR; please review #4390 for the combined scope.

@doudouOUC doudouOUC closed this May 21, 2026
doudouOUC added a commit that referenced this pull request May 21, 2026
…ry safety

Adopts 7 review findings from wenshao on #4390 (+ duplicates from now-closed
#4393). Critical bugs first, polish second.

CRITICAL:

1. tsc TS2322 — wrapper return type incompatible with Anthropic SDK Fetch.
   `typeof fetch` (Node WHATWG, 2 overloads) is not structurally assignable
   to Anthropic's narrower `Fetch = (input: RequestInfo, init?) => ...`,
   even though they're call-compatible at runtime. Make wrapper generic
   `<TFetch extends FetchLikeLoose>` so callers preserve their exact fetch
   signature; cast the Anthropic call site through `unknown` with a comment
   explaining why.

2. tsc TS2352 / TS2493 — `baseFetch.mock.calls[0]![1] as RequestInit` was
   out-of-bounds when wrapped was called with no init arg. Replaced with a
   `makeFetchMock()` helper returning typed accessors.

3. normalizeOtlpPrefix catch fallback was DANGEROUS — a config of `"http"`
   produced prefix `"http"` which `startsWith`-matched every outbound HTTP
   request → silently disabled ALL instrumentation (no client spans, no
   correlation header — defeats the entire feature). Fixed: catch returns
   undefined + diag.warn. Misconfigured endpoint loses its feedback-loop
   guard (acceptable) instead of disabling all guards (catastrophic).

4. `url.startsWith(prefix)` matching was NOT boundary-safe — port collision
   (`:4318` matches `:43180`), hostname suffix collision (`otlp.example.com`
   matches `otlp.example.com.evil.net`), path-segment collision (`/v1`
   matches `/v1foo/x`). Replaced with origin-equality + path-prefix +
   boundary-char check (next char must be `/`, `?`, `#`, or end-of-string).

5. HttpInstrumentation also lacked the OTLP feedback-loop guard. The OTLP
   HTTP exporter (`@opentelemetry/exporter-trace-otlp-http`) uses node:http
   (patched by HttpInstrumentation, NOT undici). Without this, every OTLP
   upload batch creates a parasitic client span → feedback loop. Added
   `ignoreOutgoingRequestHook` that reuses the same `matchesOtlpPrefix` /
   `stripPathSuffix` helpers as the undici instrumentation.

SAFETY:

6. Request input + undefined init dropped the Request's own headers
   (Authorization etc.) because `new Headers(undefined)` → `{...init, headers}`
   replaced them with just our session header. Fix: when input is a Request
   and init.headers is unset, seed from input.headers before adding ours.

7. Wrapped fetch had no try/catch — a throwing Config getter or Headers
   constructor would propagate as TypeError and break the LLM request path.
   Wrapped header construction in try/catch; on failure, fall through to
   baseFetch with original init (no header) + diag.warn. Telemetry must
   never break the model call.

COVERAGE:

- 3 new sdk.test.ts boundary tests (port/host/path)
- 1 new sdk.test.ts normalizeOtlpPrefix catch-branch coverage
- 1 new sdk.test.ts HttpInstrumentation OTLP guard test
- 1 new sdk.test.ts proxy-mode wrapped-fetch test (default.test.ts)
- 1 new anthropic test asserting wrapped fetch installed on Anthropic SDK
- 2 new llm-correlation-fetch.test.ts (Request-headers preservation + try/catch fall-through)

All 668 tests pass (1 pre-existing Anthropic User-Agent failure on main is
unrelated). tsc clean.

Declined: #10 DRY-refactor of baseFetch extraction across 3 sites — the
duplication was pre-existing (default/dashscope buildClient was already
near-identical), refactoring is a separate cleanup PR not gated by this
feature. Will reply on the thread.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
doudouOUC added a commit that referenced this pull request May 21, 2026
…ry safety

Adopts 7 review findings from wenshao on #4390 (+ duplicates from now-closed
#4393). Critical bugs first, polish second.

CRITICAL:

1. tsc TS2322 — wrapper return type incompatible with Anthropic SDK Fetch.
   `typeof fetch` (Node WHATWG, 2 overloads) is not structurally assignable
   to Anthropic's narrower `Fetch = (input: RequestInfo, init?) => ...`,
   even though they're call-compatible at runtime. Make wrapper generic
   `<TFetch extends FetchLikeLoose>` so callers preserve their exact fetch
   signature; cast the Anthropic call site through `unknown` with a comment
   explaining why.

2. tsc TS2352 / TS2493 — `baseFetch.mock.calls[0]![1] as RequestInit` was
   out-of-bounds when wrapped was called with no init arg. Replaced with a
   `makeFetchMock()` helper returning typed accessors.

3. normalizeOtlpPrefix catch fallback was DANGEROUS — a config of `"http"`
   produced prefix `"http"` which `startsWith`-matched every outbound HTTP
   request → silently disabled ALL instrumentation (no client spans, no
   correlation header — defeats the entire feature). Fixed: catch returns
   undefined + diag.warn. Misconfigured endpoint loses its feedback-loop
   guard (acceptable) instead of disabling all guards (catastrophic).

4. `url.startsWith(prefix)` matching was NOT boundary-safe — port collision
   (`:4318` matches `:43180`), hostname suffix collision (`otlp.example.com`
   matches `otlp.example.com.evil.net`), path-segment collision (`/v1`
   matches `/v1foo/x`). Replaced with origin-equality + path-prefix +
   boundary-char check (next char must be `/`, `?`, `#`, or end-of-string).

5. HttpInstrumentation also lacked the OTLP feedback-loop guard. The OTLP
   HTTP exporter (`@opentelemetry/exporter-trace-otlp-http`) uses node:http
   (patched by HttpInstrumentation, NOT undici). Without this, every OTLP
   upload batch creates a parasitic client span → feedback loop. Added
   `ignoreOutgoingRequestHook` that reuses the same `matchesOtlpPrefix` /
   `stripPathSuffix` helpers as the undici instrumentation.

SAFETY:

6. Request input + undefined init dropped the Request's own headers
   (Authorization etc.) because `new Headers(undefined)` → `{...init, headers}`
   replaced them with just our session header. Fix: when input is a Request
   and init.headers is unset, seed from input.headers before adding ours.

7. Wrapped fetch had no try/catch — a throwing Config getter or Headers
   constructor would propagate as TypeError and break the LLM request path.
   Wrapped header construction in try/catch; on failure, fall through to
   baseFetch with original init (no header) + diag.warn. Telemetry must
   never break the model call.

COVERAGE:

- 3 new sdk.test.ts boundary tests (port/host/path)
- 1 new sdk.test.ts normalizeOtlpPrefix catch-branch coverage
- 1 new sdk.test.ts HttpInstrumentation OTLP guard test
- 1 new sdk.test.ts proxy-mode wrapped-fetch test (default.test.ts)
- 1 new anthropic test asserting wrapped fetch installed on Anthropic SDK
- 2 new llm-correlation-fetch.test.ts (Request-headers preservation + try/catch fall-through)

All 668 tests pass (1 pre-existing Anthropic User-Agent failure on main is
unrelated). tsc clean.

Declined: #10 DRY-refactor of baseFetch extraction across 3 sites — the
duplication was pre-existing (default/dashscope buildClient was already
near-identical), refactoring is a separate cleanup PR not gated by this
feature. Will reply on the thread.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
wenshao pushed a commit that referenced this pull request May 25, 2026
…gation (#4384) (#4390)

* feat(telemetry): propagate W3C traceparent on outbound LLM requests

Part 1 of #4384 (sub-issue of #3731 P3 deeper observability).

Today qwen-code's only OTel instrumentation is `HttpInstrumentation`,
which only patches Node's `http`/`https` modules. The `openai` and
`@google/genai` SDKs use `globalThis.fetch` (undici), so outbound LLM
requests carry no `traceparent` header and trace context dies at the
qwen-code process boundary.

Adds `@opentelemetry/instrumentation-undici@0.14.0` (peer-compatible
with the installed `@opentelemetry/instrumentation@0.203.0`) and wires
it into `initializeTelemetry()` next to the existing
`HttpInstrumentation`. Default propagator (W3C tracecontext + baggage)
remains unchanged — no explicit `textMapPropagator` needed.

`ignoreRequestHook` skips OTLP exporter endpoints to avoid the
classic feedback loop (OTel SDK uses fetch to upload OTLP data; without
the hook each upload would create a span that gets uploaded, infinitely).
Configured `otlpEndpoint` / per-signal endpoints are stripped of trailing
slash and query string for robust prefix matching against undici's
`request.origin + request.path`.

Outbound LLM calls now also produce a client-side HTTP span (separating
network TTFB / transfer time from the existing `api.generateContent`
total-duration span).

Design doc: docs/design/telemetry-outbound-propagation-design.md
(Part A — traceparent; Part B — session id header — lands in a
follow-up PR per the design's split rationale.)

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix(telemetry): harden OTLP feedback-loop guard + slim lockfile diff

Review feedback on #4390:

1. CI was failing on npm ci because the lockfile was generated with npm 11
   locally (it sprinkles `peer: true` annotations npm 10 reads differently
   and rejects). Regenerated with npm 10 (matching CI's Node 22.x default),
   so the diff vs main is now 18 lines (the actual instrumentation-undici
   entry) instead of 105 lines of npm-version drift noise.

2. (Copilot inline at sdk.ts:330) `otlpUrlPrefixes` was derived from raw
   Config strings, so a settings.json `"otlpEndpoint": "\"http://...\""`
   (quoted) or trailing `#fragment` would silently miss the prefix match
   and reintroduce the feedback loop the hook exists to prevent. Replaced
   the regex-based suffix trim with a WHATWG URL parser:
   - strips ?query, #fragment, trailing slash
   - trims symmetric ASCII quotes a user may have placed in settings.json
   - falls back to safe suffix trimming if URL parsing fails (misconfigured
     endpoint still gets SOME protection)

3. (CodeQL inline) Replaced the `/\?.*$/` regex in ignoreRequestHook with
   `indexOf('?')`/`indexOf('#')` slicing for ReDoS hygiene. The regex was
   linear in practice but flagged as polynomial — using indexOf removes
   the ambiguity and is arguably simpler.

Added 3 tests in sdk.test.ts covering the new normalizations (#fragment
on incoming path, quoted endpoint, #fragment on configured endpoint).

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* feat(telemetry): propagate X-Qwen-Code-Session-Id on outbound LLM requests

Part 2 of #4384. Stacks on top of PR #4390 (traceparent via undici).

Adds a product-namespaced HTTP header X-Qwen-Code-Session-Id to every
outbound LLM request when telemetry is enabled, so server-side ingestion
can correlate observed requests with qwen-code session metric/log records.
Pattern matched from claude-code (X-Claude-Code-Session-Id, verified at
src/services/api/client.ts:108 in their open-source repo).

Critical design decision (design doc section 4.3): the OpenAI / Anthropic
providers use a per-request fetch wrapper rather than the SDK defaultHeaders
option, because content-generator SDK clients are constructed once and NOT
recreated on /clear-triggered session resets (Config.resetSession updates
this.sessionId but the contentGenerator keeps using the stale header value).
Reading config.getSessionId() from inside the wrapper at request time gives
the live value.

Gemini provider uses static httpOptions.headers — @google/genai HttpOptions
interface does not expose a fetch hook (only headers, baseUrl, apiVersion,
timeout, extraParams). This is a known limitation: after session reset,
Gemini X-Qwen-Code-Session-Id stays stale until the contentGenerator is
recreated. Documented in telemetry.md and the design doc section 8.6;
spans/logs continue to carry the live session id for trace/log correlation.
Lazy-invalidate fix is a follow-up sub-issue.

Header is omitted when telemetry is disabled OR when getSessionId returns
an empty string (some HTTP middleware rejects empty header values).

Integration sites:
- packages/core/src/core/openaiContentGenerator/provider/default.ts
  (base class — automatically covered by deepseek/minimax/mistral/
  modelscope/openrouter subclasses; openrouter calls super.buildHeaders)
- packages/core/src/core/openaiContentGenerator/provider/dashscope.ts
  (overrides buildClient — must be touched separately; QwenContentGenerator
  inherits via this provider)
- packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts
- packages/core/src/core/geminiContentGenerator/index.ts (factory function,
  not the GeminiContentGenerator class — no signature change)

End-to-end verification (local HTTP server in tmux):
  PASS: traceparent + X-Qwen-Code-Session-Id on every LLM request
  PASS: session id refreshes after simulated /clear (staleness regression
        guarded by llm-correlation-fetch.test.ts)
  PASS: OTLP upload traffic not traced (no feedback loop — PR A
        ignoreRequestHook working)

Robot generated with Qwen Code https://github.com/QwenLM/qwen-code

* fix(telemetry): R2 review fixes — critical correctness + tsc + boundary safety

Adopts 7 review findings from wenshao on #4390 (+ duplicates from now-closed
#4393). Critical bugs first, polish second.

CRITICAL:

1. tsc TS2322 — wrapper return type incompatible with Anthropic SDK Fetch.
   `typeof fetch` (Node WHATWG, 2 overloads) is not structurally assignable
   to Anthropic's narrower `Fetch = (input: RequestInfo, init?) => ...`,
   even though they're call-compatible at runtime. Make wrapper generic
   `<TFetch extends FetchLikeLoose>` so callers preserve their exact fetch
   signature; cast the Anthropic call site through `unknown` with a comment
   explaining why.

2. tsc TS2352 / TS2493 — `baseFetch.mock.calls[0]![1] as RequestInit` was
   out-of-bounds when wrapped was called with no init arg. Replaced with a
   `makeFetchMock()` helper returning typed accessors.

3. normalizeOtlpPrefix catch fallback was DANGEROUS — a config of `"http"`
   produced prefix `"http"` which `startsWith`-matched every outbound HTTP
   request → silently disabled ALL instrumentation (no client spans, no
   correlation header — defeats the entire feature). Fixed: catch returns
   undefined + diag.warn. Misconfigured endpoint loses its feedback-loop
   guard (acceptable) instead of disabling all guards (catastrophic).

4. `url.startsWith(prefix)` matching was NOT boundary-safe — port collision
   (`:4318` matches `:43180`), hostname suffix collision (`otlp.example.com`
   matches `otlp.example.com.evil.net`), path-segment collision (`/v1`
   matches `/v1foo/x`). Replaced with origin-equality + path-prefix +
   boundary-char check (next char must be `/`, `?`, `#`, or end-of-string).

5. HttpInstrumentation also lacked the OTLP feedback-loop guard. The OTLP
   HTTP exporter (`@opentelemetry/exporter-trace-otlp-http`) uses node:http
   (patched by HttpInstrumentation, NOT undici). Without this, every OTLP
   upload batch creates a parasitic client span → feedback loop. Added
   `ignoreOutgoingRequestHook` that reuses the same `matchesOtlpPrefix` /
   `stripPathSuffix` helpers as the undici instrumentation.

SAFETY:

6. Request input + undefined init dropped the Request's own headers
   (Authorization etc.) because `new Headers(undefined)` → `{...init, headers}`
   replaced them with just our session header. Fix: when input is a Request
   and init.headers is unset, seed from input.headers before adding ours.

7. Wrapped fetch had no try/catch — a throwing Config getter or Headers
   constructor would propagate as TypeError and break the LLM request path.
   Wrapped header construction in try/catch; on failure, fall through to
   baseFetch with original init (no header) + diag.warn. Telemetry must
   never break the model call.

COVERAGE:

- 3 new sdk.test.ts boundary tests (port/host/path)
- 1 new sdk.test.ts normalizeOtlpPrefix catch-branch coverage
- 1 new sdk.test.ts HttpInstrumentation OTLP guard test
- 1 new sdk.test.ts proxy-mode wrapped-fetch test (default.test.ts)
- 1 new anthropic test asserting wrapped fetch installed on Anthropic SDK
- 2 new llm-correlation-fetch.test.ts (Request-headers preservation + try/catch fall-through)

All 668 tests pass (1 pre-existing Anthropic User-Agent failure on main is
unrelated). tsc clean.

Declined: #10 DRY-refactor of baseFetch extraction across 3 sites — the
duplication was pre-existing (default/dashscope buildClient was already
near-identical), refactoring is a separate cleanup PR not gated by this
feature. Will reply on the thread.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* chore(deps): allow patch updates for @opentelemetry/instrumentation-undici

Switch from exact pin `0.14.0` to `^0.14.0` for consistency with the rest
of the `@opentelemetry/*` deps in this block (all carated).

For 0.x semver, npm treats `^0.14.0` as `>=0.14.0 <0.15.0`, so patch
updates within the 0.14.x line — which are tied to the same
`@opentelemetry/instrumentation@0.203.x` peer — flow in via `npm update`
without requiring a manual package.json edit. A bump across the 0.x
minor (e.g. 0.15.x) would shift the instrumentation peer compatibility
and still requires explicit attention, which the caret correctly blocks.

Per review feedback on #4390 (wenshao).

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* test(telemetry): stub getTelemetryEnabled + getSessionId in Gemini factory tests

The X-Qwen-Code-Session-Id commit added a `staticCorrelationHeaders(gcConfig)`
call inside the Gemini content generator factory. That helper reads
`gcConfig.getTelemetryEnabled()` and `gcConfig.getSessionId()` per request.

Both pre-existing Gemini tests in `contentGenerator.test.ts` build a minimal
partial Config stub via `as unknown as Config` and only stub the methods the
factory used to need. The new call path now hits the unstubbed methods at
runtime, surfacing as `TypeError: config.getTelemetryEnabled is not a function`
on all three CI platforms.

Add the two missing stubs to both test cases. The Gemini factory continues
to ignore the values when telemetry is off — these stubs only have to exist,
not return anything in particular.

Local check ran the full test suite for the four directories `/loop` covers
plus `src/core/contentGenerator.test.ts` itself; all green. Also re-ran the
other test files that build partial Config mocks via the same idiom
(`client.test.ts`, `config.test.ts`, `nextSpeakerChecker.test.ts`,
`content-generator-config.test.ts`) — none exercise the new code path.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix(telemetry): R3 review fixes — port + protocol + quote + safety

Four issues found by wenshao reviewing the R2 boundary-safety pass on PR
#4390. All four close gaps where the OTLP feedback-loop guard or the
correlation-header path could fail silently.

1. **Port normalization mismatch** (sdk.ts ignoreOutgoingRequestHook):
   `normalizeOtlpPrefix` builds prefixes via `URL.origin`, which strips
   default ports (`:80` for http, `:443` for https). The hook reconstructed
   request origin manually as `${proto}://${host}${portPart}`, keeping the
   port. Result: prefix `http://collector` (no explicit port) didn't match
   a request to `http://collector:80/v1/traces` because their `.origin`
   differed → guard bypassed → feedback loop. Now the reconstructed origin
   is also routed through `URL` so both sides apply the same default-port
   stripping.

2. **HTTPS proto silent fallback** (sdk.ts ignoreOutgoingRequestHook):
   The `(req.protocol && ...) || 'http'` fallback would silently mis-bucket
   HTTPS requests as HTTP when `req.protocol` was unset, so HTTPS OTLP
   endpoints couldn't match their prefix. Changed to fail open: when proto
   can't be determined, return false (request gets instrumented). Worst
   case is a parasitic client span — observable, recoverable — versus the
   previous unbounded silent feedback loop. Picked fail-open over the bot's
   port-based heuristic because non-standard HTTPS ports break the
   heuristic but not fail-open.

3. **Quote-stripping divergence** (sdk.ts normalizeOtlpPrefix):
   `parseOtlpEndpoint` (line 109) uses `/^["']|["']$/g` which strips
   asymmetric leading/trailing quotes; `normalizeOtlpPrefix` previously
   only stripped symmetric pairs. A settings.json typo like `"value'` would
   let the exporter connect (parseOtlpEndpoint trims) but leave the guard
   returning `undefined` (normalizeOtlpPrefix rejected) → parasitic loop.
   Aligned `normalizeOtlpPrefix` to the same lenient regex.

4. **`staticCorrelationHeaders` missing try/catch** (llm-correlation-fetch.ts):
   `wrapFetchWithCorrelation` already catches all internal exceptions and
   falls through to baseFetch — same "telemetry must never break LLM path"
   contract was missing on the static-headers helper. A throw here would
   propagate up to the Gemini content-generator factory and crash
   content-generator init for the whole session. Wrapped the body in
   try/catch with `diag.warn` fall-through to `{}`.

Tests: added 4 regression tests covering each scenario:
- default-port HTTP request matched against portless prefix (1)
- hook returns false when req.protocol missing on https endpoint (2)
- asymmetric-quoted endpoint normalizes for guard parity (3)
- staticCorrelationHeaders returns {} when config getter throws (4)

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* docs(telemetry): fix misleading "BOTH" wording in wrapFetchWithCorrelation

The comment described the header-seeding logic as merging "BOTH the
init.headers AND the Request's own headers", but the two branches are
mutually exclusive — `new Headers(init?.headers)` runs unconditionally
(empty Headers when init.headers is undefined), and the Request-headers
copy only runs when init.headers is undefined. So in practice it's
either-or, not BOTH.

Reworded to match the actual logic per #4390 review feedback (wenshao).
Behavior unchanged.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix(telemetry): strip port from req.host fallback + document undici scope

Two issues found by wenshao reviewing the R3 boundary-safety fixes on PR
#4390.

1. **`req.host` may already include `:port`** (sdk.ts ignoreOutgoingRequestHook):
   When `req.hostname` is absent and `req.host` is the fallback, the value
   may already be `"collector:4318"`. Naively appending `:${req.port}`
   produced `"http://collector:4318:4318"` → `new URL()` rejects → catch
   returns false → silent guard bypass for that request. Currently
   unreachable because `@opentelemetry/otlp-exporter-base` always sets
   `hostname` from WHATWG URL parsing, but the fallback exists in the
   code and must be correct — a future OTLP transport that emits `host`
   without `hostname` would silently trigger the feedback loop. Strip the
   port when falling back; bracketed IPv6 literals like `"[::1]:443"`
   keep their bracketed host intact.

2. **Undici scope honesty** (telemetry.md):
   Previous docs framed the propagation as "outbound LLM requests", but
   `UndiciInstrumentation` actually patches `globalThis.fetch` for the
   whole process — `WebFetch`, MCP clients, IDE extension calls all get
   spans + `traceparent` injection too. Added a "Scope: all fetch() calls,
   not just LLM" subsection covering: (a) trace ID leakage to third-party
   URLs (the user-supplied destinations of `WebFetch` see our trace ID;
   not secret per W3C but worth knowing); (b) non-LLM span volume
   inflating OTLP batches with a workaround tip. Per-destination scoping
   toggle deferred as a follow-up — out of scope for this PR.

Added regression test for the host:port-fallback path. Test exercises
the previously broken combination (hostname absent, host carries port)
through the existing test harness.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* feat(telemetry): scope X-Qwen-Code-Session-Id to first-party hosts by default

Address LaZzyMan's REQUEST_CHANGES review of PR #4390.

The original design injected `X-Qwen-Code-Session-Id` on every outbound
LLM request gated only by `telemetry.enabled`. Review caught that this
broadcasts a stable cross-request client identifier to every configured
third-party provider (OpenAI, Anthropic, OpenRouter, MiniMax, ModelScope,
Mistral, vanilla Gemini, ...), which the claude-code precedent does NOT
justify — claude-code is a first-party Anthropic→Anthropic flow; qwen-code
is an open-source CLI connecting to many providers.

Fix: add a host allowlist with a deliberately narrow default. The header
is now only attached to destinations whose hostname matches:

  dashscope.aliyuncs.com
  dashscope-intl.aliyuncs.com
  *.dashscope.aliyuncs.com
  *.dashscope-intl.aliyuncs.com
  *.alibaba-inc.com
  *.aliyun-inc.com

This is exactly the set where the LLM provider, the upstream telemetry
backend (ARMS Tracing), and qwen-code itself are the same legal entity —
mirroring the first-party claude-code pattern and preserving the real
product value (server-side trace stitching against DashScope) without
exposing the session id to third parties.

Operators with broader correlation requirements override via:

  "telemetry": {
    "sessionIdHeaderHosts": ["*"]                          // restore broadcast
    "sessionIdHeaderHosts": []                              // fully disable
    "sessionIdHeaderHosts": ["api.example.com", "*.foo"]    // custom allowlist
  }

Implementation:

- NEW `telemetry/trusted-llm-hosts.ts`: `DEFAULT_SESSION_ID_HEADER_HOSTS`
  + `matchesTrustedHost(hostname, patterns)` + `extractRequestHost(input)`.
  Pattern syntax is intentionally tiny (bare hostname OR `*.suffix`,
  dot-anchored to reject `evil-alibaba-inc.com` style attacks). Unit-tested
  in dedicated test file including TLD/sub-domain attack vectors.

- `wrapFetchWithCorrelation` (openai + anthropic providers): resolves the
  allowlist at wrap time (Config snapshot), inspects each request's
  destination URL inside `correlationFetch`, falls through to baseFetch
  for non-trusted destinations. Wildcard escape hatch via `["*"]`.

- `staticCorrelationHeaders` (Gemini factory): now takes an optional
  `destinationUrl` and applies the same host gate. The Gemini SDK default
  endpoint `generativelanguage.googleapis.com` is NOT on the default
  allowlist, so vanilla Gemini calls receive no header — matching the
  "first-party only" scope. Operators who put the Gemini SDK on a
  DashScope-compatible endpoint via `baseUrl` get the header naturally.

- `Config.getTelemetrySessionIdHeaderHosts()` getter +
  `TelemetrySettings.sessionIdHeaderHosts` interface field + JSON schema
  entry in `settingsSchema.ts`. Wired through `resolveTelemetrySettings`.

- Defensive optional-chaining + try/catch on the Config getter call at
  wrap time so partial test mocks (or pre-getter Config implementations)
  fall back to the default allowlist rather than crashing buildClient.

Tests: 12 new cases covering host match/skip on default allowlist,
sub-domain handling, TLD-suffix attack rejection, `["*"]` broadcast
override, `[]` full-disable, custom operator allowlist, unparseable
destination (fail closed), and the three Gemini factory paths
(googleapis.com default → omit; DashScope `baseUrl` → inject; custom
allowlist → inject).

Docs updated in `docs/developers/development/telemetry.md` Session
correlation header section, including override examples and the new
Gemini host-gate semantics.

Closes the LaZzyMan REQUEST_CHANGES blocker. The cross-vendor
fingerprint-broadcast failure mode is now opt-in rather than default,
restoring the first-party-only semantics that make the claude-code
precedent applicable.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix(telemetry): R5 review fixups — Vertex destination + ["*"] trim + docs

Self-review pass on commit 1c8528a (host-scoped session-id header):

1. **Vertex AI destination guessing**
   (geminiContentGenerator/index.ts)
   `@google/genai` routes to `{region}-aiplatform.googleapis.com` (not
   `generativelanguage.googleapis.com`) when `vertexai: true` and no
   `baseUrl`. The previous "guess generativelanguage" default would have
   mis-bucketed Vertex traffic under any operator-supplied allowlist that
   covered the public Gemini endpoint but not the Vertex one. Today
   invisible (both off the default allowlist), but a latent gotcha for
   operators tuning `telemetry.sessionIdHeaderHosts`.
   Fix: pass `undefined` when `config.baseUrl` is unset (fail-closed —
   no header). Operators who want correlation against Google endpoints
   must set `baseUrl` explicitly, which is also the SDK's input for
   destination resolution.

2. **`["*"]` broadcast escape hatch tolerates whitespace**
   (llm-correlation-fetch.ts)
   `[" * "]` (a settings.json hand-edit with a stray space) previously
   silently fell back to "no host matches" — the opposite of operator
   intent. Now `.trim()` before comparing, so common whitespace mistakes
   still trigger broadcast.

3. **Doc note on wrap-time allowlist snapshot**
   (llm-correlation-fetch.ts JSDoc)
   The session id is read live per-request, but `trustedHosts` is
   snapshotted once at `wrapFetchWithCorrelation` call time. Spell this
   out in the JSDoc so a future maintainer doesn't read the live
   `getSessionId()` and assume the allowlist is the same shape.

4. **Defensive test coverage**
   (trusted-llm-hosts.test.ts, llm-correlation-fetch.test.ts)
   Added: extractRequestHost with explicit port / userinfo / query /
   fragment / IPv6 bracket form. Whitespace `[" * "]` broadcast test.
   IPv6 case documents the "bracketed → never matches" behavior is
   intentional fail-closed for the named-host allowlist scope.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* chore: regenerate settings.schema.json for sessionIdHeaderHosts

Lint check `Check settings schema is up-to-date` failed because the
checked-in `packages/vscode-ide-companion/schemas/settings.schema.json`
wasn't regenerated after adding `telemetry.sessionIdHeaderHosts` to
`settingsSchema.ts` in commit 1c8528a. Regenerated via
`npm run generate:settings-schema`.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* docs(design): update telemetry-outbound-propagation design for R3 host-allowlist scoping

Adds a "修订历史" header table at the top and a new §11 "R3 修订 —
Host-Allowlist Scoping for X-Qwen-Code-Session-Id" capturing what changed
after LaZzyMan's REQUEST_CHANGES review, why, and how. Inline pointers
added at §3.1, §3.2, §4.3, §4.4, §9 (claude-code comparison table) to
point readers at §11 — original prose preserved as a record of the
decision path rather than rewritten in place.

Concretely §11 covers:
- The three-step LazzyMan critique and why R1's "broadcast to all
  providers" was structurally wrong for an open-source multi-provider CLI
- The default allowlist (`DEFAULT_SESSION_ID_HEADER_HOSTS`) and its
  semantic alignment with the DashScope provider detector
- Pattern grammar (bare hostname / `*.suffix` dot-anchored), the
  TLD-suffix attack vectors it rejects, why no regex / port-aware globbing
- `wrapFetchWithCorrelation` host gate, wrap-time vs request-time
  semantics, `[" * "]` whitespace tolerance
- `staticCorrelationHeaders` `destinationUrl` parameter, Gemini factory's
  fail-closed treatment of unset `baseUrl` (avoids the Vertex
  vs `generativelanguage.googleapis.com` ambiguity)
- All R3 file changes mapped to the original §5 file-change list
- Mapping of LazzyMan's three concerns to R3's responses
- §10 future-work additions: `traceparent` per-destination toggle,
  `X-Qwen-Code-Request-Id`, IPv6 allowlist syntax

No code changes; documentation only.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix(telemetry): defensive allowlist normalization + positive proxy test

Three issues found by wenshao reviewing the R3 host-allowlist scoping.

1. **[Critical] `broadcastAll` outside safety try/catch**
   (llm-correlation-fetch.ts wrapFetchWithCorrelation)
   The try/catch only fires when `getTelemetrySessionIdHeaderHosts()`
   throws. If it returns a malformed value — a bare string (settings.json
   typo `"sessionIdHeaderHosts": "host"` instead of `["host"]`), an array
   containing `null`/`undefined`/number entries, or whitespace-padded
   entries — `.some((p) => p.trim() === '*')` throws TypeError at
   buildClient time, bricking the LLM session before the first prompt.
   `staticCorrelationHeaders` already handled this via its end-to-end
   try/catch but the sister helper diverged. Settings loader does no
   runtime schema validation so this is reachable via a single typo.

   Fix: normalize the allowlist at wrap time:
     1. catch a throwing getter (existing)
     2. reject non-array → default allowlist (NEW — bare string typo)
     3. filter out non-string elements (NEW — [null, ...] typo)
     4. trim every surviving entry uniformly (NEW — see #2 below)
   Then `trustedHosts.includes('*')` instead of `.some((p) => p.trim() === '*')`,
   since patterns are already pre-trimmed.

2. **Trim asymmetry between `*` detection and host-pattern match**
   (llm-correlation-fetch.ts)
   `[" * "]` was tolerated (trimmed before `===` compare) but
   `[" dashscope.aliyuncs.com "]` silently never matched. The
   normalization above fixes this by trimming uniformly upstream.

3. **Proxy fetch test: only negative assertions**
   (openaiContentGenerator/provider/default.test.ts)
   The test asserted `callArg.fetch !== proxyFetch` and `!== globalThis.fetch`
   but both passed for ANY wrapper, including a buggy one that
   accidentally wraps globalThis.fetch instead of proxyFetch. Added a
   positive assertion: call the wrapped fetch and verify proxyFetch was
   the delegation target.

Tests: 4 new cases — whitespace-padded host pattern, bare-string
malformed config (both wrapper and static), null/number-containing
array malformed config (both wrapper and static), positive proxy fetch
delegation. All pass; pre-existing Anthropic User-Agent failure
unrelated.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* refactor(telemetry): split outbound correlation out of telemetry scope (R4)

Address LaZzyMan round-8 follow-up review on PR #4390: even though R3's
host allowlist made the default behavior safe, the meta-architectural
concern remains: telemetry's namespace and consent flow shouldn't quietly
extend to wire-level behavior aimed at third-party LLM provider request
streams. The recipient sets differ; the consent decisions differ; they
deserve separate namespaces, separate threat models, separate PRs.

This commit (called R4 in the design doc) collapses the PR scope so it
lands ONLY telemetry observability work:

REMOVED from this PR:
  - packages/core/src/telemetry/llm-correlation-fetch.ts(.test.ts)
  - packages/core/src/telemetry/trusted-llm-hosts.ts(.test.ts)
  - telemetry.sessionIdHeaderHosts setting + Config getter +
    resolveTelemetrySettings wiring + settingsSchema entry
  - wrapFetchWithCorrelation usage from four provider construction
    points (default.ts, dashscope.ts, anthropicContentGenerator.ts,
    geminiContentGenerator/index.ts)
  - All session-id provider tests across the four providers + the
    contentGenerator.test.ts mock stub
  - "Session correlation header" section in telemetry.md

ADDED:
  - OutboundCorrelationSettings interface in packages/core/src/config/config.ts,
    standalone top-level namespace separate from TelemetrySettings —
    SECURITY-RELEVANT label, all defaults off
  - Config.getOutboundCorrelationPropagateTraceContext() getter
  - outboundCorrelation top-level entry in settingsSchema.ts with
    propagateTraceContext: { default: false } and explicit
    SECURITY-RELEVANT framing in the description
  - CLI config-load pipeline passes settings.outboundCorrelation into
    ConfigParameters
  - NOOP_PROPAGATOR (TextMapPropagator no-op) in sdk.ts, conditionally
    installed on NodeSDK when propagateTraceContext is false (default).
    When true, omits textMapPropagator from NodeSDK options so the SDK
    keeps its default W3C composite propagator
  - 2 new sdk.test.ts cases covering the propagator gate behavior

UNCHANGED:
  - UndiciInstrumentation registration + OTLP feedback-loop guard +
    HttpInstrumentation OTLP guard from R2/R3 stay intact — they are
    pure telemetry (client HTTP spans into the operator's own OTLP
    collector), no wire-level data egress
  - Documentation rewrites telemetry.md to split "client-side HTTP
    span on outbound fetch" (telemetry) from a new "Outbound
    correlation (SECURITY-RELEVANT)" top-level section
  - design doc gets R4 revision row + new §12 "R4 Scope Conflation
    Split" capturing the rationale and follow-up PR outline

The session-id apparatus (R3 code) lives in git history at commits
1c8528a / cb162e7 / 7a1b4f8 / 40e1efc / 106598c; the
follow-up PR can cherry-pick or restore those files under the new
outboundCorrelation.* namespace as LazzyMan suggested.

Vscode-ide-companion settings.schema.json regenerated.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* docs(telemetry): disclose telemetry.enabled dependency on propagateTraceContext

Self-review pass on R4 commit 9bdd3bd flagged one footgun: both
`docs/developers/development/telemetry.md` and the settingsSchema.ts
description for `outboundCorrelation.propagateTraceContext` describe
the toggle's behavior without noting that the flag is a silent no-op
when `telemetry.enabled` is false. An operator who sets only
`outboundCorrelation.propagateTraceContext: true` and forgets the
telemetry switch gets zero behavior change — no error, no warning, no
traceparent.

Fix: add the dependency disclosure to both surfaces, plus a JSON
example showing both flags wired together for the ARMS+DashScope
cross-process trace continuation use case.

Also fix a minor comment accuracy nit at `sdk.test.ts:683`: said the
SDK installs `W3CTraceContextPropagator` instance when opt-in is true,
but the actual default is `CompositePropagator(W3CTraceContextPropagator
+ W3CBaggagePropagator)` per `@opentelemetry/sdk-node` source.

Vscode-ide-companion settings.schema.json regenerated to reflect the
expanded description string.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* test(config): cover getOutboundCorrelationPropagateTraceContext defaults

R4 (commit 9bdd3bd) added the getter but the test file didn't grow a
corresponding describe block — sibling telemetry getters all have unit
tests but this new one was missed.

Add 4 cases covering the security-relevant default-to-false invariant
and explicit-set behavior:
- omitted outboundCorrelation → false
- empty outboundCorrelation: {} → false (the `?? false` collapse on the
  getter, complementing the same on the constructor)
- explicit true → true
- explicit false → false

PR #4390 review (wenshao).

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* docs(design): reflect post-R4 polish commits in §12

R4 (commit 9bdd3bd) was followed by two polish commits that the
design doc §12 didn't track:

- 0be0df2 (docs): telemetry.enabled dependency disclosure on
  propagateTraceContext — added to telemetry.md + settingsSchema
  description because a self-review pass identified the silent-no-op
  footgun (operator sets propagateTraceContext: true but forgets
  telemetry.enabled: true, sees zero behavior change with no error).
- c0352fd (test): 4 config.test.ts cases covering the
  getOutboundCorrelationPropagateTraceContext default-false invariant
  (omitted / {} / explicit true / explicit false) — wenshao review
  flagged the test gap.

Updates §12.4 with a new "Hidden dependency: telemetry.enabled" sub-
section explaining the gating relationship and pointing forward at the
follow-up PR (future outboundCorrelation.* settings inherit the same
dependency). Updates §12.5 implementation table to add the
config.test.ts row and clarify the telemetry.md / vscode-schema rows
were touched again in the polish pass.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* refactor: simplify post-R4 polish per /simplify review

/simplify review pass on commits 0be0df2 + c0352fd + 62cf6b4
flagged 4 concerns. Fix all 4:

1. **settingsSchema.ts description**: footgun warning ("Depends on
   telemetry.enabled: true") was at char 600+ of a 650-char description.
   VS Code settings UI truncates to ~300 chars inline → the most
   important warning was hidden in the most-glanced view. Hoist to first
   sentence ("Requires telemetry.enabled: true.").

2. **config.test.ts**: drop the task-narration comment
   ("PR #4390 R4: keep wire-level toggle out of telemetry namespace.")
   that just restated the change context. The remaining 2-line comment
   explaining WHY default-to-false is security-relevant survives.

3. **config.test.ts**: collapse 4 separate `it()` blocks into a single
   `it.each([...])` covering the same 4 precondition × expectation
   combinations. Removes boilerplate (`new Config({...baseParams, ...})`
   repeated 4×) without losing assertion power; case-3 ("explicit false")
   was a weak duplicate of case-2 ("empty object") since both hit the
   same `?? false` branch, but keeping all 4 in the parametric table
   documents intent more clearly than dropping case-3.

4. **design doc §12.4 + §12.5**: strip specific commit SHAs
   (`0be0df270`, `c0352fd5b`) — design docs should be evergreen, not
   doubled-up commit logs (those live in `git log`). Keep the design
   intent ("two panels both document the dependency" / "test block
   added") without naming the specific commits.

Regenerated vscode-ide-companion/schemas/settings.schema.json to
reflect the hoisted description sentence.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants