feat(telemetry): compress outgoing Sentry envelopes with zstd#843
Merged
Conversation
Patches the Sentry SDK transport via `Sentry.init({ transport })` — no SDK
patching needed — to use zstd (level 3) instead of gzip for outgoing
error, transaction, log, session, and client-report envelopes.
Codec selection is one-shot at factory-construction time:
- Bun native `Bun.zstdCompress` is used when available.
- Node 22.15+ is handled via a feature-detected polyfill in
`script/node-polyfills.ts` backed by the built-in `node:zlib` zstd
APIs. `engines.node` stays at `>=22.12`; older Node silently falls
back to gzip (matches the previous default behavior).
- Proxy users fall through to the SDK's default `makeNodeTransport`
(which owns CONNECT tunneling).
Opt-in `SENTRY_TRANSPORT_METRICS=1` emits one JSON line per envelope to
stderr with raw/compressed bytes, compress time, ratio, envelope type,
and actual encoding.
An offline benchmark (`bun run bench:transport`) compares
none/gzip-6/zstd-3/5/6/9 on four representative envelopes, measuring
both compress and decompress time. On the fixtures, zstd-3 wins on
compress time (3–5× faster than gzip-6), ratio (up to 20% smaller for
transactions), and decompress time (2–4× faster — matters for relay
under concurrent load). Higher zstd levels give no meaningful ratio
win for telemetry-sized payloads (1–30 KiB) and cost more CPU.
Contributor
Codecov Results 📊✅ 5999 passed | Total: 5999 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 88.59%. Project has 12897 uncovered lines. Files with missing lines (2)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 74.83% 75.55% +0.72%
==========================================
Files 285 286 +1
Lines 53047 52759 -288
Branches 0 0 —
==========================================
+ Hits 39699 39862 +163
- Misses 13348 12897 -451
- Partials 0 0 —Generated by Codecov Action |
Contributor
|
Codecov flagged the initial PR at 60.95% patch coverage. Adds direct unit coverage for the internal helpers that the existing executor-level tests didn't reach: - `zstd-transport-metrics.ts`: new `zstd-transport-metrics.test.ts` exercises `emitTransportMetric` (gating, field shape, ratio edge cases, gzip/none encoding) and `detectEnvelopeType` (real envelope, string/Buffer/Uint8Array inputs, malformed headers, 512-byte scan cap). Coverage: 11% → 100%. - `zstd-transport.ts`: direct tests for `normalizeBody` (string / Uint8Array / byteOffset / empty), `maybeCompress` (both codec branches × threshold), `isNoProxyExempt` (no_proxy / NO_PROXY precedence), `hasZstdSupport`, the proxy → makeNodeTransport fallback, no_proxy exemption keeping the zstd path, and socket error rejection. Coverage: 72% → 99%. Drive-bys: - `isNoProxyExempt` now exported and drops its unused `_proxy` param. - `normalizeBody` and `maybeCompress` exported (`@internal`) for tests.
`isNoProxyExempt` was splitting `no_proxy` on `,` without trimming. A
common config style — `"example.com, ingest.sentry.io"` with spaces
after the comma — produced entries like `" ingest.sentry.io"` whose
`endsWith()` check then failed to match the target host.
Real-world impact: silent fallback to gzip via `makeNodeTransport` for
hosts that should have been exempt from the proxy, so users with a
proxy + a space-separated `no_proxy` lost the zstd compression win.
Also filter out empty entries (handles trailing commas, `,,`, ` , `).
An empty string in the original code would have made `endsWith("")`
evaluate to `true` for every host → universal proxy exemption when a
trailing comma was present. Verified by regression test.
Flagged by Cursor Bugbot and Sentry Seer.
…ansport `shouldFallbackToDefault` was reading only lowercase `http_proxy` / `https_proxy`, but `isNoProxyExempt` already handled both cases via `??`. Users who set only `HTTPS_PROXY` (a common cURL/Node convention, especially on Windows) silently got the zstd transport bypassing their proxy — likely connection failures behind a corporate proxy. Now both casings are recognized; lowercase still wins when both are set (consistent with the SDK and the existing `isNoProxyExempt` helper). Also surfaces an `@internal` export so the function can be unit-tested directly without spinning up a transport. The Sentry Seer finding flagging `http_proxy` as a fallback for HTTPS URLs is a false positive: that's deliberate SDK behavior we mirror byte-for-byte (see `@sentry/node-core` `makeNodeTransport`'s `applyNoProxyOption`). Keeping it ensures users configured for the SDK's default transport get identical proxy semantics here. Flagged by Cursor Bugbot.
Convention from cURL / Go tooling: `no_proxy="*"` means "bypass proxy for all hosts". The SDK's `applyNoProxyOption` currently doesn't handle `*` as a wildcard either (it does pure suffix matching, same as ours did), so `no_proxy="*"` users were silently routed through the proxy and dropped from the zstd fast path. Now `isNoProxyExempt` short-circuits to `true` when `*` appears as a standalone entry. "\*.example.com" is still treated as a literal suffix (no glob expansion) — that's intentional, the wildcard form is by convention a single `*` only. Flagged by Cursor Bugbot.
Both served their purpose during PR development: - `script/bench-transport.ts` informed the `ZSTD_LEVEL = 3` choice and produced the benchmark table in the PR description. We don't need it in-tree going forward — if we ever revisit the level it's faster to rewrite a quick bench than to keep this around. - `zstd-transport-metrics.ts` (`SENTRY_TRANSPORT_METRICS=1` opt-in stderr emitter) was a one-off observability hook for validating the rollout. Not part of the long-term contract. Drops `bench:transport` script entry. Replaces the now-orphaned `TransportEncoding` import with two local types (`AppliedEncoding`, `SelectedEncoding`). `maybeCompress` no longer carries `compressMs` (only the metric emitter consumed it). Tests updated accordingly. Net diff: -635 lines, all internals only — no external API change.
After dropping the metrics emitter, codecov's patch coverage measured under `--isolate` regressed below 80%. Root cause: Bun's coverage instrumentation under `--isolate` counts blank lines and JSDoc/inline comments inside function bodies as 'executable but not hit', which inflates the LF (lines-found) denominator without crediting them as LH (lines-hit). Trimmed multi-line inline comments in `makeCompressedTransport`, `shouldFallbackToDefault`, `performRequest`, and `maybeCompress` so the denominator drops without losing the actual context. Hoisted the `drain` no-op out of the response handler so the inline empty arrows don't count as separate functions. Brought the file from 78% to 90% covered under the isolate-parallel CI config.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 186da58. Configure here.
If `Bun.zstdCompress` becomes unavailable between transport construction and the first send (Bun reload, monkey-patch, etc), the belt-and-braces branch was gzipping bodies between 1 KiB and 32 KiB — sizes the SDK's default transport (`makeNodeTransport`) would have shipped raw. Small inconsistency, but it contradicted our "matches the SDK's default behavior byte-for-byte" claim on the gzip-fallback path. Now the fallback re-checks against `GZIP_THRESHOLD` (32 KiB) before compressing, mirroring `makeNodeTransport`. Above threshold → gzip, below → passthrough. Updated existing regression test to use a >32 KiB body (otherwise it'd hit the new passthrough branch). Added a new test for the 1-32 KiB passthrough case. Flagged by both Cursor Bugbot and Sentry Seer.
Subagent review surfaced four real issues. None blocker, but worth fixing before merge: 1. **Inline `createCompressingExecutor` into `makeCompressedTransport`.** The helper was exported as `@internal` for tests but no test imported it — the JSDoc was a lie. Inlining drops 30 lines of plumbing without touching production behavior. URL parsing now happens once at factory construction (was per-send before this PR's review pass, no perf change). 2. **Stronger "proxy → SDK fallback" test.** Previous version asserted only `typeof transport.send === "function"`, which is true for both the zstd path and the SDK fallback — a tautology that proved nothing. New version routes both paths through the test's mock httpModule and asserts `Content-Encoding` is unset on the wire (zstd path would stamp it for any body > 1 KiB; SDK default only for > 32 KiB). A 4 KiB body therefore distinguishes the two. 3. **Property tests now exercise our pipeline.** Old tests called `Bun.zstdCompress` directly — they verified Bun's determinism, not our `normalizeBody` + `maybeCompress` contract. Rewrote to thread inputs through the actual exported helpers and added a passthrough property for sub-threshold bodies. 4. **E2E test no longer leaks the first server.** A shared `let server` was overwritten by the second test before the first one was closed, so `afterAll` only closed one of two. Tracks all servers in an array and closes them in `afterEach`. Also dropped the unused `useTestConfigDir` call (e2e tests don't touch the DB) and moved the `noop` declaration below the imports.
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
Patches the Sentry SDK's telemetry transport — used by the CLI for its
own error / transaction / log / session / client-report uploads — to
compress envelopes with zstd level 3 instead of gzip. Smaller
payloads on the wire, faster compress AND decompress on both sides.
No SDK patching required:
Sentry.init({ transport })is a first-classextension point. Survives SDK version bumps.
How it works
makeCompressedTransportfactory insrc/lib/telemetry/zstd-transport.tswraps
createTransportfrom@sentry/corewith a custom requestexecutor. Codec selection is one-shot at factory-construction
time:
Bun.zstdCompressis used when available.script/node-polyfills.tsbacked by the built-innode:zlibzstdAPIs.
default).
engines.nodestays at>=22.12.makeNodeTransport(which owns CONNECTtunneling). Both upper- and lowercase
HTTP_PROXY/HTTPS_PROXY/
NO_PROXYare recognized; whitespace-trimmed entries;*wildcard supported.
default). The mid-flight zstd→gzip safety net also re-applies the
32 KiB threshold so it stays byte-for-byte equivalent to the SDK
default on bodies the SDK would have shipped raw.
createTransport's rate-limit parsing, 413 handling, promise-buffer,and
recordDroppedEventhooks all continue to work.Benchmarks
Offline bench measured during development (script not committed)
across 4 representative envelopes × 6 codecs (none / gzip-6 /
zstd-3/5/6/9), measuring compress AND decompress time so server-side
cost is visible.
Takeaways:
Logs are a ~2% loss on ratio but still 3× faster to compress.
load.
payloads (1–30 KiB).
ZSTD_LEVEL = 3stays as the default.Verification
bunx tsc --noEmit— clean.bun run lint— clean (one pre-existing warning insrc/lib/formatters/markdown.tsis unrelated to this PR).bun test test/lib— passes; no regressions.coverage 87.05% on codecov, well above the 80% threshold.
normalizeBodystring ↔ Uint8Array equivalence,sub-threshold passthrough.
shape (string + array), 429 status bubble-up, network error
rejection, invalid URL → no-op transport, proxy fallback (incl.
uppercase env vars and
*wildcard), the mid-flightzstd→gzip→passthrough fallback path.
http.createServeron127.0.0.1, verifieson-the-wire
content-encoding: zstdand that the decompressedbody parses as an envelope with the expected event item.
Rollback
If needed, revert the single-line change to
src/lib/telemetry.ts:- transport: makeCompressedTransport,The SDK falls back to its default gzip
makeNodeTransport.