Skip to content

feat(cli): Cache telemetry that fails to send and retry it on the next run - #1963

Open
sentry-junior[bot] wants to merge 8 commits into
mainfrom
junior/telemetry-offline-store
Open

sentry-junior[bot] wants to merge 8 commits into
mainfrom
junior/telemetry-offline-store

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Refs #1956

Stacked on #1960 (base junior/1956-telemetry-request-deadline); merge it after #1960.

With ingest stalled, #1960 gets a command out in about 6.6 s, and the telemetry it tried to send is dropped. This PR keeps that telemetry on disk and retries it later. It also stops hitting the network after the first failure, so a command's exit waits for one request deadline instead of two.

Design

  • Store (telemetry/offline-store.ts, new telemetry_envelope_queue table, schema v21).
    • Envelopes are written with serializeEnvelope as BLOBs, since attachments can be binary, and read back with parseEnvelope. A row that no longer parses is dropped.
    • is_error marks envelopes that contain an event item. Each row's max age depends on it.
    • Each write drops expired rows and the rows over the caps, in one window query. Envelopes without an error go first, oldest first; errors are dropped only after those.
    • Reads claim rows with DELETE ... WHERE id IN (SELECT ... LIMIT n) RETURNING, so two CLI processes never get the same envelope.
    • A claimed envelope that fails again goes back with its original age, so a retry never extends it.
    • Store errors are logged at debug level and never reach the command. The table is a cache table: repair may drop it.
  • Transport (telemetry/offline-transport.ts). makeOfflineTransport from @sentry/core wraps the zstd transport, which keeps fix(cli): Let the CLI exit on its own instead of forcing it on macOS #1960's 3 s request deadline. Redaction still runs first, so stored envelopes are already redacted.
  • Metrics aren't stored. shouldStore returns false for trace_metric envelopes. They're sent as usual and dropped on failure, or skipped while the breaker is open. A long-running command emits runtime metrics every 5 s, and old ones are worth little. When they were stored, they filled the store during an outage and pushed sessions and transactions out of the caps. A metrics envelope can still be the probe that closes the breaker.
  • Breaker, per transport:
    • The first network error or timeout opens it. While it's open, shouldSend returns false and envelopes go straight to disk.
    • After BREAKER_COOLDOWN_MS (5 minutes), the next envelope goes out as a probe. Envelopes sent while the probe is in flight go to disk.
    • Any HTTP response to the probe closes the breaker and sends the next 10 stored envelopes. A failure re-opens it for another 5 minutes.
    • Why 5 minutes: short commands never live that long, so they keep one deadline and then disk. A long-running process pays one deadline-bound request every 5 minutes while ingest is down, and starts delivering what it stored within 5 minutes of recovery, far inside the 4-day age limit.
  • Drain. Early in withTelemetry(), the CLI claims up to 10 stored envelopes and sends them without awaiting. The close(3000) on exit waits for them. Each resend gets a fresh sent_at: Relay shifts item timestamps by the gap between sent_at and receipt, so a stale sent_at would move them forward.
  • sentry mcp uses the store only when it reports to the CLI's project, that is with no --sentry-dsn or SENTRY_DSN (useCliDsn). Then it wraps fix(cli): Let the CLI exit on its own instead of forcing it on macOS #1960's makeNodeTransportWithDeadline, sends up to 10 stored envelopes at startup, and recovers through the breaker. close(5000) on disconnect is unchanged. With an override, it keeps the plain transport. The gate is small on purpose: removing the override is happening in a separate PR on main.
  • Opt-out. When telemetry is off, withTelemetry() and sentry mcp delete the stored envelopes. They check for rows first, so an opted-out run against a read-only database prints no warning.
  • Library mode never touches the store. It keeps the plain zstd transport, with no drain and no clearing.

SDK behavior this relies on (@sentry/core 11.6.0, transports/offline.js and base.js):

  • A send that throws, including our abort, is stored and resolves {}. If shouldSend returns false, the SDK throws internally, so that envelope is stored too.
  • Any HTTP response is returned as is, so a 4xx or 5xx is dropped and not stored. base.js records an outcome only for 413 (send_error) and for thrown errors (network_error). In the CLI project over the last 30 days, client reports show network_error on about 82k errors, 15k sessions, 13k transactions, and 106 logs, and send_error (413) on 88 transactions. The loss is network errors, which the store covers.
  • If shouldStore returns false (our metrics rule), the SDK rethrows the error instead of storing. client.sendEnvelope catches it.
  • Client reports are never stored. With the breaker open, they fail fast instead of waiting for the network.
  • createTransport drops rate-limited items before the send, so they are never stored.
  • The store's shift always returns nothing, which turns off the SDK's timer-driven retries. Those send one envelope 100 ms after each success, or on a backoff starting at 5 s. When one fires during the exit flush, its request keeps the process alive past the flush, and such requests can chain. sendStored replaces them.

