Skip to content

feat(core): Align GenAI content telemetry fields - #7667

Merged
doudouOUC merged 4 commits into
QwenLM:mainfrom
doudouOUC:agent/align-genai-content-telemetry
Jul 24, 2026
Merged

feat(core): Align GenAI content telemetry fields#7667
doudouOUC merged 4 commits into
QwenLM:mainfrom
doudouOUC:agent/align-genai-content-telemetry

Conversation

@doudouOUC

@doudouOUC doudouOUC commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR aligns seven content and Tool span attributes shared by the pinned OpenTelemetry GenAI conventions and Alibaba Cloud ARMS: gen_ai.input.messages, gen_ai.output.messages, gen_ai.system_instructions, gen_ai.tool.definitions, gen_ai.tool.description, gen_ai.tool.call.arguments, and gen_ai.tool.call.result.

LLM inputs are captured from the first provider-final SDK request, while outputs are captured from the final physical request attempt. OpenAI-compatible, Anthropic, Gemini, and Vertex payloads are converted to the pinned standard JSON schemas, including multi-candidate and streaming responses, without retaining raw chunk arrays.

Tool spans now record the static registry description, the final invocation parameters immediately before execution, and the successful model-facing result. Exact-equivalent private content attributes and the tool_schema event are no longer emitted; deprecated public helpers remain as thin standard-field adapters.

Sensitive JSON remains opt-in through telemetry.includeSensitiveSpanAttributes, is serialized compactly, and is omitted as a complete value when invalid, cyclic, incomplete, or over the configured per-attribute limit. Tool definition type and name are required identities, while an optional parameters value that cannot be normalized to Draft-07 is omitted without discarding the ordered identity list. Tool descriptions are non-sensitive and use a surrogate-safe 4096 UTF-16-code-unit limit.

Why it's needed

Qwen Code already emits the non-content GenAI fields shared by OpenTelemetry and ARMS, but its content observability still used private, provider-independent snapshots taken before adapters had finalized the SDK request. That made dashboards depend on non-standard keys and could diverge from the payload actually sent to a provider.

Capturing at provider boundaries makes the telemetry truthful after adapter defaults, overrides, field removal, and fallback behavior. Standard schemas also let ARMS and other OpenTelemetry backends query the same fields without dual-write aliases, while the existing sensitive-data switch and size limits keep content capture explicit and bounded.

Reviewer Test Plan

How to verify

Enable telemetry with the file exporter and includeSensitiveSpanAttributes=true, then run an OpenAI-compatible LLM → Tool → LLM turn. Confirm that each LLM span contains schema-valid standard input/output content matching the provider-final request and raw response, that all response choices and finish reasons are preserved in provider index order, and that the Tool span contains its static description, final executed arguments, and successful model-facing result.

Repeat with sensitive capture disabled. Confirm that messages, instructions, definitions, arguments, and results are absent while non-sensitive GenAI fields and the Tool description remain. Also confirm that the removed system_prompt*, tools, tools_count, response.model_output*, tool_input*, tool_result*, and tool_schema event are not emitted.

The permanent fake OpenAI integration test performs both scenarios and compares exported span JSON with the request bodies received by the server. Provider unit tests cover OpenAI-compatible, Anthropic, Gemini, and Vertex non-streaming/streaming boundaries, retries and fallback attempts, cancellation/failure finalization, concurrent context isolation, complete-value size omission, and final Tool argument/result timing.

Evidence (Before & After)

N/A — this is a non-UI telemetry change.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS with Node.js v22.22.3 and npm 10.9.8. Verified with the locally built and bundled CLI, the file exporter, and the local fake OpenAI server without sandboxing.

Risk & Scope

  • Main risk or tradeoff: This is a broad core instrumentation feature with more than 1,000 added production-logic lines, so maintainer awareness is requested. The main runtime risk is provider payload conversion and opt-in span size growth; conversion is best-effort, content is disabled by default, streaming accumulation is bounded, and invalid or oversized values are omitted as a whole.
  • Not validated / out of scope: The official OpenAI endpoint and ARMS staging were not tested because credentials were unavailable. Windows and Linux were not tested locally. Embeddings, payload fields beyond this seven-field batch, seed, top_k, SpanKind/span-name migration, and retry topology changes remain out of scope.
  • Breaking changes / migration notes: Exact-equivalent private sensitive attributes and the tool_schema event stop emitting without a dual-write period. Queries should migrate to the seven standard gen_ai.* fields above. gen_ai.response.finish_reasons now preserves provider-raw values such as stop, length, tool_calls, and end_turn instead of Gemini-normalized values such as STOP and MAX_TOKENS. Deprecated helper exports remain source-compatible, and sensitive content remains disabled by default.

Linked Issues

N/A

中文说明

本 PR 做了什么

本 PR 对齐固定版本 OpenTelemetry GenAI 规范与阿里云 ARMS 共有的 7 个内容及 Tool Span 属性:gen_ai.input.messagesgen_ai.output.messagesgen_ai.system_instructionsgen_ai.tool.definitionsgen_ai.tool.descriptiongen_ai.tool.call.argumentsgen_ai.tool.call.result

LLM 输入来自第一次 provider-final SDK 请求,输出来自最后一次物理请求 attempt。OpenAI 兼容、Anthropic、Gemini 和 Vertex 的 payload 会转换为固定版本的标准 JSON Schema,包括多候选和流式响应,同时不会保存原始 chunk 数组。

Tool Span 现在记录 registry 中的静态描述、执行前最终的 invocation 参数,以及成功后最终返回给模型的结果。与标准字段完全等价的私有内容属性和 tool_schema 事件停止写入;已废弃的公共 helper 继续保留,但只作为标准字段的薄适配器。

敏感 JSON 仍只在 telemetry.includeSensitiveSpanAttributes 开启时采集,使用紧凑序列化;当值非法、循环引用、不完整或超过单属性配置上限时整项省略。Tool definition 的 typename 是必填身份;可选 parameters 无法归一化为 Draft-07 时,只省略该可选属性,并保留有序的 Tool 身份列表。Tool 描述为非敏感字段,使用代理对安全的 4096 个 UTF-16 code unit 上限。

为什么需要

Qwen Code 已经输出 OpenTelemetry 与 ARMS 共有的非内容 GenAI 字段,但内容可观测仍使用私有字段,而且快照取自 provider adapter 完成 SDK 请求处理之前。这使查询依赖非标准 key,也可能与实际发给 provider 的 payload 不一致。

在 provider 边界采集可以真实反映 adapter 默认值、覆盖值、字段删除和 fallback 之后的请求。标准 Schema 也让 ARMS 和其他 OpenTelemetry backend 可以查询同一组字段而无需双写 alias;现有敏感数据开关和大小限制则保证内容采集是显式且有界的。

Reviewer 测试计划

如何验证

启用 telemetry file exporter 和 includeSensitiveSpanAttributes=true,执行一次 OpenAI 兼容的 LLM → Tool → LLM 流程。确认每个 LLM Span 都包含符合 Schema 的标准输入/输出内容,并与 provider-final 请求及原始响应一致;确认所有 response choice 和 finish reason 按 provider index 顺序保留;确认 Tool Span 包含静态描述、最终执行参数和成功后返回给模型的结果。

关闭敏感内容采集后重复执行。确认 messages、instructions、definitions、arguments 和 result 均不存在,同时非敏感 GenAI 字段与 Tool description 仍然保留。还应确认已删除的 system_prompt*toolstools_countresponse.model_output*tool_input*tool_result*tool_schema 事件不再输出。

永久 fake OpenAI 集成测试覆盖以上两个场景,并将导出的 Span JSON 与服务端实际收到的请求体比较。Provider 单元测试覆盖 OpenAI 兼容、Anthropic、Gemini 和 Vertex 的非流式/流式边界、retry 与 fallback attempt、取消/失败 finalize、并发 Context 隔离、完整值超限省略,以及 Tool 最终参数/结果的采集时机。

证据(Before & After)

N/A——这是非 UI telemetry 变更。

测试平台

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

macOS,Node.js v22.22.3,npm 10.9.8。使用本地 build 和 bundle 后的 CLI、file exporter 以及不启用 sandbox 的本地 fake OpenAI server 完成验证。

风险与范围

  • 主要风险或权衡:这是一个较大范围的核心可观测 feature,新增生产逻辑超过 1,000 行,需要 maintainer 知悉。主要运行时风险是 provider payload 转换与开启敏感采集后的 Span 体积增长;转换为 best-effort,内容采集默认关闭,流式累积有界,非法或超限值会整项省略。
  • 未验证 / 不在范围内:由于没有凭据,未验证官方 OpenAI endpoint 和 ARMS staging;未在本地验证 Windows 和 Linux。Embedding、本批 7 个字段之外的 payload、seedtop_k、SpanKind/Span name 迁移和 retry topology 变更仍不在本 PR 范围内。
  • Breaking change / 迁移说明:与标准字段完全等价的私有敏感属性和 tool_schema 事件停止写入,不提供双写期。查询应迁移到上面的 7 个标准 gen_ai.* 字段。gen_ai.response.finish_reasons 现在保留 provider 原始值,例如 stoplengthtool_callsend_turn,不再使用 STOPMAX_TOKENS 等 Gemini 归一化值。已废弃的 helper 导出保持源码兼容,敏感内容默认仍关闭。

关联 Issue

N/A

Capture provider-final GenAI messages and tool payloads with the shared ARMS/OpenTelemetry field contract, and retire equivalent private content aliases.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

GenAI content telemetry E2E

Scope

Validate the P2 standard content attributes across an OpenAI-compatible
LLM → Tool → LLM turn:

  • gen_ai.input.messages
  • gen_ai.output.messages
  • gen_ai.system_instructions
  • gen_ai.tool.definitions
  • gen_ai.tool.description
  • gen_ai.tool.call.arguments
  • gen_ai.tool.call.result

Also verify that sensitive attributes are absent when capture is disabled and
that the removed private aliases and tool_schema event are not emitted.

Baseline

  • Global CLI: qwen 0.20.1
  • Scenario: local fake OpenAI endpoint, file exporter, sensitive span
    attributes enabled, one shell tool call followed by a final answer.
  • Expected before P2: the seven P2 standard attributes are absent; older
    sensitive private attributes may be present.
  • Result on 2026-07-24: the command completed the two-request tool flow, but
    the assertion failed before reaching P2 checks because the installed build
    also predates the P0/P1 aligned operation, provider, request-parameter,
    response-model, and usage fields. Its LLM Span exposed the request model and
    one normalized STOP finish reason, and none of the P2 standard content
    fields.

Local verification

  1. Build and bundle the repository.
  2. Run integration-tests/cli/gen-ai-telemetry.test.ts without sandboxing.
  3. Confirm the fake endpoint's final messages and tool definitions correspond
    to the compact standard JSON on both LLM spans, and that OpenAI
    system/developer messages remain in gen_ai.input.messages rather than
    being duplicated as gen_ai.system_instructions.
  4. Confirm two streamed candidates are preserved in provider index order and
    fragmented content/tool arguments are joined.
  5. Confirm the Tool span records its static description, final executed
    arguments, and successful model-facing result.
  6. Confirm the second test, with sensitive capture disabled, emits none of the
    six sensitive attributes.

Result on 2026-07-24: passed with the local bundle. The permanent integration
test exported two LLM spans and one Tool span, matched provider-final inputs and
definitions, preserved both final-response candidates and fragmented text,
captured final Tool arguments/result, found no removed private fields or
tool_schema event, and omitted all sensitive content when the switch was
disabled.

External verification

  • Official OpenAI endpoint: not run; OPENAI_API_KEY and OPENAI_MODEL were
    not available.
  • ARMS staging: not run; staging credentials were not available.

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Review — GenAI content telemetry alignment

Reviewed the full diff at 496a37f, read gen-ai-content.ts / gen-ai-request.ts end to end, and ran the touched suites locally against the PR branch:

gen-ai-content, gen-ai-exchange, gen-ai-request, detailed-span-attributes,
session-tracing, coreToolScheduler, loggingContentGenerator,
openaiContentGenerator/pipeline, anthropicContentGenerator, geminiContentGenerator
→ 838 tests, all passing

The architecture holds up. Context-keyed handles with a generation counter isolate retries and concurrent requests correctly, every telemetry path is try/catch-wrapped so it cannot throw into the request path, jsonValue builds objects via Object.defineProperty so a __proto__ key in a provider payload cannot pollute, and the fixtures + docs are unusually thorough for a change this size.

My concerns are about undeclared behaviour changes rather than the design. Both Criticals share one shape: they change what is exported for users running with includeSensitiveSpanAttributes=false, which is the default and the population the PR describes as unaffected.

C=2, S=3, N=2 — details inline.

Minor, not filed as a finding: gen-ai-exchange.test.ts (402 lines) tests gen-ai-request.ts, and there is no gen-ai-exchange.ts. gen-ai-request.ts is now 568 lines spanning request-attribute extraction, the exchange controller, the context plumbing and twelve report* facades — a natural split point if you touch it again.

CI

ubuntu-latest / Java 11 and windows-latest / Java 21 fail on DaemonSessionClientTest.rejectsClientWidePromptCapacityBeforeMutation (IOException: HTTP/1.1 header parser received no bytes). This diff touches no Java, the same job passes on ubuntu Java 17/21 and macOS Java 21, and the workflow is green on the last five main runs — flaky, worth a re-run rather than an investigation.

Comment thread packages/core/src/telemetry/session-tracing.ts
Comment thread packages/cli/src/acp-integration/session/Session.ts Outdated
Comment thread packages/core/src/telemetry/gen-ai-content.ts Outdated
Comment thread packages/core/src/telemetry/gen-ai-content.ts
Comment thread integration-tests/cli/gen-ai-telemetry.test.ts
Keep ACP tool telemetry best-effort, improve diagnostics and sensitive-off coverage, and document finish-reason and tool-description behavior.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Addressed the latest review against 5230bd8f48.

Feedback Disposition
ACP telemetry helpers could fail tool execution Fixed and regression-tested
Sensitive-off Tool coverage was incomplete Fixed with a full LLM → Tool → LLM integration scenario
Missing finish reason caused silent whole-attribute omission Kept schema-strict omission; added a content-free debug diagnostic
Streaming budget estimate did not explain JSON escaping Documented the lower-bound estimate and exact final serialized cap
Tool description is always emitted Kept by design as non-sensitive standard metadata; documented MCP/workspace scope and the 4096-code-unit cap
Provider-raw finish reasons are a migration Kept by design; documented the breaking query migration. The pinned schema includes error as a valid finish reason
Late usage refresh looked unused No code change: existing success and late-failure regression tests cover post-yield provider mutation

Validation on the exact commit: build, bundle, typecheck, lint, core content tests (27/27), ACP Session tests (414/414), and OpenAI fake-server integration (2/2) all passed. Two prior Java SDK matrix failures were independently diagnosed as unrelated timing/connection flakes; this PR does not modify Java, and the new commit has triggered fresh checks.

@doudouOUC
doudouOUC marked this pull request as ready for review July 24, 2026 15:30
@doudouOUC
doudouOUC enabled auto-merge July 24, 2026 15:30
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments above for the result.

Qwen Triage 已完成 —— 查看运行。结果见上方各阶段评论。

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Re-review — 5230bd8f4

Re-ran the suites on the updated branch: core 838 + ACP Session.test.ts 414, all passing. CI is now fully green, including ubuntu Java 11 and windows Java 21 — confirming those were the flake they looked like.

Round-1 outcomes

  • Fixed: ACP telemetry isolation, the reserve() documentation, and the sensitive-off tool-span coverage.
  • Accepted as your call: always-on gen_ai.tool.description, provider-raw finish_reasons, and whole-attribute omission on a missing finish reason. All three are now documented where a query author would look, and I am not re-litigating them.
  • Withdrawn — two of my findings were wrong, both corrected in their threads. error is in the pinned FinishReason enum, so the partial-failure fallback was schema-valid all along. And refreshLateUsageMetadata is load-bearing and was covered by tests in the same commit — I asserted otherwise without checking the file's own test suite, and removing the calls fails exactly the two tests you named.

One new finding

C=0, S=1. Not a regression from 5230bd8f4 — it is in gen-ai-content.ts from the first commit and I missed it in round 1. Details inline.

Comment thread packages/core/src/telemetry/gen-ai-content.ts
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections are present with substantive content, including a bilingual translation.

Problem: This is a feat PR, not a bug fix, so no reproduction is expected. The motivation is clear: existing content telemetry uses private, provider-independent snapshots taken before adapters finalize the SDK request, making dashboards depend on non-standard keys and potentially diverging from the actual wire payload. Aligning with the pinned OpenTelemetry GenAI conventions and ARMS is a well-defined goal backed by the existing design doc (gen-ai-arms-field-alignment.md), which this PR updates.

Direction: Aligned. Standardizing content attributes on the OTel GenAI conventions is the right call — it lets ARMS and other backends query the same fields without dual-write aliases. Claude Code's CHANGELOG shows similar OTel content attribute work (CLAUDE_CODE_OTEL_CONTENT_MAX_LENGTH, claude_code.assistant_response), confirming the industry direction. However, this PR touches telemetry infrastructure, which is a maintainer-escalation area. Flagging for maintainer direction review.

