Skip to content

fix(telemetry): ignore unsupported OTel exporter selectors - #8703

Merged
wenshao merged 2 commits into
QwenLM:mainfrom
zjunothing:fix/issue-8697-otel-metrics-env
Aug 8, 2026
Merged

fix(telemetry): ignore unsupported OTel exporter selectors#8703
wenshao merged 2 commits into
QwenLM:mainfrom
zjunothing:fix/issue-8697-otel-metrics-env

Conversation

@zjunothing

Copy link
Copy Markdown
Collaborator

What this PR does

This PR prevents sdk-node from reading the standard OTEL_TRACES_EXPORTER, OTEL_LOGS_EXPORTER, and OTEL_METRICS_EXPORTER selector variables while Qwen Code starts its telemetry SDK. The variables are restored immediately after the synchronous SDK start call.

Why it's needed

Qwen Code already supplies explicit exporters for each enabled signal. In the bundled CLI, sdk-node's environment auto-configuration can nevertheless construct an omitted OTLP exporter when OTEL_METRICS_EXPORTER=otlp is present. That constructor throws after partial SDK setup, so Qwen Code's own telemetry remains uninitialized and metrics are silently lost while unrelated instrumentation can continue exporting.

Fixes #8697

Reviewer Test Plan

How to verify

  1. Build the CLI bundle with npm run build -- --cli-only && npm run bundle.
  2. Set OTEL_METRICS_EXPORTER=otlp and an explicit Qwen Code gRPC OTLP endpoint.
  3. Start telemetry through the bundled SDK facade.
  4. Confirm initialization succeeds and the caller's OTEL_*_EXPORTER values are unchanged after startup.

Before this change, the bundled SDK fails with qwen-code bundles @opentelemetry/sdk-node without @opentelemetry/exporter-metrics-otlp-proto ... Attempted to construct: OTLPMetricExporter. After this change, the same invocation reports initialized:true.

Evidence (Before & After)

N/A — this is a non-visual telemetry startup behavior. Exact command output and test results are included in the verification report below.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

  • macOS arm64
  • Node.js v22.23.1
  • npm 10.9.8

Risk & Scope

  • Main risk or tradeoff: Qwen Code intentionally ignores external OTel exporter selector variables because its settings already select explicit exporters; this preserves the existing Qwen Code telemetry configuration rather than merging two exporter-selection surfaces.
  • Not validated / out of scope: honoring external exporter selectors as an alternative way to configure Qwen Code; Windows/Linux runtime execution; a live collector connection.
  • Breaking changes / migration notes: none. The three environment variables remain visible to the caller and are restored after synchronous SDK startup.

Linked Issues

Fixes #8697

中文说明

What this PR does

本 PR 防止 sdk-node 在 Qwen Code 启动 telemetry SDK 时读取标准的 OTEL_TRACES_EXPORTEROTEL_LOGS_EXPORTEROTEL_METRICS_EXPORTER 选择变量。变量会在同步 SDK 启动调用结束后立即恢复。

Why it's needed

Qwen Code 已经为每个启用的信号显式提供 exporter,但在 bundle CLI 中,即使设置了 OTEL_METRICS_EXPORTER=otlp,sdk-node 的环境变量自动配置仍可能构造一个被 bundle 排除的 OTLP exporter。该构造器会在 SDK 部分初始化后抛错,导致 Qwen Code 自身的 telemetry 未初始化,metrics 静默丢失,而无关的 instrumentation 仍可能继续导出。

Fixes #8697

Reviewer Test Plan

How to verify

  1. 使用 npm run build -- --cli-only && npm run bundle 构建 CLI bundle。
  2. 设置 OTEL_METRICS_EXPORTER=otlp 和显式的 Qwen Code gRPC OTLP endpoint。
  3. 通过 bundle SDK facade 启动 telemetry。
  4. 确认初始化成功,并确认调用方的 OTEL_*_EXPORTER 值在启动后保持不变。

修复前,bundle SDK 报错:qwen-code bundles @opentelemetry/sdk-node without @opentelemetry/exporter-metrics-otlp-proto ... Attempted to construct: OTLPMetricExporter。修复后,同一调用输出 initialized:true

Evidence (Before & After)

不适用——这是非视觉 telemetry 启动行为。精确命令输出和测试结果会在下面的验证报告中列出。

Tested on

OS Status
🍏 macOS ✅ 已测试
🪟 Windows 不适用
🐧 Linux 不适用

Environment (optional)

  • macOS arm64
  • Node.js v22.23.1
  • npm 10.9.8

Risk & Scope

  • Main risk or tradeoff:Qwen Code 有自己的 settings 来选择显式 exporter,因此有意忽略外部 OTel exporter 选择变量;这保留了现有 Qwen Code telemetry 配置,而不是合并两套 exporter 选择入口。
  • Not validated / out of scope:将外部 exporter 选择变量作为 Qwen Code 的替代配置入口;Windows/Linux 运行时验证;真实 collector 连接。
  • Breaking changes / migration notes:无。三个环境变量对调用方仍可见,并会在同步 SDK 启动后恢复。

Linked Issues

Fixes #8697

@zjunothing

Copy link
Copy Markdown
Collaborator Author

Verification report

Environment

  • macOS arm64
  • Node.js v22.23.1
  • npm 10.9.8
  • Commit fa207ae

Reproduction and result

Failure-first reproduction on the current upstream bundle:

$ OTEL_METRICS_EXPORTER=otlp node .../sdk-impl-*.js
SDK_START=FAIL
Error: qwen-code bundles @opentelemetry/sdk-node without @opentelemetry/exporter-metrics-otlp-proto ... Attempted to construct: OTLPMetricExporter

After the fix, the same OTEL_METRICS_EXPORTER=otlp environment was exercised through the bundled SDK facade:

{"initialized":true,"metricsExporterEnv":"otlp"}

The facade removes all three external exporter selectors only during synchronous sdk.start(), then restores them. The focused regression test also verifies that all three are absent inside start() and restored afterward.

Tests executed

  • npm -w packages/core run test -- src/telemetry/sdk.test.ts -t 'ignores external exporter selectors' — PASS (1 passed)
  • npm -w packages/core run test -- src/telemetry/sdk.test.ts — PASS (64/64)
  • npm run build -- --cli-only — PASS
  • npm run bundle — PASS
  • npm run typecheck — PASS
  • npm -w packages/core run typecheck — PASS
  • npm run lint:ci — PASS
  • npx eslint packages/core/src/telemetry/sdk.ts packages/core/src/telemetry/sdk.test.ts — PASS
  • git diff --check — PASS
  • repository pre-commit (prettier --write + staged-file ESLint) — PASS

Evidence

No screenshot is applicable: this is non-visual telemetry startup behavior. The failure and fixed bundle outputs above are real local command results.

中文验证报告

验证报告

环境

  • macOS arm64
  • Node.js v22.23.1
  • npm 10.9.8
  • 提交 fa207ae

复现与结果

在当前 upstream bundle 上进行了修复前失败复现:

$ OTEL_METRICS_EXPORTER=otlp node .../sdk-impl-*.js
SDK_START=FAIL
Error: qwen-code bundles @opentelemetry/sdk-node without @opentelemetry/exporter-metrics-otlp-proto ... Attempted to construct: OTLPMetricExporter

修复后,在同样设置 OTEL_METRICS_EXPORTER=otlp 的环境中通过 bundle SDK facade 验证:

{"initialized":true,"metricsExporterEnv":"otlp"}

facade 只在同步 sdk.start() 期间移除三个外部 exporter 选择变量,随后恢复。聚焦回归测试也验证了三个变量在 start() 内均不可见,调用结束后完整恢复。

已执行测试

  • npm -w packages/core run test -- src/telemetry/sdk.test.ts -t 'ignores external exporter selectors' — 通过(1 项)
  • npm -w packages/core run test -- src/telemetry/sdk.test.ts — 通过(64/64)
  • npm run build -- --cli-only — 通过
  • npm run bundle — 通过
  • npm run typecheck — 通过
  • npm -w packages/core run typecheck — 通过
  • npm run lint:ci — 通过
  • npx eslint packages/core/src/telemetry/sdk.ts packages/core/src/telemetry/sdk.test.ts — 通过
  • git diff --check — 通过
  • 仓库 pre-commit(prettier --write + 暂存文件 ESLint)— 通过

证据

这是非视觉 telemetry 启动行为,不适用截图。上方的失败和修复后 bundle 输出均来自本地实际命令。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@wenshao

wenshao commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 7, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Re-run on commit ad1a0932 (test guards + invariant comment), triggered by @wenshao after the autofix round addressed the findings from his end-to-end verification report.

  • Template: complete ✓
  • Problem: observed, heavily evidenced — and now independently reproduced. OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697 carries the exact error string, a clean A/B reproduction, and 30 days of collector-side data. Since the prior run, @wenshao reproduced it end-to-end with two source-built bundles against a real OTLP/gRPC collector (six-arm A/B matrix): the base bundle aborts mid-start() with the issue's error verbatim, this branch starts cleanly and exports all signals.
  • Direction: aligned, and the prior run's telemetry-policy escalation is resolved. The bundle deliberately omits the OTLP exporter chains (Cold-start follow-ups: remaining lazy-loading candidates from the ACP eager-closure audit #7264/perf(telemetry): lazy-load the SDK and split OTLP exporter chains by protocol #7276), so of the issue's two options only "cleanly ignore the selectors during startup" is minimal — and the maintainer has now signed off on exactly that, in practice and via an approving review on this head.
  • Size: core paths (packages/core/src/telemetry/) — 41 production lines (sdk.ts +34/−1; sdk-impl.ts +5/−1, comment-only) + 80 test lines. Well under every threshold; fix type, no Tier 1 concern.
  • Approach: minimal, and this commit adds zero production logic. The fix itself is unchanged from the prior run: one helper wrapping the single sdk.start() call site, scrub-and-restore in finally. The new commit only closes the review findings — the F1 test restructured so it actually fails without the fix, a new F2 throw-path test pinning the finally restore, and a four-line invariant comment at the new NodeSDK site. Nothing to cut.
  • Risk: no high-risk path matches; no elevated signals remaining.

Moving on to code review. 🔍

中文说明

在提交 ad1a0932(测试守护 + 不变量注释)上重跑,由 @wenshao 在 autofix 轮次处理完其端到端验证报告中的问题后触发。

  • 模板: 完整 ✓
  • 问题:已观测、证据充分——且已被独立复现。 OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697 提供了原始报错、干净的 A/B 复现和 30 天的 collector 侧数据。上次运行之后,@wenshao 用两份从源码构建的 bundle 对接真实 OTLP/gRPC collector 做了端到端复现(六组 A/B):base bundle 在 start() 中途以 issue 中的原始报错中断,本分支正常启动并导出全部信号。
  • 方向:对齐,且上次运行的 telemetry 策略升级已解决。 bundle 有意不包含 OTLP exporter 链(Cold-start follow-ups: remaining lazy-loading candidates from the ACP eager-closure audit #7264/perf(telemetry): lazy-load the SDK and split OTLP exporter chains by protocol #7276),issue 的两个选项中只有"启动期间干净地忽略选择变量"是最小解——maintainer 现已用实际行动和对本 head 的批准评审完成了签核。
  • 规模: 核心路径(packages/core/src/telemetry/)——41 行生产代码(sdk.ts +34/−1;sdk-impl.ts +5/−1,纯注释)+ 80 行测试。远低于所有阈值;fix 类型,无 Tier 1 问题。
  • 方案:最小化,且本提交零生产逻辑改动。 修复本体与上次运行一致:一个 helper 包裹唯一的 sdk.start() 调用点,finally 中清除并还原。新提交只闭环评审意见——重构 F1 测试使其在修复缺失时真正失败、新增 F2 抛错路径测试钉住 finally 还原、在 new NodeSDK 处补四行不变量注释。无可删减。
  • 风险: 无高风险路径命中;无遗留升级信号。

进入代码审查 🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Code review

Independent proposal for this commit, written before reading its diff: F1 is fixed by recording the observed env values inside the mocked start() and asserting on them only after initializeTelemetry resolves (assertions thrown inside the mock are swallowed by init's catch — that was the false green); F2 needs a throw-path test asserting the three variables come back; the constructor-time invariant belongs in a comment at the new NodeSDK site. The commit does exactly these three things and nothing else — I verified line by line that no production logic changed this round.

Both new tests are genuine guards now. I can't mutation-test here (triage never executes PR code), so I checked statically:

  • F1 test: with the fix reverted to a plain sdk.start(), the mocked start records the set values into observedDuringStart, and the post-resolve expect(...).toBeUndefined() fails outside initializeTelemetry's catch — the exact hole @wenshao's report described is closed. The added comment explaining why the assertions must live outside the mock is the right kind of "why" comment.
  • F2 test: the finally is load-bearing — without it, any unrelated start() throw would leave the caller's three OTEL_*_EXPORTER vars permanently deleted for every child process spawned afterwards. The new test pins exactly that contract: mock throws → telemetry stays uninitialized → all three variables restored.
  • sdk-impl.ts comment: the four added lines accurately record what I verified against @opentelemetry/sdk-node@0.203.0 — the logs env fallback runs in the NodeSDK constructor, which sits outside the scrub window around start(), so spanProcessors/logRecordProcessors must stay unconditional arrays. Correct placement, correct content.

No blockers, and no new observations. The prior run's two non-blocking notes still stand as recorded (the no-metricReader edge case aligns runtime with the already-declared unsupported-env contract; the restructured test + throw-path test supersede the old false-green concern).

Testing

Evidence basis: unattended CI run — no code was built or executed here. The table quotes the PR's own CI checks on the reviewed commit via the API.

One red artifact to name plainly: the .github/workflows/qwen-code-pr-review.yml workflow run on this head reports failure — but it spawned zero jobs and failed at startup (created and updated timestamps identical). It is the review-bot orchestration workflow, not build/test CI; the later run of the same workflow (from @wenshao's /review) consumed its full 10800 s budget and timed out, per the notice in this thread. Classification: infra noise, not caused by this PR.

The behavioral claim no longer rests on the author's word, so there is no sandboxed-lane line this round: @wenshao's independent six-arm A/B (real source-built bundles, real OTLP/gRPC collector, otlp/prometheus/unset arms on both base and PR heads, on Linux) reproduced the failure on base and the repair on this branch — a superset of what a /verify run would produce. The author's earlier macOS reproduction is superseded by it, not relied upon.

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
label ✅ success
route ✅ success
Remind on force-push ✅ success

Completed checks on the reviewed commit; skipped checks (macOS/Windows test matrix and integrations — gated for fork PRs) omitted. / reviewed commit 上已完成的 check;省略 skipped(fork PR 受限的 macOS/Windows 测试矩阵与集成测试)。

中文说明

代码审查

读 diff 之前先写独立方案:F1 的正确修法是在被 mock 的 start() 内记录观察到的环境变量值、等 initializeTelemetry 返回后再断言(mock 内部的断言会被 init 的 catch 吞掉——这正是"假绿"的成因);F2 需要一个抛错路径测试断言三个变量被还原;构造函数期的不变量应写成 new NodeSDK 处的注释。本提交恰好做了这三件事、别无其他——逐行核实本轮零生产逻辑改动。

两个新测试现在都是真正的守护。triage 不执行 PR 代码,无法做 mutation 测试,故以静态方式核对:

  • F1 测试: 把修复回退为裸 sdk.start(),被 mock 的 start 会把已设置的值记入 observedDuringStart,随后在 initializeTelemetrycatch 之外执行的 expect(...).toBeUndefined() 会失败——@wenshao 报告指出的缺口已闭合。新增注释解释了断言为何必须放在 mock 外部,是恰当的"why"注释。
  • F2 测试: 这个 finally 有实际作用——没有它,任何无关的 start() 抛错都会让调用方的三个 OTEL_*_EXPORTER 变量在进程余生内一直处于被删除状态,之后拉起的每个子进程都会继承被裁剪的环境。新测试钉住的正是该契约:mock 抛错 → telemetry 保持未初始化 → 三个变量全部还原。
  • sdk-impl.ts 注释: 新增四行准确记录了我对照 @opentelemetry/sdk-node@0.203.0 核实过的事实——logs 的 env 回退在 NodeSDK 构造函数中执行,位于包住 start() 的清理窗口之外,因此 spanProcessors/logRecordProcessors 必须保持无条件数组。位置正确,内容正确。

无阻塞项,也无新观察。上次运行的两条非阻塞记录仍然有效(无 metricReader 的边界情形与已声明的"env 入口不支持"契约一致;重构后的测试 + 抛错路径测试取代了原先的假绿问题)。

测试

证据基础: 无人值守 CI 运行——此处未构建或执行任何代码。表格通过 API 引用 reviewed commit 上 PR 自身的 CI 检查结果。

一个需要明说的红色项:本 head 上的 .github/workflows/qwen-code-pr-review.yml 工作流运行报告 failure——但它一个 job 都没有产生,在启动瞬间即失败(created 与 updated 时间戳相同)。那是评审机器人编排工作流,不是构建/测试 CI;同一工作流的后续运行(@wenshao/review 触发)跑满 10800 秒预算后超时,见本线程通知。判定:基础设施噪音,非本 PR 所致。

行为结论不再依赖作者自述,因此本轮不再列沙箱验证通道:@wenshao 的独立六组 A/B(真实源码构建 bundle、真实 OTLP/gRPC collector、base 与 PR head 各跑 otlp/prometheus/未设置,Linux 环境)在 base 上复现了失败、在本分支上确认了修复——这是 /verify 所能产出的超集。作者早先在 macOS 上的复现被其取代,而非被引用为据。

CI 结果见英文版表格。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — everything the gate can check is clean on the new head; the only remainder is the two follow-up items from #8697 that the maintainer explicitly said should not block.

The prior run deferred with one open question: who owns the telemetry-policy call? The answer came in the strongest form possible — @wenshao built both bundles from source, ran a six-arm A/B against a real OTLP/gRPC collector, reproduced the bug on base and confirmed the repair on this branch (including the prometheus variant and real-process caller-env restore), and then flagged the two test gaps. This commit closes both exactly as prescribed, and I verified statically that the restructured tests are genuine mutation guards: each now fails when the corresponding production code is reverted, where before both stayed green. The production fix itself was already the minimal correct shape; this round touched no production logic at all.

Stepping back: the root cause is independently verified, the fix is the smallest one that works, the tests now pin it, the maintainer approved this exact commit, and the only red CI artifact is a zero-job startup failure of the review-bot workflow itself. The one standing nit: #8697's other two asks — a warning visible without QWEN_DEBUG_LOG_FILE=1, and not letting a metric-provider failure abort the rest of SDK startup — deserve a follow-up issue so they aren't silently dropped. They do not belong in this PR.

Approving, pinned to the reviewed commit. ✅

中文说明

置信度:4/5 —— 新 head 上门禁能查的一切均干净;唯一遗留是 #8697 的两个后续项,maintainer 已明确表示不应阻塞本 PR。

上次运行带着一个未决问题搁置:telemetry 策略决定由谁拍板?答案以最有力的形式给出——@wenshao 从源码构建了两份 bundle,对接真实 OTLP/gRPC collector 跑了六组 A/B,在 base 上复现了 bug、在本分支上确认了修复(包括 prometheus 变体和真实进程中的调用方环境还原),随后指出了两个测试缺口。本提交完全按规定闭合了这两点,并且我静态核实了重构后的测试是真正的 mutation 守护:把对应生产代码回退,两者现在都会失败,而此前都是绿的。生产修复本身已是最小正确形态;本轮零生产逻辑改动。

退一步看:根因已被独立核实,修复是可行的最小方案,测试现在钉住了它,maintainer 已批准这个确切的提交,唯一的红色 CI 项是评审机器人工作流自身的零 job 启动失败。唯一的遗留小项:#8697 的另外两点诉求——不依赖 QWEN_DEBUG_LOG_FILE=1 的告警、以及不让 metric provider 失败中断整个 SDK 启动——值得建一个后续 issue 以免被悄悄丢弃,但不属于本 PR。

予以批准,钉在 reviewed commit 上。✅

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Resolved (re-run, 2026-08-08): the sign-off this deferral asked for has arrived — @wenshao verified the fix end-to-end (six-arm A/B, real bundles, real OTLP/gRPC collector), his two test findings were addressed and mutation-checked in commit ad1a0932, and he approved that head. The re-run triage passed all stages and approved, pinned to the reviewed commit. Original deferral below for the record.


⏸️ Deferring to @wenshao — this PR is a clean, minimal, well-evidenced fix for #8697, but it settles a telemetry-policy question (standard OTEL_*_EXPORTER selectors are ignored while qwen-code starts its SDK, rather than honored), and that surface is maintainer-owned. Two things would help the call:

  1. Sign-off on the "ignore during startup" policy (the only minimal option given the bundle omits the exporter chains — see my Stage 1 note).
  2. Optionally, @qwen-code /verify as a sponsored run to independently confirm the bundled SDK initializes with OTEL_METRICS_EXPORTER=otlp set (the current end-to-end evidence is the author's macOS reproduction).

No code blockers found; CI was still running at review time. Needs a human call on this one.

中文说明

已解决(重跑,2026-08-08): 本次搁置所等待的签核已经给出——@wenshao 端到端验证了修复(六组 A/B、真实 bundle、真实 OTLP/gRPC collector),他提出的两个测试问题已在提交 ad1a0932 中处理并完成 mutation 核对,他本人已批准该 head。重跑的 triage 通过全部阶段并已批准(钉在 reviewed commit 上)。原始搁置说明保留如下。


⏸️ 转交 @wenshao —— 本 PR 是对 #8697 干净、最小且证据充分的修复,但它确定了一个 telemetry 策略问题(qwen-code 启动 SDK 期间忽略标准 OTEL_*_EXPORTER 选择变量,而非尊重它们),该领域由 maintainer 负责。两点有助于决策:

  1. 对"启动期间忽略"策略拍板(鉴于 bundle 不包含 exporter 链,这是唯一的最小选项——见我的 Stage 1 说明)。
  2. 可选:以赞助运行方式触发 @qwen-code /verify,独立确认 bundle SDK 在设置 OTEL_METRICS_EXPORTER=otlp 时能完成初始化(目前端到端证据是作者在 macOS 上的复现)。

未发现代码阻塞项;review 时 CI 仍在运行。这一项需要人工决定。

Qwen Code · qwen3.8-max

@qwen-code-dev-bot

qwen-code-dev-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 2 finishedview run. See this round's report below.

中文说明

AutoFix 第 2 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round: no action needed

No actionable feedback arrived since the last evaluation, so this round makes no changes.

What was checked

  • No new review findings and no inline review comments to triage.
  • No failed or still-red CI checks; every completed check on the current head reports success.
  • The only new issue-level comment is a verification report confirming the fix: a failure-first reproduction with OTEL_METRICS_EXPORTER=otlp fails on the base bundle and succeeds after the fix, and the focused regression test, the full sdk.test.ts suite, build, bundle, typecheck, and lint all pass on commit fa207ae.

Result

No code changes were made. The PR head remains at fa207ae (fix(telemetry): ignore unsupported OTel exporter selectors (#8697)), and no conflict resolution was required.

中文说明

Autofix 评审轮次:无需处理

自上次评估以来没有收到需要处理的反馈,因此本轮不做任何改动。

已检查内容

  • 没有新的评审意见,也没有需要分诊的行内评审评论。
  • 没有失败或持续失败的 CI 检查;当前 head 上所有已完成的检查均为成功。
  • 唯一新增的 issue 级评论是一份验证报告,确认修复有效:按"先复现失败"的方式设置 OTEL_METRICS_EXPORTER=otlp 在 base bundle 上会失败,修复后同样环境运行成功;聚焦回归测试、完整的 sdk.test.ts 套件、build、bundle、typecheck 和 lint 在提交 fa207ae 上全部通过。

结果

未做任何代码改动。PR 的 head 仍为 fa207aefix(telemetry): ignore unsupported OTel exporter selectors (#8697)),也无需解决冲突。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@wenshao

wenshao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Verification report — real bundled CLI against a real OTLP/gRPC collector

Verdict: the fix is correct and I reproduced #8697 and its repair end-to-end. Recommend merge, with one non-blocking follow-up: the new unit test does not actually guard the fix (it passes with the fix reverted).

Harness

Not a mock of the SDK — the actual shipped artifact:

  • Two bundles built from source with the PR's own build steps (npm run build -- --cli-only && npm run bundle): main @ 4ec0371 (merge-base) and PR fix(telemetry): ignore unsupported OTel exporter selectors #8703 @ fa207ae. The bug only exists in the bundle, because esbuild.config.js stubs the @opentelemetry/exporter-* packages when sdk-node imports them.
  • A real OTLP/gRPC collector (@grpc/grpc-js server implementing TraceService / MetricsService / LogsService, decoding with the OTLP protobuf definitions) on 127.0.0.1:4317, logging every span, metric instrument and log record it receives.
  • A real CLI session driven by a mock OpenAI-compatible model server: one run_shell_command tool round plus a held-open second round, so the session outlives the 5 s span-batch tick and the 10 s metric-reader tick — i.e. it behaves like an interactive session, not a 2-second smoke test.
  • Environment as described in OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697: OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_PROTOCOL=grpc, OTEL_TRACES_EXPORTER=otlp, OTEL_LOGS_EXPORTER=otlp, OTEL_METRIC_EXPORT_INTERVAL=10000, QWEN_DEBUG_LOG_FILE=1; settings.json with telemetry.enabled + otlpEndpoint + otlpProtocol: grpc.

Linux (Debian 13, kernel 6.12), Node v22.22.2, npm 10.9.7 — so this also covers the 🐧 Linux row the PR left as N/A.

A/B results

# Bundle OTEL_METRICS_EXPORTER SDK start spans received distinct metric instruments log records
1 main 4ec0371 otlp ERROR 1 (POST only) 0 0
2 main 4ec0371 unset ok 9 15 10
3 PR fa207ae otlp ok 9 15 10
4 PR fa207ae unset ok 9 15 10
5 main 4ec0371 prometheus ERROR 1 0 0
6 PR fa207ae prometheus ok 9 15 10

Arm 1 reproduced the issue's error line verbatim:

[ERROR] [OTEL] Error starting OpenTelemetry SDK: Error: qwen-code bundles @opentelemetry/sdk-node
without @opentelemetry/exporter-metrics-otlp-grpc (env-based exporter selection is unsupported;
configure telemetry via qwen-code settings instead). Attempted to construct: OTLPMetricExporter

before/after

Two things worth flagging for merge priority:

  1. The blast radius is larger than "metrics are lost." In arm 1 the collector received exactly one span — the auto-instrumentation POST — and zero log records. Because telemetryInitialized stays false, every hot-path call gated on isTelemetrySdkInitialized() is skipped, so qwen-code.llm_request, qwen-code.tool, qwen-code.interaction and all GenAI log records disappear too. Only the HTTP/undici instrumentation, registered before the throw, keeps exporting. From the collector's point of view service.name=qwen-code still looks alive, which is exactly why OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697 was hard to spot.
  2. Arms 5/6 show the fix is not otlp-specific. OTEL_METRICS_EXPORTER=prometheus hits the same abort on main (different stub message: @opentelemetry/exporter-prometheus) and is equally repaired by the PR.

Arms 2 and 4 also confirm that OTEL_TRACES_EXPORTER=otlp / OTEL_LOGS_EXPORTER=otlp were already inert: they were set in every arm, and the SDK starts fine as long as OTEL_METRICS_EXPORTER is absent. That matches the code — spanProcessors and logRecordProcessors are always passed as arrays, so getSpanProcessorsFromEnv() and configureLoggerProviderFromEnv() are never reached. Scrubbing those two is defence-in-depth, which I think is the right call.

Caller environment is genuinely restored

Verified in the real process rather than only in a unit test: a child process spawned by the run_shell_command tool after telemetry startup prints all three variables intact, byte-identical to main.

PR #8703 : OTEL_METRICS_EXPORTER=[otlp] OTEL_TRACES_EXPORTER=[otlp] OTEL_LOGS_EXPORTER=[otlp]
main     : OTEL_METRICS_EXPORTER=[otlp] OTEL_TRACES_EXPORTER=[otlp] OTEL_LOGS_EXPORTER=[otlp]

NodeSDK.start() is synchronous, so the delete/restore window contains no await and no spawn — nothing in the process can observe it. This matters for qwen-code specifically, since shell tools, hooks and stdio MCP servers all inherit process.env.

Static checks (PR worktree)

  • vitest run packages/core/src/telemetry/849 passed / 28 files
  • tsc --noEmit -p packages/core → clean; eslint → clean; prettier --check → clean

Findings (non-blocking)

F1 — the new test is a false green. packages/core/src/telemetry/sdk.test.ts:193 passes with and without the fix. I reverted startSdkWithExplicitExporters(sdk) back to sdk.start() and the file still reported 64/64. The reason: the expect(process.env[name]).toBeUndefined() calls live inside the mocked start(), which runs inside initializeTelemetry's try { … } catch (error) { debugLogger.error(…) }. The assertion error is swallowed as an init failure, and startCalled = true was already set on the line above, so the test sails through.

Recording inside the mock and asserting after initializeTelemetry resolves makes it a real guard — verified: it fails on the mutation (expected 'console' to be undefined) and passes again once the fix is restored.

+      const observed: Record<string, string | undefined> = {};
       vi.mocked(NodeSDK.prototype.start).mockImplementationOnce(() => {
         startCalled = true;
         for (const name of Object.keys(exporterEnv)) {
-          expect(process.env[name]).toBeUndefined();
+          observed[name] = process.env[name];
         }
       });

       try {
         await initializeTelemetry(mockConfig);
         expect(startCalled).toBe(true);
+        for (const name of Object.keys(exporterEnv)) {
+          expect(observed[name]).toBeUndefined();
+        }

F2 — the finally restore path is untested. Replacing try { … } finally { … } in startSdkWithExplicitExporters with two plain blocks (no restore when start() throws) also leaves 64/64 green. The finally is load-bearing: if start() ever throws for an unrelated reason, without it the three variables would stay deleted from process.env for the rest of the process, and every child process spawned afterwards would silently inherit the stripped environment. Worth a case that makes start() throw and then asserts the variables are back.

test mutation

Scope notes (not objections)

  • OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697 asked for three things; this PR delivers the first (skip env metric auto-config). The other two — a warning visible without QWEN_DEBUG_LOG_FILE=1, and not letting metric-provider setup abort the rest of SDK startup — still stand. The specific trigger is gone, so I would not block on them, but any other start() failure remains as silent as this one was. Good follow-up issue.
  • configureLoggerProviderFromEnv() runs in the NodeSDK constructor, which is in startTelemetrySdk() — outside this PR's scrub window. It is unreachable today only because logRecordProcessors is always an array. If that ever becomes conditional, the guard as placed would not catch it; a comment at the new NodeSDK({…}) site recording the invariant would help.
  • I also probed the metricReader === undefined path (telemetry.enabled, no endpoint, OTEL_METRICS_EXPORTER=console, 2 s export interval), where env-derived readers were previously the only readers. No observable difference between main and the PR in the bundled CLI — nothing regressed there.
中文完整版

验证报告 —— 真实 bundle CLI 对接真实 OTLP/gRPC collector

结论:修复正确,我端到端复现了 #8697 并验证了修复效果,建议合入。 有一个不阻塞合入的后续项:新增的单元测试实际上并不能守护这个修复(把修复回退掉,它照样通过)。

验证环境

不是对 SDK 打桩,而是真实的发布产物:

  • 用 PR 自己的构建步骤从源码构建了两份 bundlenpm run build -- --cli-only && npm run bundle):main @ 4ec0371(merge-base)与 PR fix(telemetry): ignore unsupported OTel exporter selectors #8703 @ fa207ae。这个 bug 只存在于 bundle 中,因为 esbuild.config.js 会在 sdk-node 引入 @opentelemetry/exporter-* 时把它们替换成会抛错的桩。
  • 一个真实的 OTLP/gRPC collector(用 @grpc/grpc-js 实现 TraceService / MetricsService / LogsService,用 OTLP protobuf 定义解码),监听 127.0.0.1:4317,把收到的每一个 span、metric instrument 和 log record 都记录下来。
  • 一次真实的 CLI 会话,模型侧用 mock OpenAI 兼容服务:先跑一轮 run_shell_command 工具调用,再把第二轮响应挂住,让会话活过 5s 的 span 批量导出周期和 10s 的 metric reader 周期 —— 也就是模拟真实交互会话,而不是 2 秒就退出的冒烟测试。
  • 环境变量按 OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697 描述配置:OTEL_EXPORTER_OTLP_ENDPOINTOTEL_EXPORTER_OTLP_PROTOCOL=grpcOTEL_TRACES_EXPORTER=otlpOTEL_LOGS_EXPORTER=otlpOTEL_METRIC_EXPORT_INTERVAL=10000QWEN_DEBUG_LOG_FILE=1settings.json 配置 telemetry.enabled + otlpEndpoint + otlpProtocol: grpc

Linux(Debian 13,内核 6.12)、Node v22.22.2、npm 10.9.7 —— 因此也补上了 PR 中标为 N/A 的 🐧 Linux 一行。

A/B 结果

# Bundle OTEL_METRICS_EXPORTER SDK 启动 收到 spans 不同 metric 指标数 log records
1 main 4ec0371 otlp ERROR 1(仅 POST 0 0
2 main 4ec0371 未设置 ok 9 15 10
3 PR fa207ae otlp ok 9 15 10
4 PR fa207ae 未设置 ok 9 15 10
5 main 4ec0371 prometheus ERROR 1 0 0
6 PR fa207ae prometheus ok 9 15 10

第 1 组一字不差地复现了 issue 中的报错:

[ERROR] [OTEL] Error starting OpenTelemetry SDK: Error: qwen-code bundles @opentelemetry/sdk-node
without @opentelemetry/exporter-metrics-otlp-grpc (env-based exporter selection is unsupported;
configure telemetry via qwen-code settings instead). Attempted to construct: OTLPMetricExporter

有两点值得在合入优先级上强调:

  1. 影响面比"metrics 丢失"更大。 第 1 组中 collector 只收到了一个 span —— 自动埋点的 POST,log records 为 0。因为 telemetryInitialized 一直是 false,所有以 isTelemetrySdkInitialized() 为前置判断的热路径调用都会被跳过,于是 qwen-code.llm_requestqwen-code.toolqwen-code.interaction 以及全部 GenAI log records 也一并消失。只有在抛错之前就完成注册的 HTTP/undici instrumentation 还在继续导出。从 collector 视角看 service.name=qwen-code 仍然"活着",这正是 OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697 难以被发现的原因。
  2. 第 5/6 组说明修复并不局限于 otlp OTEL_METRICS_EXPORTER=prometheusmain 上触发同样的中断(桩报错换成了 @opentelemetry/exporter-prometheus),也同样被本 PR 修复。

第 2、4 组同时证明 OTEL_TRACES_EXPORTER=otlp / OTEL_LOGS_EXPORTER=otlp 本来就是惰性的:它们在所有组里都设置了,只要不设 OTEL_METRICS_EXPORTER,SDK 就能正常启动。这与代码一致 —— spanProcessorslogRecordProcessors 始终以数组形式传入,因此 getSpanProcessorsFromEnv()configureLoggerProviderFromEnv() 永远不会被走到。把这两个变量一起清掉属于纵深防御,我认为是合理的选择。

调用方环境确实被还原了

不只靠单元测试,而是在真实进程里验证:telemetry 启动之后run_shell_command 工具拉起的子进程打印出的三个变量完好无损,与 main 完全一致。

PR #8703 : OTEL_METRICS_EXPORTER=[otlp] OTEL_TRACES_EXPORTER=[otlp] OTEL_LOGS_EXPORTER=[otlp]
main     : OTEL_METRICS_EXPORTER=[otlp] OTEL_TRACES_EXPORTER=[otlp] OTEL_LOGS_EXPORTER=[otlp]

NodeSDK.start() 是同步的,所以 delete/restore 窗口中不存在任何 awaitspawn,进程内没有任何代码能观察到它。这一点对 qwen-code 尤其重要,因为 shell 工具、hooks 和 stdio MCP server 都会继承 process.env

静态检查(PR worktree)

  • vitest run packages/core/src/telemetry/849 通过 / 28 个文件
  • tsc --noEmit -p packages/core → 干净;eslint → 干净;prettier --check → 干净

发现的问题(不阻塞合入)

F1 —— 新增测试是"假绿"。 packages/core/src/telemetry/sdk.test.ts:193 在有无修复的情况下都通过。我把 startSdkWithExplicitExporters(sdk) 回退为 sdk.start(),该文件仍然报 64/64 全绿。原因是:expect(process.env[name]).toBeUndefined() 写在被 mock 的 start() 内部,而它是在 initializeTelemetrytry { … } catch (error) { debugLogger.error(…) } 里执行的。断言抛出的错误被当作初始化失败吞掉了,而 startCalled = true 在上一行就已经赋值,于是测试一路通过。

改成在 mock 内部记录、在 initializeTelemetry 返回之后再断言,就成了真正的守护 —— 已验证:mutation 下会失败(expected 'console' to be undefined),恢复修复后重新通过。

+      const observed: Record<string, string | undefined> = {};
       vi.mocked(NodeSDK.prototype.start).mockImplementationOnce(() => {
         startCalled = true;
         for (const name of Object.keys(exporterEnv)) {
-          expect(process.env[name]).toBeUndefined();
+          observed[name] = process.env[name];
         }
       });

       try {
         await initializeTelemetry(mockConfig);
         expect(startCalled).toBe(true);
+        for (const name of Object.keys(exporterEnv)) {
+          expect(observed[name]).toBeUndefined();
+        }

F2 —— finally 还原路径没有被测试覆盖。startSdkWithExplicitExporters 里的 try { … } finally { … } 换成两个普通代码块(即 start() 抛错时不还原),同样是 64/64 全绿。这个 finally 是有实际作用的:如果 start() 因为其它原因抛错,没有它,这三个变量就会在整个进程生命周期内一直处于被删除状态,之后拉起的每一个子进程都会静默继承被裁剪过的环境。建议补一个"让 start() 抛错、然后断言变量已恢复"的用例。

范围说明(不是反对意见)

  • OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing #8697 提了三点诉求,本 PR 完成了第一点(跳过基于环境变量的 metric 自动配置)。另外两点 —— 不依赖 QWEN_DEBUG_LOG_FILE=1 也能看到的告警、以及不让 metric provider 初始化失败中断整个 SDK 启动 —— 仍然存在。既然具体触发条件已被消除,我不建议以此阻塞合入;但其它任何 start() 失败依然会像这次一样悄无声息。适合作为后续 issue。
  • configureLoggerProviderFromEnv() 是在 NodeSDK 构造函数里执行的,而构造发生在 startTelemetrySdk() 中,不在本 PR 的清理窗口内。它今天不可达,仅仅是因为 logRecordProcessors 始终是数组。若将来这里变成条件传入,当前位置的防护就覆盖不到;建议在 new NodeSDK({…}) 处加一行注释把这个不变量记下来。
  • 我还探测了 metricReader === undefined 的分支(telemetry.enabled、不配 endpoint、OTEL_METRICS_EXPORTER=console、导出周期 2s),这条路径上环境变量派生的 reader 原本是唯一的 reader。在 bundle CLI 中 main 与 PR 没有可观察到的差异,没有回归。

…enLM#8697)

The env-scrub assertions ran inside the mocked start(), where
initializeTelemetry's init-failure catch swallowed them — the test
passed even with the fix reverted. Record observations during start()
and assert after init resolves, add a throw-path test proving the
finally block restores the caller's environment, and note in sdk-impl
that spanProcessors/logRecordProcessors must stay unconditional arrays
because the sdk-node logs env fallback runs in the NodeSDK constructor,
outside the scrub window around start().
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round — PR #8703

The round's feedback is @wenshao's end-to-end verification report (real bundled CLI against a real OTLP/gRPC collector), which confirms the fix is correct and reproducible, and raises two non-blocking findings plus one suggestion. All three are addressed; both test findings were additionally mutation-verified locally.

Feedback points and dispositions

F1 — the new unit test is a false green (fixed). Verified against the code: the expect(...) calls ran inside the mocked start(), which executes within initializeTelemetry's try/catch, so an assertion failure there was swallowed as an init failure while startCalled = true had already been set. The test now records the observed env values during start() into observedDuringStart and asserts they were undefined after initializeTelemetry resolves (plus the post-start restore assertion in the same loop). A short comment records why the assertions must live outside the mock. Mutation check: reverting startSdkWithExplicitExporters(sdk) to sdk.start() now fails the test with expected undefined, received "console", matching the report's prediction.

F2 — the finally restore path was untested (fixed). Added restores external exporter selectors when sdk.start() throws: it sets the three exporter env vars, makes the mocked start() throw, and asserts telemetry stays uninitialized while all three variables are back to their original values. Mutation check: replacing the try/finally with plain blocks (no restore on throw) now fails this test with received: undefined, while the success-path test still passes — exactly the gap the report described.

Invariant comment at the new NodeSDK site (added). Verified against the @opentelemetry/sdk-node@0.203.0 source first: configureLoggerProviderFromEnv() runs in the constructor (outside the scrub window, which only wraps start()), while getSpanProcessorsFromEnv()/configureMetricProviderFromEnv() run inside start(). The existing comment above ...(metricReader && { metricReader }) in sdk-impl.ts is extended with four lines recording that spanProcessors/logRecordProcessors must stay unconditional arrays for exactly that reason. No code change.

Scope notes — warning without QWEN_DEBUG_LOG_FILE, and not letting metric-provider setup abort SDK startup (deferred, no code change). The reviewer explicitly marked these as not blocking and as follow-up issue material; they are out of scope for this fix. Recording them here so they are visible for a follow-up issue rather than silently dropped.

The remaining report content (A/B reproduction, blast-radius analysis, caller-environment restore verification, metricReader === undefined probe) is positive verification of the existing commit and required no action.

Changes

  • packages/core/src/telemetry/sdk.test.ts — F1 assertion restructure + new throw-path test (tests only).
  • packages/core/src/telemetry/sdk-impl.ts — invariant comment extension (comment only).

No production logic changed this round. Commit: ad1a0932d4 test(telemetry): guard exporter env scrub and its finally restore (#8697).

Verification

Commands actually run (from the repository root unless noted):

  • npx vitest run src/telemetry/sdk.test.ts (in packages/core) — passed, 65 tests (was 64; +1 new throw-path test)
  • Mutation check 1 (F1): temporarily reverted the fix to plain sdk.start() — the restructured test failed as expected (expected undefined, received "console"); fix restored
  • Mutation check 2 (F2): temporarily replaced the try/finally with plain blocks — the new throw-path test failed as expected (received: undefined); fix restored
  • npx vitest run src/telemetry/ (in packages/core) — passed, 28 files / 850 tests (baseline 849)
  • npm run buildpassed
  • npm run typecheckpassed
  • npm run lintpassed
  • npx prettier --check on the two touched files — passed
  • Re-ran src/telemetry/sdk.test.ts after committing — passed, 65 tests; working tree clean

Integration tests were not run: this round changed tests and comments only, with zero production-behavior change; the bundled-CLI A/B behavior was already verified end-to-end by the maintainer's report against the previous commit. npm run generate:settings-schema was not needed (no settings source touched).

中文说明

Autofix 评审轮次 — PR #8703

本轮反馈是 @wenshao 的端到端验证报告(真实 bundle CLI 对接真实 OTLP/gRPC collector)。该报告确认修复正确且可复现,并提出了两个不阻塞合入的问题和一条建议。三者均已处理;两个测试问题还在本地额外做了 mutation 验证。

反馈点及处理

F1 —— 新增单元测试是"假绿"(已修复)。 对照代码核实:expect(...) 调用写在被 mock 的 start() 内部,而它在 initializeTelemetrytry/catch 中执行,因此断言失败会被当作初始化失败吞掉,而 startCalled = true 在此之前已经置位。现在测试改为在 start() 期间把观察到的环境变量值记录到 observedDuringStart,并在 initializeTelemetry 返回之后断言它们为 undefined(同一循环中保留了启动后还原的断言)。并加了一条简短注释说明断言为何必须放在 mock 外部。Mutation 验证:把 startSdkWithExplicitExporters(sdk) 回退为 sdk.start() 后,该测试如报告预测的那样失败(expected undefined, received "console")。

F2 —— finally 还原路径没有测试覆盖(已修复)。 新增用例 restores external exporter selectors when sdk.start() throws:设置三个 exporter 环境变量,让被 mock 的 start() 抛错,然后断言 telemetry 保持未初始化状态、且三个变量都恢复为原始值。Mutation 验证:把 try/finally 换成普通代码块(抛错时不还原)后,该新用例如报告所述失败(received: undefined),而成功路径的测试仍然通过 —— 正是报告指出的缺口。

new NodeSDK 处的不变量注释(已添加)。 先对照 @opentelemetry/sdk-node@0.203.0 源码核实:configureLoggerProviderFromEnv() 在构造函数中执行(位于清理窗口之外,清理窗口只包住 start()),而 getSpanProcessorsFromEnv()/configureMetricProviderFromEnv()start() 内执行。sdk-impl.ts...(metricReader && { metricReader }) 上方的既有注释扩展了四行,正是出于这个原因记录 spanProcessors/logRecordProcessors 必须保持为无条件数组。无代码改动。

范围说明 —— 不依赖 QWEN_DEBUG_LOG_FILE 的告警、以及不让 metric provider 初始化失败中断 SDK 启动(搁置,无代码改动)。 评审人明确标注这些不阻塞合入、适合作为后续 issue;超出本修复的范围。在此记录以便后续建 issue 时可见,而不是被悄悄丢弃。

报告中的其余内容(A/B 复现、影响面分析、调用方环境还原验证、metricReader === undefined 路径探测)是对既有提交的正向验证,无需处理。

改动内容

  • packages/core/src/telemetry/sdk.test.ts —— F1 断言重构 + 新增抛错路径用例(纯测试改动)。
  • packages/core/src/telemetry/sdk-impl.ts —— 不变量注释扩展(纯注释改动)。

本轮没有改动任何生产逻辑。提交:ad1a0932d4 test(telemetry): guard exporter env scrub and its finally restore (#8697)

验证

实际执行的命令(如无特别说明均在仓库根目录执行):

  • npx vitest run src/telemetry/sdk.test.ts(在 packages/core 内)—— 通过,65 个用例(原为 64 个;新增 1 个抛错路径用例)
  • Mutation 验证 1(F1):临时将修复回退为普通 sdk.start() —— 重构后的测试按预期失败expected undefined, received "console");随后恢复修复
  • Mutation 验证 2(F2):临时把 try/finally 换成普通代码块 —— 新增的抛错路径用例按预期失败received: undefined);随后恢复修复
  • npx vitest run src/telemetry/(在 packages/core 内)—— 通过,28 个文件 / 850 个用例(基线 849)
  • npm run build —— 通过
  • npm run typecheck —— 通过
  • npm run lint —— 通过
  • 对两个改动文件执行 npx prettier --check —— 通过
  • 提交后重新运行 src/telemetry/sdk.test.ts —— 通过,65 个用例;工作区干净

未运行集成测试:本轮只改了测试和注释,生产行为零变化;bundle CLI 的 A/B 行为已由评审人的报告在上一个提交上完成端到端验证。未执行 npm run generate:settings-schema(未触及任何 settings 源)。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@wenshao

wenshao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /review

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor
_Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/31243238704)._

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: @qwen-code /review --timeout=360. See workflow logs.

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round: no changes

The only new feedback since the last evaluation is the automated reviewer's own notice that its review run timed out after 10800 seconds. That is an infrastructure event, not a code finding:

  • It reports no defect in this PR, and no maintainer or reviewer requested a change — there is nothing to fix in the diff.
  • The suggested response, re-running the review via @qwen-code /review --timeout=360, is a GitHub comment that re-triggers a workflow. The autofix agent holds no GitHub credentials and does not post comments or start workflows; if the deeper review is still wanted, a maintainer can re-trigger it.
  • All CI checks on the current head completed as SUCCESS or SKIPPED, and there is no base conflict.

The branch is unchanged: fa207ae (ignore unsupported OTel exporter selectors) plus ad1a093 (test guard for the env scrub and its restore).

中文说明

Autofix 审查轮次:无变更

自上次评估以来,唯一的新反馈是自动审查器自身发出的通知,说明其审查运行在 10800 秒后超时。这是一次基础设施事件,而不是代码问题:

  • 它没有报告本 PR 中的任何缺陷,也没有维护者或审查者要求更改——diff 中没有任何需要修复的内容。
  • 建议的应对方式,即通过 @qwen-code /review --timeout=360 重新运行审查,是一条用于重新触发工作流的 GitHub 评论。Autofix 代理不持有任何 GitHub 凭据,不会发表评论,也不会启动工作流;如果仍需要更深入的审查,可由维护者重新触发。
  • 当前 head 上的所有 CI 检查均以 SUCCESS 或 SKIPPED 完成,且不存在与基线分支的冲突。

分支保持不变:fa207ae(忽略不受支持的 OTel exporter 选择器)加上 ad1a093(为 env 变量清理及其恢复添加测试守护)。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@wenshao

wenshao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao
wenshao enabled auto-merge August 8, 2026 22:47
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 85 passed · 0 failed · 85 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:85 通过 · 0 失败 · 85 总计

Verification report

PR 8703 — fix(telemetry): ignore unsupported OTel exporter selectors

Verdict: merge-ready — 85/85 scripted assertions passed (47 A/B + 25 restore-probe + 13 tally), 0 unexpected failures. Verified head: ad1a0932d4e7dda7a737afe8bbfeba4526dc9297 (base 6ddb0307ac739e6b8dd07fb11ba64089541a3e98). Round 1 (no previous report).

中文摘要
  • 结论merge-ready。85/85 脚本化断言全部通过,无意外失败,无阻塞性发现。
  • A/B 结论(见 "Central claim" 表与 01-ab-base-vs-head-bundle-cells.png):bundled CLI 在 OTEL_METRICS_EXPORTER=otlp(以及兄弟值 prometheus)下,base 构建 telemetry 静默初始化失败(telemetry 记录 0 条、debug 日志出现 stub 抛错、进程仍 exit 0);head 构建完全免疫(20 条记录、无错误),且三个 OTEL_*_EXPORTER 变量在启动后被逐字节恢复(真实路径子进程探针可见)。
  • 测试有效性:mutation 矩阵(02-mutation-matrix-all-mutants-red.png)证明两个新测试分别钉住 scrub 与 throw 后恢复两个性质;M1(= base 代码)使新测试按预期变红,即新测试对 base 行为非空转。
  • 未覆盖:per-commit 归因(shallow checkout)、OTLP wire 路径(用 outfile 代替,机制与 exporter 无关)、Windows/macOS、真实路径 throw 恢复(单测覆盖)、OTEL_SDK_DISABLED 相邻变量、对当前 main 的 trial merge(无网络)。

Central claim + A/B

Central claim: while the bundled CLI starts its telemetry SDK, @opentelemetry/sdk-node must not act on the OTEL_{TRACES,LOGS,METRICS}_EXPORTER selector variables, because sdk-node's env auto-configuration constructs exporter packages the bundle deliberately stubs out — the constructor throws inside start(), telemetry stays uninitialized, and metrics are silently lost.

Mechanism confirmed from installed @opentelemetry/sdk-node@0.203.0 source: NodeSDK.start() calls configureMetricProviderFromEnv() unconditionally and constructs the env exporter before the explicit-metricReader check, so the stub throws even when qwen-code supplies its own reader. The logs env fallback runs in the constructor (suppressed by qwen-code's always-passed logRecordProcessors: []), traces in start() (suppressed by spanProcessors: []) — so metrics-in-start() is the only live env door, exactly what the PR's scrub window wraps.

Harness: real bundled CLI (dist/cli.js), fake OpenAI loopback server driving a scripted tool turn (run_shell_command writes an env probe), telemetry in outfile mode, QWEN_DEBUG_LOG_FILE=1 for the init-failure oracle. Control build = same tree/deps/pipeline with only sdk.ts+sdk-impl.ts reverted to HEAD^1 (PR touches no lockfile; single-tree control avoids workspace-symlink confounds; head chunks grep-verified to contain startSdkWithExplicitExporters, base chunks verified not to, stub present in both).

Cell build selector env telemetry records stub throw in debug log env probe (child of CLI, post-init)
C1 base OTEL_METRICS_EXPORTER=otlp 0 Attempted to construct: OTLPMetricExporter + init-failure catch otlp (never scrubbed)
C2 head OTEL_METRICS_EXPORTER=otlp 20 none otlp restored
C3 head all three =otlp 20 none all three restored
C4 base neutral 20 none
C5 head neutral 20 none
C6 base =none 20 none — (none short-circuits before construction)
C7 base =prometheus 0 Attempted to construct: PrometheusExporter
C8 head =prometheus 20 none prometheus restored

All cells exit 0 — the base failure is silent at the process level, exactly the reported symptom. Witness: evidence/01-ab-base-vs-head-bundle-cells.png (re-run capture; its records=40 rows are the appended second run over the same cell dirs — the authoritative first-run counts are the 20/0 above; the 0-vs->0 flip and all 47 assertions held in both runs).

Secondary claim 1 — exact env restore. Real-path probe through the real (dev-tree) initializeTelemetry with real sdk-node, 5 combos × {absent, value, empty string, comma list}: 25/25 — absent stays absent (delete, not undefined-string), empty string stays empty string, values byte-for-byte. Witness: evidence/03-restore-probe-real-path-matrix.png.

Secondary claim 2 — the new tests are load-bearing. Mutation matrix on sdk.ts (evidence/02-mutation-matrix-all-mutants-red.png, raw tails in mutant-outputs/):

Mutant Result killed by
M0 head unmutated 65/65 green
M1 scrub-delete (≡ base code) 1 red ignores external exporter selectors…expected 'console' to be undefined at the during-start observation
M2 restore-delete 2 red both new tests — expected undefined to be 'console' at post-init/post-throw restore
M3 scrub sets '' instead of delete 1 red during-start observation — expected '' to be undefined
M4 positive control (telemetryInitialized = false) 9 red pre-existing lifecycle tests (suite can go red)

Partitioning nuance (completeness, not a defect): M1 kills only test 1 and M2 kills both — test 1 pins the scrub, test 2 pins the finally-restore on throw; together they cover the property, and M1 being exactly base code also proves the new test fails against the pre-fix source for the intended reason.

Targeted gate: packages/core src/telemetry at head — 28 files, 850/850 pass (evidence/04-telemetry-suite-gate-850.png).

Reviewer Test Plan walk: (1) bundle built ✓ (via npm run bundle on the pre-built tree; esbuild bundles from TS source so the --cli-only nuance is immaterial); (2) hostile OTEL_METRICS_EXPORTER=otlp set ✓ — endpoint shape substituted outfile for gRPC OTLP (see Not covered; the throw is exporter-agnostic and C1/C7 prove it fires with an explicit reader present, which is the unconditional construction the gRPC repro relies on); (3) telemetry started through the bundled CLI's real bootstrap path ✓; (4) init success + caller env unchanged after startup ✓ (records/spans + child-process env probe). The PR's stated before/after strings reproduced verbatim in C1's debug log.

Findings

F1 (informational, pre-existing, non-blocking): the telemetry init failure is observable only in the QWEN_DEBUG_LOG_FILE debug log, which is off by default — with default settings the base failure is completely invisible (exit 0, no console output, empty telemetry file). The catch { debugLogger.error(...) } in initializeTelemetry predates this PR and the PR removes the only bundled trigger rather than the observability gap; no code change requested.

F2 (positive): the sibling door OTEL_METRICS_EXPORTER=prometheus (also a stubbed package) was measured broken on base (C7) and is closed by the same scrub on head (C8) — the fix generalizes beyond the reported otlp shape.

Not covered

  • Per-commit attribution: metadata lists 2 commits; the depth-2 checkout makes only 1 of them locally reachable (rev-list silently returned 1; --is-shallow-repository=true). The aggregate HEAD^1..HEAD diff is what was verified.
  • OTLP wire path: A/B used outfile telemetry, not a gRPC/HTTP collector. The failure mechanism (unconditional env-exporter construction in start()) is exporter-agnostic — C1/C7 threw with an explicit FileMetricExporter reader present — but no live collector connection was exercised.
  • Real-path throw-restore: the finally-restore-on-throw was verified via M2 + code reading; constructing a genuine throw through the real (non-bundled) start() was not done.
  • Platforms: Linux container only (PR author tested macOS; Windows untested by either).
  • OTEL_SDK_DISABLED sibling: not scrubbed; on head it makes start() a no-op while qwen-code still marks itself initialized (silent no-telemetry). Pre-existing, different mechanism, out of this PR's scope; not exercised.
  • Base staleness / trial merge: snapshot baseRefOid=4ec0371e… drifted from local HEAD^1=6ddb0307ac… (main moved); the local merge ref is the verified baseline. No trial merge into current main (no network in this environment).
  • Repo-wide gates: full core/repo test, lint, typecheck not re-run here; the PR's own CI covers them. The telemetry directory suite is the targeted gate cited above.

Methodology

Environment: node:22-bookworm CI container, refs/pull/8703/merge checkout (depth 2), npm ci+npm run build pre-run. Head bundle built with npm run bundle; base control built by reverting only packages/core/src/telemetry/{sdk.ts,sdk-impl.ts} to HEAD^1 and re-bundling (snapshots in bundles/), then restoring (tree left clean, git status empty). The A/B harness (ab-harness.mjs + fake-openai.mjs, ported from integration-tests/fake-openai-server.ts) spawns the real bundled CLI per cell with a sandboxed HOME/QWEN_HOME, a loopback fake OpenAI server issuing a scripted tool turn, and asserts on telemetry.log record counts, debug-log stub errors, child-process env probes, and exit codes. The restore probe (restore-probe.mjs/-child.mjs) drives the dev-tree dist through real initializeTelemetry. Mutation matrix (mutation-matrix.sh) applies one-point mutations, runs sdk.test.ts, restores, and verifies cleanliness per mutant. Raw logs under cells/, mutant-outputs/; summaries in ab-summary.json, restore-probe.json; counts aggregated by tally.mjs into assertions.json (85/85). Captures via scripts/verify-capture.mjs.

Evidence images

01-ab-base-vs-head-bundle-cells

02-mutation-matrix-all-mutants-red

03-restore-probe-real-path-matrix

04-telemetry-suite-gate-850

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round: no action taken

No changes were made to PR #8703 in this round.

Feedback triage:

  • Reviews: one new review since the last evaluation — an approval with no comments, so there is nothing to address.
  • Inline comments: none.
  • Issue-level comments: the only new comment is the automated verification bot's own "sandboxed verification is running" status marker, which is a workflow status update rather than feedback.
  • Failed / still-red checks: none — all check runs completed as SUCCESS or SKIPPED.

Since there is no actionable feedback, no code changes are needed and no commit was created. The branch remains at its current head commit.

中文说明

Autofix 审查轮次:未采取任何操作

本轮未对 PR #8703 做任何修改。

反馈分类结果:

  • 审查(Reviews):自上次评估以来仅有一条新审查——一个不带任何评论的批准(approval),因此没有需要处理的内容。
  • 行内评论(Inline comments):无。
  • Issue 级评论:唯一的新评论是自动化验证机器人自己发布的"沙箱验证正在运行"状态标记,属于工作流状态更新,而非反馈意见。
  • 失败 / 持续失败的检查:无——所有检查运行均以 SUCCESS 或 SKIPPED 完成。

由于没有可处理的反馈,无需修改代码,也未创建任何提交。分支保持在当前 head 提交不变。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@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.

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Aug 8, 2026
Merged via the queue into QwenLM:main with commit 1774771 Aug 8, 2026
36 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OTEL_METRICS_EXPORTER=otlp in environment silently disables metrics export while traces keep flowing

4 participants