refactor(telemetry): remove dead useCollector setting and unreachable TelemetryTarget.QWEN#4061
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Removes unused/invalid telemetry configuration surface area from the core and CLI by deleting the dead useCollector setting and the unreachable TelemetryTarget.QWEN enum member, and then updating tests + docs accordingly.
Changes:
- Removed
TelemetrySettings.useCollectorplumbing (config resolution, Config API, tests, docs). - Removed
TelemetryTarget.QWENfrom the exported enum (was never accepted by config parsing). - Updated related unit tests and documentation tables to match the reduced configuration surface.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/telemetry/sdk.test.ts | Removes useCollector-related mocking and a now-obsolete test case. |
| packages/core/src/telemetry/index.ts | Removes the exported TelemetryTarget.QWEN enum member. |
| packages/core/src/telemetry/config.ts | Stops resolving/returning useCollector from env/settings. |
| packages/core/src/telemetry/config.test.ts | Updates expectations to no longer include useCollector. |
| packages/core/src/config/config.ts | Removes useCollector from TelemetrySettings and drops the getter. |
| packages/core/src/config/config.test.ts | Removes constructor/getter tests for useCollector. |
| packages/cli/src/config/config.test.ts | Removes env-var precedence test for QWEN_TELEMETRY_USE_COLLECTOR. |
| docs/users/configuration/settings.md | Removes telemetry.useCollector and QWEN_TELEMETRY_USE_COLLECTOR documentation rows. |
| docs/developers/development/telemetry.md | Removes useCollector from developer-facing telemetry configuration docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… TelemetryTarget.QWEN useCollector was plumbed through config (interface, constructor, getter, env var resolution) but never consumed by the telemetry SDK — the setting had no runtime effect. TelemetryTarget.QWEN existed in the enum but parseTelemetryTargetValue() only accepted 'local' and 'gcp', making 'qwen' unreachable (it would throw FatalConfigError). Remove both dead code paths along with their tests and documentation. Part of #3731
3422364 to
a2d4d4c
Compare
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. |
wenshao
approved these changes
May 11, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — deepseek-v4-pro via Qwen Code /review
wenshao
approved these changes
May 11, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. Clean removal of dead code — useCollector telemetry setting and unreachable TelemetryTarget.QWEN enum value. All references (interface fields, getter methods, env var parsing, tests, docs) consistently removed with no dangling imports or stale references. LGTM! ✅ — deepseek-v4-pro via Qwen Code /review
BZ-D
added a commit
that referenced
this pull request
May 12, 2026
Conflicts resolved: - docs/users/configuration/settings.md — main removed the QWEN_TELEMETRY_USE_COLLECTOR row in #4061. Took main's table and re-inserted the FORCE_HYPERLINK / QWEN_DISABLE_HYPERLINKS rows after NO_COLOR. - packages/cli/src/ui/utils/TableRenderer.test.tsx — both sides added a new describe block in the same spot (`OSC 8 markdown links in cells` on this branch, `horizontal/vertical mode threshold` on main). Kept both, OSC 8 describe first, then h/v describe. Auto-merge fix: - packages/channels/base/src/index.ts — git auto-merge dropped main's `resolvePath` re-export (added in #4045) even though this branch never touched the file, breaking the channel config-utils import. Reset to main's content.
wenshao
pushed a commit
that referenced
this pull request
May 13, 2026
… and CLI flags (#4066) * docs(telemetry): align config and docs semantics for target, outfile, and CLI flags - Remove stale warning note "This feature requires corresponding code changes" — the OTLP implementation is now complete (#3779, #4061) - Clarify that `target` is an informational destination label and does not control exporter routing; `otlpEndpoint` or `outfile` must be set to configure where data is sent - Mark `--telemetry-target` CLI flag as deprecated in the configuration table to match the deprecateOption() call in cli/src/config/config.ts - Fix `outfile` / `QWEN_TELEMETRY_OUTFILE` descriptions: remove the incorrect "when target is local" qualifier — outfile overrides OTLP export regardless of the target value - Simplify the file-based output example by removing the now-redundant `"target": "local"` and `"otlpEndpoint": ""` fields Closes the "Align telemetry config and docs semantics for target, useCollector, otlpEndpoint, otlpProtocol, and outfile" checklist item in #3731. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(telemetry): address Copilot review comments on outfile and target descriptions - Fix outfile table row in telemetry.md: "overrides `otlpEndpoint`" → "overrides OTLP export" (outfile disables all OTLP exporting, not just the base endpoint) - Use fully-qualified setting names (`telemetry.otlpEndpoint`, `telemetry.outfile`) in the target description in settings.md for consistency with the rest of the table 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(telemetry): update QWEN_TELEMETRY_TARGET env var description and add outfile note - Align QWEN_TELEMETRY_TARGET env var description with the updated telemetry.target setting semantics (informational label, not routing) - Add a note after the file-based output example clarifying that outfile automatically disables OTLP export 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
TaimoorSiddiquiOfficial
pushed a commit
to TaimoorSiddiquiOfficial/HopCode
that referenced
this pull request
May 13, 2026
… and CLI flags (QwenLM#4066) * docs(telemetry): align config and docs semantics for target, outfile, and CLI flags - Remove stale warning note "This feature requires corresponding code changes" — the OTLP implementation is now complete (QwenLM#3779, QwenLM#4061) - Clarify that `target` is an informational destination label and does not control exporter routing; `otlpEndpoint` or `outfile` must be set to configure where data is sent - Mark `--telemetry-target` CLI flag as deprecated in the configuration table to match the deprecateOption() call in cli/src/config/config.ts - Fix `outfile` / `QWEN_TELEMETRY_OUTFILE` descriptions: remove the incorrect "when target is local" qualifier — outfile overrides OTLP export regardless of the target value - Simplify the file-based output example by removing the now-redundant `"target": "local"` and `"otlpEndpoint": ""` fields Closes the "Align telemetry config and docs semantics for target, useCollector, otlpEndpoint, otlpProtocol, and outfile" checklist item in QwenLM#3731. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(telemetry): address Copilot review comments on outfile and target descriptions - Fix outfile table row in telemetry.md: "overrides `otlpEndpoint`" → "overrides OTLP export" (outfile disables all OTLP exporting, not just the base endpoint) - Use fully-qualified setting names (`telemetry.otlpEndpoint`, `telemetry.outfile`) in the target description in settings.md for consistency with the rest of the table 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(telemetry): update QWEN_TELEMETRY_TARGET env var description and add outfile note - Align QWEN_TELEMETRY_TARGET env var description with the updated telemetry.target setting semantics (informational label, not routing) - Add a note after the file-based output example clarifying that outfile automatically disables OTLP export 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
wenshao
pushed a commit
that referenced
this pull request
May 17, 2026
… TelemetryTarget.QWEN (#4061) useCollector was plumbed through config (interface, constructor, getter, env var resolution) but never consumed by the telemetry SDK — the setting had no runtime effect. TelemetryTarget.QWEN existed in the enum but parseTelemetryTargetValue() only accepted 'local' and 'gcp', making 'qwen' unreachable (it would throw FatalConfigError). Remove both dead code paths along with their tests and documentation. Part of #3731
wenshao
pushed a commit
that referenced
this pull request
May 17, 2026
… and CLI flags (#4066) * docs(telemetry): align config and docs semantics for target, outfile, and CLI flags - Remove stale warning note "This feature requires corresponding code changes" — the OTLP implementation is now complete (#3779, #4061) - Clarify that `target` is an informational destination label and does not control exporter routing; `otlpEndpoint` or `outfile` must be set to configure where data is sent - Mark `--telemetry-target` CLI flag as deprecated in the configuration table to match the deprecateOption() call in cli/src/config/config.ts - Fix `outfile` / `QWEN_TELEMETRY_OUTFILE` descriptions: remove the incorrect "when target is local" qualifier — outfile overrides OTLP export regardless of the target value - Simplify the file-based output example by removing the now-redundant `"target": "local"` and `"otlpEndpoint": ""` fields Closes the "Align telemetry config and docs semantics for target, useCollector, otlpEndpoint, otlpProtocol, and outfile" checklist item in #3731. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(telemetry): address Copilot review comments on outfile and target descriptions - Fix outfile table row in telemetry.md: "overrides `otlpEndpoint`" → "overrides OTLP export" (outfile disables all OTLP exporting, not just the base endpoint) - Use fully-qualified setting names (`telemetry.otlpEndpoint`, `telemetry.outfile`) in the target description in settings.md for consistency with the rest of the table 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(telemetry): update QWEN_TELEMETRY_TARGET env var description and add outfile note - Align QWEN_TELEMETRY_TARGET env var description with the updated telemetry.target setting semantics (informational label, not routing) - Add a note after the file-based output example clarifying that outfile automatically disables OTLP export 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
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.
Summary
useCollectordead config setting — plumbed through interface, constructor, getter, env var resolution, and docs, but never consumed by the telemetry SDK (initializeTelemetrynever calledgetTelemetryUseCollector())TelemetryTarget.QWENunreachable enum value —parseTelemetryTargetValue()only acceptslocal/gcp, passing'qwen'throwsFatalConfigErrorPart of #3731 (P1: Configuration semantics cleanup)
Test plan
packages/core— config, telemetry/config, telemetry/sdk tests all pass (174 tests)packages/cli— config tests pass (199 tests)useCollector,USE_COLLECTOR, orTelemetryTarget.QWEN🤖 Generated with Qwen Code