Limits

  • At most 100 envelopes and 4 MiB. A single envelope larger than 4 MiB is dropped instead of stored. Before, storing one emptied the store; Bugbot found it.
  • Envelopes with an error event keep 30 days. Relay sets an event's max_secs_in_past to the project's retention (90 days by default, relay-server/src/processing/utils/event.rs), and past that it only replaces the timestamp with the received time.
  • Everything else keeps 4 days, a day under Relay's 5-day limits:
    • Older sessions are dropped (max_session_secs_in_past).
    • Transactions must fall inside the metrics aggregator's max_secs_in_past.

Measured on the rebuilt npm bundle (dist/bin.cjs), Node 22, Linux:

  • Ingest was an HTTPS CONNECT proxy, so the runs used the SDK's proxy transport path.
  • In the stalled runs, the proxy accepted the tunnel and then stayed silent. In the healthy runs, it terminated TLS with a test CA (NODE_EXTRA_CA_CERTS) and answered 200.
Ingest Command #1960 This PR
stalled help 6.19–6.34 s 3.22–3.43 s
stalled cli defaults 6.31–6.40 s 3.32–3.41 s
healthy help 0.23–0.36 s 0.23–0.37 s
  • Short runs: 8 stalled runs stored 16 envelopes. The next healthy run sent 10 of them plus its own. The run after that sent the other 6 and emptied the store.
  • sentry local serve, stopped before the cooldown:
    • The store started with 6 envelopes, and ingest was stalled.
    • The startup drain timed out and requeued them.
    • Ingest was switched to healthy at 10 s, and SIGINT came at 20 s.
    • The server exited in 0.02 s with nothing sent, because the breaker was still open, and stored its exit telemetry.
  • sentry local serve, run past the cooldown (re-run after metrics stopped being stored):
    • Four stalled help runs (3.21–3.38 s each) left 8 envelopes in the store: 4 transactions and 4 sessions.
    • local serve started with ingest stalled, and ingest turned healthy at 20 s. The startup drain timed out and requeued its envelopes. With the breaker open, the store grew to only 9; the metrics emitted every 5 s were dropped.
    • At about 305 s, a metrics envelope went out as the probe and succeeded. All 9 stored envelopes followed at once, with a fresh sent_at: sessions, transactions and a log.
    • At 330 s, SIGINT exited in 0.13 s. The store ended empty, and ingest received 19 envelopes.
    • Before this change, the same scenario ended with 63 envelopes still stored, 60 of them trace_metric.
  • Opt-out: a run with SENTRY_CLI_NO_TELEMETRY=1 after stalled runs left the store empty.
  • sentry mcp: on this branch the bundle sends no telemetry, because of a separate bug on main: Sentry.init runs inside buildCommand's pre-init exec span, so the client is lost. fix(cli): Bind the sentry mcp telemetry client so it sends anything #1970 fixes it. Until fix(cli): Bind the sentry mcp telemetry client so it sends anything #1970 merges, this PR's MCP path is covered by unit tests only.

Not run locally: six unrelated permission tests (EACCES/EPERM) fail in this sandbox because it runs with full capabilities. They fail the same way on #1960.

Open questions:

  • Completion timing metrics (completion_telemetry_queue) are also trace_metric, so an outage now drops them, as it did before this PR. OK to leave them out of the store?

via @BYK.

--

View Session in Sentry

@sentry-junior
sentry-junior Bot added this pull request to stack #1964 November 4, 2026 22:10
@vercel

vercel Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
sentry-local Skipped Skipped Nov 5, 2026 12:03am UTC

Request Review

@github-actions github-actions Bot added the risk: medium PR risk score: medium label Nov 4, 2026

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

BYK added a commit that referenced this pull request Nov 4, 2026
## Summary

Remove every way to override the DSN for the server's own telemetry in
`sentry mcp` and `@sentry/mcp-server`:

- the `--sentry-dsn` flag;
- the `SENTRY_DSN` and `DEFAULT_SENTRY_DSN` env reads;
- the `sentryDsn` config plumbing.

MCP telemetry now always goes to `SENTRY_CLI_DSN`. Only the CLI's
telemetry opt-outs decide whether it's on.

`SENTRY_DSN` is the variable people set for their own apps. An MCP
server started from a project shell or IDE inherited it and sent our
telemetry to the user's own Sentry project.

