DX-2977: identify the cli with Upstash-Telemetry headers - #18
Merged
Conversation
The coordinator cannot tell the cli apart from a raw API-key caller: both send only Basic auth. Send the same Upstash-Telemetry-* trio the JS SDKs and @upstash/mcp-server already send, so requests can be attributed. Claude-Session: https://claude.ai/code/session_016dGHwYmqdtkQswYYbrgudB
Adds the opt-out alongside the headers themselves, following what the
other Upstash SDKs and comparable CLIs do:
upstash telemetry disable | enable | status persisted, as with
`vercel telemetry disable`
and `fly settings analytics`
UPSTASH_DISABLE_TELEMETRY=1 read by redis-js, qstash-js,
vector-js and the rest
The env var beats the saved preference, so a CI job opts out without
writing config.
The headers are now resolved per request instead of at import. `cli.ts`
calls dotenv only after the module graph is evaluated, so a value read at
import time would miss a `.env` file.
The saved preference lives beside the credentials in config.json, so
`writeConfig` merges rather than overwrites and `deleteConfig` keeps the
preference behind: logging out must not quietly turn telemetry back on.
It also now reports whether credentials were actually removed, so logout
does not claim one when only the preference was stored.
Claude-Session: https://claude.ai/code/session_016dGHwYmqdtkQswYYbrgudB
CahidArda
force-pushed
the
DX-2977-telemetry-headers
branch
from
September 1, 2026 07:59
7154913 to
610c051
Compare
alitariksahin
approved these changes
Sep 2, 2026
There was a problem hiding this comment.
🟡 Changes recommended
deleteConfig() can leave legacy ~/.upstash.json credentials on disk (especially after writing a telemetry-only new config), causing logout to report no credentials while not actually logging the user out.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds opt-out telemetry identification so Upstash can distinguish the CLI from raw API-key callers by attaching the same Upstash-Telemetry-* headers used by other Upstash SDKs, plus a CLI command and documentation for managing the setting.
Changes:
- Add
src/telemetry.tsto compute theUpstash-Telemetry-Sdk/Runtime/Platformheaders and honorUPSTASH_DISABLE_TELEMETRYplus a persisted preference. - Persist
telemetry_disabledalongside credentials in the config file and introduceupstash telemetry {status,enable,disable}. - Extend the HTTP client to attach telemetry headers, with unit tests and README documentation.
File summaries
| File | Description |
|---|---|
| tests/unit/telemetry.test.ts | New unit tests for telemetry status/headers and persistence behavior. |
| tests/unit/client.test.ts | New unit test asserting requests include the telemetry header trio. |
| src/telemetry.ts | Implements telemetry status resolution and header construction. |
| src/config.ts | Stores/reads telemetry_disabled in config; preserves preference across login/logout. |
| src/commands/telemetry.ts | Adds upstash telemetry command group to manage the preference. |
| src/client.ts | Adds telemetry headers to every API request. |
| src/cli.ts | Registers the new telemetry command with the main CLI program. |
| README.md | Documents what telemetry headers are sent and how to disable/enable them. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
100
to
+110
| export function deleteConfig(): boolean { | ||
| const path = getConfigPath(); | ||
| if (!existsSync(path)) return false; | ||
| rmSync(path); | ||
| return true; | ||
| const existing = readRawConfig(path); | ||
| if (!existing) return false; | ||
| const hadCredentials = Boolean(existing.email && (existing.api_key ?? existing.apiKey)); | ||
| if (existing.telemetry_disabled === undefined) { | ||
| rmSync(path); | ||
| } else { | ||
| writeStoredConfig({ telemetry_disabled: existing.telemetry_disabled }); | ||
| } | ||
| return hadCredentials; |
| expect(deleteConfig()).toBe(false); | ||
| }); | ||
|
|
||
| it("keeps the config file readable only by the owner", async () => { |
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.
The coordinator cannot tell the cli apart from a raw API-key caller: both send only Basic auth. Send the same Upstash-Telemetry-* trio the JS SDKs and @upstash/mcp-server already send, so requests can be attributed.
Claude-Session: https://claude.ai/code/session_016dGHwYmqdtkQswYYbrgudB