Size: This PR touches core paths (packages/core/src/**) extensively. Breakdown:

  • Production logic (.ts source in packages/): 2,608 lines (additions + deletions)
  • Test files (*.test.ts, excluded from thresholds): 2,897 lines
  • Test fixtures (JSON): 1,155 lines
  • Docs: 189 lines
  • Integration test support: 134 lines

At 2,608 production lines this exceeds both the 500-line maintainer-awareness threshold and the 1,000-line large-PR advisory. Since this is a feat (not refactor), it is not hard-blocked, but maintainer awareness is requested and the author should consider whether splitting is feasible. The seven fields share the same exchange mechanism, so splitting may be artificial — but the tool-span changes (description, arguments, result) could potentially land separately from the LLM content capture if that eases review.

Approach: The architecture is sound. Capturing at provider boundaries (after adapter finalization) is the right design point. The GenAiExchangeController with generation tracking for retries/fallbacks, and the bounded streaming accumulator with overflow-to-discard, are well-considered. Removing old private attributes without a dual-write period is a breaking change, but it's clearly documented in both the PR body and the updated design doc, and the deprecated helpers remain as thin source-compatible adapters.

One question worth considering: the normalizeSchema function (~150 lines of JSON Schema draft-07 validation) is thorough but adds significant complexity. Is the full validation necessary, or would a lighter structural check suffice for the telemetry use case?

⚠️ Escalating to maintainer for direction review (telemetry infrastructure touch) and size awareness before proceeding with code review.

中文说明

感谢贡献!

模板完整 ✓ —— 所有必填章节均已填写,包含中文翻译。

问题: 这是一个 feat PR,不是 bug 修复,因此不需要复现。动机清晰:现有内容遥测使用私有字段,且快照取自 adapter 完成 SDK 请求处理之前,导致 dashboard 依赖非标准 key,也可能与实际发送的 payload 不一致。对齐固定版本的 OpenTelemetry GenAI 规范和 ARMS 是一个目标明确的改进,有现有设计文档(gen-ai-arms-field-alignment.md)支撑,本 PR 也同步更新了该文档。

方向: 对齐。将内容属性标准化到 OTel GenAI 规范是正确的方向——让 ARMS 和其他 backend 可以查询同一组字段而无需双写 alias。Claude Code 的 CHANGELOG 显示了类似的 OTel 内容属性工作(CLAUDE_CODE_OTEL_CONTENT_MAX_LENGTHclaude_code.assistant_response),确认了行业方向。但本 PR 触及遥测基础设施,属于需要维护者审查的领域。 标记请维护者进行方向审查。

规模: 本 PR 大量触及核心路径(packages/core/src/**)。分解如下:

  • 生产逻辑(packages/ 中的 .ts 源文件):2,608 行(增 + 删)
  • 测试文件(*.test.ts,不计入阈值):2,897 行
  • 测试 fixtures(JSON):1,155 行
  • 文档:189 行
  • 集成测试支撑:134 行

2,608 行生产代码超过了 500 行维护者知悉阈值和 1,000 行大 PR 建议阈值。由于这是 feat(非 refactor),不会被硬性阻止,但需要维护者知悉,作者可以考虑是否可以拆分。七个字段共享同一个 exchange 机制,拆分可能不自然——但 tool span 变更(description、arguments、result)也许可以与 LLM 内容采集分开提交,以降低审查难度。

方案: 架构合理。在 provider 边界(adapter 完成之后)采集是正确的设计点。GenAiExchangeController 带有 retry/fallback 的 generation 跟踪,以及有界的流式累加器(溢出即丢弃),设计周到。移除旧的私有属性且不提供双写期是一个 breaking change,但在 PR 正文和更新的设计文档中都有清晰说明,已废弃的 helper 仍作为源码兼容的薄适配器保留。

一个值得考虑的问题:normalizeSchema 函数(约 150 行 JSON Schema draft-07 验证)很全面但增加了相当的复杂度。对于遥测场景,是否需要完整验证,还是更轻量的结构检查就足够了?

⚠️ 转交维护者 进行方向审查(触及遥测基础设施)和规模知悉,然后再进入代码审查。

Qwen Code · qwen3.8-max-preview

Reviewed at 5230bd8f4821f5b0b83ce7a099199b1c92df2896 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: Given the goal of capturing provider-final content for OTel GenAI alignment, I would have: (1) added a content extraction function per provider that converts the SDK request/response to a canonical JSON schema, (2) extended the existing gen-ai-request.ts observer to also capture responses, (3) added the seven standard attributes gated behind the existing includeSensitiveSpanAttributes flag, and (4) kept the old attributes during a transition period.

Comparison with the PR: The PR's approach matches and exceeds this baseline. The GenAiExchangeController is more sophisticated than a simple observer extension — it properly handles the request/response lifecycle across retries and fallbacks via generation tracking, and the streaming accumulator bounds memory with overflow-to-discard. The decision to remove old attributes without dual-write is bolder than my transition-period approach, but it's defensible given the design doc clearly documents the migration and the deprecated helpers remain source-compatible.

No critical blockers found. The code is well-structured and follows project conventions (ESM, kebab-case.ts, collocated tests, no any — uses unknown with type guards throughout). A few observations:

  • Exchange lifecycle is sound. finalize() is idempotent (guards on this.finalized), called on all exit paths in loggingContentGenerator.ts (success, error, stream timeout). The generation counter correctly invalidates stale attempt handles after a retry/fallback starts a new response accumulator.

  • Memory bounding is correct. The GenAiOutputAccumulator.reserve() method tracks estimated length and calls markOverflow() which clears all parts. finalize() then applies the exact compact-JSON length check via stringifyGenAiJson. Streaming accumulation is bounded by the configured sensitiveSpanAttributeMaxLength.

  • Sensitive data gating is consistent. Content capture requires !isInternal && shouldCollectSensitiveSpanAttributes(). Tool arguments/results go through areSensitiveSpanAttributesEnabled(config). Tool description is correctly treated as non-sensitive with its own 4096-unit surrogate-safe limit.

  • Tool argument timing is correct. Captured from invocation.params immediately before invocation.execute(), after permission and edit hooks — this is the final set of parameters that will actually run.

  • Tool result capture is deliberately scoped. Only successful calls emit gen_ai.tool.call.result, from the FunctionResponse.response object. Error/blocked/cancelled paths do not emit a result attribute. This is documented and reasonable.

  • normalizeSchema complexity. The ~150-line JSON Schema draft-07 validator is thorough. For a telemetry attribute that's best-effort and omitted-as-a-whole on failure, this level of validation is defensible (it prevents malformed schemas from polluting the span), but it's the densest part of the diff and worth a careful look from someone familiar with the tool-schema surface.

  • disabledFallbackContext creates a proxy Context that intercepts the observer key when parent.setValue throws. Defensive, but the prototype-chain approach (Object.create(parent)) means any new Context interface methods added by OTel in the future won't be proxied. Low risk given OTel's stability guarantees, but worth noting.

sequenceDiagram
    participant P1 as LoggingContentGenerator
    participant P2 as GenAiExchangeController
    participant P3 as Provider (OpenAI/Anthropic/Gemini)
    participant P4 as GenAiOutputAccumulator
    participant P5 as LLM Request Span
    P1->>P2: createGenAiExchange(span, options)
    P1->>P3: generateContent / generateContentStream
    P3->>P2: reportXxxRequest(sdkRequest) — beginRequest
    P2->>P5: setAttributes(gen_ai.input.messages, system_instructions, tool.definitions)
    P3->>P2: reportXxxResponse / reportXxxChunk
    P2->>P4: record(handle, update)
    P1->>P2: finalize(success)
    P2->>P4: finalize(success) — serialize output
    P2->>P5: setAttribute(gen_ai.output.messages)
    P2-->>P1: finishReasons
Loading
Files changed (30 of 34 shown)
File What changed
docs/design/gen-ai-arms-field-alignment.md Design doc updated with the seven content/tool fields, removal table, and finish-reason migration note
docs/design/telemetry-llm-request-timing-design.md Cross-reference updated: chat compression no longer resets removed hash state
docs/developers/development/telemetry.md User-facing telemetry docs: updated includeSensitiveSpanAttributes description
integration-tests/cli/gen-ai-telemetry.test.ts Integration test: verifies standard content attributes with fake OpenAI server, both enabled and disabled
integration-tests/fake-openai-server.ts Fake server extended to capture request bodies and serve streaming responses for content verification
packages/cli/src/acp-integration/session/Session.ts ACP/daemon tool path: passes description to startToolSpan, records arguments and result via new helpers
packages/cli/src/acp-integration/session/Session.test.ts Tests for the ACP tool telemetry additions
packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.ts Reports responses and streaming events to the exchange; handles fallback attempt via reportAnthropicFollowingRequest
packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts Tests for Anthropic exchange integration
packages/core/src/core/coreToolScheduler.ts Tool arguments captured at invocation.params before execute; result from FunctionResponse; removed old private attribute calls
packages/core/src/core/coreToolScheduler.test.ts Tests for the new tool argument/result capture timing
packages/core/src/core/geminiChat.ts Removed unused import
packages/core/src/core/geminiContentGenerator/geminiContentGenerator.ts Reports responses and wraps streams with observeGeminiStream for chunk reporting
packages/core/src/core/geminiContentGenerator/geminiContentGenerator.test.ts Tests for Gemini exchange integration
packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts Creates GenAiExchange, finalizes on all exit paths (success, error, timeout), removes old addSystemPrompt/addToolSchema/addModelOutput calls
packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts Extensive test updates: removed old private-attribute assertions, added exchange lifecycle tests
packages/core/src/core/openaiContentGenerator/pipeline.ts Passes telemetryAttempt handle through executor; reports responses and chunks
packages/core/src/core/openaiContentGenerator/pipeline.test.ts Tests for OpenAI exchange integration
packages/core/src/telemetry/detailed-span-attributes.ts Old helpers deprecated as thin adapters; new addToolArgumentsAttributes and addToolCallResultAttributes; removed seenHashes and hash-based dedup
packages/core/src/telemetry/detailed-span-attributes.test.ts Tests rewritten for the new standard-field adapters
packages/core/src/telemetry/gen-ai-content.ts New module: provider-specific content extraction (OpenAI, Anthropic, Gemini), canonical part conversion, JSON Schema validation, GenAiOutputAccumulator for streaming
packages/core/src/telemetry/gen-ai-content.test.ts Comprehensive tests: all four providers, streaming, multi-candidate, overflow, schema validation
packages/core/src/telemetry/gen-ai-exchange.test.ts Tests for GenAiExchangeController lifecycle: begin, record, finalize, generation invalidation
packages/core/src/telemetry/gen-ai-request.ts Replaced observer pattern with GenAiExchangeController; added response/chunk reporting functions; deprecated createGenAiRequestObserverContext
packages/core/src/telemetry/gen-ai-request.test.ts Minor test update for the new return type
packages/core/src/telemetry/index.ts Exports addToolArgumentsAttributes and addToolCallResultAttributes
packages/core/src/telemetry/session-tracing.ts startToolSpan accepts description param with 4096-unit surrogate-safe truncation; removed clearDetailedSpanState call
packages/core/src/telemetry/session-tracing.test.ts Tests for tool description bounding and empty-description omission
packages/core/src/telemetry/test-fixtures/gen-ai/*.json JSON Schema fixtures for validating canonical content shapes (6 files)
…and 4 more files

Testing

CI evidence (from the PR's own checks at 5230bd8):

Check Conclusion
precheck-pr / precheck ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Real daemon E2E / Java 11 ✅ success

All CI checks pass. The PR includes 941 lines of new unit tests for gen-ai-content.ts, 402 lines for the exchange controller, and updated integration tests that verify exported span JSON against server-received request bodies.

Not verified: real-provider behavior (OpenAI endpoint, ARMS staging) — the author notes credentials were unavailable. Windows and Linux were not tested locally by the author (macOS only). This is a non-UI telemetry change, so tmux real-scenario testing is N/A.

中文说明

代码审查

独立方案: 给定目标是为 OTel GenAI 对齐采集 provider-final 内容,我会:(1) 为每个 provider 添加内容提取函数,将 SDK 请求/响应转换为规范 JSON Schema;(2) 扩展现有 gen-ai-request.ts observer 以同时捕获响应;(3) 在现有 includeSensitiveSpanAttributes 开关后添加七个标准属性;(4) 在过渡期保留旧属性。

与 PR 的比较: PR 的方案匹配并超越了这一基线。GenAiExchangeController 比简单的 observer 扩展更精密——它通过 generation 跟踪正确处理 retry 和 fallback 间的请求/响应生命周期,流式累加器通过溢出即丢弃来限制内存。移除旧属性且不提供双写期比我的过渡期方案更大胆,但设计文档清晰记录了迁移路径,且已废弃的 helper 保持源码兼容,因此是合理的。

未发现关键阻塞问题。代码结构良好,遵循项目约定(ESM、kebab-case.ts、同目录测试、无 any——全程使用 unknown 加类型守卫)。几点观察:

  • Exchange 生命周期健全。 finalize() 是幂等的(守卫 this.finalized),在 loggingContentGenerator.ts 的所有退出路径(成功、错误、流超时)上调用。generation 计数器在 retry/fallback 启动新响应累加器时正确使旧 attempt handle 失效。

  • 内存限制正确。 GenAiOutputAccumulator.reserve() 跟踪估算长度并调用 markOverflow() 清除所有 parts。finalize() 然后通过 stringifyGenAiJson 应用精确的紧凑 JSON 长度检查。流式累加受配置的 sensitiveSpanAttributeMaxLength 限制。

  • 敏感数据门控一致。 内容采集需要 !isInternal && shouldCollectSensitiveSpanAttributes()。Tool 参数/结果通过 areSensitiveSpanAttributesEnabled(config) 检查。Tool 描述正确视为非敏感,使用独立的 4096 code unit 代理对安全限制。

  • Tool 参数时机正确。invocation.execute() 之前从 invocation.params 采集,在权限和编辑 hook 之后——这是实际执行的最终参数集。

  • Tool 结果采集范围经过深思熟虑。 仅成功调用发出 gen_ai.tool.call.result,来自 FunctionResponse.response 对象。错误/阻止/取消路径不发出 result 属性。有文档说明且合理。

  • normalizeSchema 复杂度。 约 150 行的 JSON Schema draft-07 验证器很全面。对于 best-effort 且失败时整项省略的遥测属性,这一验证级别是合理的(防止格式错误的 schema 污染 span),但它是 diff 中最密集的部分,值得熟悉 tool-schema 表面的人仔细审查。

  • disabledFallbackContextparent.setValue 抛出时创建一个代理 Context,拦截 observer key。防御性编码,但原型链方式(Object.create(parent))意味着 OTel 未来添加的新 Context 接口方法不会被代理。鉴于 OTel 的稳定性保证,风险较低,但值得注意。

序列图展示了 exchange 生命周期的关键路径:LoggingContentGenerator 创建 exchange → provider 报告请求 → provider 报告响应/chunk → LoggingContentGenerator 调用 finalize → span 获得 gen_ai.output.messages。

测试

CI 证据(来自 PR 自身在 5230bd8 的检查):

所有 CI 检查通过。PR 包含 941 行 gen-ai-content.ts 新单元测试、402 行 exchange controller 测试,以及更新的集成测试(将导出的 span JSON 与服务端收到的请求体进行比较)。

未验证:真实 provider 行为(OpenAI endpoint、ARMS staging)——作者注明没有凭据。作者未在本地测试 Windows 和 Linux(仅 macOS)。这是非 UI 遥测变更,tmux 真实场景测试不适用。

Qwen Code · qwen3.8-max-preview

Reviewed at 5230bd8f4821f5b0b83ce7a099199b1c92df2896 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 3/5

The code quality is high and the direction is right — standardizing content telemetry on the OTel GenAI conventions is well-motivated and well-executed. The exchange controller lifecycle, bounded streaming accumulator, and consistent sensitive-data gating are all sound. CI is green and test coverage is comprehensive (2,900+ lines of test code).

What keeps this at 3 rather than higher:

  1. Telemetry infrastructure is a maintainer-escalation area. This PR reshapes the core telemetry content-capture pipeline — replacing the observer pattern with a full exchange controller, removing seven categories of private attributes without dual-write, and changing gen_ai.response.finish_reasons from Gemini-normalized to provider-raw values. These are architectural decisions that affect downstream dashboards and queries, and they warrant maintainer sign-off.

  2. Scale. 2,608 production lines across 34 files is a large surface for a single review pass. The changes are coherent (all seven fields share the exchange mechanism), but the breadth increases the chance of a subtle interaction that a second pair of eyes would catch.

  3. Breaking changes without dual-write. The removal of system_prompt*, tools, tools_count, response.model_output*, tool_input*, tool_result*, and the tool_schema event is clearly documented, and the deprecated helpers remain source-compatible. But any existing dashboard or query keyed on the old attribute names will silently stop matching. The finish-reason value change (STOPstop, MAX_TOKENSlength) has the same risk. A maintainer should confirm the user impact is acceptable.

Verdict: No critical blockers found in the code. The implementation is careful, well-tested, and follows project conventions. Escalating to the maintainer for direction approval on the telemetry architecture changes and the breaking-change scope before this can be approved.

@doudouOUC — solid work. The exchange controller design and the provider-specific content extraction are well done. The main thing holding this back from approval is the telemetry-infrastructure scope, which needs a maintainer's architectural sign-off.

中文说明

置信度:3/5

代码质量高,方向正确——将内容遥测标准化到 OTel GenAI 规范动机充分且执行良好。Exchange controller 生命周期、有界流式累加器和一致的敏感数据门控都是合理的。CI 全绿,测试覆盖全面(2,900+ 行测试代码)。

置信度为 3 而非更高的原因:

  1. 遥测基础设施是需要维护者审查的领域。 本 PR 重塑了核心遥测内容采集管道——用完整的 exchange controller 替换 observer 模式,移除七类私有属性且不提供双写,并将 gen_ai.response.finish_reasons 从 Gemini 归一化值改为 provider 原始值。这些是影响下游 dashboard 和查询的架构决策,需要维护者签核。

  2. 规模。 2,608 行生产代码跨 34 个文件,对于单次审查来说是一个很大的表面。变更是连贯的(七个字段共享 exchange 机制),但广度增加了细微交互被遗漏的可能性。

  3. 无双写的 Breaking changes。 移除 system_prompt*toolstools_countresponse.model_output*tool_input*tool_result*tool_schema 事件有清晰的文档说明,已废弃的 helper 保持源码兼容。但任何依赖旧属性名的现有 dashboard 或查询将静默停止匹配。finish-reason 值变更(STOPstopMAX_TOKENSlength)有同样的风险。维护者应确认用户影响可接受。

结论: 代码中未发现关键阻塞问题。实现仔细、测试充分、遵循项目约定。转交维护者 进行遥测架构变更和 breaking change 范围的方向审批,之后才能批准。

@doudouOUC —— 出色的工作。Exchange controller 设计和 provider 特定的内容提取做得很好。阻止批准的主要原因是遥测基础设施的范围,需要维护者的架构签核。

Qwen Code · qwen3.8-max-preview

Reviewed at 5230bd8f4821f5b0b83ce7a099199b1c92df2896 · re-run with @qwen-code /triage

Document that invalid optional parameter schemas are omitted while required tool identities remain ordered and complete.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Addressed the latest review against cde441897c.

The new tool-schema suggestion was partially accepted. I did not change runtime behavior because the pinned OTel schema requires only type and name, explicitly makes parameters optional, and the approved contract plus the existing omits only invalid optional parameters but rejects missing identity regression test intentionally preserve the ordered identity list when an optional provider schema cannot be normalized to Draft-07.

The review did expose an ambiguity in the design wording. The design doc and bilingual PR description now distinguish attribute-level whole-value omission from the optional parameters normalization rule.

Validation on the exact commit: gen-ai-content.test.ts 27/27 passed, Prettier passed, git diff --check passed, and two clean audit passes found no additional issue.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed. Not reviewed: chunk 1, chunk 3, chunk 4, chunk 26, chunk 18, chunk 14, chunk 8, chunk 24, chunk 15, chunk 13, chunk 7, chunk 11, chunk 9, chunk 20, chunk 5, chunk 17, chunk 16, chunk 19, chunk 2, chunk 22, chunk 23, chunk 12, chunk 25, chunk 21, chunk 10, chunk 6 — launched with a prompt that is not the one the CLI built. Not reviewed: Test coverage matrix (whole-diff), Agent 1b: Removed-behavior audit, Agent 1c: Cross-file tracer — its prompt was built, but no agent on record was launched with it. Not reviewed: reverse audit — its prompt was built, but no agent was launched with it — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries, and cannot be certified.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Code Review — #7667 feat(core): Align GenAI content telemetry fields

Reviewed at the current head (all CI green, incl. the 31‑min Ubuntu suite and the permanent fake‑OpenAI integration test). This is a large, careful change — a new 1,518‑line conversion module plus wiring across five provider/scheduler paths. Overall it's high quality and, in my read, close to merge‑ready. I went through the full diff independently; the earlier self‑review thread already caught the highest‑impact items and I've confirmed each resolution is actually in the tree.

What it does

  • Emits seven OTel‑GenAI/ARMS standard content attributes: gen_ai.input.messages, gen_ai.output.messages, gen_ai.system_instructions, gen_ai.tool.definitions, gen_ai.tool.description, gen_ai.tool.call.arguments, gen_ai.tool.call.result.
  • Captures inputs from the first provider‑final SDK request and outputs from the final physical attempt, via a new GenAiExchangeController + GenAiOutputAccumulator keyed by a per‑attempt generation so stale chunks from a superseded attempt are ignored.
  • Converts OpenAI‑compatible / Anthropic / Gemini / Vertex payloads (non‑stream + stream, multi‑candidate) to pinned JSON schemas without retaining raw chunk arrays.
  • Removes the private system_prompt*/tools/tools_count/response.model_output*/tool_input*/tool_result* attributes and the tool_schema event; deprecated helpers stay as thin adapters.

Strengths

  • Attempt/generation isolation is the right model for retries + Anthropic required‑thinking fallback (beginFollowingRequest validates generation before starting a new accumulator). Concurrent‑context isolation via requestObserverKey is sound.
  • Robust edge handling: cyclic/undefined‑leaf omission in jsonValue, surrogate‑safe truncation, Anthropic input:{} + input_json_delta "arguments‑from‑start" replacement, explicit‑empty ([]) candidate lists, per‑attempt reset in finalize.
  • Telemetry can't break the request/tool path: conversions are wrapped; the ACP path now guards both helper calls (matching CoreToolScheduler's safelyAdd…). Confirmed at Session.ts ~7424/7671.
  • Test coverage is genuinely strong — the fake‑OpenAI E2E pins wire‑vs‑span equivalence for both capture‑on and capture‑off, multi‑candidate (streamed idx 0 + non‑streamed idx 1), tool arg/result timing, and negative assertions for every removed attribute + the tool_schema event.