The override dates back to #120 (Apr 2025), which added Sentry to the
original stdio package with `--sentry-dsn`/`SENTRY_DSN`. In that design,
an empty `--sentry-dsn=` was the documented opt-out. #1401 (`sentry
mcp`) and #1731 (the slim `@sentry/mcp-server` bundle) carried it over.

### Key changes

- **`packages/cli/src/lib/mcp/`:**
  - `args.ts` drops the flag type, the env reads and the merge.
  - `config.ts` drops `sentryDsn`.
- `server.ts` always calls `init({ dsn: SENTRY_CLI_DSN, enabled:
isTelemetryEnabled() })`.
- The early return for an empty `--sentry-dsn=` is gone, and so is the
rule that an explicit DSN ignored the CLI opt-out.
- **`commands/mcp.ts`:** the `--sentry-dsn` flag and the `SENTRY_DSN`
help line are removed. The skill reference is regenerated with
`generate:skill`.
- **`mcp-test-client`:** stops passing `--sentry-dsn` and `SENTRY_DSN`
to the server it launches. Its own `--sentry-dsn`, for the test client's
own Sentry client, stays, with a clearer description.
- **`mcp-core/tsdown.config.ts`:** drops the unused `DEFAULT_SENTRY_DSN`
define. Nothing in mcp-core reads it; it was the old stdio package's
build-time DSN.
- **Docs:** `docs/operations/monitoring.md`, `docs/testing/stdio.md`,
`TELEMETRY.md`, the mcp-core README and the stdio setup page now point
to the opt-out instead.
- The remote/Cloudflare `VITE_SENTRY_DSN` is unchanged. That's
deployment config, not a user override.

### Breaking changes

**For `@sentry/mcp-server` and `sentry mcp` users:**

- **`--sentry-dsn` is now an unknown flag.** A config that passes it,
including the documented opt-out `--sentry-dsn=`, fails before the
server starts. I checked on the rebuilt bundle (`node
dist/sentry-mcp.cjs --sentry-dsn=` and `--sentry-dsn=https://…`): stderr
shows `No flag registered for --sentry-dsn` and the exit code is 252.
- An error is safer than silently ignoring the flag. Ignoring it would
quietly turn telemetry back on for anyone who had opted out.
- **`SENTRY_DSN` and `DEFAULT_SENTRY_DSN` are silently ignored.** Anyone
who used them to send MCP telemetry to their own Sentry instance now
reports to the Sentry CLI's project instead.

**Migration:** remove `--sentry-dsn` from the MCP config. To turn
telemetry off, set `SENTRY_CLI_NO_TELEMETRY=1` or `DO_NOT_TRACK=1` in
the server's `env`, or run `sentry cli defaults telemetry off` once
(shared config dir).

```json
{ "command": "npx", "args": ["@sentry/mcp-server@latest"], "env": { "SENTRY_CLI_NO_TELEMETRY": "1" } }
```

`computeTelemetryEffective()` in `packages/cli/src/lib/telemetry.ts`
checks the env vars for an exact `"1"`, then the stored preference.

**Release note:** the title has no `!`. `.craft.yml` uses
`changelog.policy: auto` and `versioning.policy: auto`, there's no
`.github/release.yml`, and Craft's default categories map `type!:` to a
major bump. A `!` would most likely cut CLI 1.0.0 (inferred from Craft's
docs, not tried). `docs/releases/stdio.md` has no separate
breaking-change process for `@sentry/mcp-server`, which ships at the
CLI's version. Please call out the break in the release notes, or add
`!` if a major bump is intended.

### Tests

- `test/lib/mcp/telemetry.test.ts`:
  - init always uses `SENTRY_CLI_DSN`;
  - `SENTRY_CLI_NO_TELEMETRY=1` and `DO_NOT_TRACK=1` each disable it;
- with `SENTRY_DSN` and `DEFAULT_SENTRY_DSN` set in the env, the DSN is
still the CLI's (the env leak behind this change).
- `test/lib/mcp/args.test.ts`: `parseMcpEnv` ignores both env vars, and
`--sentry-dsn=` is rejected.
- `packages/mcp-server/test/bundle.test.mjs`:
  - the bundle refuses `--sentry-dsn=`;
- the "telemetry on" exit test turns telemetry on with
`SENTRY_CLI_NO_TELEMETRY=0` and sends it to a dead `https_proxy`, so
nothing leaves the machine.

### Screenshot

The stdio setup page (`packages/mcp-cloudflare`, `?transport=stdio`),
from a local `vite build`:

