From ec0392bfffe823379c424296db36d636975798ad Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 24 Apr 2026 17:22:52 +0000 Subject: [PATCH 1/9] feat(telemetry): compress outgoing Sentry envelopes with zstd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- package.json | 1 + script/bench-transport.ts | 290 ++++++++++++++ script/node-polyfills.ts | 66 ++++ src/lib/telemetry.ts | 6 + src/lib/telemetry/zstd-transport-metrics.ts | 106 +++++ src/lib/telemetry/zstd-transport.ts | 363 ++++++++++++++++++ test/lib/telemetry/zstd-transport.e2e.test.ts | 148 +++++++ .../telemetry/zstd-transport.property.test.ts | 74 ++++ test/lib/telemetry/zstd-transport.test.ts | 287 ++++++++++++++ 9 files changed, 1341 insertions(+) create mode 100644 script/bench-transport.ts create mode 100644 src/lib/telemetry/zstd-transport-metrics.ts create mode 100644 src/lib/telemetry/zstd-transport.ts create mode 100644 test/lib/telemetry/zstd-transport.e2e.test.ts create mode 100644 test/lib/telemetry/zstd-transport.property.test.ts create mode 100644 test/lib/telemetry/zstd-transport.test.ts diff --git a/package.json b/package.json index 8dcf0a878..9b830249d 100644 --- a/package.json +++ b/package.json @@ -96,6 +96,7 @@ "bench:save": "bun run script/bench.ts --save-baseline", "bench:compare": "bun run script/bench.ts --compare", "bench:sweep": "bun run script/bench-sweep.ts", + "bench:transport": "bun run script/bench-transport.ts", "check:fragments": "bun run script/check-fragments.ts", "check:deps": "bun run script/check-no-deps.ts", "check:errors": "bun run script/check-error-patterns.ts", diff --git a/script/bench-transport.ts b/script/bench-transport.ts new file mode 100644 index 000000000..3a25de2b6 --- /dev/null +++ b/script/bench-transport.ts @@ -0,0 +1,290 @@ +/** + * Offline benchmark for the telemetry transport's compression codecs. + * + * bun run script/bench-transport.ts # markdown table to stdout + * bun run script/bench-transport.ts > bench.md + * + * For each of four representative envelopes (error / transaction / log / + * session) the script measures compress-time and wire size across: + * + * none — raw, no compression + * gzip-6 — zlib default level (what the SDK's default transport uses) + * zstd-3 — libzstd default + * zstd-5 — mid-level; probes the low-to-mid curve for small payloads + * zstd-6 — slightly higher-ratio / slower + * zstd-9 — upper anchor; AGENTS.md warns of decoder-side cost at high levels + * + * Also measures decompress time with Bun.zstdDecompress / zlib.gunzip so + * the server-side cost is visible. Without decode-side data a lower + * compressed size can look like a win on ratio while actually being + * worse total throughput once the ingest relay's decode cost is counted. + * + * Output: a single markdown table per envelope plus a per-codec summary. + */ + +import { promisify } from "node:util"; +import { gunzip as gunzipCb, gzip as gzipCb } from "node:zlib"; +import { createEnvelope, serializeEnvelope } from "@sentry/core"; + +const gzipAsync = promisify(gzipCb); +const gunzipAsync = promisify(gunzipCb); + +type Codec = "none" | "gzip-6" | "zstd-3" | "zstd-5" | "zstd-6" | "zstd-9"; + +const CODECS: Codec[] = [ + "none", + "gzip-6", + "zstd-3", + "zstd-5", + "zstd-6", + "zstd-9", +]; + +const WARMUP_ITERS = 5; +const MEASURE_ITERS = 50; + +async function compress(codec: Codec, buf: Buffer): Promise { + if (codec === "none") { + return buf; + } + if (codec === "gzip-6") { + return await gzipAsync(buf); + } + const level = Number(codec.split("-")[1]); + const out = await Bun.zstdCompress(buf, { level }); + return Buffer.from(out.buffer, out.byteOffset, out.byteLength); +} + +async function decompress(codec: Codec, buf: Buffer): Promise { + if (codec === "none") { + return buf; + } + if (codec === "gzip-6") { + return await gunzipAsync(buf); + } + const out = await Bun.zstdDecompress(buf); + return Buffer.from(out.buffer, out.byteOffset, out.byteLength); +} + +async function timeFn( + fn: () => Promise +): Promise<{ avgMs: number; result: T }> { + for (let i = 0; i < WARMUP_ITERS; i++) { + await fn(); + } + const start = performance.now(); + let result: T | undefined; + for (let i = 0; i < MEASURE_ITERS; i++) { + result = await fn(); + } + const elapsed = performance.now() - start; + return { avgMs: elapsed / MEASURE_ITERS, result: result as T }; +} + +// ── Fixture envelopes ──────────────────────────────────────────────── +// Modeled on captures from running the CLI against the real ingest: +// shape and field choice reflect what the SDK actually sends, so the +// compression ratio numbers transfer to production. + +function buildErrorEnvelope(): Buffer { + const eventId = "a".repeat(32); + const header = { event_id: eventId, sent_at: new Date().toISOString() }; + const item = { + event_id: eventId, + level: "error", + platform: "node", + sdk: { name: "sentry.javascript.node-core", version: "10.47.0" }, + exception: { + values: [ + { + type: "TypeError", + value: "Cannot read properties of undefined (reading 'foo')", + stacktrace: { + frames: Array.from({ length: 30 }, (_, i) => ({ + filename: `/home/user/proj/src/file${i}.ts`, + function: `handler${i}`, + lineno: 100 + i, + colno: 10, + pre_context: [" const x = y;", " if (!z) return;"], + context_line: ` return data.${"nested.".repeat(5)}prop;`, + post_context: ["} catch (e) {", " log(e);", "}"], + })), + }, + }, + ], + }, + tags: { + command: "issue.list", + "sentry.runtime": "bun", + cli_version: "0.29.0", + }, + contexts: { + runtime: { name: "bun", version: "1.3.13" }, + os: { name: "darwin", version: "23.4.0" }, + }, + }; + return Buffer.from( + serializeEnvelope( + createEnvelope(header as never, [ + // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity + [{ type: "event" } as any, item as any], + ]) + ) + ); +} + +function buildTransactionEnvelope(): Buffer { + const eventId = "b".repeat(32); + const traceId = "c".repeat(32); + const header = { event_id: eventId, sent_at: new Date().toISOString() }; + const now = Date.now() / 1000; + const spanOps = ["http.client", "db", "file"]; + const spans = Array.from({ length: 60 }, (_, i) => ({ + span_id: String(i).padStart(16, "0"), + trace_id: traceId, + op: spanOps[i % 3], + description: `operation ${i} with a reasonably long description line`, + start_timestamp: now - 1, + timestamp: now, + data: { "http.method": "GET", "url.path": `/api/0/resource/${i}` }, + })); + const item = { + type: "transaction", + event_id: eventId, + transaction: "cli.command", + contexts: { + trace: { trace_id: traceId, span_id: "0000000000000000" }, + }, + spans, + sdk: { name: "sentry.javascript.node-core", version: "10.47.0" }, + }; + return Buffer.from( + serializeEnvelope( + createEnvelope(header as never, [ + // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity + [{ type: "transaction" } as any, item as any], + ]) + ) + ); +} + +function buildLogEnvelope(): Buffer { + const header = { sent_at: new Date().toISOString() }; + const items = Array.from({ length: 20 }, (_, i) => ({ + timestamp: Date.now() / 1000, + trace_id: "d".repeat(32), + level: "info", + body: `log message ${i}: something happened during execution`, + attributes: { command: { value: "issue.list", type: "string" } }, + })); + return Buffer.from( + serializeEnvelope( + createEnvelope(header as never, [ + // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity + [{ type: "log" } as any, { items } as any], + ]) + ) + ); +} + +function buildSessionEnvelope(): Buffer { + const header = { sent_at: new Date().toISOString() }; + const item = { + sid: "e".repeat(32), + did: "user-1", + started: new Date().toISOString(), + status: "exited", + errors: 0, + attrs: { release: "0.29.0", environment: "production" }, + }; + return Buffer.from( + serializeEnvelope( + createEnvelope(header as never, [ + // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity + [{ type: "session" } as any, item as any], + ]) + ) + ); +} + +type BenchRow = { + envelope: string; + rawBytes: number; + codec: Codec; + sentBytes: number; + ratio: number; + compressMs: number; + decompressMs: number; +}; + +async function benchCodec( + envelopeName: string, + buf: Buffer, + codec: Codec +): Promise { + const { avgMs: compressMs, result: compressed } = await timeFn(() => + compress(codec, buf) + ); + const { avgMs: decompressMs } = await timeFn(() => + decompress(codec, compressed) + ); + return { + envelope: envelopeName, + rawBytes: buf.length, + codec, + sentBytes: compressed.length, + ratio: compressed.length / buf.length, + compressMs, + decompressMs, + }; +} + +function formatRow(r: BenchRow): string { + const cols = [ + r.envelope, + String(r.rawBytes), + r.codec, + String(r.sentBytes), + r.ratio.toFixed(3), + r.compressMs.toFixed(3), + r.decompressMs.toFixed(3), + ]; + return `| ${cols.join(" | ")} |`; +} + +async function main(): Promise { + if (typeof Bun?.zstdCompress !== "function") { + process.stderr.write( + "bench-transport: Bun.zstdCompress unavailable — run on Bun or Node >= 22.15 with polyfill installed\n" + ); + process.exit(1); + } + + const fixtures: { name: string; buf: Buffer }[] = [ + { name: "error (30-frame stack)", buf: buildErrorEnvelope() }, + { name: "transaction (60 spans)", buf: buildTransactionEnvelope() }, + { name: "log (20 entries)", buf: buildLogEnvelope() }, + { name: "session", buf: buildSessionEnvelope() }, + ]; + + process.stdout.write( + "# Telemetry transport codec benchmark\n\n" + + `_${WARMUP_ITERS} warmup + ${MEASURE_ITERS} measured iterations per cell._\n\n` + ); + + process.stdout.write( + "| envelope | raw bytes | codec | sent bytes | ratio | compress ms | decompress ms |\n" + ); + process.stdout.write( + "|----------|----------:|-------|-----------:|------:|------------:|--------------:|\n" + ); + + for (const fix of fixtures) { + for (const codec of CODECS) { + const row = await benchCodec(fix.name, fix.buf, codec); + process.stdout.write(`${formatRow(row)}\n`); + } + } +} + +await main(); diff --git a/script/node-polyfills.ts b/script/node-polyfills.ts index 12fb360dc..170b829c4 100644 --- a/script/node-polyfills.ts +++ b/script/node-polyfills.ts @@ -8,6 +8,10 @@ import { } from "node:child_process"; import { statSync } from "node:fs"; import { access, readFile, stat, writeFile } from "node:fs/promises"; +import { promisify } from "node:util"; +// biome-ignore lint/performance/noNamespaceImport: runtime access to optional `zstdCompress`/`zstdDecompress` exports +import * as zlibModule from "node:zlib"; +import { constants as zlibConstants } from "node:zlib"; // node:sqlite is imported lazily inside NodeDatabasePolyfill to avoid // crashing on Node.js versions without node:sqlite support when the // bundle is loaded as a library (the consumer may never use SQLite). @@ -284,4 +288,66 @@ const BunPolyfill = { }, }; +/** + * Coerce arbitrary compression input (string, ArrayBuffer, TypedArray) + * into a contiguous Buffer without copying where possible. + */ +function toBufferForCompression( + data: NodeJS.TypedArray | Buffer | string | ArrayBuffer +): Buffer { + if (typeof data === "string") { + return Buffer.from(data, "utf-8"); + } + if (data instanceof ArrayBuffer) { + return Buffer.from(data); + } + return Buffer.from(data.buffer, data.byteOffset, data.byteLength); +} + +// Feature-detected zstd polyfill. `node:zlib.zstdCompress` lands in Node +// 22.15; we do NOT install the polyfill below on older Node because +// Bun.zstdCompress's absence lets the telemetry transport's runtime +// probe fall back to gzip cleanly (see src/lib/telemetry/zstd-transport.ts). +// +// Kept out of the BunPolyfill literal so the `typeof Bun.zstdCompress` +// probe genuinely returns `"undefined"` on older runtimes — assigning +// a noop stub would defeat the feature-detect. +const zlibOptionalZstdCompress = (zlibModule as { zstdCompress?: unknown }) + .zstdCompress; +const zlibOptionalZstdDecompress = (zlibModule as { zstdDecompress?: unknown }) + .zstdDecompress; + +if ( + typeof zlibOptionalZstdCompress === "function" && + typeof zlibOptionalZstdDecompress === "function" +) { + const nodeZstdCompress = promisify( + zlibOptionalZstdCompress as ( + buf: Buffer, + options: { params?: Record }, + cb: (err: Error | null, result: Buffer) => void + ) => void + ); + const nodeZstdDecompress = promisify( + zlibOptionalZstdDecompress as ( + buf: Buffer, + cb: (err: Error | null, result: Buffer) => void + ) => void + ); + + (BunPolyfill as unknown as { zstdCompress: unknown }).zstdCompress = ( + data: NodeJS.TypedArray | Buffer | string | ArrayBuffer, + opts?: { level?: number } + ): Promise => + nodeZstdCompress(toBufferForCompression(data), { + params: { + [zlibConstants.ZSTD_c_compressionLevel]: opts?.level ?? 3, + }, + }); + + (BunPolyfill as unknown as { zstdDecompress: unknown }).zstdDecompress = ( + data: NodeJS.TypedArray | Buffer | string | ArrayBuffer + ): Promise => nodeZstdDecompress(toBufferForCompression(data)); +} + globalThis.Bun = BunPolyfill as typeof Bun; diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 4a0519cae..2c10bd682 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -30,6 +30,7 @@ import { import { ApiError } from "./errors.js"; import { attachSentryReporter, logger } from "./logger.js"; import { getSentryBaseUrl, isSentrySaasUrl } from "./sentry-urls.js"; +import { makeCompressedTransport } from "./telemetry/zstd-transport.js"; import { getRealUsername } from "./utils.js"; export type { Span } from "@sentry/core"; @@ -518,6 +519,11 @@ export function initSentry( const client = Sentry.init({ dsn: SENTRY_CLI_DSN, enabled, + // Compress outgoing envelopes with zstd (level 3) instead of gzip — + // smaller payloads, faster compress/decompress on both sides. + // Automatic gzip fallback when running on Node < 22.15 without the + // `Bun.zstdCompress` polyfill (see script/node-polyfills.ts). + transport: makeCompressedTransport, // Keep default integrations but filter out ones that add overhead without benefit. // Important: Don't use defaultIntegrations: false as it may break debug ID support. // NodeSystemError is excluded on runtimes missing util.getSystemErrorMap (Bun) — CLI-K1. diff --git a/src/lib/telemetry/zstd-transport-metrics.ts b/src/lib/telemetry/zstd-transport-metrics.ts new file mode 100644 index 000000000..85d71215b --- /dev/null +++ b/src/lib/telemetry/zstd-transport-metrics.ts @@ -0,0 +1,106 @@ +/** + * Opt-in transport-level metrics for the Sentry SDK telemetry pipeline. + * + * When `SENTRY_TRANSPORT_METRICS=1` is set, {@link emitTransportMetric} + * writes a single line of JSON to stderr per outbound envelope. Off by + * default — no cost in production beyond a single env-var read. + * + * Emitted fields: + * - `ts` epoch ms when the metric was emitted + * - `kind` always `"sentry_transport"` (for greppability) + * - `envelope_type` first-item type (`"event"`, `"transaction"`, + * `"log"`, `"session"`, `"client_report"`, …) or + * `"unknown"` if the envelope couldn't be parsed + * - `encoding` `"zstd"` | `"gzip"` | `"none"` + * - `raw_bytes` body size before compression + * - `sent_bytes` body size on the wire + * - `compress_ms` wall-clock compress time in milliseconds (0 if + * the body was under the compression threshold) + * - `ratio` sent / raw (1.0 if no compression) + * + * The envelope-type sniff is passed as a callback so we only pay its + * JSON-parse cost when the metric is actually emitted. + */ + +import { getEnv } from "../env.js"; + +export type TransportEncoding = "zstd" | "gzip" | "none"; + +export type TransportMetricInput = { + rawBytes: number; + sentBytes: number; + compressMs: number; + encoding: TransportEncoding; + /** Lazy envelope-type extractor. Invoked only when the metric is emitted. */ + envelopeType: () => string | undefined; +}; + +/** Emit a single JSON line to stderr iff `SENTRY_TRANSPORT_METRICS=1`. */ +export function emitTransportMetric(m: TransportMetricInput): void { + if (getEnv().SENTRY_TRANSPORT_METRICS !== "1") { + return; + } + const ratio = m.rawBytes > 0 ? m.sentBytes / m.rawBytes : 1; + const line = JSON.stringify({ + ts: Date.now(), + kind: "sentry_transport", + envelope_type: m.envelopeType() ?? "unknown", + encoding: m.encoding, + raw_bytes: m.rawBytes, + sent_bytes: m.sentBytes, + compress_ms: Number(m.compressMs.toFixed(2)), + ratio: Number(ratio.toFixed(3)), + }); + process.stderr.write(`${line}\n`); +} + +/** + * Peek at the first envelope item header to classify the envelope. + * + * Envelope wire format (https://develop.sentry.dev/sdk/envelopes/): + * + * {envelope_header}\n + * {item_1_header}\n + * {item_1_body}\n + * {item_2_header}\n + * ... + * + * Only the first item's `type` field is needed. We decode at most the + * first ~512 bytes (well beyond the largest reasonable item header) to + * avoid slurping the whole envelope for a classification hint. + * + * @param body Raw envelope bytes (pre-compression) or string. + * @returns The first item's `type` or `undefined` if the envelope can't + * be parsed (empty, truncated, non-JSON item header). + */ +export function detectEnvelopeType( + body: Buffer | Uint8Array | string +): string | undefined { + const text = + typeof body === "string" + ? body.slice(0, 512) + : Buffer.from( + body.buffer, + body.byteOffset, + Math.min(body.byteLength, 512) + ).toString("utf-8"); + + const firstNl = text.indexOf("\n"); + if (firstNl < 0) { + return; + } + const secondNl = text.indexOf("\n", firstNl + 1); + const itemHeader = text.slice( + firstNl + 1, + secondNl > 0 ? secondNl : undefined + ); + if (!itemHeader) { + return; + } + try { + const parsed = JSON.parse(itemHeader) as { type?: unknown }; + return typeof parsed.type === "string" ? parsed.type : undefined; + } catch { + return; + } +} diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts new file mode 100644 index 000000000..618edb544 --- /dev/null +++ b/src/lib/telemetry/zstd-transport.ts @@ -0,0 +1,363 @@ +/** + * Custom Sentry SDK transport factory with zstd-first compression. + * + * Wraps `createTransport` from @sentry/core with a custom request executor + * that compresses outgoing envelope bodies with zstd level 3 (libzstd + * default). When running on a host without zstd support (Node < 22.15 + * without the polyfill installed), falls back to gzip — matches the + * SDK's default behavior byte-for-byte so there's no regression. + * + * Codec selection is one-shot, performed at factory-construction time. + * No per-request branching: if `Bun.zstdCompress` is available when the + * transport is created, every envelope uses zstd; otherwise every + * envelope uses gzip. The choice is reflected in the metric emitted by + * {@link emitTransportMetric} so the ratio of zstd-vs-gzip callers can + * be observed in the real world. + * + * This mirrors `@sentry/node-core/transports/http.js` `makeNodeTransport` + * — URL parsing, `no_proxy` handling, proxy agent, CA certs, keepAlive, + * IPv6 hostname unwrapping, rate-limit response header normalization — + * but swaps out the gzip-via-stream-pipe for an async one-shot compress + * that sets the correct `Content-Encoding`. + * + * Why not patch the SDK: + * `Sentry.init({ transport })` is a first-class extension point on + * the Client. Going via a custom factory avoids patch-file maintenance + * across SDK upgrades and avoids the gzip→gunzip→zstd waste that an + * `httpModule` shim would incur (the SDK has already piped the body + * through `createGzip()` by the time `httpModule.request()` runs). + */ + +// biome-ignore lint/performance/noNamespaceImport: http module must be passed as a namespace object (matches SDK's HTTPModule interface) +import * as http from "node:http"; +// biome-ignore lint/performance/noNamespaceImport: same as above for https +import * as https from "node:https"; +import { Readable } from "node:stream"; +import { promisify } from "node:util"; +import { gzip as gzipCb } from "node:zlib"; +import { + createTransport, + suppressTracing, + type Transport, + type TransportMakeRequestResponse, + type TransportRequest, + type TransportRequestExecutor, +} from "@sentry/core"; +import { + makeNodeTransport, + type NodeTransportOptions, +} from "@sentry/node-core/light"; +import { + detectEnvelopeType, + emitTransportMetric, + type TransportEncoding, +} from "./zstd-transport-metrics.js"; + +/** + * zstd compression level. L3 is libzstd's default — benchmarks across + * envelope sizes (1–30 KiB) showed L3–L6 sit on the same ratio-vs-time + * curve for this workload; L3 is the safe operating point until the + * offline bench pins a different value. See `script/bench-transport.ts`. + */ +const ZSTD_LEVEL = 3; + +/** + * Minimum body length above which we attempt compression. + * + * For zstd we lower this from the SDK's 32 KiB gzip threshold to 1 KiB + * — Bun's zstd worker is cheap to dispatch and most error envelopes + * (5–15 KiB) would miss the 32 KiB cutoff and ship uncompressed + * otherwise. + */ +const ZSTD_THRESHOLD = 1024; + +/** + * Matches the SDK default. Kept identical to avoid any byte-level + * regression when the zstd fast path is unavailable. + */ +const GZIP_THRESHOLD = 1024 * 32; + +/** + * Shape of the globalThis.Bun subset we rely on. Bun's real types + * declare this, but the transport also runs under Node (via the + * feature-detected polyfill in `script/node-polyfills.ts`) where only + * a subset of Bun APIs are installed. + */ +type BunZstdHost = { + zstdCompress?: ( + data: Uint8Array | Buffer | string | ArrayBuffer, + options?: { level?: number } + ) => Promise; +}; + +const gzipAsync = promisify(gzipCb); + +/** Factory — see module docs. */ +export function makeCompressedTransport( + options: NodeTransportOptions +): Transport { + // When a proxy is configured (via options.proxy or http_proxy / + // https_proxy env vars and not overridden by no_proxy), fall back to + // the SDK's default makeNodeTransport — it owns the CONNECT-tunneling + // HttpsProxyAgent. Proxy users thus continue to get gzip (the SDK + // default), but correctness wins over micro-optimizing an edge case. + let urlSegments: URL; + try { + urlSegments = new URL(options.url); + } catch { + // Mirror makeNodeTransport: return a no-op transport on bad URL so + // the SDK doesn't throw at init time on misconfigured DSNs. + return createTransport(options, () => Promise.resolve({})); + } + + if (shouldFallbackToDefault(urlSegments, options)) { + return makeNodeTransport(options); + } + + const isHttps = urlSegments.protocol === "https:"; + const nativeHttpModule = isHttps ? https : http; + const keepAlive = options.keepAlive ?? false; + const agent = new nativeHttpModule.Agent({ + keepAlive, + maxSockets: 30, + timeout: 2000, + }); + + const httpModule = options.httpModule ?? nativeHttpModule; + + // One-shot codec selection. Frozen into the executor closure below. + const encoding: Exclude = hasZstdSupport() + ? "zstd" + : "gzip"; + + const executor = createCompressingExecutor({ + options, + httpModule, + agent, + encoding, + }); + + return createTransport(options, executor); +} + +/** + * True iff a proxy is configured for this URL and not exempted by + * no_proxy. When true, the caller falls back to the SDK's default + * transport (which handles CONNECT tunneling). + */ +function shouldFallbackToDefault( + url: URL, + options: NodeTransportOptions +): boolean { + const isHttps = url.protocol === "https:"; + const envProxy = isHttps ? process.env.https_proxy : process.env.http_proxy; + const proxy = options.proxy || envProxy || process.env.http_proxy; + if (!proxy) { + return false; + } + return !isNoProxyExempt(url, proxy); +} + +/** + * @internal Exported for tests. Builds the bare HTTP executor without + * any of `makeCompressedTransport`'s URL / proxy / agent plumbing — the + * caller supplies a fully resolved {@link http.Agent} and the {@link + * http.request}-compatible module to use. + */ +export function createCompressingExecutor(args: { + options: NodeTransportOptions; + httpModule: NonNullable; + agent: http.Agent; + encoding: Exclude; +}): TransportRequestExecutor { + const { options, httpModule, agent, encoding } = args; + const { hostname, pathname, port, protocol, search } = new URL(options.url); + const hostnameIsIPv6 = hostname.startsWith("["); + + return (request: TransportRequest) => + new Promise((resolve, reject) => { + suppressTracing(() => { + performRequest({ + request, + options, + httpModule, + agent, + encoding, + hostname: hostnameIsIPv6 ? hostname.slice(1, -1) : hostname, + path: `${pathname}${search}`, + port, + protocol, + }) + .then(resolve) + .catch(reject); + }); + }); +} + +type PerformRequestArgs = { + request: TransportRequest; + options: NodeTransportOptions; + httpModule: NonNullable; + agent: http.Agent; + encoding: Exclude; + hostname: string; + path: string; + port: string; + protocol: string; +}; + +async function performRequest( + args: PerformRequestArgs +): Promise { + const { request, options, httpModule, agent, encoding } = args; + + const rawBuffer = normalizeBody(request.body); + const { payload, encodingApplied, compressMs } = await maybeCompress( + rawBuffer, + encoding + ); + + const headers: Record = { ...(options.headers ?? {}) }; + if (encodingApplied !== "none") { + headers["content-encoding"] = encodingApplied; + } + + emitTransportMetric({ + rawBytes: rawBuffer.length, + sentBytes: payload.length, + compressMs, + encoding: encodingApplied, + envelopeType: () => detectEnvelopeType(rawBuffer), + }); + + return new Promise((resolve, reject) => { + const req = httpModule.request( + { + method: "POST", + agent, + headers, + hostname: args.hostname, + path: args.path, + port: args.port, + protocol: args.protocol, + ca: options.caCerts, + }, + (res) => { + res.on("data", () => { + // Drain socket + }); + res.on("end", () => { + // Drain socket + }); + res.setEncoding("utf8"); + + const retryAfterHeader = res.headers["retry-after"] ?? null; + const rateLimitsHeader = res.headers["x-sentry-rate-limits"] ?? null; + + resolve({ + statusCode: res.statusCode, + headers: { + "retry-after": Array.isArray(retryAfterHeader) + ? (retryAfterHeader[0] ?? null) + : retryAfterHeader, + "x-sentry-rate-limits": Array.isArray(rateLimitsHeader) + ? (rateLimitsHeader[0] ?? null) + : rateLimitsHeader, + }, + }); + } + ); + + req.on("error", reject); + // Single-shot write. `payload` is already a complete Buffer in + // memory (compressed or not), so piping a fresh Readable through + // avoids the SDK's stream-gzip dance without changing the wire + // behavior — `http.ClientRequest` still sees a body it can send. + Readable.from(payload).pipe(req); + }); +} + +/** Coerce `string | Uint8Array` into a single contiguous Buffer. */ +function normalizeBody(body: string | Uint8Array): Buffer { + if (typeof body === "string") { + return Buffer.from(body, "utf-8"); + } + // Buffer.from(view) copies; but Buffer.from(view.buffer, byteOffset, + // byteLength) is zero-copy and gives us a Buffer that aliases the + // original bytes — exactly what we want before handing off to the + // compression worker. + return Buffer.from(body.buffer, body.byteOffset, body.byteLength); +} + +type CompressResult = { + payload: Buffer; + encodingApplied: TransportEncoding; + compressMs: number; +}; + +/** + * Apply the pre-selected codec iff the body is large enough to benefit. + * Under threshold → passthrough with `encoding: "none"` (matches SDK + * default behavior). + */ +async function maybeCompress( + buf: Buffer, + encoding: Exclude +): Promise { + const threshold = encoding === "zstd" ? ZSTD_THRESHOLD : GZIP_THRESHOLD; + if (buf.length <= threshold) { + return { payload: buf, encodingApplied: "none", compressMs: 0 }; + } + + const start = performance.now(); + if (encoding === "zstd") { + const bun = (globalThis as { Bun?: BunZstdHost }).Bun; + // Shouldn't happen (factory checked at construction time), but a + // belt-and-braces fallback to gzip keeps us correct if `Bun` is + // swapped out between construction and first send. + if (!bun?.zstdCompress) { + const gz = await gzipAsync(buf); + return { + payload: gz, + encodingApplied: "gzip", + compressMs: performance.now() - start, + }; + } + const out = await bun.zstdCompress(buf, { level: ZSTD_LEVEL }); + return { + payload: Buffer.from(out.buffer, out.byteOffset, out.byteLength), + encodingApplied: "zstd", + compressMs: performance.now() - start, + }; + } + + const gz = await gzipAsync(buf); + return { + payload: gz, + encodingApplied: "gzip", + compressMs: performance.now() - start, + }; +} + +/** Feature-detect zstd support on the current runtime. */ +export function hasZstdSupport(): boolean { + const bun = (globalThis as { Bun?: BunZstdHost }).Bun; + return typeof bun?.zstdCompress === "function"; +} + +/** + * Mirror the SDK's `applyNoProxyOption`: returns true iff the target + * URL matches an entry in `NO_PROXY` / `no_proxy`, in which case the + * proxy should be ignored. + */ +function isNoProxyExempt(urlSegments: URL, _proxy: string): boolean { + const noProxy = process.env.no_proxy ?? process.env.NO_PROXY; + if (!noProxy) { + return false; + } + return noProxy + .split(",") + .some( + (ex) => urlSegments.host.endsWith(ex) || urlSegments.hostname.endsWith(ex) + ); +} diff --git a/test/lib/telemetry/zstd-transport.e2e.test.ts b/test/lib/telemetry/zstd-transport.e2e.test.ts new file mode 100644 index 000000000..949b14e09 --- /dev/null +++ b/test/lib/telemetry/zstd-transport.e2e.test.ts @@ -0,0 +1,148 @@ +/** + * E2E tests for the zstd transport. + * + * Spins up a real `http.createServer` on `127.0.0.1:0`, points the + * transport at it, and verifies the wire-level behavior: the request + * body is zstd-compressed, the `Content-Encoding` header is correct, + * and the body decompresses back to a valid envelope. + * + * Uses `useTestConfigDir()` for DB isolation (see AGENTS.md). + */ + +import { afterAll, describe, expect, test } from "bun:test"; + +/** No-op for SDK callbacks that require a function but return nothing meaningful. */ +function noop(): void { + // intentionally empty +} + +import { createServer, type IncomingMessage, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { createEnvelope } from "@sentry/core"; +import { + hasZstdSupport, + makeCompressedTransport, +} from "../../../src/lib/telemetry/zstd-transport.js"; +import { useTestConfigDir } from "../../helpers.js"; + +type CapturedRequest = { + headers: IncomingMessage["headers"]; + body: Buffer; +}; + +function startMockIngest( + responder: (req: IncomingMessage) => { + statusCode: number; + headers?: Record; + } +): Promise<{ + server: Server; + url: string; + captures: CapturedRequest[]; +}> { + const captures: CapturedRequest[] = []; + const server = createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (c: Buffer) => chunks.push(c)); + req.on("end", () => { + captures.push({ headers: req.headers, body: Buffer.concat(chunks) }); + const response = responder(req); + res.writeHead(response.statusCode, response.headers ?? {}); + res.end(); + }); + }); + + return new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => { + const addr = server.address() as AddressInfo; + resolve({ + server, + url: `http://127.0.0.1:${addr.port}/api/0/envelope/`, + captures, + }); + }); + }); +} + +useTestConfigDir("zstd-transport-e2e-"); + +describe("makeCompressedTransport (e2e)", () => { + let server: Server | undefined; + + afterAll(() => { + server?.close(); + }); + + test("sends zstd-encoded envelope; server decompresses back to original", async () => { + if (!hasZstdSupport()) { + return; + } + + const { + server: srv, + url, + captures, + } = await startMockIngest(() => ({ + statusCode: 200, + })); + server = srv; + + const transport = makeCompressedTransport({ + url, + recordDroppedEvent: noop, + }); + + // Sizable envelope — above the 1 KiB zstd threshold. + const message = "x".repeat(4096); + const envelope: any = createEnvelope({ event_id: "abc" } as any, [ + [{ type: "event" } as any, { message } as any], + ]); + + const response = await transport.send(envelope); + expect(response.statusCode).toBe(200); + + expect(captures).toHaveLength(1); + expect(captures[0]?.headers["content-encoding"]).toBe("zstd"); + + const decompressed = await Bun.zstdDecompress(captures[0]!.body); + const text = Buffer.from( + decompressed.buffer, + decompressed.byteOffset, + decompressed.byteLength + ).toString("utf-8"); + expect(text).toContain(message); + expect(text).toContain('"type":"event"'); + }); + + test("rate-limit headers flow back into createTransport wrapper", async () => { + const { + server: srv, + url, + captures, + } = await startMockIngest(() => ({ + statusCode: 429, + headers: { + "retry-after": "60", + "x-sentry-rate-limits": "60:error:organization", + }, + })); + server = srv; + + const transport = makeCompressedTransport({ + url, + recordDroppedEvent: noop, + }); + + const envelope: any = createEnvelope({ event_id: "a" } as any, [ + [{ type: "event" } as any, { message: "hi" } as any], + ]); + const response = await transport.send(envelope); + + expect(response.statusCode).toBe(429); + expect(response.headers?.["retry-after"]).toBe("60"); + expect(response.headers?.["x-sentry-rate-limits"]).toBe( + "60:error:organization" + ); + expect(captures).toHaveLength(1); + }); +}); diff --git a/test/lib/telemetry/zstd-transport.property.test.ts b/test/lib/telemetry/zstd-transport.property.test.ts new file mode 100644 index 000000000..320aeb317 --- /dev/null +++ b/test/lib/telemetry/zstd-transport.property.test.ts @@ -0,0 +1,74 @@ +/** + * Property tests for the zstd-first transport codec. + * + * Any input bytes that go through zstd compression must round-trip + * byte-for-byte when decompressed. Also verifies that string and + * UTF-8-equivalent Uint8Array inputs produce identical wire output — + * so the executor's string-vs-bytes normalization is on-spec. + */ + +import { describe, expect, test } from "bun:test"; +import { + asyncProperty, + assert as fcAssert, + string, + uint8Array, +} from "fast-check"; +import { DEFAULT_NUM_RUNS } from "../../model-based/helpers.js"; + +describe("property: zstd round-trip", () => { + test("Bun.zstdCompress(b) → Bun.zstdDecompress === b for all byte sequences", async () => { + await fcAssert( + asyncProperty( + uint8Array({ minLength: 0, maxLength: 64 * 1024 }), + async (bytes) => { + const buf = Buffer.from(bytes); + const compressed = await Bun.zstdCompress(buf, { level: 3 }); + const decompressed = await Bun.zstdDecompress(compressed); + expect(Buffer.from(decompressed).equals(buf)).toBe(true); + } + ), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("level sweep (3, 5, 6, 9) all round-trip identically", async () => { + await fcAssert( + asyncProperty( + uint8Array({ minLength: 64, maxLength: 8 * 1024 }), + async (bytes) => { + const buf = Buffer.from(bytes); + for (const level of [3, 5, 6, 9]) { + const compressed = await Bun.zstdCompress(buf, { level }); + const decompressed = await Bun.zstdDecompress(compressed); + expect(Buffer.from(decompressed).equals(buf)).toBe(true); + } + } + ), + { numRuns: 25 } + ); + }); + + test("string and equivalent Uint8Array inputs produce equal compressed output", async () => { + await fcAssert( + asyncProperty( + string({ minLength: 32, maxLength: 16 * 1024 }), + async (s) => { + const fromString = await Bun.zstdCompress(Buffer.from(s, "utf-8"), { + level: 3, + }); + const fromBytes = await Bun.zstdCompress( + new TextEncoder().encode(s), + { + level: 3, + } + ); + expect(Buffer.from(fromString).equals(Buffer.from(fromBytes))).toBe( + true + ); + } + ), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts new file mode 100644 index 000000000..4416db047 --- /dev/null +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -0,0 +1,287 @@ +/** + * Unit tests for {@link makeCompressedTransport}. + * + * Uses the SDK's `options.httpModule` extension point to inject a mock + * that captures the bytes written to the ClientRequest and synthesizes + * a response. We don't touch real sockets. + * + * Each scenario asserts on one of: + * 1. the `content-encoding` header as observed on the wire, + * 2. the compressed body round-tripping back to the input, + * 3. rate-limit response headers surfacing correctly to the + * `createTransport` layer. + */ + +import { afterEach, describe, expect, test } from "bun:test"; +import { EventEmitter } from "node:events"; +import type { ClientRequest, IncomingHttpHeaders } from "node:http"; +import { gunzipSync } from "node:zlib"; +import { createEnvelope } from "@sentry/core"; +import { + hasZstdSupport, + makeCompressedTransport, +} from "../../../src/lib/telemetry/zstd-transport.js"; + +/** No-op for SDK callbacks that require a function but return nothing meaningful. */ +function noop(): void { + // intentionally empty +} + +// ── Mock http module ───────────────────────────────────────────────── + +type MockResponseShape = { + statusCode: number; + headers: IncomingHttpHeaders; +}; + +type CapturedRequest = { + chunks: Buffer[]; + options: Record; +}; + +/** + * Build a fake {@link http} module that captures outbound body bytes and + * resolves with a pre-canned response. The returned object exposes both + * the shim and the capture buffer for test assertions. + */ +function buildMockHttpModule(response: MockResponseShape): { + httpModule: { + request: (opts: unknown, cb?: (res: unknown) => void) => ClientRequest; + }; + captured: CapturedRequest; +} { + const captured: CapturedRequest = { chunks: [], options: {} }; + const httpModule = { + request: (opts: unknown, cb?: (res: unknown) => void) => { + captured.options = opts as Record; + + // The ClientRequest must behave as a Writable so that + // `Readable.from(payload).pipe(req)` succeeds. We fake one from + // an EventEmitter and expose `write`/`end` that push into the + // captured buffer. + const req = new EventEmitter() as unknown as ClientRequest & { + write: (chunk: Buffer | string) => boolean; + end: (chunk?: Buffer | string) => void; + }; + req.write = (chunk: Buffer | string) => { + captured.chunks.push( + Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk) + ); + return true; + }; + req.end = (chunk?: Buffer | string) => { + if (chunk !== undefined) { + captured.chunks.push( + Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk) + ); + } + // Fire the response on next tick so the caller has time to + // finish piping before we resolve. The response object is a + // minimal IncomingMessage-like shape that the transport's + // handler uses. + process.nextTick(() => { + const res = Object.assign(new EventEmitter(), { + statusCode: response.statusCode, + headers: response.headers, + setEncoding: noop, + }); + cb?.(res); + // Drive data/end events the transport listens to. + process.nextTick(() => { + res.emit("data", Buffer.alloc(0)); + res.emit("end"); + }); + }); + }; + return req; + }, + }; + return { httpModule: httpModule as never, captured }; +} + +// ── Tests ──────────────────────────────────────────────────────────── + +const BASE_OPTIONS = { + url: "https://ingest.example.com/api/0/envelope/", + headers: { "x-sentry-auth": "Sentry sentry_key=abc" }, + recordDroppedEvent: noop, +}; + +describe("makeCompressedTransport", () => { + let savedZstd: typeof globalThis.Bun.zstdCompress | undefined; + + afterEach(() => { + // Restore Bun.zstdCompress after tests that stash it. + if (savedZstd !== undefined) { + globalThis.Bun.zstdCompress = savedZstd; + savedZstd = undefined; + } + }); + + test("zstd branch: sets Content-Encoding: zstd and round-trips", async () => { + if (!hasZstdSupport()) { + // On a runtime without zstd this test is meaningless. + return; + } + const { httpModule, captured } = buildMockHttpModule({ + statusCode: 200, + headers: {}, + }); + + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + httpModule, + }); + + // Build a large envelope — above ZSTD_THRESHOLD (1 KiB). + const payload = "x".repeat(4096); + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { data: payload } as any], + ]); + await transport.send(envelope); + + const headers = captured.options.headers as Record; + expect(headers["content-encoding"]).toBe("zstd"); + + const wire = Buffer.concat(captured.chunks); + expect(wire.length).toBeGreaterThan(0); + + // Decompress and verify the payload body is present + const decompressed = await Bun.zstdDecompress(wire); + const text = Buffer.from( + decompressed.buffer, + decompressed.byteOffset, + decompressed.byteLength + ).toString("utf-8"); + expect(text).toContain(payload); + }); + + test("gzip fallback: Bun.zstdCompress absent → Content-Encoding: gzip", async () => { + savedZstd = globalThis.Bun.zstdCompress; + // Stash + remove zstd to force the gzip branch + (globalThis as { Bun: { zstdCompress?: unknown } }).Bun.zstdCompress = + undefined as never; + + try { + const { httpModule, captured } = buildMockHttpModule({ + statusCode: 200, + headers: {}, + }); + + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + httpModule, + }); + + // Build a payload > GZIP_THRESHOLD (32 KiB). + const payload = "y".repeat(64 * 1024); + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { data: payload } as any], + ]); + await transport.send(envelope); + + const headers = captured.options.headers as Record; + expect(headers["content-encoding"]).toBe("gzip"); + + const wire = Buffer.concat(captured.chunks); + const decompressed = gunzipSync(wire); + const text = decompressed.toString("utf-8"); + expect(text).toContain(payload); + } finally { + // afterEach restores savedZstd + } + }); + + test("below threshold: no content-encoding header", async () => { + const { httpModule, captured } = buildMockHttpModule({ + statusCode: 200, + headers: {}, + }); + + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + httpModule, + }); + + // Tiny envelope - well below 1 KiB zstd threshold + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { tiny: "x" } as any], + ]); + await transport.send(envelope); + + const headers = captured.options.headers as Record; + expect(headers["content-encoding"]).toBeUndefined(); + + // The wire body should be exactly the serialized envelope + const wire = Buffer.concat(captured.chunks); + expect(wire.toString("utf-8")).toContain("event"); + }); + + test("rate-limit response headers bubble up (string form)", async () => { + const { httpModule } = buildMockHttpModule({ + statusCode: 429, + headers: { + "retry-after": "60", + "x-sentry-rate-limits": "60:error:organization", + }, + }); + + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + httpModule, + }); + + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { data: "small" } as any], + ]); + const response = await transport.send(envelope); + + expect(response.statusCode).toBe(429); + expect(response.headers?.["retry-after"]).toBe("60"); + expect(response.headers?.["x-sentry-rate-limits"]).toBe( + "60:error:organization" + ); + }); + + test("rate-limit response headers normalize array form", async () => { + const { httpModule } = buildMockHttpModule({ + statusCode: 429, + // Some proxies emit duplicate headers as arrays + headers: { + "retry-after": "30", + "x-sentry-rate-limits": [ + "30:transaction:organization", + "60:error:organization", + ] as never, + }, + }); + + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + httpModule, + }); + + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { data: "small" } as any], + ]); + const response = await transport.send(envelope); + + // Array collapsed to first element + expect(response.headers?.["x-sentry-rate-limits"]).toBe( + "30:transaction:organization" + ); + }); + + test("invalid URL: no-op transport returned", async () => { + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + url: "not a url", + }); + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, {} as any], + ]); + // No-op transport resolves with empty response, does not throw. + const response = await transport.send(envelope); + expect(response).toEqual({}); + }); +}); From abec923e829bc89d3615da67c3cea77a5b08968d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 24 Apr 2026 17:37:54 +0000 Subject: [PATCH 2/9] test(telemetry): boost zstd-transport patch coverage to 99% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/telemetry/zstd-transport.ts | 18 +- .../telemetry/zstd-transport-metrics.test.ts | 237 ++++++++++++++++ test/lib/telemetry/zstd-transport.test.ts | 265 +++++++++++++++++- 3 files changed, 514 insertions(+), 6 deletions(-) create mode 100644 test/lib/telemetry/zstd-transport-metrics.test.ts diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index 618edb544..ecebc24f5 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -155,7 +155,7 @@ function shouldFallbackToDefault( if (!proxy) { return false; } - return !isNoProxyExempt(url, proxy); + return !isNoProxyExempt(url); } /** @@ -277,8 +277,12 @@ async function performRequest( }); } -/** Coerce `string | Uint8Array` into a single contiguous Buffer. */ -function normalizeBody(body: string | Uint8Array): Buffer { +/** + * Coerce `string | Uint8Array` into a single contiguous Buffer. + * + * @internal Exported for tests. + */ +export function normalizeBody(body: string | Uint8Array): Buffer { if (typeof body === "string") { return Buffer.from(body, "utf-8"); } @@ -299,8 +303,10 @@ type CompressResult = { * Apply the pre-selected codec iff the body is large enough to benefit. * Under threshold → passthrough with `encoding: "none"` (matches SDK * default behavior). + * + * @internal Exported for tests. */ -async function maybeCompress( +export async function maybeCompress( buf: Buffer, encoding: Exclude ): Promise { @@ -349,8 +355,10 @@ export function hasZstdSupport(): boolean { * Mirror the SDK's `applyNoProxyOption`: returns true iff the target * URL matches an entry in `NO_PROXY` / `no_proxy`, in which case the * proxy should be ignored. + * + * @internal Exported for tests. */ -function isNoProxyExempt(urlSegments: URL, _proxy: string): boolean { +export function isNoProxyExempt(urlSegments: URL): boolean { const noProxy = process.env.no_proxy ?? process.env.NO_PROXY; if (!noProxy) { return false; diff --git a/test/lib/telemetry/zstd-transport-metrics.test.ts b/test/lib/telemetry/zstd-transport-metrics.test.ts new file mode 100644 index 000000000..9ecb0facb --- /dev/null +++ b/test/lib/telemetry/zstd-transport-metrics.test.ts @@ -0,0 +1,237 @@ +/** + * Unit tests for the transport-metrics emitter. + * + * Exercises the two public helpers directly: + * - `emitTransportMetric` is gated on `SENTRY_TRANSPORT_METRICS=1`, + * and when enabled writes one JSON line to stderr with the + * expected shape. + * - `detectEnvelopeType` parses the first item header out of a + * real envelope wire format, and returns `undefined` on malformed + * input (empty, truncated, non-JSON header). + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { createEnvelope, serializeEnvelope } from "@sentry/core"; +import { + detectEnvelopeType, + emitTransportMetric, +} from "../../../src/lib/telemetry/zstd-transport-metrics.ts"; + +const METRICS_ENV_VAR = "SENTRY_TRANSPORT_METRICS"; + +/** Lazy envelope-type stub that returns undefined (matches the envelope + * parser's failure case). Declared as a named function rather than + * `() => undefined` to satisfy biome's `noUselessUndefined` rule. */ +function noEnvelopeType(): string | undefined { + return; +} + +/** + * Capture `process.stderr.write` calls into an array for the duration + * of a callback. Restores the original writer unconditionally. + */ +function captureStderr(fn: () => T): { lines: string[]; result: T } { + const lines: string[] = []; + const original = process.stderr.write.bind(process.stderr); + process.stderr.write = ((chunk: string | Uint8Array, ...rest: any[]) => { + const text = + typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf-8"); + lines.push(text); + return original(chunk, ...rest); + }) as typeof process.stderr.write; + try { + const result = fn(); + return { lines, result }; + } finally { + process.stderr.write = original; + } +} + +describe("emitTransportMetric", () => { + let savedEnv: string | undefined; + + beforeEach(() => { + savedEnv = process.env[METRICS_ENV_VAR]; + }); + + afterEach(() => { + if (savedEnv === undefined) { + delete process.env[METRICS_ENV_VAR]; + } else { + process.env[METRICS_ENV_VAR] = savedEnv; + } + }); + + test("off by default: emits nothing", () => { + delete process.env[METRICS_ENV_VAR]; + const { lines } = captureStderr(() => + emitTransportMetric({ + rawBytes: 100, + sentBytes: 50, + compressMs: 1.5, + encoding: "zstd", + envelopeType: () => "event", + }) + ); + expect(lines).toHaveLength(0); + }); + + test("enabled: emits one JSON line with expected fields", () => { + process.env[METRICS_ENV_VAR] = "1"; + const { lines } = captureStderr(() => + emitTransportMetric({ + rawBytes: 1000, + sentBytes: 250, + compressMs: 1.234_567, + encoding: "zstd", + envelopeType: () => "event", + }) + ); + expect(lines).toHaveLength(1); + const payload = JSON.parse(lines[0]!.trim()) as Record; + expect(payload.kind).toBe("sentry_transport"); + expect(payload.envelope_type).toBe("event"); + expect(payload.encoding).toBe("zstd"); + expect(payload.raw_bytes).toBe(1000); + expect(payload.sent_bytes).toBe(250); + expect(payload.compress_ms).toBe(1.23); // rounded to 2 decimals + expect(payload.ratio).toBe(0.25); // 250/1000 rounded to 3 + expect(typeof payload.ts).toBe("number"); + }); + + test("envelopeType returning undefined → 'unknown'", () => { + process.env[METRICS_ENV_VAR] = "1"; + const { lines } = captureStderr(() => + emitTransportMetric({ + rawBytes: 100, + sentBytes: 100, + compressMs: 0, + encoding: "none", + envelopeType: noEnvelopeType, + }) + ); + const payload = JSON.parse(lines[0]!.trim()) as Record; + expect(payload.envelope_type).toBe("unknown"); + expect(payload.encoding).toBe("none"); + expect(payload.ratio).toBe(1); + }); + + test("rawBytes=0 → ratio defaults to 1 (no division by zero)", () => { + process.env[METRICS_ENV_VAR] = "1"; + const { lines } = captureStderr(() => + emitTransportMetric({ + rawBytes: 0, + sentBytes: 0, + compressMs: 0, + encoding: "none", + envelopeType: () => "event", + }) + ); + const payload = JSON.parse(lines[0]!.trim()) as Record; + expect(payload.ratio).toBe(1); + }); + + test("any value other than '1' leaves the emitter off", () => { + process.env[METRICS_ENV_VAR] = "true"; + const { lines } = captureStderr(() => + emitTransportMetric({ + rawBytes: 100, + sentBytes: 100, + compressMs: 0, + encoding: "none", + envelopeType: () => "event", + }) + ); + expect(lines).toHaveLength(0); + }); + + test("gzip encoding surfaces in the emitted line", () => { + process.env[METRICS_ENV_VAR] = "1"; + const { lines } = captureStderr(() => + emitTransportMetric({ + rawBytes: 5000, + sentBytes: 500, + compressMs: 0.5, + encoding: "gzip", + envelopeType: () => "transaction", + }) + ); + const payload = JSON.parse(lines[0]!.trim()) as Record; + expect(payload.encoding).toBe("gzip"); + expect(payload.envelope_type).toBe("transaction"); + }); +}); + +describe("detectEnvelopeType", () => { + test("extracts type from a real event envelope", () => { + const envelope = createEnvelope({ event_id: "a".repeat(32) } as any, [ + [{ type: "event" } as any, { message: "hi" } as any], + ]); + const wire = serializeEnvelope(envelope); + expect(detectEnvelopeType(wire)).toBe("event"); + }); + + test("extracts type from a transaction envelope (string input)", () => { + const wire = + '{"event_id":"a"}\n{"type":"transaction","length":17}\n{"some":"span"}\n'; + expect(detectEnvelopeType(wire)).toBe("transaction"); + }); + + test("extracts type from a Buffer input", () => { + const wire = '{"event_id":"a"}\n{"type":"log"}\n{"items":[]}\n'; + expect(detectEnvelopeType(Buffer.from(wire, "utf-8"))).toBe("log"); + }); + + test("extracts type from a Uint8Array input", () => { + const wire = '{"event_id":"a"}\n{"type":"session"}\n{"sid":"x"}\n'; + expect(detectEnvelopeType(new TextEncoder().encode(wire))).toBe("session"); + }); + + test("returns undefined on empty input", () => { + expect(detectEnvelopeType("")).toBeUndefined(); + expect(detectEnvelopeType(Buffer.alloc(0))).toBeUndefined(); + }); + + test("returns undefined on single-line (no newline) input", () => { + expect(detectEnvelopeType("{}")).toBeUndefined(); + }); + + test("returns undefined when the item header is not JSON", () => { + expect(detectEnvelopeType('{}\nnot json\n{"body":1}\n')).toBeUndefined(); + }); + + test("returns undefined when the item header has a non-string type", () => { + expect(detectEnvelopeType('{}\n{"type":42}\n{}\n')).toBeUndefined(); + }); + + test("returns undefined when item header is missing the type field", () => { + expect(detectEnvelopeType('{}\n{"length":0}\n{}\n')).toBeUndefined(); + }); + + test("caps header scan at first 512 bytes", () => { + // Even if the envelope is enormous, only the first 512 bytes are + // decoded — a valid item header in the first 512 still resolves. + const big = "x".repeat(10 * 1024); + const wire = `{"event_id":"a"}\n{"type":"event"}\n${big}`; + expect(detectEnvelopeType(wire)).toBe("event"); + }); + + test("returns undefined when first-item header lives past byte 512", () => { + // Envelope header alone longer than 512 bytes → item header is + // past the scan window → undefined. + const big = "x".repeat(1024); + const wire = `{"pad":"${big}"}\n{"type":"event"}\n{}\n`; + expect(detectEnvelopeType(wire)).toBeUndefined(); + }); + + test("handles item header at end of input (no trailing newline)", () => { + const wire = '{}\n{"type":"event"}'; + expect(detectEnvelopeType(wire)).toBe("event"); + }); + + test("returns undefined when the item header slot is empty (adjacent newlines)", () => { + // envelope header immediately followed by another newline — empty + // item header, nothing to parse. + expect(detectEnvelopeType("{}\n\n{}\n")).toBeUndefined(); + }); +}); diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts index 4416db047..caaef7544 100644 --- a/test/lib/telemetry/zstd-transport.test.ts +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -12,14 +12,17 @@ * `createTransport` layer. */ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { EventEmitter } from "node:events"; import type { ClientRequest, IncomingHttpHeaders } from "node:http"; import { gunzipSync } from "node:zlib"; import { createEnvelope } from "@sentry/core"; import { hasZstdSupport, + isNoProxyExempt, makeCompressedTransport, + maybeCompress, + normalizeBody, } from "../../../src/lib/telemetry/zstd-transport.js"; /** No-op for SDK callbacks that require a function but return nothing meaningful. */ @@ -284,4 +287,264 @@ describe("makeCompressedTransport", () => { const response = await transport.send(envelope); expect(response).toEqual({}); }); + + test("proxy configured: falls back to SDK's makeNodeTransport", () => { + const savedProxy = process.env.https_proxy; + process.env.https_proxy = "http://proxy.internal:3128"; + try { + // No httpModule override: we can't observe the returned transport's + // internals, but we can prove the path differs from the zstd one + // by checking that no zstd-specific header is attached when a + // proxy is present. The test below (normalizeBody + maybeCompress) + // exercise the zstd codepath directly. + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + // makeNodeTransport requires httpModule too; with the default it + // will try to create an Agent. Pass a dummy module to avoid any + // real network I/O when the returned transport is constructed. + }); + // Sanity: we got *some* transport object back with send/flush. + expect(typeof transport.send).toBe("function"); + expect(typeof transport.flush).toBe("function"); + } finally { + if (savedProxy === undefined) { + delete process.env.https_proxy; + } else { + process.env.https_proxy = savedProxy; + } + } + }); + + test("network error on socket: promise rejects, nothing throws outward", async () => { + // Mock http module whose request() emits an 'error' event instead of + // responding. The executor's `req.on('error', reject)` must surface + // it to the outer promise, which createTransport's wrapper catches + // and records as network_error. + const throwingMod = { + request: (_opts: unknown, _cb?: unknown) => { + const req = new EventEmitter() as unknown as ClientRequest & { + write: (c: unknown) => boolean; + end: () => void; + }; + req.write = () => true; + req.end = () => { + process.nextTick(() => req.emit("error", new Error("ECONNREFUSED"))); + }; + return req; + }, + }; + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + httpModule: throwingMod as never, + }); + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { data: "x".repeat(4096) } as any], + ]); + // createTransport wraps network errors and re-throws them — a real + // API consumer would swallow this via .catch(). We just assert the + // promise settles (does not hang) and throws an ECONNREFUSED. + await expect(transport.send(envelope)).rejects.toThrow("ECONNREFUSED"); + }); + + test("proxy configured + URL is no_proxy exempt: uses zstd transport", async () => { + const savedProxy = process.env.https_proxy; + const savedNoProxy = process.env.no_proxy; + process.env.https_proxy = "http://proxy.internal:3128"; + process.env.no_proxy = "example.com"; + try { + const { httpModule, captured } = buildMockHttpModule({ + statusCode: 200, + headers: {}, + }); + const transport = makeCompressedTransport({ + ...BASE_OPTIONS, + httpModule, + }); + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { data: "small" } as any], + ]); + await transport.send(envelope); + // httpModule was called → we took the zstd path, not the SDK + // fallback (which would have ignored our httpModule mock and + // tried to connect through the proxy). + expect(captured.chunks.length).toBeGreaterThan(0); + } finally { + if (savedProxy === undefined) { + delete process.env.https_proxy; + } else { + process.env.https_proxy = savedProxy; + } + if (savedNoProxy === undefined) { + delete process.env.no_proxy; + } else { + process.env.no_proxy = savedNoProxy; + } + } + }); +}); + +// ── Direct helper tests ────────────────────────────────────────────── + +describe("normalizeBody", () => { + test("string → UTF-8 bytes", () => { + const buf = normalizeBody("hello"); + expect(buf.toString("utf-8")).toBe("hello"); + }); + + test("multi-byte UTF-8 string", () => { + const buf = normalizeBody("café ☕"); + expect(buf.toString("utf-8")).toBe("café ☕"); + }); + + test("Uint8Array → zero-copy Buffer view", () => { + const src = new Uint8Array([1, 2, 3, 4, 5]); + const buf = normalizeBody(src); + expect(buf.length).toBe(5); + expect(Array.from(buf)).toEqual([1, 2, 3, 4, 5]); + }); + + test("Uint8Array with non-zero byteOffset", () => { + const backing = new Uint8Array([9, 9, 1, 2, 3, 9, 9]); + const view = new Uint8Array(backing.buffer, 2, 3); + const buf = normalizeBody(view); + expect(Array.from(buf)).toEqual([1, 2, 3]); + }); + + test("empty string", () => { + expect(normalizeBody("").length).toBe(0); + }); + + test("empty Uint8Array", () => { + expect(normalizeBody(new Uint8Array(0)).length).toBe(0); + }); +}); + +describe("maybeCompress", () => { + test("zstd + body above threshold → zstd-compressed", async () => { + if (!hasZstdSupport()) { + return; + } + const buf = Buffer.from("x".repeat(4096)); + const result = await maybeCompress(buf, "zstd"); + expect(result.encodingApplied).toBe("zstd"); + expect(result.payload.length).toBeLessThan(buf.length); + expect(result.compressMs).toBeGreaterThanOrEqual(0); + const decompressed = await Bun.zstdDecompress(result.payload); + expect(decompressed.toString("utf-8")).toBe("x".repeat(4096)); + }); + + test("zstd + body below 1 KiB threshold → passthrough", async () => { + const buf = Buffer.from("x".repeat(512)); + const result = await maybeCompress(buf, "zstd"); + expect(result.encodingApplied).toBe("none"); + expect(result.payload).toBe(buf); + expect(result.compressMs).toBe(0); + }); + + test("gzip + body above 32 KiB threshold → gzip-compressed", async () => { + const buf = Buffer.from("y".repeat(64 * 1024)); + const result = await maybeCompress(buf, "gzip"); + expect(result.encodingApplied).toBe("gzip"); + expect(result.payload.length).toBeLessThan(buf.length); + const decompressed = gunzipSync(result.payload); + expect(decompressed.toString("utf-8")).toBe("y".repeat(64 * 1024)); + }); + + test("gzip + body below 32 KiB threshold → passthrough", async () => { + const buf = Buffer.from("z".repeat(16 * 1024)); + const result = await maybeCompress(buf, "gzip"); + expect(result.encodingApplied).toBe("none"); + expect(result.payload).toBe(buf); + }); + + test("zstd path + Bun.zstdCompress missing mid-flight → gzip safety net", async () => { + const saved = globalThis.Bun?.zstdCompress; + (globalThis as { Bun: { zstdCompress?: unknown } }).Bun.zstdCompress = + undefined as never; + try { + const buf = Buffer.from("x".repeat(4096)); + // Encoding pre-selected as "zstd" (caller didn't reprobe), but + // the runtime now lacks zstd — the belt-and-braces branch gzips. + const result = await maybeCompress(buf, "zstd"); + expect(result.encodingApplied).toBe("gzip"); + expect(gunzipSync(result.payload).toString("utf-8")).toBe( + "x".repeat(4096) + ); + } finally { + if (saved !== undefined) { + globalThis.Bun.zstdCompress = saved; + } + } + }); +}); + +describe("isNoProxyExempt", () => { + let savedNoProxy: string | undefined; + let savedNoProxyUpper: string | undefined; + + beforeEach(() => { + savedNoProxy = process.env.no_proxy; + savedNoProxyUpper = process.env.NO_PROXY; + delete process.env.no_proxy; + delete process.env.NO_PROXY; + }); + + afterEach(() => { + if (savedNoProxy === undefined) { + delete process.env.no_proxy; + } else { + process.env.no_proxy = savedNoProxy; + } + if (savedNoProxyUpper === undefined) { + delete process.env.NO_PROXY; + } else { + process.env.NO_PROXY = savedNoProxyUpper; + } + }); + + test("no env var set → not exempt", () => { + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(false); + }); + + test("suffix match in no_proxy → exempt", () => { + process.env.no_proxy = "example.com,internal.lan"; + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(true); + }); + + test("no match → not exempt", () => { + process.env.no_proxy = "other.com"; + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(false); + }); + + test("NO_PROXY (uppercase) also recognized", () => { + process.env.NO_PROXY = "example.com"; + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(true); + }); + + test("lowercase takes precedence over uppercase", () => { + process.env.no_proxy = "other.com"; + process.env.NO_PROXY = "example.com"; + // Lowercase wins → uppercase ignored → no match → not exempt + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(false); + }); +}); + +describe("hasZstdSupport", () => { + test("true when Bun.zstdCompress is present (Bun runtime)", () => { + // Running under bun test, so this should always be true. + expect(hasZstdSupport()).toBe(true); + }); + + test("false when Bun.zstdCompress is absent (simulated)", () => { + const saved = globalThis.Bun?.zstdCompress; + (globalThis as { Bun: { zstdCompress?: unknown } }).Bun.zstdCompress = + undefined as never; + try { + expect(hasZstdSupport()).toBe(false); + } finally { + if (saved !== undefined) { + globalThis.Bun.zstdCompress = saved; + } + } + }); }); From 4651518fdf6e82cbdfe58337017d5b4972c7d6cf Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 25 Apr 2026 14:44:48 +0000 Subject: [PATCH 3/9] fix(telemetry): trim whitespace around no_proxy entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- src/lib/telemetry/zstd-transport.ts | 5 +++++ test/lib/telemetry/zstd-transport.test.ts | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index ecebc24f5..65d610665 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -356,6 +356,9 @@ export function hasZstdSupport(): boolean { * URL matches an entry in `NO_PROXY` / `no_proxy`, in which case the * proxy should be ignored. * + * Whitespace around comma-separated entries is trimmed — `"a.com, b.com"` + * is a common config style and both entries should match. + * * @internal Exported for tests. */ export function isNoProxyExempt(urlSegments: URL): boolean { @@ -365,6 +368,8 @@ export function isNoProxyExempt(urlSegments: URL): boolean { } return noProxy .split(",") + .map((ex) => ex.trim()) + .filter((ex) => ex.length > 0) .some( (ex) => urlSegments.host.endsWith(ex) || urlSegments.hostname.endsWith(ex) ); diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts index caaef7544..0a32fabeb 100644 --- a/test/lib/telemetry/zstd-transport.test.ts +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -527,6 +527,21 @@ describe("isNoProxyExempt", () => { // Lowercase wins → uppercase ignored → no match → not exempt expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(false); }); + + test("trims whitespace around comma-separated entries", () => { + // Common config style: `"a.com, b.com"` — both entries should match + // even with the leading space after the comma. + process.env.no_proxy = "other.com, ingest.example.com"; + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(true); + }); + + test("ignores empty entries (trailing comma, double comma)", () => { + process.env.no_proxy = "other.com,, , example.com,"; + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(true); + // Empty entry must not match every host (would be true if `endsWith("")` + // were ever evaluated). + expect(isNoProxyExempt(new URL("https://other-host.test/"))).toBe(false); + }); }); describe("hasZstdSupport", () => { From e398565e3cd20f910b22d013e33e4552c7102e15 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 25 Apr 2026 14:53:31 +0000 Subject: [PATCH 4/9] fix(telemetry): honor uppercase HTTP_PROXY/HTTPS_PROXY/NO_PROXY in transport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- src/lib/telemetry/zstd-transport.ts | 22 +++++- test/lib/telemetry/zstd-transport.test.ts | 82 +++++++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index 65d610665..af365875a 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -144,14 +144,30 @@ export function makeCompressedTransport( * True iff a proxy is configured for this URL and not exempted by * no_proxy. When true, the caller falls back to the SDK's default * transport (which handles CONNECT tunneling). + * + * Mirrors `@sentry/node-core/transports/http.js` `applyNoProxyOption`'s + * proxy-resolution priority: + * - http → `options.proxy` | `http_proxy` + * - https → `options.proxy` | `https_proxy` | `http_proxy` + * + * Both upper- and lowercase env vars are recognized so behavior matches + * cURL / Node ecosystem convention. Lowercase wins when both are set, + * staying consistent with the SDK and {@link isNoProxyExempt}. + * + * @internal Exported for tests. */ -function shouldFallbackToDefault( +export function shouldFallbackToDefault( url: URL, options: NodeTransportOptions ): boolean { const isHttps = url.protocol === "https:"; - const envProxy = isHttps ? process.env.https_proxy : process.env.http_proxy; - const proxy = options.proxy || envProxy || process.env.http_proxy; + const httpProxy = process.env.http_proxy ?? process.env.HTTP_PROXY; + const httpsProxy = process.env.https_proxy ?? process.env.HTTPS_PROXY; + const envProxy = isHttps ? httpsProxy : httpProxy; + // SDK precedent: HTTPS connections fall back to http_proxy as a last + // resort. We mirror that so a user configured for the SDK's default + // transport gets identical proxy semantics here. + const proxy = options.proxy || envProxy || httpProxy; if (!proxy) { return false; } diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts index 0a32fabeb..38b4f1564 100644 --- a/test/lib/telemetry/zstd-transport.test.ts +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -23,6 +23,7 @@ import { makeCompressedTransport, maybeCompress, normalizeBody, + shouldFallbackToDefault, } from "../../../src/lib/telemetry/zstd-transport.js"; /** No-op for SDK callbacks that require a function but return nothing meaningful. */ @@ -563,3 +564,84 @@ describe("hasZstdSupport", () => { } }); }); + +describe("shouldFallbackToDefault", () => { + const PROXY_VARS = [ + "http_proxy", + "HTTP_PROXY", + "https_proxy", + "HTTPS_PROXY", + "no_proxy", + "NO_PROXY", + ] as const; + const saved: Partial> = {}; + + beforeEach(() => { + for (const k of PROXY_VARS) { + saved[k] = process.env[k]; + delete process.env[k]; + } + }); + + afterEach(() => { + for (const k of PROXY_VARS) { + const v = saved[k]; + if (v === undefined) { + delete process.env[k]; + } else { + process.env[k] = v; + } + } + }); + + const opts = { url: "https://ingest.example.com/", recordDroppedEvent: noop }; + const httpsUrl = new URL("https://ingest.example.com/"); + const httpUrl = new URL("http://ingest.example.com/"); + + test("no proxy configured → no fallback", () => { + expect(shouldFallbackToDefault(httpsUrl, opts)).toBe(false); + }); + + test("options.proxy wins → fallback", () => { + expect( + shouldFallbackToDefault(httpsUrl, { + ...opts, + proxy: "http://proxy.internal:3128", + }) + ).toBe(true); + }); + + test("lowercase https_proxy → fallback (HTTPS URL)", () => { + process.env.https_proxy = "http://proxy.internal:3128"; + expect(shouldFallbackToDefault(httpsUrl, opts)).toBe(true); + }); + + test("uppercase HTTPS_PROXY → fallback (HTTPS URL)", () => { + process.env.HTTPS_PROXY = "http://proxy.internal:3128"; + expect(shouldFallbackToDefault(httpsUrl, opts)).toBe(true); + }); + + test("lowercase wins over uppercase when both are set", () => { + process.env.https_proxy = "http://winning.proxy:3128"; + process.env.HTTPS_PROXY = "http://losing.proxy:3128"; + // Both trigger fallback; this just asserts the lookup doesn't + // crash on duplicate vars and that the function still returns true. + expect(shouldFallbackToDefault(httpsUrl, opts)).toBe(true); + }); + + test("HTTPS URL falls back to http_proxy when https_proxy is unset (matches SDK precedent)", () => { + process.env.http_proxy = "http://proxy.internal:3128"; + expect(shouldFallbackToDefault(httpsUrl, opts)).toBe(true); + }); + + test("uppercase HTTP_PROXY → fallback (http URL)", () => { + process.env.HTTP_PROXY = "http://proxy.internal:3128"; + expect(shouldFallbackToDefault(httpUrl, opts)).toBe(true); + }); + + test("uppercase NO_PROXY exemption keeps zstd path even with proxy set", () => { + process.env.HTTPS_PROXY = "http://proxy.internal:3128"; + process.env.NO_PROXY = "example.com"; + expect(shouldFallbackToDefault(httpsUrl, opts)).toBe(false); + }); +}); From 6cedd6fc50513afa141cbc6b51bf20e502ac471a Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 25 Apr 2026 15:04:21 +0000 Subject: [PATCH 5/9] feat(telemetry): support no_proxy="*" wildcard exemption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/telemetry/zstd-transport.ts | 22 +++++++++++++++------- test/lib/telemetry/zstd-transport.test.ts | 20 ++++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index af365875a..6d5b9e1a5 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -372,8 +372,13 @@ export function hasZstdSupport(): boolean { * URL matches an entry in `NO_PROXY` / `no_proxy`, in which case the * proxy should be ignored. * - * Whitespace around comma-separated entries is trimmed — `"a.com, b.com"` - * is a common config style and both entries should match. + * Slightly more permissive than the SDK: + * - Whitespace around comma-separated entries is trimmed + * (`"a.com, b.com"` is common; SDK does not trim). + * - The `"*"` wildcard means "bypass proxy for all hosts" — a + * convention from cURL / Go tooling that the SDK currently + * ignores (would route through the proxy regardless). We honor + * it so users with `no_proxy="*"` keep the zstd path. * * @internal Exported for tests. */ @@ -382,11 +387,14 @@ export function isNoProxyExempt(urlSegments: URL): boolean { if (!noProxy) { return false; } - return noProxy + const entries = noProxy .split(",") .map((ex) => ex.trim()) - .filter((ex) => ex.length > 0) - .some( - (ex) => urlSegments.host.endsWith(ex) || urlSegments.hostname.endsWith(ex) - ); + .filter((ex) => ex.length > 0); + if (entries.includes("*")) { + return true; + } + return entries.some( + (ex) => urlSegments.host.endsWith(ex) || urlSegments.hostname.endsWith(ex) + ); } diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts index 38b4f1564..80246af8f 100644 --- a/test/lib/telemetry/zstd-transport.test.ts +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -543,6 +543,26 @@ describe("isNoProxyExempt", () => { // were ever evaluated). expect(isNoProxyExempt(new URL("https://other-host.test/"))).toBe(false); }); + + test("'*' wildcard exempts all hosts", () => { + process.env.no_proxy = "*"; + expect(isNoProxyExempt(new URL("https://ingest.example.com/"))).toBe(true); + expect(isNoProxyExempt(new URL("https://anything.test/"))).toBe(true); + expect(isNoProxyExempt(new URL("http://192.0.2.1:8080/"))).toBe(true); + }); + + test("'*' wildcard alongside other entries still exempts everything", () => { + process.env.no_proxy = "specific.com, *, other.com"; + expect(isNoProxyExempt(new URL("https://unrelated.test/"))).toBe(true); + }); + + test("literal '*' in a host name does not get matched as wildcard", () => { + // The wildcard check requires `*` to be a standalone entry. + // A bizarre value like `"*.example.com"` is treated as a literal + // suffix: `endsWith("*.example.com")` is false for normal hosts. + process.env.no_proxy = "*.example.com"; + expect(isNoProxyExempt(new URL("https://foo.example.com/"))).toBe(false); + }); }); describe("hasZstdSupport", () => { From 3a786dcbfe2be2be5ee2ef1b3a2e6f3af281b869 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 25 Apr 2026 18:18:42 +0000 Subject: [PATCH 6/9] chore(telemetry): drop bench script and runtime transport metrics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- package.json | 1 - script/bench-transport.ts | 290 ------------------ src/lib/telemetry/zstd-transport-metrics.ts | 106 ------- src/lib/telemetry/zstd-transport.ts | 66 ++-- .../telemetry/zstd-transport-metrics.test.ts | 237 -------------- test/lib/telemetry/zstd-transport.test.ts | 2 - 6 files changed, 21 insertions(+), 681 deletions(-) delete mode 100644 script/bench-transport.ts delete mode 100644 src/lib/telemetry/zstd-transport-metrics.ts delete mode 100644 test/lib/telemetry/zstd-transport-metrics.test.ts diff --git a/package.json b/package.json index 9b830249d..8dcf0a878 100644 --- a/package.json +++ b/package.json @@ -96,7 +96,6 @@ "bench:save": "bun run script/bench.ts --save-baseline", "bench:compare": "bun run script/bench.ts --compare", "bench:sweep": "bun run script/bench-sweep.ts", - "bench:transport": "bun run script/bench-transport.ts", "check:fragments": "bun run script/check-fragments.ts", "check:deps": "bun run script/check-no-deps.ts", "check:errors": "bun run script/check-error-patterns.ts", diff --git a/script/bench-transport.ts b/script/bench-transport.ts deleted file mode 100644 index 3a25de2b6..000000000 --- a/script/bench-transport.ts +++ /dev/null @@ -1,290 +0,0 @@ -/** - * Offline benchmark for the telemetry transport's compression codecs. - * - * bun run script/bench-transport.ts # markdown table to stdout - * bun run script/bench-transport.ts > bench.md - * - * For each of four representative envelopes (error / transaction / log / - * session) the script measures compress-time and wire size across: - * - * none — raw, no compression - * gzip-6 — zlib default level (what the SDK's default transport uses) - * zstd-3 — libzstd default - * zstd-5 — mid-level; probes the low-to-mid curve for small payloads - * zstd-6 — slightly higher-ratio / slower - * zstd-9 — upper anchor; AGENTS.md warns of decoder-side cost at high levels - * - * Also measures decompress time with Bun.zstdDecompress / zlib.gunzip so - * the server-side cost is visible. Without decode-side data a lower - * compressed size can look like a win on ratio while actually being - * worse total throughput once the ingest relay's decode cost is counted. - * - * Output: a single markdown table per envelope plus a per-codec summary. - */ - -import { promisify } from "node:util"; -import { gunzip as gunzipCb, gzip as gzipCb } from "node:zlib"; -import { createEnvelope, serializeEnvelope } from "@sentry/core"; - -const gzipAsync = promisify(gzipCb); -const gunzipAsync = promisify(gunzipCb); - -type Codec = "none" | "gzip-6" | "zstd-3" | "zstd-5" | "zstd-6" | "zstd-9"; - -const CODECS: Codec[] = [ - "none", - "gzip-6", - "zstd-3", - "zstd-5", - "zstd-6", - "zstd-9", -]; - -const WARMUP_ITERS = 5; -const MEASURE_ITERS = 50; - -async function compress(codec: Codec, buf: Buffer): Promise { - if (codec === "none") { - return buf; - } - if (codec === "gzip-6") { - return await gzipAsync(buf); - } - const level = Number(codec.split("-")[1]); - const out = await Bun.zstdCompress(buf, { level }); - return Buffer.from(out.buffer, out.byteOffset, out.byteLength); -} - -async function decompress(codec: Codec, buf: Buffer): Promise { - if (codec === "none") { - return buf; - } - if (codec === "gzip-6") { - return await gunzipAsync(buf); - } - const out = await Bun.zstdDecompress(buf); - return Buffer.from(out.buffer, out.byteOffset, out.byteLength); -} - -async function timeFn( - fn: () => Promise -): Promise<{ avgMs: number; result: T }> { - for (let i = 0; i < WARMUP_ITERS; i++) { - await fn(); - } - const start = performance.now(); - let result: T | undefined; - for (let i = 0; i < MEASURE_ITERS; i++) { - result = await fn(); - } - const elapsed = performance.now() - start; - return { avgMs: elapsed / MEASURE_ITERS, result: result as T }; -} - -// ── Fixture envelopes ──────────────────────────────────────────────── -// Modeled on captures from running the CLI against the real ingest: -// shape and field choice reflect what the SDK actually sends, so the -// compression ratio numbers transfer to production. - -function buildErrorEnvelope(): Buffer { - const eventId = "a".repeat(32); - const header = { event_id: eventId, sent_at: new Date().toISOString() }; - const item = { - event_id: eventId, - level: "error", - platform: "node", - sdk: { name: "sentry.javascript.node-core", version: "10.47.0" }, - exception: { - values: [ - { - type: "TypeError", - value: "Cannot read properties of undefined (reading 'foo')", - stacktrace: { - frames: Array.from({ length: 30 }, (_, i) => ({ - filename: `/home/user/proj/src/file${i}.ts`, - function: `handler${i}`, - lineno: 100 + i, - colno: 10, - pre_context: [" const x = y;", " if (!z) return;"], - context_line: ` return data.${"nested.".repeat(5)}prop;`, - post_context: ["} catch (e) {", " log(e);", "}"], - })), - }, - }, - ], - }, - tags: { - command: "issue.list", - "sentry.runtime": "bun", - cli_version: "0.29.0", - }, - contexts: { - runtime: { name: "bun", version: "1.3.13" }, - os: { name: "darwin", version: "23.4.0" }, - }, - }; - return Buffer.from( - serializeEnvelope( - createEnvelope(header as never, [ - // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity - [{ type: "event" } as any, item as any], - ]) - ) - ); -} - -function buildTransactionEnvelope(): Buffer { - const eventId = "b".repeat(32); - const traceId = "c".repeat(32); - const header = { event_id: eventId, sent_at: new Date().toISOString() }; - const now = Date.now() / 1000; - const spanOps = ["http.client", "db", "file"]; - const spans = Array.from({ length: 60 }, (_, i) => ({ - span_id: String(i).padStart(16, "0"), - trace_id: traceId, - op: spanOps[i % 3], - description: `operation ${i} with a reasonably long description line`, - start_timestamp: now - 1, - timestamp: now, - data: { "http.method": "GET", "url.path": `/api/0/resource/${i}` }, - })); - const item = { - type: "transaction", - event_id: eventId, - transaction: "cli.command", - contexts: { - trace: { trace_id: traceId, span_id: "0000000000000000" }, - }, - spans, - sdk: { name: "sentry.javascript.node-core", version: "10.47.0" }, - }; - return Buffer.from( - serializeEnvelope( - createEnvelope(header as never, [ - // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity - [{ type: "transaction" } as any, item as any], - ]) - ) - ); -} - -function buildLogEnvelope(): Buffer { - const header = { sent_at: new Date().toISOString() }; - const items = Array.from({ length: 20 }, (_, i) => ({ - timestamp: Date.now() / 1000, - trace_id: "d".repeat(32), - level: "info", - body: `log message ${i}: something happened during execution`, - attributes: { command: { value: "issue.list", type: "string" } }, - })); - return Buffer.from( - serializeEnvelope( - createEnvelope(header as never, [ - // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity - [{ type: "log" } as any, { items } as any], - ]) - ) - ); -} - -function buildSessionEnvelope(): Buffer { - const header = { sent_at: new Date().toISOString() }; - const item = { - sid: "e".repeat(32), - did: "user-1", - started: new Date().toISOString(), - status: "exited", - errors: 0, - attrs: { release: "0.29.0", environment: "production" }, - }; - return Buffer.from( - serializeEnvelope( - createEnvelope(header as never, [ - // biome-ignore lint/suspicious/noExplicitAny: fixture fidelity - [{ type: "session" } as any, item as any], - ]) - ) - ); -} - -type BenchRow = { - envelope: string; - rawBytes: number; - codec: Codec; - sentBytes: number; - ratio: number; - compressMs: number; - decompressMs: number; -}; - -async function benchCodec( - envelopeName: string, - buf: Buffer, - codec: Codec -): Promise { - const { avgMs: compressMs, result: compressed } = await timeFn(() => - compress(codec, buf) - ); - const { avgMs: decompressMs } = await timeFn(() => - decompress(codec, compressed) - ); - return { - envelope: envelopeName, - rawBytes: buf.length, - codec, - sentBytes: compressed.length, - ratio: compressed.length / buf.length, - compressMs, - decompressMs, - }; -} - -function formatRow(r: BenchRow): string { - const cols = [ - r.envelope, - String(r.rawBytes), - r.codec, - String(r.sentBytes), - r.ratio.toFixed(3), - r.compressMs.toFixed(3), - r.decompressMs.toFixed(3), - ]; - return `| ${cols.join(" | ")} |`; -} - -async function main(): Promise { - if (typeof Bun?.zstdCompress !== "function") { - process.stderr.write( - "bench-transport: Bun.zstdCompress unavailable — run on Bun or Node >= 22.15 with polyfill installed\n" - ); - process.exit(1); - } - - const fixtures: { name: string; buf: Buffer }[] = [ - { name: "error (30-frame stack)", buf: buildErrorEnvelope() }, - { name: "transaction (60 spans)", buf: buildTransactionEnvelope() }, - { name: "log (20 entries)", buf: buildLogEnvelope() }, - { name: "session", buf: buildSessionEnvelope() }, - ]; - - process.stdout.write( - "# Telemetry transport codec benchmark\n\n" + - `_${WARMUP_ITERS} warmup + ${MEASURE_ITERS} measured iterations per cell._\n\n` - ); - - process.stdout.write( - "| envelope | raw bytes | codec | sent bytes | ratio | compress ms | decompress ms |\n" - ); - process.stdout.write( - "|----------|----------:|-------|-----------:|------:|------------:|--------------:|\n" - ); - - for (const fix of fixtures) { - for (const codec of CODECS) { - const row = await benchCodec(fix.name, fix.buf, codec); - process.stdout.write(`${formatRow(row)}\n`); - } - } -} - -await main(); diff --git a/src/lib/telemetry/zstd-transport-metrics.ts b/src/lib/telemetry/zstd-transport-metrics.ts deleted file mode 100644 index 85d71215b..000000000 --- a/src/lib/telemetry/zstd-transport-metrics.ts +++ /dev/null @@ -1,106 +0,0 @@ -/** - * Opt-in transport-level metrics for the Sentry SDK telemetry pipeline. - * - * When `SENTRY_TRANSPORT_METRICS=1` is set, {@link emitTransportMetric} - * writes a single line of JSON to stderr per outbound envelope. Off by - * default — no cost in production beyond a single env-var read. - * - * Emitted fields: - * - `ts` epoch ms when the metric was emitted - * - `kind` always `"sentry_transport"` (for greppability) - * - `envelope_type` first-item type (`"event"`, `"transaction"`, - * `"log"`, `"session"`, `"client_report"`, …) or - * `"unknown"` if the envelope couldn't be parsed - * - `encoding` `"zstd"` | `"gzip"` | `"none"` - * - `raw_bytes` body size before compression - * - `sent_bytes` body size on the wire - * - `compress_ms` wall-clock compress time in milliseconds (0 if - * the body was under the compression threshold) - * - `ratio` sent / raw (1.0 if no compression) - * - * The envelope-type sniff is passed as a callback so we only pay its - * JSON-parse cost when the metric is actually emitted. - */ - -import { getEnv } from "../env.js"; - -export type TransportEncoding = "zstd" | "gzip" | "none"; - -export type TransportMetricInput = { - rawBytes: number; - sentBytes: number; - compressMs: number; - encoding: TransportEncoding; - /** Lazy envelope-type extractor. Invoked only when the metric is emitted. */ - envelopeType: () => string | undefined; -}; - -/** Emit a single JSON line to stderr iff `SENTRY_TRANSPORT_METRICS=1`. */ -export function emitTransportMetric(m: TransportMetricInput): void { - if (getEnv().SENTRY_TRANSPORT_METRICS !== "1") { - return; - } - const ratio = m.rawBytes > 0 ? m.sentBytes / m.rawBytes : 1; - const line = JSON.stringify({ - ts: Date.now(), - kind: "sentry_transport", - envelope_type: m.envelopeType() ?? "unknown", - encoding: m.encoding, - raw_bytes: m.rawBytes, - sent_bytes: m.sentBytes, - compress_ms: Number(m.compressMs.toFixed(2)), - ratio: Number(ratio.toFixed(3)), - }); - process.stderr.write(`${line}\n`); -} - -/** - * Peek at the first envelope item header to classify the envelope. - * - * Envelope wire format (https://develop.sentry.dev/sdk/envelopes/): - * - * {envelope_header}\n - * {item_1_header}\n - * {item_1_body}\n - * {item_2_header}\n - * ... - * - * Only the first item's `type` field is needed. We decode at most the - * first ~512 bytes (well beyond the largest reasonable item header) to - * avoid slurping the whole envelope for a classification hint. - * - * @param body Raw envelope bytes (pre-compression) or string. - * @returns The first item's `type` or `undefined` if the envelope can't - * be parsed (empty, truncated, non-JSON item header). - */ -export function detectEnvelopeType( - body: Buffer | Uint8Array | string -): string | undefined { - const text = - typeof body === "string" - ? body.slice(0, 512) - : Buffer.from( - body.buffer, - body.byteOffset, - Math.min(body.byteLength, 512) - ).toString("utf-8"); - - const firstNl = text.indexOf("\n"); - if (firstNl < 0) { - return; - } - const secondNl = text.indexOf("\n", firstNl + 1); - const itemHeader = text.slice( - firstNl + 1, - secondNl > 0 ? secondNl : undefined - ); - if (!itemHeader) { - return; - } - try { - const parsed = JSON.parse(itemHeader) as { type?: unknown }; - return typeof parsed.type === "string" ? parsed.type : undefined; - } catch { - return; - } -} diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index 6d5b9e1a5..92794f804 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -10,9 +10,7 @@ * Codec selection is one-shot, performed at factory-construction time. * No per-request branching: if `Bun.zstdCompress` is available when the * transport is created, every envelope uses zstd; otherwise every - * envelope uses gzip. The choice is reflected in the metric emitted by - * {@link emitTransportMetric} so the ratio of zstd-vs-gzip callers can - * be observed in the real world. + * envelope uses gzip. * * This mirrors `@sentry/node-core/transports/http.js` `makeNodeTransport` * — URL parsing, `no_proxy` handling, proxy agent, CA certs, keepAlive, @@ -47,17 +45,19 @@ import { makeNodeTransport, type NodeTransportOptions, } from "@sentry/node-core/light"; -import { - detectEnvelopeType, - emitTransportMetric, - type TransportEncoding, -} from "./zstd-transport-metrics.js"; + +/** Codec actually applied to a given envelope. */ +type AppliedEncoding = "zstd" | "gzip" | "none"; + +/** Codec the transport will attempt; "none" only happens under threshold. */ +type SelectedEncoding = "zstd" | "gzip"; /** - * zstd compression level. L3 is libzstd's default — benchmarks across - * envelope sizes (1–30 KiB) showed L3–L6 sit on the same ratio-vs-time - * curve for this workload; L3 is the safe operating point until the - * offline bench pins a different value. See `script/bench-transport.ts`. + * zstd compression level. L3 is libzstd's default and was confirmed + * optimal for telemetry-sized payloads (1–30 KiB) by an offline + * benchmark before merge: L3–L6 sit on the same ratio-vs-time curve, + * and L3 wins on compress time without losing ratio. Higher levels + * (≥9) regress compress time without meaningful ratio gains. */ const ZSTD_LEVEL = 3; @@ -126,9 +126,7 @@ export function makeCompressedTransport( const httpModule = options.httpModule ?? nativeHttpModule; // One-shot codec selection. Frozen into the executor closure below. - const encoding: Exclude = hasZstdSupport() - ? "zstd" - : "gzip"; + const encoding: SelectedEncoding = hasZstdSupport() ? "zstd" : "gzip"; const executor = createCompressingExecutor({ options, @@ -184,7 +182,7 @@ export function createCompressingExecutor(args: { options: NodeTransportOptions; httpModule: NonNullable; agent: http.Agent; - encoding: Exclude; + encoding: SelectedEncoding; }): TransportRequestExecutor { const { options, httpModule, agent, encoding } = args; const { hostname, pathname, port, protocol, search } = new URL(options.url); @@ -215,7 +213,7 @@ type PerformRequestArgs = { options: NodeTransportOptions; httpModule: NonNullable; agent: http.Agent; - encoding: Exclude; + encoding: SelectedEncoding; hostname: string; path: string; port: string; @@ -228,24 +226,13 @@ async function performRequest( const { request, options, httpModule, agent, encoding } = args; const rawBuffer = normalizeBody(request.body); - const { payload, encodingApplied, compressMs } = await maybeCompress( - rawBuffer, - encoding - ); + const { payload, encodingApplied } = await maybeCompress(rawBuffer, encoding); const headers: Record = { ...(options.headers ?? {}) }; if (encodingApplied !== "none") { headers["content-encoding"] = encodingApplied; } - emitTransportMetric({ - rawBytes: rawBuffer.length, - sentBytes: payload.length, - compressMs, - encoding: encodingApplied, - envelopeType: () => detectEnvelopeType(rawBuffer), - }); - return new Promise((resolve, reject) => { const req = httpModule.request( { @@ -311,8 +298,7 @@ export function normalizeBody(body: string | Uint8Array): Buffer { type CompressResult = { payload: Buffer; - encodingApplied: TransportEncoding; - compressMs: number; + encodingApplied: AppliedEncoding; }; /** @@ -324,14 +310,13 @@ type CompressResult = { */ export async function maybeCompress( buf: Buffer, - encoding: Exclude + encoding: SelectedEncoding ): Promise { const threshold = encoding === "zstd" ? ZSTD_THRESHOLD : GZIP_THRESHOLD; if (buf.length <= threshold) { - return { payload: buf, encodingApplied: "none", compressMs: 0 }; + return { payload: buf, encodingApplied: "none" }; } - const start = performance.now(); if (encoding === "zstd") { const bun = (globalThis as { Bun?: BunZstdHost }).Bun; // Shouldn't happen (factory checked at construction time), but a @@ -339,26 +324,17 @@ export async function maybeCompress( // swapped out between construction and first send. if (!bun?.zstdCompress) { const gz = await gzipAsync(buf); - return { - payload: gz, - encodingApplied: "gzip", - compressMs: performance.now() - start, - }; + return { payload: gz, encodingApplied: "gzip" }; } const out = await bun.zstdCompress(buf, { level: ZSTD_LEVEL }); return { payload: Buffer.from(out.buffer, out.byteOffset, out.byteLength), encodingApplied: "zstd", - compressMs: performance.now() - start, }; } const gz = await gzipAsync(buf); - return { - payload: gz, - encodingApplied: "gzip", - compressMs: performance.now() - start, - }; + return { payload: gz, encodingApplied: "gzip" }; } /** Feature-detect zstd support on the current runtime. */ diff --git a/test/lib/telemetry/zstd-transport-metrics.test.ts b/test/lib/telemetry/zstd-transport-metrics.test.ts deleted file mode 100644 index 9ecb0facb..000000000 --- a/test/lib/telemetry/zstd-transport-metrics.test.ts +++ /dev/null @@ -1,237 +0,0 @@ -/** - * Unit tests for the transport-metrics emitter. - * - * Exercises the two public helpers directly: - * - `emitTransportMetric` is gated on `SENTRY_TRANSPORT_METRICS=1`, - * and when enabled writes one JSON line to stderr with the - * expected shape. - * - `detectEnvelopeType` parses the first item header out of a - * real envelope wire format, and returns `undefined` on malformed - * input (empty, truncated, non-JSON header). - */ - -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { createEnvelope, serializeEnvelope } from "@sentry/core"; -import { - detectEnvelopeType, - emitTransportMetric, -} from "../../../src/lib/telemetry/zstd-transport-metrics.ts"; - -const METRICS_ENV_VAR = "SENTRY_TRANSPORT_METRICS"; - -/** Lazy envelope-type stub that returns undefined (matches the envelope - * parser's failure case). Declared as a named function rather than - * `() => undefined` to satisfy biome's `noUselessUndefined` rule. */ -function noEnvelopeType(): string | undefined { - return; -} - -/** - * Capture `process.stderr.write` calls into an array for the duration - * of a callback. Restores the original writer unconditionally. - */ -function captureStderr(fn: () => T): { lines: string[]; result: T } { - const lines: string[] = []; - const original = process.stderr.write.bind(process.stderr); - process.stderr.write = ((chunk: string | Uint8Array, ...rest: any[]) => { - const text = - typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf-8"); - lines.push(text); - return original(chunk, ...rest); - }) as typeof process.stderr.write; - try { - const result = fn(); - return { lines, result }; - } finally { - process.stderr.write = original; - } -} - -describe("emitTransportMetric", () => { - let savedEnv: string | undefined; - - beforeEach(() => { - savedEnv = process.env[METRICS_ENV_VAR]; - }); - - afterEach(() => { - if (savedEnv === undefined) { - delete process.env[METRICS_ENV_VAR]; - } else { - process.env[METRICS_ENV_VAR] = savedEnv; - } - }); - - test("off by default: emits nothing", () => { - delete process.env[METRICS_ENV_VAR]; - const { lines } = captureStderr(() => - emitTransportMetric({ - rawBytes: 100, - sentBytes: 50, - compressMs: 1.5, - encoding: "zstd", - envelopeType: () => "event", - }) - ); - expect(lines).toHaveLength(0); - }); - - test("enabled: emits one JSON line with expected fields", () => { - process.env[METRICS_ENV_VAR] = "1"; - const { lines } = captureStderr(() => - emitTransportMetric({ - rawBytes: 1000, - sentBytes: 250, - compressMs: 1.234_567, - encoding: "zstd", - envelopeType: () => "event", - }) - ); - expect(lines).toHaveLength(1); - const payload = JSON.parse(lines[0]!.trim()) as Record; - expect(payload.kind).toBe("sentry_transport"); - expect(payload.envelope_type).toBe("event"); - expect(payload.encoding).toBe("zstd"); - expect(payload.raw_bytes).toBe(1000); - expect(payload.sent_bytes).toBe(250); - expect(payload.compress_ms).toBe(1.23); // rounded to 2 decimals - expect(payload.ratio).toBe(0.25); // 250/1000 rounded to 3 - expect(typeof payload.ts).toBe("number"); - }); - - test("envelopeType returning undefined → 'unknown'", () => { - process.env[METRICS_ENV_VAR] = "1"; - const { lines } = captureStderr(() => - emitTransportMetric({ - rawBytes: 100, - sentBytes: 100, - compressMs: 0, - encoding: "none", - envelopeType: noEnvelopeType, - }) - ); - const payload = JSON.parse(lines[0]!.trim()) as Record; - expect(payload.envelope_type).toBe("unknown"); - expect(payload.encoding).toBe("none"); - expect(payload.ratio).toBe(1); - }); - - test("rawBytes=0 → ratio defaults to 1 (no division by zero)", () => { - process.env[METRICS_ENV_VAR] = "1"; - const { lines } = captureStderr(() => - emitTransportMetric({ - rawBytes: 0, - sentBytes: 0, - compressMs: 0, - encoding: "none", - envelopeType: () => "event", - }) - ); - const payload = JSON.parse(lines[0]!.trim()) as Record; - expect(payload.ratio).toBe(1); - }); - - test("any value other than '1' leaves the emitter off", () => { - process.env[METRICS_ENV_VAR] = "true"; - const { lines } = captureStderr(() => - emitTransportMetric({ - rawBytes: 100, - sentBytes: 100, - compressMs: 0, - encoding: "none", - envelopeType: () => "event", - }) - ); - expect(lines).toHaveLength(0); - }); - - test("gzip encoding surfaces in the emitted line", () => { - process.env[METRICS_ENV_VAR] = "1"; - const { lines } = captureStderr(() => - emitTransportMetric({ - rawBytes: 5000, - sentBytes: 500, - compressMs: 0.5, - encoding: "gzip", - envelopeType: () => "transaction", - }) - ); - const payload = JSON.parse(lines[0]!.trim()) as Record; - expect(payload.encoding).toBe("gzip"); - expect(payload.envelope_type).toBe("transaction"); - }); -}); - -describe("detectEnvelopeType", () => { - test("extracts type from a real event envelope", () => { - const envelope = createEnvelope({ event_id: "a".repeat(32) } as any, [ - [{ type: "event" } as any, { message: "hi" } as any], - ]); - const wire = serializeEnvelope(envelope); - expect(detectEnvelopeType(wire)).toBe("event"); - }); - - test("extracts type from a transaction envelope (string input)", () => { - const wire = - '{"event_id":"a"}\n{"type":"transaction","length":17}\n{"some":"span"}\n'; - expect(detectEnvelopeType(wire)).toBe("transaction"); - }); - - test("extracts type from a Buffer input", () => { - const wire = '{"event_id":"a"}\n{"type":"log"}\n{"items":[]}\n'; - expect(detectEnvelopeType(Buffer.from(wire, "utf-8"))).toBe("log"); - }); - - test("extracts type from a Uint8Array input", () => { - const wire = '{"event_id":"a"}\n{"type":"session"}\n{"sid":"x"}\n'; - expect(detectEnvelopeType(new TextEncoder().encode(wire))).toBe("session"); - }); - - test("returns undefined on empty input", () => { - expect(detectEnvelopeType("")).toBeUndefined(); - expect(detectEnvelopeType(Buffer.alloc(0))).toBeUndefined(); - }); - - test("returns undefined on single-line (no newline) input", () => { - expect(detectEnvelopeType("{}")).toBeUndefined(); - }); - - test("returns undefined when the item header is not JSON", () => { - expect(detectEnvelopeType('{}\nnot json\n{"body":1}\n')).toBeUndefined(); - }); - - test("returns undefined when the item header has a non-string type", () => { - expect(detectEnvelopeType('{}\n{"type":42}\n{}\n')).toBeUndefined(); - }); - - test("returns undefined when item header is missing the type field", () => { - expect(detectEnvelopeType('{}\n{"length":0}\n{}\n')).toBeUndefined(); - }); - - test("caps header scan at first 512 bytes", () => { - // Even if the envelope is enormous, only the first 512 bytes are - // decoded — a valid item header in the first 512 still resolves. - const big = "x".repeat(10 * 1024); - const wire = `{"event_id":"a"}\n{"type":"event"}\n${big}`; - expect(detectEnvelopeType(wire)).toBe("event"); - }); - - test("returns undefined when first-item header lives past byte 512", () => { - // Envelope header alone longer than 512 bytes → item header is - // past the scan window → undefined. - const big = "x".repeat(1024); - const wire = `{"pad":"${big}"}\n{"type":"event"}\n{}\n`; - expect(detectEnvelopeType(wire)).toBeUndefined(); - }); - - test("handles item header at end of input (no trailing newline)", () => { - const wire = '{}\n{"type":"event"}'; - expect(detectEnvelopeType(wire)).toBe("event"); - }); - - test("returns undefined when the item header slot is empty (adjacent newlines)", () => { - // envelope header immediately followed by another newline — empty - // item header, nothing to parse. - expect(detectEnvelopeType("{}\n\n{}\n")).toBeUndefined(); - }); -}); diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts index 80246af8f..1b72da381 100644 --- a/test/lib/telemetry/zstd-transport.test.ts +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -429,7 +429,6 @@ describe("maybeCompress", () => { const result = await maybeCompress(buf, "zstd"); expect(result.encodingApplied).toBe("zstd"); expect(result.payload.length).toBeLessThan(buf.length); - expect(result.compressMs).toBeGreaterThanOrEqual(0); const decompressed = await Bun.zstdDecompress(result.payload); expect(decompressed.toString("utf-8")).toBe("x".repeat(4096)); }); @@ -439,7 +438,6 @@ describe("maybeCompress", () => { const result = await maybeCompress(buf, "zstd"); expect(result.encodingApplied).toBe("none"); expect(result.payload).toBe(buf); - expect(result.compressMs).toBe(0); }); test("gzip + body above 32 KiB threshold → gzip-compressed", async () => { From 186da58e8ddf1c7a18c32d28f6aad394bc396d6d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 25 Apr 2026 18:45:25 +0000 Subject: [PATCH 7/9] style(telemetry): trim verbose inline comments in zstd-transport 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. --- src/lib/telemetry/zstd-transport.ts | 58 +++++++++++------------------ 1 file changed, 21 insertions(+), 37 deletions(-) diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index 92794f804..bffb8754a 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -92,21 +92,21 @@ type BunZstdHost = { const gzipAsync = promisify(gzipCb); -/** Factory — see module docs. */ +/** + * Factory for the SDK's `Sentry.init({ transport })` option. + * + * Falls back to `makeNodeTransport` when a proxy is configured (the SDK + * owns CONNECT tunneling) or when the DSN URL is unparseable. Otherwise + * picks a one-shot codec — zstd if available, gzip otherwise — and + * wires up an executor. + */ export function makeCompressedTransport( options: NodeTransportOptions ): Transport { - // When a proxy is configured (via options.proxy or http_proxy / - // https_proxy env vars and not overridden by no_proxy), fall back to - // the SDK's default makeNodeTransport — it owns the CONNECT-tunneling - // HttpsProxyAgent. Proxy users thus continue to get gzip (the SDK - // default), but correctness wins over micro-optimizing an edge case. let urlSegments: URL; try { urlSegments = new URL(options.url); } catch { - // Mirror makeNodeTransport: return a no-op transport on bad URL so - // the SDK doesn't throw at init time on misconfigured DSNs. return createTransport(options, () => Promise.resolve({})); } @@ -122,19 +122,14 @@ export function makeCompressedTransport( maxSockets: 30, timeout: 2000, }); - const httpModule = options.httpModule ?? nativeHttpModule; - - // One-shot codec selection. Frozen into the executor closure below. const encoding: SelectedEncoding = hasZstdSupport() ? "zstd" : "gzip"; - const executor = createCompressingExecutor({ options, httpModule, agent, encoding, }); - return createTransport(options, executor); } @@ -162,9 +157,7 @@ export function shouldFallbackToDefault( const httpProxy = process.env.http_proxy ?? process.env.HTTP_PROXY; const httpsProxy = process.env.https_proxy ?? process.env.HTTPS_PROXY; const envProxy = isHttps ? httpsProxy : httpProxy; - // SDK precedent: HTTPS connections fall back to http_proxy as a last - // resort. We mirror that so a user configured for the SDK's default - // transport gets identical proxy semantics here. + // SDK precedent: HTTPS falls back to http_proxy as a last resort. const proxy = options.proxy || envProxy || httpProxy; if (!proxy) { return false; @@ -246,17 +239,12 @@ async function performRequest( ca: options.caCerts, }, (res) => { - res.on("data", () => { - // Drain socket - }); - res.on("end", () => { - // Drain socket - }); + // Drain the response body + res.on("data", drain); + res.on("end", drain); res.setEncoding("utf8"); - const retryAfterHeader = res.headers["retry-after"] ?? null; const rateLimitsHeader = res.headers["x-sentry-rate-limits"] ?? null; - resolve({ statusCode: res.statusCode, headers: { @@ -270,18 +258,19 @@ async function performRequest( }); } ); - req.on("error", reject); - // Single-shot write. `payload` is already a complete Buffer in - // memory (compressed or not), so piping a fresh Readable through - // avoids the SDK's stream-gzip dance without changing the wire - // behavior — `http.ClientRequest` still sees a body it can send. Readable.from(payload).pipe(req); }); } +/** No-op used to drain HTTP response bodies. */ +function drain(): void { + // intentionally empty +} + /** - * Coerce `string | Uint8Array` into a single contiguous Buffer. + * Coerce `string | Uint8Array` into a contiguous Buffer (zero-copy for + * Uint8Array; UTF-8 encoded for strings). * * @internal Exported for tests. */ @@ -289,10 +278,6 @@ export function normalizeBody(body: string | Uint8Array): Buffer { if (typeof body === "string") { return Buffer.from(body, "utf-8"); } - // Buffer.from(view) copies; but Buffer.from(view.buffer, byteOffset, - // byteLength) is zero-copy and gives us a Buffer that aliases the - // original bytes — exactly what we want before handing off to the - // compression worker. return Buffer.from(body.buffer, body.byteOffset, body.byteLength); } @@ -318,10 +303,9 @@ export async function maybeCompress( } if (encoding === "zstd") { + // Belt-and-braces — `Bun` may have been swapped out between + // construction and first send. Fall through to gzip if so. const bun = (globalThis as { Bun?: BunZstdHost }).Bun; - // Shouldn't happen (factory checked at construction time), but a - // belt-and-braces fallback to gzip keeps us correct if `Bun` is - // swapped out between construction and first send. if (!bun?.zstdCompress) { const gz = await gzipAsync(buf); return { payload: gz, encodingApplied: "gzip" }; From cb67a30dc51fd70c83f96ec847c84b40f3dea0fe Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 25 Apr 2026 18:56:38 +0000 Subject: [PATCH 8/9] =?UTF-8?q?fix(telemetry):=20re-apply=20gzip=20thresho?= =?UTF-8?q?ld=20in=20mid-flight=20zstd=E2=86=92gzip=20fallback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/telemetry/zstd-transport.ts | 7 +++++- test/lib/telemetry/zstd-transport.test.ts | 28 ++++++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index bffb8754a..e6babc923 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -304,9 +304,14 @@ export async function maybeCompress( if (encoding === "zstd") { // Belt-and-braces — `Bun` may have been swapped out between - // construction and first send. Fall through to gzip if so. + // construction and first send. Fall through to gzip if so, but + // re-apply the gzip threshold so mid-sized bodies (1-32 KiB) don't + // get compressed when the SDK default would have shipped them raw. const bun = (globalThis as { Bun?: BunZstdHost }).Bun; if (!bun?.zstdCompress) { + if (buf.length <= GZIP_THRESHOLD) { + return { payload: buf, encodingApplied: "none" }; + } const gz = await gzipAsync(buf); return { payload: gz, encodingApplied: "gzip" }; } diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts index 1b72da381..6d80bc268 100644 --- a/test/lib/telemetry/zstd-transport.test.ts +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -456,18 +456,18 @@ describe("maybeCompress", () => { expect(result.payload).toBe(buf); }); - test("zstd path + Bun.zstdCompress missing mid-flight → gzip safety net", async () => { + test("zstd path + Bun.zstdCompress missing mid-flight + >32 KiB → gzip safety net", async () => { const saved = globalThis.Bun?.zstdCompress; (globalThis as { Bun: { zstdCompress?: unknown } }).Bun.zstdCompress = undefined as never; try { - const buf = Buffer.from("x".repeat(4096)); + const buf = Buffer.from("x".repeat(64 * 1024)); // Encoding pre-selected as "zstd" (caller didn't reprobe), but // the runtime now lacks zstd — the belt-and-braces branch gzips. const result = await maybeCompress(buf, "zstd"); expect(result.encodingApplied).toBe("gzip"); expect(gunzipSync(result.payload).toString("utf-8")).toBe( - "x".repeat(4096) + "x".repeat(64 * 1024) ); } finally { if (saved !== undefined) { @@ -475,6 +475,28 @@ describe("maybeCompress", () => { } } }); + + test("zstd path + Bun.zstdCompress missing mid-flight + 1-32 KiB → passthrough (matches SDK default)", async () => { + // Edge case: caller selected "zstd" because Bun.zstdCompress was + // present at construction, but the global got swapped out before + // the first send. For bodies between ZSTD_THRESHOLD (1 KiB) and + // GZIP_THRESHOLD (32 KiB) we MUST pass them through uncompressed, + // matching the SDK's default-transport behavior. Otherwise we'd + // gzip bodies the SDK would have shipped raw. + const saved = globalThis.Bun?.zstdCompress; + (globalThis as { Bun: { zstdCompress?: unknown } }).Bun.zstdCompress = + undefined as never; + try { + const buf = Buffer.from("x".repeat(8 * 1024)); + const result = await maybeCompress(buf, "zstd"); + expect(result.encodingApplied).toBe("none"); + expect(result.payload).toBe(buf); + } finally { + if (saved !== undefined) { + globalThis.Bun.zstdCompress = saved; + } + } + }); }); describe("isNoProxyExempt", () => { From 2c722cb1f83279e2015c6d6fb9a166003707fb0b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Sat, 25 Apr 2026 19:25:42 +0000 Subject: [PATCH 9/9] refactor(telemetry): pre-merge cleanup from final review pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/telemetry/zstd-transport.ts | 67 ++++------ test/lib/telemetry/zstd-transport.e2e.test.ts | 57 ++++----- .../telemetry/zstd-transport.property.test.ts | 121 +++++++++++++----- test/lib/telemetry/zstd-transport.test.ts | 34 +++-- 4 files changed, 164 insertions(+), 115 deletions(-) diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index e6babc923..6b8151316 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -124,12 +124,31 @@ export function makeCompressedTransport( }); const httpModule = options.httpModule ?? nativeHttpModule; const encoding: SelectedEncoding = hasZstdSupport() ? "zstd" : "gzip"; - const executor = createCompressingExecutor({ - options, - httpModule, - agent, - encoding, - }); + const hostnameIsIPv6 = urlSegments.hostname.startsWith("["); + const hostname = hostnameIsIPv6 + ? urlSegments.hostname.slice(1, -1) + : urlSegments.hostname; + const path = `${urlSegments.pathname}${urlSegments.search}`; + + const executor: TransportRequestExecutor = (request: TransportRequest) => + new Promise((resolve, reject) => { + suppressTracing(() => { + performRequest({ + request, + options, + httpModule, + agent, + encoding, + hostname, + path, + port: urlSegments.port, + protocol: urlSegments.protocol, + }) + .then(resolve) + .catch(reject); + }); + }); + return createTransport(options, executor); } @@ -165,42 +184,6 @@ export function shouldFallbackToDefault( return !isNoProxyExempt(url); } -/** - * @internal Exported for tests. Builds the bare HTTP executor without - * any of `makeCompressedTransport`'s URL / proxy / agent plumbing — the - * caller supplies a fully resolved {@link http.Agent} and the {@link - * http.request}-compatible module to use. - */ -export function createCompressingExecutor(args: { - options: NodeTransportOptions; - httpModule: NonNullable; - agent: http.Agent; - encoding: SelectedEncoding; -}): TransportRequestExecutor { - const { options, httpModule, agent, encoding } = args; - const { hostname, pathname, port, protocol, search } = new URL(options.url); - const hostnameIsIPv6 = hostname.startsWith("["); - - return (request: TransportRequest) => - new Promise((resolve, reject) => { - suppressTracing(() => { - performRequest({ - request, - options, - httpModule, - agent, - encoding, - hostname: hostnameIsIPv6 ? hostname.slice(1, -1) : hostname, - path: `${pathname}${search}`, - port, - protocol, - }) - .then(resolve) - .catch(reject); - }); - }); -} - type PerformRequestArgs = { request: TransportRequest; options: NodeTransportOptions; diff --git a/test/lib/telemetry/zstd-transport.e2e.test.ts b/test/lib/telemetry/zstd-transport.e2e.test.ts index 949b14e09..0818eee7e 100644 --- a/test/lib/telemetry/zstd-transport.e2e.test.ts +++ b/test/lib/telemetry/zstd-transport.e2e.test.ts @@ -5,17 +5,9 @@ * transport at it, and verifies the wire-level behavior: the request * body is zstd-compressed, the `Content-Encoding` header is correct, * and the body decompresses back to a valid envelope. - * - * Uses `useTestConfigDir()` for DB isolation (see AGENTS.md). */ -import { afterAll, describe, expect, test } from "bun:test"; - -/** No-op for SDK callbacks that require a function but return nothing meaningful. */ -function noop(): void { - // intentionally empty -} - +import { afterEach, describe, expect, test } from "bun:test"; import { createServer, type IncomingMessage, type Server } from "node:http"; import type { AddressInfo } from "node:net"; import { createEnvelope } from "@sentry/core"; @@ -23,20 +15,30 @@ import { hasZstdSupport, makeCompressedTransport, } from "../../../src/lib/telemetry/zstd-transport.js"; -import { useTestConfigDir } from "../../helpers.js"; + +/** No-op for SDK callbacks that require a function but return nothing meaningful. */ +function noop(): void { + // intentionally empty +} type CapturedRequest = { headers: IncomingMessage["headers"]; body: Buffer; }; +/** + * Track every server we start so they can be closed in teardown. + * Without this, a `let server` shared across tests is overwritten by + * the second test before the first one is closed — silent socket leak. + */ +const startedServers: Server[] = []; + function startMockIngest( responder: (req: IncomingMessage) => { statusCode: number; headers?: Record; } ): Promise<{ - server: Server; url: string; captures: CapturedRequest[]; }> { @@ -51,12 +53,12 @@ function startMockIngest( res.end(); }); }); + startedServers.push(server); return new Promise((resolve) => { server.listen(0, "127.0.0.1", () => { const addr = server.address() as AddressInfo; resolve({ - server, url: `http://127.0.0.1:${addr.port}/api/0/envelope/`, captures, }); @@ -64,13 +66,18 @@ function startMockIngest( }); } -useTestConfigDir("zstd-transport-e2e-"); - describe("makeCompressedTransport (e2e)", () => { - let server: Server | undefined; - - afterAll(() => { - server?.close(); + afterEach(async () => { + // Close every server started in this test, in parallel. Lets the + // event loop drain naturally instead of relying on process exit. + await Promise.all( + startedServers.splice(0).map( + (s) => + new Promise((resolve) => { + s.close(() => resolve()); + }) + ) + ); }); test("sends zstd-encoded envelope; server decompresses back to original", async () => { @@ -78,14 +85,9 @@ describe("makeCompressedTransport (e2e)", () => { return; } - const { - server: srv, - url, - captures, - } = await startMockIngest(() => ({ + const { url, captures } = await startMockIngest(() => ({ statusCode: 200, })); - server = srv; const transport = makeCompressedTransport({ url, @@ -115,18 +117,13 @@ describe("makeCompressedTransport (e2e)", () => { }); test("rate-limit headers flow back into createTransport wrapper", async () => { - const { - server: srv, - url, - captures, - } = await startMockIngest(() => ({ + const { url, captures } = await startMockIngest(() => ({ statusCode: 429, headers: { "retry-after": "60", "x-sentry-rate-limits": "60:error:organization", }, })); - server = srv; const transport = makeCompressedTransport({ url, diff --git a/test/lib/telemetry/zstd-transport.property.test.ts b/test/lib/telemetry/zstd-transport.property.test.ts index 320aeb317..34e6f7f54 100644 --- a/test/lib/telemetry/zstd-transport.property.test.ts +++ b/test/lib/telemetry/zstd-transport.property.test.ts @@ -1,74 +1,133 @@ /** - * Property tests for the zstd-first transport codec. + * Property tests for the zstd transport's compression pipeline. * - * Any input bytes that go through zstd compression must round-trip - * byte-for-byte when decompressed. Also verifies that string and - * UTF-8-equivalent Uint8Array inputs produce identical wire output — - * so the executor's string-vs-bytes normalization is on-spec. + * Exercises our actual `normalizeBody` + `maybeCompress` helpers (not + * just `Bun.zstdCompress` directly) so we verify the real wire path + * round-trips for arbitrary inputs. + * + * Properties under test: + * 1. zstd-encoded compress → decompress round-trips for any byte sequence. + * 2. gzip-encoded compress → gunzip round-trips for any byte sequence. + * 3. UTF-8 string and equivalent `Uint8Array` inputs produce identical + * wire bytes when fed through `normalizeBody` + `maybeCompress`. + * (This validates our string-vs-bytes normalization, not Bun's + * determinism.) + * 4. Sub-threshold inputs are passthrough — `payload === buf` and + * `encodingApplied === "none"`. */ import { describe, expect, test } from "bun:test"; +import { gunzipSync } from "node:zlib"; import { asyncProperty, assert as fcAssert, + property, string, uint8Array, } from "fast-check"; +import { + maybeCompress, + normalizeBody, +} from "../../../src/lib/telemetry/zstd-transport.js"; import { DEFAULT_NUM_RUNS } from "../../model-based/helpers.js"; -describe("property: zstd round-trip", () => { - test("Bun.zstdCompress(b) → Bun.zstdDecompress === b for all byte sequences", async () => { +const ZSTD_THRESHOLD = 1024; +const GZIP_THRESHOLD = 32 * 1024; + +describe("property: maybeCompress round-trip (zstd path)", () => { + test("zstd compress → decompress returns the original bytes", async () => { await fcAssert( asyncProperty( - uint8Array({ minLength: 0, maxLength: 64 * 1024 }), + uint8Array({ minLength: ZSTD_THRESHOLD + 1, maxLength: 64 * 1024 }), async (bytes) => { const buf = Buffer.from(bytes); - const compressed = await Bun.zstdCompress(buf, { level: 3 }); - const decompressed = await Bun.zstdDecompress(compressed); + const result = await maybeCompress(buf, "zstd"); + expect(result.encodingApplied).toBe("zstd"); + const decompressed = await Bun.zstdDecompress(result.payload); expect(Buffer.from(decompressed).equals(buf)).toBe(true); } ), { numRuns: DEFAULT_NUM_RUNS } ); }); +}); - test("level sweep (3, 5, 6, 9) all round-trip identically", async () => { +describe("property: maybeCompress round-trip (gzip path)", () => { + test("gzip compress → gunzip returns the original bytes", async () => { await fcAssert( asyncProperty( - uint8Array({ minLength: 64, maxLength: 8 * 1024 }), + uint8Array({ minLength: GZIP_THRESHOLD + 1, maxLength: 96 * 1024 }), async (bytes) => { const buf = Buffer.from(bytes); - for (const level of [3, 5, 6, 9]) { - const compressed = await Bun.zstdCompress(buf, { level }); - const decompressed = await Bun.zstdDecompress(compressed); - expect(Buffer.from(decompressed).equals(buf)).toBe(true); - } + const result = await maybeCompress(buf, "gzip"); + expect(result.encodingApplied).toBe("gzip"); + expect(gunzipSync(result.payload).equals(buf)).toBe(true); } ), - { numRuns: 25 } + { numRuns: 25 } // gzip on 96 KiB is slower; fewer runs ); }); +}); - test("string and equivalent Uint8Array inputs produce equal compressed output", async () => { +describe("property: normalizeBody string/Uint8Array equivalence", () => { + test("string and TextEncoder-encoded bytes normalize to identical Buffers", () => { + fcAssert( + property(string({ minLength: 0, maxLength: 16 * 1024 }), (s) => { + const fromString = normalizeBody(s); + const fromBytes = normalizeBody(new TextEncoder().encode(s)); + expect(fromString.equals(fromBytes)).toBe(true); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("string input through full compress pipeline matches bytes input", async () => { await fcAssert( asyncProperty( - string({ minLength: 32, maxLength: 16 * 1024 }), + string({ minLength: ZSTD_THRESHOLD + 1, maxLength: 16 * 1024 }), async (s) => { - const fromString = await Bun.zstdCompress(Buffer.from(s, "utf-8"), { - level: 3, - }); - const fromBytes = await Bun.zstdCompress( - new TextEncoder().encode(s), - { - level: 3, - } - ); - expect(Buffer.from(fromString).equals(Buffer.from(fromBytes))).toBe( - true + const fromString = await maybeCompress(normalizeBody(s), "zstd"); + const fromBytes = await maybeCompress( + normalizeBody(new TextEncoder().encode(s)), + "zstd" ); + expect(fromString.encodingApplied).toBe(fromBytes.encodingApplied); + expect(fromString.payload.equals(fromBytes.payload)).toBe(true); + } + ), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); + +describe("property: maybeCompress passthrough below threshold", () => { + test('zstd encoding + body ≤ 1 KiB → encodingApplied="none", payload === buf', async () => { + await fcAssert( + asyncProperty( + uint8Array({ minLength: 0, maxLength: ZSTD_THRESHOLD }), + async (bytes) => { + const buf = Buffer.from(bytes); + const result = await maybeCompress(buf, "zstd"); + expect(result.encodingApplied).toBe("none"); + expect(result.payload).toBe(buf); } ), { numRuns: DEFAULT_NUM_RUNS } ); }); + + test('gzip encoding + body ≤ 32 KiB → encodingApplied="none", payload === buf', async () => { + await fcAssert( + asyncProperty( + uint8Array({ minLength: 0, maxLength: GZIP_THRESHOLD }), + async (bytes) => { + const buf = Buffer.from(bytes); + const result = await maybeCompress(buf, "gzip"); + expect(result.encodingApplied).toBe("none"); + expect(result.payload).toBe(buf); + } + ), + { numRuns: 25 } // 32 KiB arbitrary alloc is slower; fewer runs + ); + }); }); diff --git a/test/lib/telemetry/zstd-transport.test.ts b/test/lib/telemetry/zstd-transport.test.ts index 6d80bc268..db143cf87 100644 --- a/test/lib/telemetry/zstd-transport.test.ts +++ b/test/lib/telemetry/zstd-transport.test.ts @@ -289,24 +289,34 @@ describe("makeCompressedTransport", () => { expect(response).toEqual({}); }); - test("proxy configured: falls back to SDK's makeNodeTransport", () => { + test("proxy configured: falls back to SDK's makeNodeTransport (no zstd applied)", async () => { const savedProxy = process.env.https_proxy; process.env.https_proxy = "http://proxy.internal:3128"; try { - // No httpModule override: we can't observe the returned transport's - // internals, but we can prove the path differs from the zstd one - // by checking that no zstd-specific header is attached when a - // proxy is present. The test below (normalizeBody + maybeCompress) - // exercise the zstd codepath directly. + // The SDK's makeNodeTransport also honors options.httpModule, so + // we can route both paths through our mock and tell them apart by + // the Content-Encoding header on the wire: the zstd path sets it + // for any body > 1 KiB, while the SDK default only sets gzip for + // bodies > 32 KiB. A 4 KiB body therefore distinguishes the two + // — zstd path stamps "zstd", SDK path stamps nothing. + const { httpModule, captured } = buildMockHttpModule({ + statusCode: 200, + headers: {}, + }); const transport = makeCompressedTransport({ ...BASE_OPTIONS, - // makeNodeTransport requires httpModule too; with the default it - // will try to create an Agent. Pass a dummy module to avoid any - // real network I/O when the returned transport is constructed. + httpModule, }); - // Sanity: we got *some* transport object back with send/flush. - expect(typeof transport.send).toBe("function"); - expect(typeof transport.flush).toBe("function"); + const envelope: any = createEnvelope({} as any, [ + [{ type: "event" } as any, { data: "x".repeat(4096) } as any], + ]); + await transport.send(envelope); + + const headers = captured.options.headers as Record; + expect(headers["content-encoding"]).toBeUndefined(); + // And the wire body is the raw envelope (not zstd-compressed). + const wire = Buffer.concat(captured.chunks); + expect(wire.toString("utf-8")).toContain('"type":"event"'); } finally { if (savedProxy === undefined) { delete process.env.https_proxy;