Skip to content

Consolidate the global-fetch integrations into @sentry/server-utils #24344

Description

@isaacs

Follow-up to #24121 (deno fetch instrumentation). That PR lands a fourth near-identical copy of the same integration. This issue tracks folding all four into one shared implementation and fixing the drift they have already accumulated.

Problem

Four packages carry the same ~90 lines around instrumentFetchRequest:

They are identical apart from the table below. Every fix needs four edits, which is why the drift exists.

bun cloudflare vercel-edge deno
integration name Fetch Fetch WinterCGFetch Fetch
export fetchIntegration fetchIntegration winterCGFetchIntegration fetchIntegration
spanOrigin auto.http.fetch auto.http.fetch auto.http.wintercg_fetch auto.http.fetch
propagateTraceparent yes yes no yes
options type exported no yes (Options) yes (Options) no
options type name FetchOptions Options Options FetchOptions
breadcrumbs option yes yes yes no (see below)
breadcrumbs field optional required + Partial<> required + Partial<> n/a
unit tests none yes yes yes (deno)

The propagateTraceparent row is a live bug: vercel-edge never sends the traceparent header, because that one copy was not updated.

Two more defects are shared by all four:

  1. setupOnce pins the first instance's options. setupOnce runs once per integration name per process (packages/core/src/integration.ts:117), and the handler closes over the first instance's shouldCreateSpanForRequest, LRU caches, and spans map. A later init() reaches only setup(), which flips a boolean in HAS_CLIENT_MAP. Its options are dropped. Verified on deno: in a fresh process fetchIntegration({ shouldCreateSpanForRequest: () => false }) works; after any earlier init() the callback is never called and the span is created anyway.
  2. No way to turn propagation off without turning spans off. shouldCreateSpanForRequest suppresses the span but still injects headers. Node solves this with a tracePropagation boolean (packages/node/src/integrations/node-fetch/types.ts:112-119); none of the four has one. On deno this also means denoHttpIntegration({ tracePropagation: false }) mutes node:http and silently leaves fetch propagating.

Where it goes

packages/server-utils/src/, exported from src/exports.ts, so it is reachable from both @sentry/server-utils and @sentry/server-utils/no-diagnostic-channels.

Reasons:

  • All four packages already depend on @sentry/server-utils.
  • src/exports.ts is the surface with no node: builtins. cloudflare (workerd) and vercel-edge import only through /no-diagnostic-channels, so the shared code must live there, not in src/index.ts.
  • @sentry/core is the wrong home: instrumentFetchRequest and addFetchInstrumentationHandler already live there, and this is the server-runtime integration wrapper around them.

Suggested path: packages/server-utils/src/integrations/fetch.ts.

Shared API

createFetchIntegration({
  name, // 'Fetch' | 'WinterCGFetch'
  spanOrigin, // 'auto.http.fetch' | 'auto.http.wintercg_fetch'
  breadcrumbs, // default for the option; deno passes false
});

Returns an IntegrationFn whose options are:

interface FetchIntegrationOptions {
  breadcrumbs?: boolean; // default true
  shouldCreateSpanForRequest?: (url: string) => boolean;
  tracePropagation?: boolean; // default true, new (see defect 2)
}

The factory keeps the existing internals: the isSentryRequestUrl guard, the two LRUMap caches, the spans record, propagateTraceparent read from client options, and the breadcrumb builder.

Fix defect 1 while moving it: replace WeakMap<Client, boolean> with WeakMap<Client, ResolvedOptions> populated in setup(client), and have the single setupOnce handler read the current client's config from that map. Options then follow the client instead of the first init().

Per-package changes

packages/server-utils

  • Add src/integrations/fetch.ts with the factory and the breadcrumb builder (lift from packages/bun/src/integrations/fetch.ts, which has the most complete copy).
  • Export createFetchIntegration and FetchIntegrationOptions from src/exports.ts.
  • Add the unit tests (below).

packages/bun

  • Replace src/integrations/fetch.ts with a call to the factory.
  • Export the options type from src/index.ts (currently missing).

packages/cloudflare

  • Replace src/integrations/fetch.ts with a call to the factory.
  • Rename the exported Options to FetchIntegrationOptions (keep the old name as a deprecated alias) and drop the Partial<Options> / required-field awkwardness.
  • Drop the dead client?.getOptions() || {} optional chaining; the guard above it already narrows client.
  • Retitle test/integrations/fetch.test.ts, whose describe block still says "WinterCGFetch instrumentation" although the integration is named Fetch.