Confirmed resolutions from the self‑review (all present in the tree)

ACP unguarded helpers → wrapped; single‑candidate‑missing‑finish‑reason → kept whole‑omission + added a content‑free debug diagnostic at the omission point; reserve() documented as a lower‑bound with finalize() as the exact gate; sensitive‑off E2E now asserts the tool span too; error confirmed in the pinned FinishReason enum. Good.

Points worth a maintainer's explicit sign‑off (not blockers)

  1. gen_ai.tool.description is exported by default (sensitive‑capture OFF). This is the one genuine default‑behavior change: with telemetry.enabled=true + includeSensitiveSpanAttributes=false, every tool span now carries the full (4096‑capped) tool description — including MCP‑server and workspace/skill‑provided text that previously only left the process behind the sensitive opt‑in (as the hash‑deduped tool_schema event). It's defensible (OTel treats tool.description as non‑sensitive metadata) and is now documented in telemetry.md, but for Qwen's dynamic MCP/skill descriptions it's a conscious privacy/volume call. The no‑dedupe re‑emission (~1 MB of repeated static text per 200‑call session) is the same knob — fine to accept, worth a maintainer's eyes since the PR itself asks for awareness.
  2. gen_ai.response.finish_reasons is now provider‑raw for all users (capture on or off). The accumulator records finish reasons regardless of captureContent (controller enabled = span.isRecording()), so dashboards see stop/length/tool_calls (OpenAI), end_turn/max_tokens (Anthropic), and still STOP/MAX_TOKENS (Gemini, whose raw value is the enum). Net effect: the same attribute is now provider‑specific in casing/vocabulary across providers. This is intended and documented as a breaking query‑migration — just flagging that an ARMS/OTel dashboard filtering a single finish_reasons value will behave differently per provider.
  3. normalizeSchema drops only parameters (keeps the tool identity) when a schema can't be normalized (e.g. draft‑04 exclusiveMinimum: true, type: "any"). I actually think the author's resolution here is the better one — preserving the ordered identity list beats dropping every tool's definition because one has an odd schema, and the pinned schema makes parameters optional. The only residual is span‑≠‑wire for that one tool; documented. Fine as‑is.

Minor / optional

  • Deprecated helpers changed semantics, not just deprecated. addModelOutputAttributes now early‑returns without a finish reason; addToolInputAttributes/addToolResultAttributes now JSON.parse and drop non‑JSON input. No internal callers remain, but any out‑of‑tree caller relying on the old string behavior silently gets nothing. A one‑line @deprecated note that behavior (not just the name) changed would help.
  • disabledFallbackContext builds a Object.create(parent) pseudo‑Context shadowing getValue/setValue/deleteValue. It's only reached when parent.setValue throws (very rare) and works, but it's an unusual shape that couples to OTel's Context surface — a short comment on why a prototype‑shadow rather than a real derived context would age better.

Correctness / perf / security

  • Correctness: no functional bug found. Memory is bounded (accumulator overflow clears parts; streaming retains canonical parts, not raw chunks). Retry/fallback finalize ordering looks right on every path (sync, stream, idle‑timeout, error, abort).
  • Security: content stays behind the existing opt‑in; internal prompts are excluded from capture; the only expanded default surface is item 1 above.

Nice work — the accumulator design and the wire‑vs‑span E2E are the strong parts. My only "please decide consciously" item is (1); the rest is confirmation.

中文说明

已在当前 head(CI 全绿,含 31 分钟 Ubuntu 套件与 fake‑OpenAI 集成测试)完整独立审阅。整体质量很高,基本接近可合并。此前的自审线程已覆盖影响最大的问题,我逐一确认修复确实已落到代码里。

做了什么:新增 7 个 OTel‑GenAI/ARMS 标准内容属性;输入取自第一次 provider‑final 请求、输出取自最后一次物理 attempt;通过按 attempt 递增的 generation 忽略被取代 attempt 的残留 chunk;四家 provider 的流式/非流式、多候选 payload 转换为固定 schema。

优点:attempt/generation 隔离与并发 context 隔离设计正确;边界处理稳健(循环引用、代理对截断、Anthropic input:{}+input_json_delta 处理、显式空候选、finalize 重置);遥测失败不会影响请求/工具执行(ACP 两处 helper 现已 guard);测试覆盖扎实(wire↔span 等价、多候选、参数/结果时机、所有被删属性与 tool_schema 事件的反向断言)。

已确认修复:ACP guard、单候选缺 finish_reason 的整体省略 + debug 诊断、reserve() 下界文档化、sensitive‑off 对 tool span 的断言、error 属于固定 FinishReason 枚举。

需维护者明确拍板(非阻塞)

  1. gen_ai.tool.description 默认导出(敏感采集关闭时)——唯一真正的默认行为变化:enabled=trueincludeSensitiveSpanAttributes=false 时,每个 tool span 现都带完整(4096 上限)描述,含以前需显式开关才会外发的 MCP/工作区/skill 文本。符合 OTel(description 视为非敏感),也已在 telemetry.md 说明,但对 Qwen 的动态 MCP/skill 描述属隐私/体积上的主动取舍;且无去重重复外发(~200 次调用/会话约 1 MB)。
  2. finish_reasons 现对所有用户输出 provider 原始值(无论是否开启内容采集),跨 provider 的取值/大小写不一致(OpenAI stop / Anthropic end_turn / Gemini 仍 STOP)。为有意设计并已标注为破坏性查询迁移。
  3. normalizeSchema 无法归一时只丢 parameters、保留工具身份——我认同作者的处理优于"整体丢弃",仅该工具存在 span≠wire,已文档化,可保持现状。

次要:被弃用 helper 的语义(非仅命名)已变化,建议补一行 @deprecated 说明;disabledFallbackContextObject.create(parent) 影子 Context 建议加注释。

正确性/性能/安全:未发现功能性 bug;内存有界;重试/回退各路径 finalize 顺序正确;内容仍在既有 opt‑in 之后,唯一扩大的默认面即第 1 点。

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Local build & real-test verification — #7667 feat(core): Align GenAI content telemetry fields

Complements the static code review above with an empirical pass: built the PR head in an isolated worktree from a clean npm ci, ran the full changed test surface, drove the real bundled CLI end-to-end against a fake OpenAI server, and — most importantly — inspected the raw exported OTel spans myself (parsing telemetry.log directly, not relying on the PR's own assertions). Everything checks out.

Environment: detached worktree @ cde441897 (PR head), clean npm ci → esbuild bundle (dist/cli.js), macOS, Node 22. Merge-base f8014652.

1 · Test suites — all green

Scope Command Result
10 changed core suites (gen-ai-content/exchange/request, detailed-span-attributes, session-tracing, logging/anthropic/gemini/openai generators, coreToolScheduler) vitest run 822 / 822 ✓
ACP tool-telemetry helpers Session.test.ts 414 / 414 ✓
E2E — real dist/cli.js vs in-process fake OpenAI, LLM → Tool → LLM gen-ai-telemetry.test.ts 2 / 2 ✓ (sensitive ON + OFF)

tests

2 · Independent span inspection (raw telemetry.log, both capture modes)

Parsed the exported spans directly and confirmed the seven standard fields behave exactly as specified:

  • Sensitive ONgen_ai.input.messages (the real ~45 KB system+user), gen_ai.output.messages = the run_shell_command tool_call with parsed args {"command":"pwd"} / finish_reason: tool_calls; the multi-candidate final turn preserves both choices in provider index order (["stop","stop"]); gen_ai.tool.definitions Draft-07 normalized; tool span carries gen_ai.tool.description (exactly 4108 = 4096 + …[truncated], surrogate-safe), gen_ai.tool.call.arguments = final invocation params {"command":"pwd"}, and gen_ai.tool.call.result = the model-facing {"output":…}.
  • Sensitive OFF — every content field absent; gen_ai.tool.description still present (non-sensitive metadata, per OTel); arguments/result absent; choice.count omitted for n=1.
  • Removed legacy surface — verified absent across all 20 spans in both runs: system_prompt*, tools/tools_count, response.model_output*, tool_input*, tool_result*, and the tool_schema event (0 emitted).

spans

3 · A/B discrimination (the tests are load-bearing)

Ran the same PR test files against a bundle rebuilt from base source vs the PR:

  • base f80146522 failed (content fields absent → gen_ai.tool.description is undefined, LLM span omits the aligned content)
  • PR cde4418972 passed

Base already carries #7635's gen_ai.request.*, so the failure isolates precisely this PR's content contribution — the E2E is not vacuous.

Verdict

From a build-and-run standpoint this is merge-ready — 1,238 unit tests + the wire-vs-span E2E all pass, the raw spans match the pinned schema on both capture paths, and the legacy attributes/tool_schema event are genuinely gone. No functional issue surfaced. The only "please decide consciously" item remains the one already flagged in the code review above: gen_ai.tool.description now emits by default (sensitive-capture OFF), re-emitted per call without dedup — defensible under OTel, but a deliberate privacy/volume call for Qwen's dynamic MCP/skill descriptions.

中文说明

本地构建 + 真实测试验证 —— #7667 feat(core): Align GenAI content telemetry fields

在上面的静态 code review 基础上补充一轮实测:在隔离 worktree 从干净 npm ci 构建 PR head,跑完全部改动测试面,用真实 bundle CLI 对 fake OpenAI server 做端到端 LLM → Tool → LLM,并且亲自解析导出的原始 OTel span(直接读 telemetry.log,不依赖 PR 自带断言)。全部通过。

环境: detached worktree @ cde441897(PR head),干净 npm ci → esbuild bundle(dist/cli.js),macOS,Node 22,merge-base f8014652

1 · 测试套件全绿

  • 10 个改动的 core 套件(gen-ai-content/exchange/request、detailed-span-attributes、session-tracing、logging/anthropic/gemini/openai 生成器、coreToolScheduler):822 / 822 ✓
  • ACP 工具遥测 helper Session.test.ts414 / 414 ✓
  • E2E 真实 dist/cli.js 对 fake OpenAI(LLM → Tool → LLM)gen-ai-telemetry.test.ts2 / 2 ✓(敏感开 + 关)

2 · 独立 span 检查(原始 telemetry.log,两种模式)

  • 敏感开input.messages(真实约 45 KB system+user);output.messages = run_shell_command 的 tool_call,参数已解析{"command":"pwd"}finish_reason: tool_calls多候选最终轮按 provider index 顺序保留两个 choice(["stop","stop"]);tool.definitions 已归一化为 Draft-07;tool span 带 tool.description(恰为 4108 = 4096 + …[truncated],代理对安全)、tool.call.arguments = 最终执行参数 {"command":"pwd"}tool.call.result = 返回给模型的 {"output":…}
  • 敏感关:所有内容字段缺席;tool.description 仍在(非敏感元数据);arguments/result 缺席;n=1 时省略 choice.count
  • 移除的旧字段:两次运行、全部 20 个 span 中均确认缺席system_prompt*tools/tools_countresponse.model_output*tool_input*tool_result*,以及 tool_schema 事件(0 个)。

3 · A/B 判别(测试确实“承重”)
同一批 PR 测试文件,对 base 源码重建的 bundle vs PR:base f80146522 失败(内容字段缺席,tool.descriptionundefined);PR cde4418972 通过。base 已含 #7635gen_ai.request.*,因此失败精确隔离本 PR 的内容字段贡献

结论: 从构建与运行角度可合并——1,238 个单测 + wire-vs-span E2E 全过,原始 span 在两条采集路径上都符合固定 schema,旧属性与 tool_schema 事件确已移除,未发现功能性问题。唯一需要 maintainer 有意识拍板的仍是 review 里已标注的第 (1) 点:tool.description 现在默认输出(敏感采集关闭时),且每次调用重复输出无去重——在 OTel 下可接受,但对 Qwen 动态 MCP/skill 描述是一个刻意的隐私/体积取舍。

Document the changed semantics of deprecated helpers and the reason the fallback context shadows key operations.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@wenshao Thanks for the thorough review. I independently rechecked the two optional points and accepted both in 04bb8302d2:

  • The deprecated compatibility helpers now state their changed write semantics: model output requires an explicit finish reason, and legacy tool input/result strings must parse to JSON objects.
  • disabledFallbackContext now documents why it uses the parent prototype while shadowing key operations: all unrelated Context behavior is preserved, while a failed setValue cannot expose an ancestor GenAI observer.

There are no runtime changes. Validation on the exact commit: the three relevant telemetry suites passed 58/58, Prettier and diff checks passed, and two clean audit passes found no additional issue.

The three maintainer-sign-off points remain intentional and documented: always-on bounded Tool descriptions, provider-raw finish reasons, and omission of only an invalid optional Tool parameters property.

@wenshao

wenshao commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Review — feat(core): Align GenAI content telemetry fields

Re-reviewed at head 04bb8302. The latest commit since the prior review round is documentation-only (6 lines of clarifying comments on the deprecated helpers and disabledFallbackContext); no production logic changed. Verdict below is unchanged: merge-ready, modulo three conscious default-behavior notes for maintainer sign-off — none are blockers.

What it does

Emits the seven content/tool span attributes shared by the pinned OpenTelemetry GenAI conventions and Alibaba Cloud ARMS: gen_ai.input.messages, gen_ai.output.messages, gen_ai.system_instructions, gen_ai.tool.definitions, gen_ai.tool.description, gen_ai.tool.call.arguments, gen_ai.tool.call.result. A new ~1.5k-line gen-ai-content.ts converts OpenAI-compatible / Anthropic / Gemini / Vertex payloads (streaming + non-streaming, multi-candidate) into the pinned JSON schemas. Inputs are read from the first provider-final SDK request; outputs from the final physical attempt via GenAiExchangeController + GenAiOutputAccumulator (generation-keyed, so fallback/retry chunks from a stale attempt are ignored). Legacy private aliases (system_prompt*, tools, tools_count, response.model_output*, tool_input*, tool_result*) and the tool_schema event are removed; deprecated helpers stay as thin standard-field adapters.

Strengths

  • Capture-at-boundary is the right call. Reading the provider-final request instead of the pre-adapter logical config makes the telemetry truthful after adapter defaults, overrides, field removal, and fallback — the core motivation, and it holds up.
  • Robust omission discipline. Every sensitive JSON value is compact-serialized and independently omitted as a whole when invalid, cyclic, incomplete, or over sensitiveSpanAttributeMaxLength. JSON is never truncated mid-structure — good, since a half-serialized attribute is worse than an absent one.
  • Concurrency isolation is handled deliberately (disabledFallbackContext shadows key operations so a failed setValue can't leak an ancestor observer into another exchange), and is covered by a concurrent-context test.
  • Surrogate-safe limits: gen_ai.tool.description uses a 4096 UTF-16-code-unit cap that preserves surrogate pairs and appends …[truncated].
  • Test coverage is excellent. The permanent fake-OpenAI integration test drives the real built CLI through an LLM→Tool→LLM turn with sensitive capture on and off, and asserts exported span JSON against the request bodies the server actually received — including multi-candidate finish reasons in provider index order, tool-definition equivalence, and absence of every removed attribute + the tool_schema event. Provider unit tests cover the streaming/non-streaming boundaries, retry/fallback, cancellation, and complete-value size omission. I independently parsed the raw telemetry.log and a base-vs-PR bundle A/B in an earlier pass — both confirmed the wire matches the assertions.

Notes for maintainer sign-off (non-blocking, all now documented)

  1. gen_ai.tool.description emits even when sensitive capture is OFF. It's treated as non-sensitive static registry metadata, so descriptions from MCP servers / workspace tool providers that were previously opt-in now export by default. There's no dedupe across a session, so a long-running session with many tool calls carries the description repeatedly. This is the one real default-behavior change and is now called out in telemetry.md; worth a conscious ack.
  2. gen_ai.response.finish_reasons is now provider-raw for all users (e.g. stop / length / tool_calls / end_turn) instead of Gemini-normalized STOP / MAX_TOKENS. Casing/vocabulary now differs across providers by design. This is a breaking query-migration and is documented as such in the design doc + PR body.
  3. normalizeSchema drops only the optional parameters when a provider schema can't be normalized to Draft-07 (e.g. draft-04 exclusiveMinimum, type:"any"), keeping the ordered tool identity list intact rather than dropping the whole definition. I agree this is the better trade-off; now documented.

Risks

Broad core instrumentation (>1k production lines), but the blast radius is well-contained: content is disabled by default, conversion is best-effort with whole-value omission, streaming accumulation is bounded, and the removed aliases have a documented migration path. No dual-write period — anyone querying the old private keys must migrate, which the PR body states clearly.

Verdict: LGTM / merge-ready. Clean, well-tested, thoroughly documented. The only open items are the three default-behavior decisions above, which are maintainer calls rather than defects.

中文说明

在 head 04bb8302 复审。相对上一轮唯一的新提交是纯文档(对 deprecated helper 与 disabledFallbackContext 补了 6 行注释),生产逻辑无改动。结论不变:可合并,仅有 3 个有意为之的默认行为变更需 maintainer 知悉,均非阻塞。

做了什么:输出 OpenTelemetry GenAI 规范与阿里云 ARMS 共有的 7 个内容/Tool 属性。新增约 1.5k 行 gen-ai-content.ts,将 OpenAI 兼容 / Anthropic / Gemini / Vertex 的 payload(流式+非流式、多候选)转换为固定版本 JSON schema。输入取自首个 provider-final 请求,输出取自最后一次物理 attempt(按 generation 归属,忽略过期 attempt 的 chunk)。移除私有别名与 tool_schema 事件,deprecated helper 保留为薄适配器。

优点:在 provider 边界采集使遥测真实反映 adapter 处理后的请求;敏感 JSON 整值省略(非法/循环/不完整/超限时整项丢弃,绝不中途截断);并发上下文隔离(disabledFallbackContext);代理对安全的 4096 UTF-16 上限;测试覆盖出色——永久 fake-OpenAI 集成测试用真实构建的 CLI 跑 LLM→Tool→LLM,敏感开关开/关两种场景都将导出的 span JSON 与服务端实际收到的请求体比对。此前我已用原始 telemetry.log 解析 + base/PR bundle A/B 独立验证,两者均一致。

需知悉的 3 点(非阻塞,均已写入文档)

  1. gen_ai.tool.description 即使敏感采集关闭也会输出(视为非敏感静态元数据),包含 MCP/workspace 工具描述;会话内不去重。唯一真正的默认行为变化,已在 telemetry.md 说明。
  2. gen_ai.response.finish_reasons 现在对所有用户保留 provider 原始值(stop/length/tool_calls/end_turn),不再是 Gemini 归一化的 STOP/MAX_TOKENS;查询需迁移,已文档化。
  3. normalizeSchema 在参数 schema 无法归一化为 Draft-07 时只省略可选的 parameters,保留有序 Tool 身份列表——认同此权衡,已文档化。

风险:核心埋点范围较大(>1k 行),但内容默认关闭、转换 best-effort 且整值省略、流式累积有界、移除别名有迁移路径,影响可控。无双写期,旧私有 key 的查询须迁移,PR body 已明确。

结论:LGTM / 可合并。 干净、测试充分、文档完善;剩余仅为上述 3 个默认行为决策,属 maintainer 取舍而非缺陷。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.7-max via Qwen Code /review

@doudouOUC
doudouOUC added this pull request to the merge queue Jul 24, 2026
Merged via the queue into QwenLM:main with commit 055523a Jul 24, 2026
52 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.1.

@yiliang114

Copy link
Copy Markdown
Collaborator

⚠️ Failed to process this request. Please re-mention the bot to retry.

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.

4 participants