![stdio setup
configuration](https://junior-prod.sentry.dev/public/artifacts/4526371a-a2e8-473e-8a21-e6d26b99bd79.png)

### Interaction with #1963

#1963 stacks the offline telemetry store on #1960. It gates the store in
`mcp/server.ts` on `useCliDsn`, which is true only without an override.
Once this merges, that gate is always true and becomes dead code.
Whichever PR lands second removes it and always uses the store.

<!-- junior-request-attribution:start -->
via @BYK.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:agent%3A11063284b105b81f348414f8900e5707 -->

--

[View Session in
Sentry](https://sentry.sentry.io/explore/conversations/agent%3A11063284b105b81f348414f8900e5707/?project=4510944073809921)

<!-- junior-session-footer:end -->

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: BYK <126780+BYK@users.noreply.github.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/cli/src/lib/mcp/server.ts Resolved
Base automatically changed from junior/1956-telemetry-request-deadline to main November 4, 2026 23:16
@sentry-junior
sentry-junior Bot force-pushed the junior/telemetry-offline-store branch from ce21980 to 39b47e7 Compare November 4, 2026 23:19
@sentry-junior
sentry-junior Bot force-pushed the junior/telemetry-offline-store branch from 39b47e7 to 5992dab Compare November 4, 2026 23:20
@sentry-junior
sentry-junior Bot force-pushed the junior/telemetry-offline-store branch from 5992dab to 39b47e7 Compare November 4, 2026 23:21

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 39b47e7. Configure here.

sentry-junior Bot and others added 7 commits November 4, 2026 23:37
…t run

With ingest stalled, telemetry sent during a command timed out, and the
exit flush then sent new data that timed out again: about 6.6 s to exit,
with everything dropped.

Wrap the telemetry transport with the SDK's makeOfflineTransport, backed
by a new SQLite table (schema v21). An envelope whose send fails or times
out, or gets a 5xx, is stored, and a per-process breaker sends every later
envelope straight to disk. The next run claims up to 10 stored envelopes
with DELETE ... RETURNING and sends them in the background; the exit
flush waits for them. The store keeps at most 100 envelopes and 4 MiB,
and drops anything older than 4 days, under Relay's 5-day limit for
sessions and transactions. Opting out clears it; library mode never
uses it.

With ingest stalled, a command now exits in about 3.3 s, and a later run
delivers what it stored.

Refs #1956
… cache sentry mcp telemetry

The breaker now half-opens 5 minutes after it opens. The next envelope is
sent as a probe: a response closes the breaker and sends a batch of stored
envelopes, and a failure opens it for another 5 minutes. Short commands
never live that long, so they still wait for one deadline and then write
to disk. Long-running ones, such as `sentry mcp` and `sentry local`, now
deliver what they stored once ingest comes back.

A 5xx response is dropped like a 4xx instead of being stored and opening
the breaker. The SDK records no outcome for it, and the CLI's client
reports show network errors as the real loss.

Envelopes with an error event keep 30 days, since Relay accepts errors
for the project's retention; everything else keeps 4 days. Over a cap,
the store drops envelopes without an error first.

`sentry mcp` uses the store when it reports to the CLI's project (no
`--sentry-dsn` or `SENTRY_DSN`): it sends stored envelopes at startup and
clears them when telemetry is off.

Refs #1956

Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
Inserting an envelope over the byte cap put every row over it in the
trim query, so the store deleted everything it held. Drop the oversized
envelope instead; it could never be kept.

Refs #1956
…g them

A long-running command emits runtime metrics every few seconds. While
ingest was down they filled the store and pushed sessions and
transactions out of its caps. They are now sent as usual and dropped on
failure; a metrics envelope can still be the probe that closes the
breaker.

Refs #1956
In a parallel drain batch, a response that arrived after another send
failed re-closed the breaker and claimed another batch, so a flaky
ingest could walk through the store, dropping any 4xx or 5xx. Only the
probe now closes the breaker.

Refs #1956
The DSN override that kept `sentry mcp` off the store is being removed
(#1969), so drop the `useCliDsn` gate: the store follows the CLI's
telemetry opt-out alone. The new lines stay clear of the DSN lines

Refs #1956
The write-path trim ranked expired rows along with live ones in the same
DELETE. Expired errors sort first, so they could push a new envelope past
the count or byte cap and delete it in the statement that removed them.
Delete expired rows in their own statement first, as claim() does.

Refs #1956
@sentry-junior
sentry-junior Bot force-pushed the junior/telemetry-offline-store branch from 2eb3ff8 to 3319004 Compare November 4, 2026 23:39
…lopes

The cooldown comment and the docs said a long-running command sends
everything it stored once ingest recovers. It sends one batch of 10 on
purpose, so each recovery stays bounded; the rest wait for later runs.

Refs #1956

This branch was previously deployed

1 inactive deployment
Preview — 4ed0efdd Deployed Nov 5, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant