feat(telemetry): expand daemon telemetry route coverage by doudouOUC · Pull Request #4682 · QwenLM/qwen-code

doudouOUC · 2026-06-01T14:12:27Z

Summary

扩展 resolveDaemonTelemetryRoute 覆盖所有 daemon 写路由（除 heartbeat），新增 recap、btw、model、shell、detach、approval-mode、metadata、sessions/delete、workspace/init、workspace MCP 路由的 telemetry span
修复尾部斜杠不匹配问题：normalize req.path 后再做 regex 匹配，避免 /session/abc/prompt/ 这类请求静默丢失 span
修复 workspace sessions regex .+ → [^/]+，防止跨路径段过度匹配

Test plan

启动 daemon，触发 recap/btw/model 等路由，确认 ARMS 中出现对应 qwen-code.daemon.request span
发送带尾部斜杠的请求（如 POST /session/:id/prompt/），确认 span 正常生成
确认 heartbeat 路由不产生 span

🤖 Generated with Qwen Code

…ing-slash handling - Add telemetry spans for previously uncovered routes: recap, btw, model, shell, detach, approval-mode, metadata (PATCH), sessions/delete, workspace/init, and workspace MCP routes (restart, add, delete) - Fix trailing-slash mismatch: normalize req.path before matching so requests like `/session/abc/prompt/` produce spans (Express routes them but the old regex missed them) - Fix workspace sessions regex: `.+` → `[^/]+` to prevent cross-segment over-matching 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

github-actions · 2026-06-01T14:15:17Z

📋 Review Summary

This PR expands daemon telemetry route coverage to include additional daemon write routes and fixes two matching issues: trailing slash handling and workspace sessions regex over-matching. The changes are focused and well-motivated, improving observability coverage while fixing edge cases that could cause silent telemetry data loss.

🔍 General Feedback

Good incremental improvement to telemetry coverage — the expanded route list aligns with the daemon's actual API surface
The trailing slash normalization fix addresses a real edge case that could cause silent span loss
The regex fix for workspace sessions (.* → [^/]+) is a good security/hardening improvement
Code style is consistent with surrounding code
The PR description clearly states the changes and test plan

🎯 Specific Feedback

🟢 Medium

File: packages/cli/src/serve/server.ts:285 - The path normalization req.path.replace(/\/$/, '') || '/' handles trailing slashes, but consider extracting this into a named helper function for clarity and reusability. A comment explaining why trailing slashes cause matching issues would help future maintainers understand the intent.
File: packages/cli/src/serve/server.ts:293 - The session action regex now includes many actions (load|resume|prompt|cancel|recap|btw|model|shell|detach|approval-mode). Consider whether this list should be maintained as a constant array that's joined into the regex, making it easier to see at a glance which actions are covered and simplifying future additions.
File: packages/cli/src/serve/server.ts:303-308 - The sessionMetadata route uses PATCH method, which is correct for partial updates. However, this is the only PATCH route in the function. Consider adding a comment or grouping it with other metadata-related routes if more are expected in the future.

🔵 Low

File: packages/cli/src/serve/server.ts:289-291 - The /sessions/delete route is added as a POST route. This follows REST conventions, but consider adding a comment explaining why this is plural (/sessions/delete) vs. singular (/session/:id) for consistency documentation.
File: packages/cli/src/serve/server.ts:336 - The workspace sessions regex test now uses [^/]+ instead of .+. Consider adding a brief comment explaining this prevents cross-segment matching (e.g., /workspace/foo/bar/sessions shouldn't match when foo is the intended workspace ID).
File: packages/cli/src/serve/server.ts:342-352 - The MCP-related routes (/workspace/mcp/:server/restart, /workspace/mcp/servers, /workspace/mcp/servers/:name) are well-covered. Consider grouping these with a comment block to indicate they're all MCP server management routes for easier navigation.

✅ Highlights

Excellent fix for the trailing slash matching issue — this is the kind of edge case that's easy to miss and causes production debugging headaches
The regex fix from .+ to [^/]+ for workspace sessions shows good attention to security/hardening details
Good test plan that covers the main scenarios: new routes, trailing slashes, and heartbeat exclusion
The changes maintain backward compatibility while expanding coverage

Copilot

Pull request overview

This PR extends daemon request telemetry route resolution so more mutation endpoints emit qwen-code.daemon.request spans, and it normalizes request paths to avoid silently missing spans due to a trailing slash.

Changes:

Normalize req.path (strip trailing /) before regex matching so routes like POST /session/:id/prompt/ still produce telemetry.
Expand resolveDaemonTelemetryRoute to include additional session/workspace routes (e.g. recap/btw/model/shell/detach/approval-mode, metadata, sessions/delete, workspace init, workspace MCP routes).
Tighten workspace sessions regex from .+ to [^/]+ to avoid cross-segment overmatching.

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

Add telemetry spans for device-flow auth and tool enable routes that were missed in the initial expansion. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

wenshao

No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review

chiga0

Code Review Overview (AI Generated)

PR: #4682 feat(telemetry): expand daemon telemetry route coverage
Type: Feature (telemetry expansion) + Bug Fix (trailing slash + regex tightening)
Change size: +52/-11 across 1 file

Multi-Round Review (Rounds 0-6): Clean — 0 findings

Round 0 (Design): Correct approach — pure additive route expansion in resolveDaemonTelemetryRoute, trailing-slash normalization, and regex tightening. All three stated goals are addressed.

Round 1 (Architecture): Single function modification, clean pattern. heartbeat correctly excluded per PR description.

Round 2 (Robustness):

req.path.replace(/\/$/, '') || '/' — correctly handles trailing slash AND root path edge case (empty string fallback to /)
All new regex patterns use [^/]+ for ID segments — no cross-segment over-matching
sessionAction regex correctly extended with recap|btw|model|shell|detach|approval-mode
sessionMetadata PATCH route, mcpRestart/mcpDelete/toolEnable/deviceFlowDelete all correctly capture and use IDs

Round 3-4 (CI/Security, Performance): N/A — no CI changes, replace() on short path string is trivial cost.

Round 5 (Telemetry): All new routes follow established patterns, sessionId/permissionRequestId correctly extracted, no attribute misassignment.

Round 6 (Undirected): Verified sessions (plural) vs session (singular) distinction intentional, :server/:name/:id placeholders consistent, no dead code paths.

LGTM!

This review was generated by QoderWork AI

Co-authored-by: Shreya <shreyakeshive@google.com>

Copilot AI review requested due to automatic review settings June 1, 2026 14:12

Copilot started reviewing on behalf of doudouOUC June 1, 2026 14:12 View session

Copilot AI reviewed Jun 1, 2026

View reviewed changes

Comment thread packages/cli/src/serve/server.ts

Comment thread packages/cli/src/serve/server.ts

feat(telemetry): add missing workspace auth and tools routes

d34f8b5

Add telemetry spans for device-flow auth and tool enable routes that were missed in the initial expansion. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

doudouOUC requested review from chiga0 and wenshao June 1, 2026 16:08

wenshao approved these changes Jun 2, 2026

View reviewed changes

chiga0 approved these changes Jun 2, 2026

View reviewed changes

doudouOUC merged commit ab400eb into daemon_mode_b_main Jun 2, 2026
6 checks passed

doudouOUC deleted the feat/daemon_local_develop branch June 2, 2026 02:40

doudouOUC mentioned this pull request Jun 4, 2026

feat(daemon): merge daemon-mode feature batch into main #4490

Merged

xaelistic pushed a commit to xaelistic/qwen-code that referenced this pull request Jun 7, 2026

Add drawer for active files in IDE mode (QwenLM#4682)

0abfe45

Co-authored-by: Shreya <shreyakeshive@google.com>

doudouOUC mentioned this pull request Jun 12, 2026

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

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(telemetry): expand daemon telemetry route coverage#4682

feat(telemetry): expand daemon telemetry route coverage#4682
doudouOUC merged 2 commits into
daemon_mode_b_mainfrom
feat/daemon_local_develop

doudouOUC commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

wenshao left a comment

Uh oh!

chiga0 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

doudouOUC commented Jun 1, 2026

Summary

Test plan

Uh oh!

github-actions Bot commented Jun 1, 2026

📋 Review Summary

🔍 General Feedback

🎯 Specific Feedback

🟢 Medium

🔵 Low

✅ Highlights

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

chiga0 left a comment

Choose a reason for hiding this comment

Code Review Overview (AI Generated)

Multi-Round Review (Rounds 0-6): Clean — 0 findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants