feat(telemetry): wire recordApiRequestBreakdown into endLLMRequestSpan (Phase 4c)#5904
Merged
Merged
Conversation
…n (Phase 4c) Activate the pre-existing recordApiRequestBreakdown histogram metric by calling it from endLLMRequestSpan. Each LLM request now records up to 3 phase durations: - REQUEST_PREPARATION (requestSetupMs) - NETWORK_LATENCY (ttftMs) - RESPONSE_PROCESSING (duration - ttftMs) The metric is gated on the optional config field in LLMRequestMetadata, so callers without Config (tests, legacy paths) silently skip recording. Changes: - Add config?: Config to LLMRequestMetadata interface - Import ApiRequestPhase + recordApiRequestBreakdown in session-tracing - Insert breakdown recording logic after setAttributes, before status - Pass config: this.config in all 5 LoggingContentGenerator call sites - Add 5 unit tests covering all phases, no-config skip, no-ttft skip, zero-sampling skip, and idempotency Closes #3731 (Phase 4c)
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required headings present, bilingual, test plan with exact commands. On direction: this is straightforward plumbing — the On approach: the scope is tight. Adding an optional Re-running full review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必需标题齐全，双语，测试计划包含精确命令。 方向：这是纯接线工作—— 方案：范围紧凑。在 重新运行完整审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewReviewed the diff against an independent proposal (import A few observations that confirm the design is sound:
No critical blockers. No AGENTS.md violations. Unit TestsAll pass on the PR branch:
Real-Scenario Testing (tmux)This is a non-user-visible metrics plumbing change, so the smoke test confirms the CLI still starts and responds without crashing. Before (installed build — qwen 0.19.2)After (this PR via dev build)CLI starts, makes an API call, and returns a response cleanly. No crashes, no unexpected errors. The warnings about QWEN_HOME are environment-specific, not related to this PR. 中文说明代码审查将 diff 与独立方案对比（导入
无关键阻塞问题，无 AGENTS.md 违规。 单元测试全部通过：session-tracing 125/125 ✓，metrics 40/40 ✓，loggingContentGenerator 58/58 ✓，tsc --noEmit 无错误 ✓。 真实场景测试（tmux）非用户可见的指标接线变更，烟雾测试确认 CLI 仍能正常启动和响应。CLI 启动、发起 API 调用、返回响应均正常，无崩溃。 — Qwen Code · qwen3.7-max |
Collaborator
|
This is clean plumbing work that does exactly what it says. The The test suite is solid — 5 new tests covering all branches (full data, config absent, partial timing, zero sampling, idempotency). The CLI smoke test confirms no regressions. If I had to maintain this in six months, I'd be glad the wiring was completed in a focused, minimal PR rather than bundled with something else. Ships it. 中文说明这是干净的接线工作，完全如其所述。 测试套件扎实——5 个新测试覆盖所有分支（完整数据、config 缺失、部分计时、零采样、幂等性）。CLI 烟雾测试确认无回归。 这是一个聚焦、最小化的 PR，适合合入。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jun 26, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
✅ Real-binary verification (maintainer) — Phase 4c breakdown metricVerdict: Verified — safe to merge. I built the real Head 1. PR's claimed checks — reproduced ✅
2. Mutation test — are the new tests real guards? (mostly ✅)Reverted only the two production files to base (
3. Real end-to-end A/B in tmux (the headline) 🎯Real binary, headless, in a FIXED (this PR) — BASE (pre-fix, two prod files reverted + core rebuilt) — metric absent: Why this A/B is trustworthy:
Notes (non-blocking)
LGTM. 👍 🇨🇳 中文版（点击展开）✅ 维护者真实二进制验证 —— Phase 4c 阶段耗时直方图结论：已验证，可以合并。 我从 PR head 构建了真实的 Head 1. 复现 PR 声称的检查 ✅
2. 变异测试 —— 新测试是不是真守卫？（大部分 ✅）只把两个生产文件回退到 base（
3. tmux 内真实端到端 A/B（重点）🎯真实二进制、headless、跑在 修复后（本 PR）—— 修复前（回退两个生产文件 + 重建 core）——指标不存在（ 为什么这个 A/B 可信：
备注（不阻塞）
LGTM。 👍 |
- Wrap Phase 4c breakdown recording in isolated try/catch so metric failures cannot affect span status/lifecycle - Gate breakdown recording on metadata.success to avoid polluting latency dashboards with failed-request timings during outages - Add test for success=false skip path
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No high-confidence review issues. Downgraded from Approve to Comment: CI still running.
All 4 low-confidence findings (inconsistent zero-value guards, variable naming, missing test for ttftMs-only path, no success gate on Phase 4c) are available in the terminal output for human review.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jun 26, 2026
Collaborator
|
@qwen-code /triage |
Collaborator
Code ReviewIndependent proposal (before reading diff): To wire Actual diff: matches this proposal exactly. Three files, +170/-0:
No issues found. The code is focused — every line serves the stated goal, no drive-by refactors, no unnecessary abstraction. The Reuse check: Test ResultsAll tests run from the PR branch in an isolated worktree:
Real-Scenario TestingThis is a non-user-visible metrics plumbing change — no CLI output or TUI behavior is affected. The The maintainer (@wenshao) has already performed comprehensive real-binary A/B testing with a fake OpenAI server and local-file telemetry export, confirming the histogram is emitted with correct per-phase durations on the PR build and absent on the base build. That verification covers the 中文说明代码审查独立方案（阅读 diff 前）：要在 LLM 请求 span 生命周期中接入 实际 diff：完全匹配此方案。三个文件，+170/-0：
未发现问题。 代码聚焦——每一行都服务于声明的目标，无顺手重构，无不必要的抽象。Phase 4c 块周围的 复用检查： 测试结果所有测试在隔离 worktree 中的 PR 分支上运行：
真实场景测试这是非用户可见的指标接线改动——CLI 输出或 TUI 行为不受影响。 维护者（@wenshao）已使用 fake OpenAI 服务和本地文件 telemetry 导出进行了全面的真实二进制 A/B 测试，确认直方图在 PR 构建中以正确的阶段耗时发射，在 base 构建中完全不存在。该验证覆盖了单测未直接涉及的 — Qwen Code · qwen3.7-max |
Collaborator
|
This is exactly what a Phase 4c completion should look like — the infrastructure was built and approved in earlier phases, this PR just flips the switch by wiring the callers. The diff is 170 lines of focused plumbing across 3 files, every line earns its place. The Phase 4c block sits in the right spot (after Tests are genuine guards — 4 of 6 fail under production-file revert (the maintainer confirmed this via mutation testing). The idempotency test validates the existing The maintainer's real-binary A/B test is the clincher: the histogram fires with correct per-phase durations that track injected delays, and is absent on the base build. That covers the Nothing to cut, nothing to add. Ships it. ✅ 中文说明这正是 Phase 4c 收尾该有的样子——基础设施在早期阶段已构建并获批，本 PR 只是通过连接调用方来开启功能。diff 是 3 个文件中 170 行的聚焦接线，每一行都有存在的理由。 Phase 4c 块位于正确的位置（ 测试是真正的守卫——维护者通过变异测试确认，6 个中有 4 个在生产文件回退下会失败。幂等性测试验证了已有的 维护者的真实二进制 A/B 测试是决定性证据：直方图以正确的阶段耗时发射，与注入的延迟吻合，在 base 构建中完全不存在。这覆盖了单测未直接涉及的 无需删减，无需添加。可以合并。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jun 26, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Activates the pre-existing
recordApiRequestBreakdownhistogram metric by calling it fromendLLMRequestSpan. Each LLM request now records up to 3 phase durations as separate histogram data points: REQUEST_PREPARATION (retry/setup overhead), NETWORK_LATENCY (time to first token), and RESPONSE_PROCESSING (output streaming time).Why it's needed
Phase 4c of the telemetry LLM request timing design (#3731). The infrastructure (
recordApiRequestBreakdownfunction,ApiRequestPhaseenum,apiRequestBreakdownHistogram) was implemented in earlier phases but had zero production callers. This PR completes the wiring so operators can analyze where LLM request latency is spent via theqwen-code.api.request.breakdownmetric.Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/telemetry/session-tracing.test.ts— all 125 tests pass including 5 new Phase 4c tests.cd packages/core && npx vitest run src/telemetry/metrics.test.ts— 40 tests pass.cd packages/core && npx vitest run src/core/loggingContentGenerator/loggingContentGenerator.test.ts— 58 tests pass (existing assertions unchanged).cd packages/core && npx tsc --noEmit— no errors in modified files.Evidence (Before & After)
N/A — non-user-visible metrics plumbing change.
Tested on
Environment (optional)
Unit tests only.
Risk & Scope
recordApiRequestBreakdownthrows unexpectedly (extremely unlikely given internal guards), span status won't be set — but span.end() still runs so no leak.config?field onLLMRequestMetadatais optional.Linked Issues
Closes #3731 (Phase 4c)
中文说明
本 PR 做了什么
将已有的
recordApiRequestBreakdown直方图指标接入endLLMRequestSpan，使每次 LLM 请求按阶段记录耗时分布：REQUEST_PREPARATION（重试/准备开销）、NETWORK_LATENCY（首 token 时间）、RESPONSE_PROCESSING（输出流式处理时间）。为什么需要
这是 telemetry LLM 请求计时设计的 Phase 4c（#3731）。基础设施已在早期阶段实现但无生产调用方。本 PR 完成最后的接线，使运维人员能通过
qwen-code.api.request.breakdown指标分析 LLM 请求延迟的时间分布。风险与范围
recordApiRequestBreakdown意外抛出异常，span status 不会设置——但 span.end() 仍然执行，不会泄漏。LLMRequestMetadata上新增的config?字段是可选的。