packages/vercel-edge

  • Replace src/integrations/wintercg-fetch.ts with a call to the factory.
  • Decide whether it keeps WinterCGFetch / auto.http.wintercg_fetch or converges on Fetch / auto.http.fetch. Converging is a breaking change: the name is public API, and dev-packages/e2e-tests/test-applications/nextjs-pages-dir/tests/middleware.test.ts:80 asserts the origin. Suggest keeping both as-is for now and passing them to the factory, then converging in the next major.
  • propagateTraceparent starts working here as a side effect. Call it out in the changelog; it is a behavior change, not just a refactor.

packages/deno

  • Replace src/integrations/fetch.ts with createFetchIntegration({ name: 'Fetch', spanOrigin: 'auto.http.fetch', breadcrumbs: false }).
  • Export the options type from src/index.ts.

Other integrations to fold or tidy

packages/deno/src/integrations/breadcrumbs.ts. Deno is the only server runtime that records fetch breadcrumbs in a separate integration; the other three do it inside the fetch integration. Two consequences worth resolving in the same pass:

  • breadcrumbs.ts:150 decides "is this a Sentry request?" with url.match(/sentry_key/) && method === 'POST', while fetch.ts:66 uses isSentryRequestUrl. One package, two rules. Move breadcrumbs.ts onto isSentryRequestUrl.
  • Once the fetch integration can record breadcrumbs (via the shared breadcrumbs option), the fetch half of breadcrumbs.ts could be deleted and deno could pass breadcrumbs: true like the others. That aligns all four runtimes but changes which integration a user disables to stop fetch breadcrumbs (breadcrumbsIntegration({ fetch: false }) today, fetchIntegration({ breadcrumbs: false }) after). Decide whether that rename is worth doing now or at the next major. If it waits, keep breadcrumbs: false on deno and leave breadcrumbs.ts alone apart from the isSentryRequestUrl fix.

Deno option-surface mismatch. denoHttpIntegration offers tracePropagation, ignoreOutgoingRequests, spans, and breadcrumbs; the fetch path will offer tracePropagation, breadcrumbs, and shouldCreateSpanForRequest. The new tracePropagation option closes the worst of the gap. Until the names converge, both docstrings should say the option covers one client path only, so nobody reads denoHttpIntegration({ tracePropagation: false }) as covering fetch.

Not in scope

Browser. packages/browser/src/tracing/request.ts cannot fold in. It is not an integration: instrumentOutgoingRequests(client, options) is called by browserTracingIntegration, shares one spans map with the XHR path, reads tracePropagationTargets from integration options through its own shouldAttachHeaders rather than from client options through shouldPropagateTraceForUrl, passes urlBase: WINDOW.location.origin, sets URL_FULL and SERVER_ADDRESS after the fact, and adds Resource Timing data. It also has no setupOnce / HAS_CLIENT_MAP lifecycle, and @sentry/browser does not depend on @sentry/server-utils. The common part is already factored out as instrumentFetchRequest in core.

Node. nativeNodeFetchIntegration instruments undici through diagnostics channels, not the global fetch function. Different mechanism, no overlap.

packages/browser/src/integrations/httpclient.ts and fetchStreamPerformance.ts. Browser-only, unrelated.

Tests

One shared suite in packages/server-utils/test/integrations/fetch.test.ts covering: span creation, shouldCreateSpanForRequest, header propagation against tracePropagationTargets, the new tracePropagation: false, propagateTraceparent, the isSentryRequestUrl skip, breadcrumbs on and off, the error path, and per-client options after a second init() (defect 1).

Per-package tests then shrink to "the integration is in the default set and is wired with the right name and origin". Bun gains its first fetch test this way. The deno test added in #24121 also lacks a case for the default traceLifecycle: 'stream'; add it here or there.

Acceptance

  • One implementation. No instrumentFetchRequest call sites outside @sentry/server-utils and @sentry/browser.
  • vercel-edge sends traceparent.
  • Integration options survive a second init().
  • tracePropagation: false stops header injection while spans stay on.
  • Options types exported from all four packages.
  • No behavior change for bun, cloudflare, or deno beyond the two new options.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

javascriptPull requests that update javascript code

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions