docs(telemetry): align config and docs semantics for target, outfile, and CLI flags#4066
Merged
Conversation
… 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)
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the telemetry documentation to match current implementation semantics, particularly clarifying that target is informational and that actual exporting is controlled by OTLP endpoint settings or outfile.
Changes:
- Remove the stale “requires corresponding code changes” note from
telemetry.md. - Clarify
telemetry.targetas an informational label (not used for exporter routing) and mark--telemetry-targetas deprecated in the telemetry config table. - Correct
outfile/QWEN_TELEMETRY_OUTFILEdescriptions to reflect that file output overrides OTLP export, and simplify the file-output example.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/users/configuration/settings.md | Updates telemetry setting and env var descriptions to reflect real routing/override semantics. |
| docs/developers/development/telemetry.md | Removes stale warning note, clarifies target semantics + CLI deprecation, and simplifies local file-output example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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. |
…t 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)
wenshao
requested changes
May 12, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Critical] Aliyun Telemetry section not updated (telemetry.md:106-140): The Aliyun Telemetry section was not updated to reflect the new target semantics. Option A still includes "target": "gcp" with no deprecation note, the sentence "Setting target: "gcp" alone…" contradicts the config table (which says target "does not control exporter routing" — no qualifier), and the old wording "overrides otlpEndpoint" is used instead of "overrides OTLP export". Please update this section to match.
[Suggestion] General settings.json example still has "target": "local" (settings.md:520): The general example at ~line 520 still includes "target": "local" without noting it is informational only, inconsistent with how the telemetry.md file-output example was cleaned up to remove target.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
… 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
reviewed
May 12, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Apologies for the omission in the previous round — now that the full review is complete, here are the remaining suggestions from a deeper pass:
Suggestion: Missing migration guidance — Now that target is documented as informational-only and --telemetry-target is deprecated, consider adding a brief migration note so users who followed the old (incorrect) docs know how to update:
| Old config | New config |
|---|---|
target: "local" |
outfile: ".qwen/telemetry.log" or otlpEndpoint: "http://localhost:4317" |
target: "gcp" |
otlpEndpoint: "https://<your-gcp-collector>" |
target: "local" + outfile |
Just outfile (remove target) |
Suggestion: Aliyun Telemetry Option A (telemetry.md:124) — The explanatory text was updated but the JSON example still includes "target": "gcp" without an inline comment. Option B (HTTP) correctly omits target. Consider removing "target": "gcp" from Option A or adding // informational label only for consistency.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
tanzhenxin
approved these changes
May 12, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Review
Docs-only cleanup that brings the telemetry config tables in line with the post-#3779/#4061 implementation: removes the stale "feature requires corresponding code changes" warning, clarifies that target is informational and does not control exporter routing, marks --telemetry-target as deprecated, fixes the outfile description to drop the misleading "when target is local" qualifier, and slims the file-based output example. Each code-grounded claim verifies against the tree (outfile short-circuits OTLP in sdk.ts, getTelemetryTarget() is unused in production routing, deprecateOption('telemetry-target', ...) is present, both predecessor PRs are merged), and the trimmed config example is genuinely the minimal working config — no required field dropped.
Verdict
APPROVE — Docs-only fix that resolves a real mismatch between behavior and documentation; all claims verify against code.
wenshao
reviewed
May 12, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] docs/users/configuration/settings.md also has a Command-Line Arguments table entry for --telemetry-target that still says it "Sets the telemetry target" without noting the new deprecated/informational-only semantics. That entry is outside the current diff, so I cannot anchor this as an inline comment, but it now conflicts with the updated telemetry docs in this PR. Consider updating that row to say the flag only sets an informational destination label and that routing is configured through --telemetry-otlp-endpoint or --telemetry-outfile.
— gpt-5.5 via Qwen Code /review
Open
53 tasks
wenshao
approved these changes
May 12, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
wenshao
reviewed
May 13, 2026
| | `QWEN_RUNTIME_DIR` | Overrides the runtime output directory (conversations, logs, todos). When unset, defaults to the `QWEN_HOME` directory. | Use this to separate ephemeral runtime data from persistent config. Useful when `QWEN_HOME` is on a shared/slow filesystem. | | ||
| | `QWEN_TELEMETRY_ENABLED` | Set to `true` or `1` to enable telemetry. Any other value is treated as disabling it. | Overrides the `telemetry.enabled` setting. | | ||
| | `QWEN_TELEMETRY_TARGET` | Sets the telemetry target (`local` or `gcp`). | Overrides the `telemetry.target` setting. | | ||
| | `QWEN_TELEMETRY_TARGET` | Sets an informational label for the telemetry destination (`local` or `gcp`). Does not control routing; use `QWEN_TELEMETRY_OTLP_ENDPOINT` or `QWEN_TELEMETRY_OUTFILE` to configure where data is sent. | Overrides the `telemetry.target` setting. | |
Collaborator
There was a problem hiding this comment.
[Suggestion] Minor wording asymmetry within this PR's own rewrite. The same target-semantics clarification was applied to three rows in this commit, but only this env-var row drops the word "exporter" (and uses "use" instead of "set"):
docs/developers/development/telemetry.md:60— "does not control exporter routing — setotlpEndpointoroutfile…"docs/users/configuration/settings.md:476— "Does not control exporter routing; settelemetry.otlpEndpointortelemetry.outfile…"- This row (
:575) — "Does not control routing; useQWEN_TELEMETRY_OTLP_ENDPOINTorQWEN_TELEMETRY_OUTFILE…"
Aligning the three rows costs nothing and keeps the new terminology consistent for future grepping:
Suggested change
| | `QWEN_TELEMETRY_TARGET` | Sets an informational label for the telemetry destination (`local` or `gcp`). Does not control routing; use `QWEN_TELEMETRY_OTLP_ENDPOINT` or `QWEN_TELEMETRY_OUTFILE` to configure where data is sent. | Overrides the `telemetry.target` setting. | | |
| | `QWEN_TELEMETRY_TARGET` | Sets an informational label for the telemetry destination (`local` or `gcp`). Does not control exporter routing; set `QWEN_TELEMETRY_OTLP_ENDPOINT` or `QWEN_TELEMETRY_OUTFILE` to configure where data is sent. | Overrides the `telemetry.target` setting. | |
— claude-opus-4-7[1m] via Qwen Code /qreview
wenshao
reviewed
May 13, 2026
| } | ||
| ``` | ||
|
|
||
| > **Note:** When `outfile` is set, OTLP export is automatically disabled. |
Collaborator
There was a problem hiding this comment.
[Suggestion] The Note explains the outfile vs OTLP semantics for users reading settings.json, but skips the override path that produces the most painful silent failure: env vars and CLI flags also feed outfile resolution.
Looking at packages/core/src/telemetry/config.ts:109-110:
const outfile =
argv.telemetryOutfile ?? env['QWEN_TELEMETRY_OUTFILE'] ?? settings.outfile;Two concrete scenarios this Note doesn't help with:
- Shadowed
settings.json. A user hasQWEN_TELEMETRY_OUTFILE=/old/path.login their shell rc from a previous experiment. They follow this example, write"outfile": ".qwen/telemetry.log"tosettings.json, see nothing in.qwen/telemetry.log, and have no hint to checkenv | grep QWEN_TELEMETRY_OUTFILE. The new minimal example actively reduces the surface area insettings.jsonthat would prompt them to look. OTEL_EXPORTER_OTLP_ENDPOINTre-route. The standard OTel env var is read as a fallback atconfig.ts:79, but the env-var table in this doc doesn't list it (only the per-signalOTEL_*variants get a passing reference at lines 93-100). A user who omitsotlpEndpointper this Note, withOTEL_EXPORTER_OTLP_ENDPOINTset system-wide, is still safe today (the!telemetryOutfilegate atsdk.ts:171holds), but they don't know that's their last line of defense.
Suggested change
| > **Note:** When `outfile` is set, OTLP export is automatically disabled. | |
| > **Note:** When `outfile` is set, OTLP export is automatically disabled. | |
| > The `target` and `otlpEndpoint` settings are not needed for file-only | |
| > output and can be safely omitted from your config. `QWEN_TELEMETRY_OUTFILE` | |
| > and `--telemetry-outfile` take precedence over `settings.json` — if | |
| > telemetry doesn't appear in the expected file, check those overrides | |
| > first. |
— claude-opus-4-7[1m] via Qwen Code /qreview
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
… 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
Closes the "Align telemetry config and docs semantics for
target,useCollector,otlpEndpoint,otlpProtocol, andoutfile" checklist item in #3731.> [!note] ⚠️ Special Note: This feature requires corresponding code changesblock intelemetry.mdis no longer accurate; the OTLP implementation is complete as of feat(telemetry): define HTTP OTLP endpoint behavior and signal routing #3779 and refactor(telemetry): remove dead useCollector setting and unreachable TelemetryTarget.QWEN #4061.targetsemantics —targetis an informational destination label ("local"/"gcp"); it does not control exporter routing. The SDK (sdk.ts) never readsgetTelemetryTarget()— actual routing is driven byotlpEndpointandoutfile. Updated the description in bothtelemetry.mdandsettings.mdto reflect this.--telemetry-targetCLI flag as deprecated in the configuration table — matches the existingdeprecateOption('telemetry-target', ...)call inpackages/cli/src/config/config.ts.outfile/QWEN_TELEMETRY_OUTFILEdescriptions — removed the incorrect "whentargetislocal" qualifier.outfileoverrides OTLP export regardless of thetargetvalue (verified insdk.ts:useOtlp = (!!parsedEndpoint || hasPerSignalEndpoint) && !telemetryOutfile)."target": "local"and"otlpEndpoint": ""fields; onlyenabledandoutfileare needed.Test plan
sdk.tsnever callsconfig.getTelemetryTarget()(search confirms no call sites in production code)outfiletakes precedence overotlpEndpointregardless oftarget(line 171 insdk.ts:const useOtlp = (!!parsedEndpoint || hasPerSignalEndpoint) && !telemetryOutfile)🤖 Generated with Qwen Code