diff --git a/MIGRATION.md b/MIGRATION.md index e44b509be415..4beda614ba23 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -614,6 +614,39 @@ options were already named `tracePropagation`. This is unrelated to `propagateTraceparent` (whether the W3C `traceparent` header is sent alongside `sentry-trace`) and `tracePropagationTargets` (which URLs receive trace headers). Both keep their names. +### Deno server transactions are dropped for some 3xx/4xx status codes + +Affected SDKs: `@sentry/deno`. + +`denoHttpIntegration` and `denoServeIntegration` now honor `ignoreStatusCodes`, using the same default list as +`httpIntegration` in the other server SDKs: incoming request transactions whose response status falls in +`[[401, 404], [301, 303], [305, 399]]` are dropped. Previously the option was declared but never read, so these +transactions were always kept. + +Each integration owns the option for the requests it instruments — `denoHttpIntegration` for `node:http`, +`denoServeIntegration` for `Deno.serve` — so setting it on one does not affect the other. Pass your own list to change +which codes are dropped, or an empty array to keep everything: + +```js +Sentry.init({ + dsn: '__DSN__', + integrations: [ + Sentry.denoHttpIntegration({ ignoreStatusCodes: [] }), + Sentry.denoServeIntegration({ ignoreStatusCodes: [] }), + ], +}); +``` + +This filter runs on transaction events (`processEvent`), so it only takes effect when `traceLifecycle` is `'static'`. +The default `'stream'` lifecycle does not produce transaction events, and typical Deno apps are unaffected. Node's +`httpIntegration` has the same limitation. + +Transactions that are kept now also carry the HTTP status in the top-level `response` context, as in the other server +SDKs. + +`denoHttpIntegration` additionally accepts the outgoing request hooks `outgoingRequestHook`, `outgoingResponseHook` and +`outgoingRequestApplyCustomAttributes`, matching `httpIntegration`. + ### `tracePropagationTargets` matching is now case-insensitive Affected SDKs: All SDKs. diff --git a/packages/core/src/integrations/http/server-transaction-event.ts b/packages/core/src/integrations/http/server-transaction-event.ts new file mode 100644 index 000000000000..7a49b04d463a --- /dev/null +++ b/packages/core/src/integrations/http/server-transaction-event.ts @@ -0,0 +1,84 @@ +/** + * Shared post-processing for transaction events produced by server span instrumentation. + * + * Node's `httpServerSpansIntegration` and Deno's `denoHttpIntegration` both create their + * server spans outside of the OTel SDK span exporter, so neither gets the exporter's + * status code handling for free. Both run this from their `processEvent` hook instead. + */ + +import { HTTP_RESPONSE_STATUS_CODE } from '@sentry/conventions/attributes'; +import { DEBUG_BUILD } from '../../debug-build'; +import type { Event } from '../../types/event'; +import { debug } from '../../utils/debug-logger'; + +/** + * Status codes for which server transactions are dropped unless `ignoreStatusCodes` says otherwise. + * + * 300 and 304 are possibly valid status codes we do not want to filter, hence the split ranges. + */ +export const DEFAULT_IGNORE_STATUS_CODES: (number | [number, number])[] = [ + [401, 404], + [301, 303], + [305, 399], +]; + +/** + * If the given status code should be filtered for the given list of status codes/ranges. + */ +export function shouldFilterStatusCode(statusCode: number, dropForStatusCodes: (number | [number, number])[]): boolean { + return dropForStatusCodes.some(code => { + if (typeof code === 'number') { + return code === statusCode; + } + + const [min, max] = code; + return statusCode >= min && statusCode <= max; + }); +} + +/** + * Drop transaction events whose HTTP status code matches `ignoreStatusCodes`, and surface the + * status as the top-level `response` context on the ones that are kept. + * + * Pass `spanOrigin` to only act on transactions produced by a specific instrumentation, so that + * an integration owning this option does not filter transactions created by a different one. + * When omitted, every transaction carrying an HTTP status code is considered. + * + * Returns `null` when the event should be dropped, otherwise the (possibly updated) event. + */ +export function processHttpServerTransactionEvent( + event: Event, + ignoreStatusCodes: (number | [number, number])[], + spanOrigin?: string, +): Event | null { + if (event.type !== 'transaction') { + return event; + } + + if (spanOrigin !== undefined && event.contexts?.trace?.origin !== spanOrigin) { + return event; + } + + const statusCode = event.contexts?.trace?.data?.[HTTP_RESPONSE_STATUS_CODE]; + if (typeof statusCode !== 'number') { + return event; + } + + if (shouldFilterStatusCode(statusCode, ignoreStatusCodes)) { + DEBUG_BUILD && debug.log('Dropping transaction due to status code', statusCode); + return null; + } + + // Surface the HTTP status as the top-level `response` context. The OTel SDK span exporter + // already does this on its path; doing it here covers transactions produced by tracer + // providers that bypass that exporter (Node's `SentryTracerProvider`, Deno's). + event.contexts = { + ...event.contexts, + response: { + ...event.contexts?.response, + status_code: statusCode, + }, + }; + + return event; +} diff --git a/packages/core/src/integrations/http/types.ts b/packages/core/src/integrations/http/types.ts index dac853d6b167..70c76fbf9256 100644 --- a/packages/core/src/integrations/http/types.ts +++ b/packages/core/src/integrations/http/types.ts @@ -265,15 +265,6 @@ export interface HttpInstrumentationOptions { */ ignoreStaticAssets?: boolean; - /** - * Do not capture spans for incoming HTTP requests with the given status codes. - * By default, spans with some 3xx and 4xx status codes are ignored (see @default). - * Expects an array of status codes or a range of status codes, e.g. [[300,399], 404] would ignore 3xx and 404 status codes. - * - * @default `[[401, 404], [301, 303], [305, 399]]` - */ - ignoreStatusCodes?: (number | [number, number])[]; - /** * A hook that can be used to mutate the span for incoming requests. * This is triggered after the span is created, but before it is recorded. diff --git a/packages/core/src/server-exports.ts b/packages/core/src/server-exports.ts index 5037dc9cb784..65fc021d3f08 100644 --- a/packages/core/src/server-exports.ts +++ b/packages/core/src/server-exports.ts @@ -41,6 +41,10 @@ export { getSqlQuerySummary as _INTERNAL_getSqlQuerySummary } from './utils/sql' export { patchHttpModuleClient } from './integrations/http/client-patch'; export { getHttpClientSubscriptions } from './integrations/http/client-subscriptions'; export { getHttpServerSubscriptions, isStaticAssetRequest } from './integrations/http/server-subscription'; +export { + DEFAULT_IGNORE_STATUS_CODES, + processHttpServerTransactionEvent, +} from './integrations/http/server-transaction-event'; export { recordRequestSession } from './integrations/http/record-request-session'; export { addOutgoingRequestBreadcrumb } from './integrations/http/add-outgoing-request-breadcrumb'; export { diff --git a/packages/core/test/lib/integrations/http/server-transaction-event.test.ts b/packages/core/test/lib/integrations/http/server-transaction-event.test.ts new file mode 100644 index 000000000000..47b48f885a49 --- /dev/null +++ b/packages/core/test/lib/integrations/http/server-transaction-event.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from 'vitest'; +import type { Event } from '../../../../src/types/event'; +import { + DEFAULT_IGNORE_STATUS_CODES, + processHttpServerTransactionEvent, + shouldFilterStatusCode, +} from '../../../../src/integrations/http/server-transaction-event'; + +function transaction(statusCode?: number, contexts: Record = {}): Event { + return { + type: 'transaction', + contexts: { + ...contexts, + trace: { data: statusCode === undefined ? {} : { 'http.response.status_code': statusCode } }, + }, + } as Event; +} + +describe('shouldFilterStatusCode', () => { + it('matches plain status codes', () => { + expect(shouldFilterStatusCode(404, [404])).toBe(true); + expect(shouldFilterStatusCode(500, [404])).toBe(false); + }); + + it('matches inclusive ranges', () => { + expect(shouldFilterStatusCode(300, [[300, 399]])).toBe(true); + expect(shouldFilterStatusCode(399, [[300, 399]])).toBe(true); + expect(shouldFilterStatusCode(400, [[300, 399]])).toBe(false); + }); + + it('matches a mix of codes and ranges', () => { + expect(shouldFilterStatusCode(404, [[300, 399], 404])).toBe(true); + }); + + it('never matches on an empty list', () => { + expect(shouldFilterStatusCode(404, [])).toBe(false); + }); + + it.each([ + [300, false], + [301, true], + [303, true], + [304, false], + [305, true], + [399, true], + [401, true], + [404, true], + [405, false], + [200, false], + [500, false], + ])('applies the default list correctly to %i', (statusCode, expected) => { + expect(shouldFilterStatusCode(statusCode, DEFAULT_IGNORE_STATUS_CODES)).toBe(expected); + }); +}); + +describe('processHttpServerTransactionEvent', () => { + it('drops transactions whose status code is ignored', () => { + expect(processHttpServerTransactionEvent(transaction(404), [404])).toBeNull(); + }); + + it('lifts the status code into the top-level `response` context', () => { + const event = processHttpServerTransactionEvent(transaction(200), []); + expect(event?.contexts?.response).toEqual({ status_code: 200 }); + }); + + it('preserves existing `response` context fields', () => { + const event = processHttpServerTransactionEvent(transaction(201, { response: { body_size: 42 } }), []); + expect(event?.contexts?.response).toEqual({ body_size: 42, status_code: 201 }); + }); + + it('leaves the event untouched when there is no status code', () => { + const event = processHttpServerTransactionEvent(transaction(undefined), []); + expect(event?.contexts?.response).toBeUndefined(); + }); + + it('ignores events from a different span origin when spanOrigin is given', () => { + const event = { + ...transaction(404), + contexts: { trace: { origin: 'auto.http.deno', data: { 'http.response.status_code': 404 } } }, + } as Event; + // Would be dropped without the gate; the origin does not match, so it passes through. + expect(processHttpServerTransactionEvent(event, [404], 'auto.http.server')).toBe(event); + }); + + it('acts on events whose span origin matches', () => { + const event = { + ...transaction(404), + contexts: { trace: { origin: 'auto.http.server', data: { 'http.response.status_code': 404 } } }, + } as Event; + expect(processHttpServerTransactionEvent(event, [404], 'auto.http.server')).toBeNull(); + }); + + it('acts on every origin when spanOrigin is omitted', () => { + const event = { + ...transaction(404), + contexts: { trace: { origin: 'auto.http.deno', data: { 'http.response.status_code': 404 } } }, + } as Event; + expect(processHttpServerTransactionEvent(event, [404])).toBeNull(); + }); + + it('leaves non-transaction events untouched, even with an ignored status code', () => { + const event = { type: undefined, contexts: { trace: { data: { 'http.response.status_code': 404 } } } } as Event; + expect(processHttpServerTransactionEvent(event, [404])).toBe(event); + }); +}); diff --git a/packages/deno/src/integrations/deno-serve.ts b/packages/deno/src/integrations/deno-serve.ts index e51009c25249..5c973cc82239 100644 --- a/packages/deno/src/integrations/deno-serve.ts +++ b/packages/deno/src/integrations/deno-serve.ts @@ -1,5 +1,5 @@ -import type { IntegrationFn, MaxRequestBodySize } from '@sentry/core'; -import { debug, defineIntegration } from '@sentry/core'; +import type { Event, IntegrationFn, MaxRequestBodySize } from '@sentry/core'; +import { debug, DEFAULT_IGNORE_STATUS_CODES, defineIntegration, processHttpServerTransactionEvent } from '@sentry/core'; import type { RequestHandlerWrapperOptions } from '../wrap-deno-request-handler'; import { wrapDenoRequestHandler } from '../wrap-deno-request-handler'; @@ -15,6 +15,21 @@ export type DenoServeIntegrationOptions = { * @default 'medium' */ maxRequestBodySize?: MaxRequestBodySize; + + /** + * Do not capture spans for incoming `Deno.serve` requests with the given status codes. + * By default, some 3xx and 4xx status codes are dropped (see @default). + * Expects an array of status codes or a range of status codes, e.g. [[300,399], 404] would ignore 3xx and 404 status codes. + * + * Applies only to spans this integration creates. `node:http` requests are covered by + * `denoHttpIntegration`'s own option of the same name. Pass `[]` to keep everything. + * + * Only takes effect with `traceLifecycle: 'static'`. The default `'stream'` lifecycle does not + * produce transaction events, so the filter does not run. + * + * @default `[[401, 404], [301, 303], [305, 399]]` + */ + ignoreStatusCodes?: (number | [number, number])[]; }; export type ServeParams = @@ -72,9 +87,16 @@ const instrumentedDenoServe = (serve: typeof Deno.serve): typeof Deno.serve => }); const _denoServeIntegration = ((options: DenoServeIntegrationOptions = {}) => { + const ignoreStatusCodes = options.ignoreStatusCodes ?? DEFAULT_IGNORE_STATUS_CODES; + return { name: INTEGRATION_NAME, maxRequestBodySize: options.maxRequestBodySize, + processEvent(event: Event): Event | null { + // Gated on this integration's own span origin so it does not filter `node:http` + // transactions, which `denoHttpIntegration` owns via its own `ignoreStatusCodes`. + return processHttpServerTransactionEvent(event, ignoreStatusCodes, 'auto.http.deno'); + }, setupOnce() { const originalServe = Deno.serve; const wrappedServe = instrumentedDenoServe(originalServe); diff --git a/packages/deno/src/integrations/http.ts b/packages/deno/src/integrations/http.ts index ee9909f9a8a1..2b3174e5b636 100644 --- a/packages/deno/src/integrations/http.ts +++ b/packages/deno/src/integrations/http.ts @@ -1,14 +1,24 @@ import { subscribe } from 'node:diagnostics_channel'; import { errorMonitor } from 'node:events'; import type { RequestOptions } from 'node:http'; -import type { HttpIncomingMessage, HttpServerResponse, Integration, IntegrationFn, Span } from '@sentry/core'; +import type { + Event, + HttpClientRequest, + HttpIncomingMessage, + HttpServerResponse, + Integration, + IntegrationFn, + Span, +} from '@sentry/core'; import { + DEFAULT_IGNORE_STATUS_CODES, defineIntegration, getHttpClientSubscriptions, getHttpServerSubscriptions, getRequestOptions, HTTP_ON_CLIENT_REQUEST, HTTP_ON_SERVER_REQUEST, + processHttpServerTransactionEvent, } from '@sentry/core'; const INTEGRATION_NAME = 'DenoHttp' as const; @@ -91,6 +101,43 @@ export interface DenoHttpIntegrationOptions { */ ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; + /** + * Do not send transaction events for incoming HTTP requests with the given status codes. + * By default, some 3xx and 4xx status codes are dropped (see @default). + * Expects an array of status codes or a range of status codes, e.g. [[300,399], 404] would ignore 3xx and 404 status codes. + * + * Applies only to spans this integration creates (`node:http`). `Deno.serve` requests are + * covered by `denoServeIntegration`'s own option of the same name. Pass `[]` to keep everything. + * + * Only takes effect with `traceLifecycle: 'static'`. The default `'stream'` lifecycle does not + * produce transaction events, so the filter does not run. Node's `httpIntegration` has the same + * limitation. + * + * @default `[[401, 404], [301, 303], [305, 399]]` + */ + ignoreStatusCodes?: (number | [number, number])[]; + + /** + * Called after the span for an outgoing request is created. + * Use this to add custom attributes to the span. + */ + outgoingRequestHook?: (span: Span, request: HttpClientRequest) => void; + + /** + * Called when the response to an outgoing request is received. + */ + outgoingResponseHook?: (span: Span, response: HttpIncomingMessage) => void; + + /** + * Called once both the outgoing request and its response are available (after the response + * ends). Useful for adding attributes based on both objects. + */ + outgoingRequestApplyCustomAttributes?: ( + span: Span, + request: HttpClientRequest, + response: HttpIncomingMessage, + ) => void; + /** * A hook that can be used to mutate the span for incoming requests. * This is triggered after the span is created, but before it is recorded. @@ -106,9 +153,15 @@ export interface DenoHttpIntegrationOptions { const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => { const breadcrumbs = options.breadcrumbs ?? true; const tracePropagation = options.tracePropagation ?? true; + const ignoreStatusCodes = options.ignoreStatusCodes ?? DEFAULT_IGNORE_STATUS_CODES; return { name: INTEGRATION_NAME, + processEvent(event: Event): Event | null { + // Gated on this integration's own span origin so it does not filter `Deno.serve` + // transactions, which `denoServeIntegration` owns via its own `ignoreStatusCodes`. + return processHttpServerTransactionEvent(event, ignoreStatusCodes, 'auto.http.server'); + }, setupOnce() { const { [HTTP_ON_SERVER_REQUEST]: onHttpServerRequest } = getHttpServerSubscriptions({ ...options, @@ -120,6 +173,7 @@ const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => { ...options, breadcrumbs, tracePropagation, + applyCustomAttributesOnSpan: options.outgoingRequestApplyCustomAttributes, ignoreOutgoingRequests: options.ignoreOutgoingRequests ? (url, request) => options.ignoreOutgoingRequests!(url, getRequestOptions(request)) : undefined, @@ -145,4 +199,8 @@ const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => { */ export const denoHttpIntegration = defineIntegration(_denoHttpIntegration) as ( options?: DenoHttpIntegrationOptions, -) => Integration & { name: 'DenoHttp'; setupOnce: () => void }; +) => Integration & { + name: 'DenoHttp'; + setupOnce: () => void; + processEvent: (event: Event) => Event | null; +}; diff --git a/packages/deno/test/deno-http-outgoing-hooks.test.ts b/packages/deno/test/deno-http-outgoing-hooks.test.ts new file mode 100644 index 000000000000..a0e60b50e147 --- /dev/null +++ b/packages/deno/test/deno-http-outgoing-hooks.test.ts @@ -0,0 +1,142 @@ +/// + +/** + * Lives in its own file because `setupOnce` runs once per process + * (`installedIntegrations` guards it) and the diagnostics channel + * subscription is global. Deno gives each test file a fresh module graph, + * so this is the only way to install `denoHttpIntegration` with the outgoing + * request hooks after another file has installed it with the defaults. + */ + +import * as http from 'node:http'; +import type { TransactionEvent } from '@sentry/core'; +import { getMainCarrier } from '@sentry/core'; +import { assertEquals } from 'https://deno.land/std@0.212.0/assert/assert_equals.ts'; +import { assertExists } from 'https://deno.land/std@0.212.0/assert/assert_exists.ts'; +import { denoHttpIntegration, init, startSpan } from '../build/esm/index.js'; + +/** + * `beforeSendTransaction` hook plus a `waitFor(predicate)` helper + * resolves when a matching transaction arrives (or has already arrived) + */ +function transactionSink(): { + transactions: TransactionEvent[]; + beforeSendTransaction: (event: TransactionEvent) => null; + waitFor: (predicate: (event: TransactionEvent) => boolean) => Promise; +} { + const transactions: TransactionEvent[] = []; + const waiters: { predicate: (e: TransactionEvent) => boolean; resolve: (e: TransactionEvent) => void }[] = []; + return { + transactions, + beforeSendTransaction(event) { + transactions.push(event); + for (let i = waiters.length - 1; i >= 0; i--) { + const w = waiters[i]!; + if (w.predicate(event)) { + waiters.splice(i, 1); + w.resolve(event); + } + } + return null; + }, + waitFor(predicate) { + const already = transactions.find(predicate); + if (already) return Promise.resolve(already); + return new Promise(resolve => { + waiters.push({ predicate, resolve }); + }); + }, + }; +} + +// Bind a promise so a real "never arrives" bug fails the test. +function withTimeout(p: Promise, ms: number, what: string): Promise { + let timer: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`Timed out waiting for ${what} after ${ms}ms`)), ms); + }); + // Clear the timer on either resolution so Deno's leak detector is happy. + return Promise.race([p, timeout]).finally(() => { + if (timer !== undefined) clearTimeout(timer); + }); +} + +const calls: string[] = []; + +Deno.test({ + name: 'denoHttpIntegration: runs outgoingRequestHook, outgoingResponseHook and outgoingRequestApplyCustomAttributes', + async fn() { + getMainCarrier().__SENTRY__ = undefined; + + const sink = transactionSink(); + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + beforeSendTransaction: sink.beforeSendTransaction, + integrations: [ + denoHttpIntegration({ + outgoingRequestHook: (span, request) => { + calls.push('outgoingRequestHook'); + span.setAttribute('outgoingRequestHook', request.method ?? 'unknown'); + }, + outgoingResponseHook: (span, response) => { + calls.push('outgoingResponseHook'); + span.setAttribute('outgoingResponseHook', response.statusCode ?? 0); + }, + outgoingRequestApplyCustomAttributes: (span, request, response) => { + calls.push('outgoingRequestApplyCustomAttributes'); + span.setAttribute('outgoingRequestApplyCustomAttributes', `${request.method}:${response.statusCode}`); + }, + }), + ], + }); + + // Use Deno.serve for the target so this test does not depend on the + // node:http server-side instrumentation. + const abortController = new AbortController(); + let onListen: ((_: unknown) => void) | undefined; + const listening = new Promise(resolve => (onListen = resolve)); + const target = Deno.serve( + { port: 0, signal: abortController.signal, onListen, hostname: '127.0.0.1' }, + () => new Response('pong'), + ); + await listening; + const targetPort = target.addr.port; + + await startSpan({ name: 'parent', op: 'test' }, async () => { + await new Promise((resolve, reject) => { + const req = http.request({ host: '127.0.0.1', port: targetPort, path: '/ping', method: 'GET' }, res => { + res.on('data', () => {}); + res.on('end', () => resolve()); + res.on('error', reject); + }); + req.on('error', reject); + req.end(); + }); + }); + + const parent = await withTimeout( + sink.waitFor(t => t.transaction === 'parent'), + 5000, + "'parent' transaction", + ); + + abortController.abort(); + await target.finished; + + const httpClientSpan = parent.spans?.find(s => s.op === 'http.client'); + assertExists( + httpClientSpan, + `expected an http.client child span, got ops: ${parent.spans?.map(s => s.op).join(', ')}`, + ); + + assertEquals(httpClientSpan!.data?.['outgoingRequestHook'], 'GET'); + assertEquals(httpClientSpan!.data?.['outgoingResponseHook'], 200); + assertEquals(httpClientSpan!.data?.['outgoingRequestApplyCustomAttributes'], 'GET:200'); + + // The request hook fires first; the apply-custom-attributes hook fires last, + // once both the request and the finished response are available. + assertEquals(calls, ['outgoingRequestHook', 'outgoingResponseHook', 'outgoingRequestApplyCustomAttributes']); + }, +}); diff --git a/packages/deno/test/deno-http.test.ts b/packages/deno/test/deno-http.test.ts index 640e7eefaa0d..e06ca0529418 100644 --- a/packages/deno/test/deno-http.test.ts +++ b/packages/deno/test/deno-http.test.ts @@ -1,4 +1,4 @@ -// +/// import * as http from 'node:http'; import type { Envelope, SessionAggregates, TransactionEvent } from '@sentry/core'; @@ -7,7 +7,7 @@ import { assert } from 'https://deno.land/std@0.212.0/assert/assert.ts'; import { assertEquals } from 'https://deno.land/std@0.212.0/assert/assert_equals.ts'; import { assertExists } from 'https://deno.land/std@0.212.0/assert/assert_exists.ts'; import type { DenoClient } from '../build/esm/index.js'; -import { init, startSpan } from '../build/esm/index.js'; +import { denoHttpIntegration, init, startSpan } from '../build/esm/index.js'; import { makeTestTransport } from './transport.ts'; function resetGlobals(): void { @@ -217,3 +217,170 @@ Deno.test({ assertEquals(httpClientSpan!.data?.['http.response.status_code'], 200); }, }); + +/** Start a node:http server that echoes the status code named by the path, e.g. `/404`. */ +async function startStatusServer(): Promise<{ port: number; close: () => Promise }> { + const server = http.createServer((req, res) => { + res.statusCode = Number(req.url?.replace('/', '')) || 200; + res.end('ok'); + }); + const port: number = await new Promise(resolve => { + server.listen(0, '127.0.0.1', () => { + resolve((server.address() as { port: number }).port); + }); + }); + return { port, close: () => new Promise(resolve => server.close(() => resolve())) }; +} + +Deno.test({ + name: 'denoHttpIntegration: drops transactions with a default-ignored status code', + async fn() { + resetGlobals(); + const sink = transactionSink(); + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + beforeSendTransaction: sink.beforeSendTransaction, + traceLifecycle: 'static', + }); + + const { port, close } = await startStatusServer(); + + // 404 is in the default ignore list, 200 is not. Request the ignored one + // first so that, by the time the 200 lands, the 404 has had its chance. + assertEquals(await (await fetch(`http://127.0.0.1:${port}/404`)).text(), 'ok'); + assertEquals(await (await fetch(`http://127.0.0.1:${port}/200`)).text(), 'ok'); + + const kept = await withTimeout( + sink.waitFor(t => t.contexts?.trace?.data?.['http.response.status_code'] === 200), + 5000, + 'http.server transaction for the 200 response', + ); + + await close(); + + assertEquals(kept.transaction, 'GET /200'); + // The shared handling also lifts the status onto the top-level `response` context. + assertEquals(kept.contexts?.response?.status_code, 200); + + const dropped = sink.transactions.filter(t => t.contexts?.trace?.data?.['http.response.status_code'] === 404); + assertEquals(dropped.length, 0, `expected the 404 transaction to be dropped, got ${dropped.length}`); + }, +}); + +Deno.test({ + name: 'denoServeIntegration: drops Deno.serve transactions with a default-ignored status code', + async fn() { + resetGlobals(); + const sink = transactionSink(); + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + beforeSendTransaction: sink.beforeSendTransaction, + traceLifecycle: 'static', + }); + + const abortController = new AbortController(); + let onListen: ((_: unknown) => void) | undefined; + const listening = new Promise(resolve => (onListen = resolve)); + const server = Deno.serve( + { port: 0, signal: abortController.signal, onListen, hostname: '127.0.0.1' }, + request => new Response('ok', { status: new URL(request.url).pathname === '/gone' ? 404 : 200 }), + ); + await listening; + const port = server.addr.port; + + assertEquals(await (await fetch(`http://127.0.0.1:${port}/gone`)).text(), 'ok'); + assertEquals(await (await fetch(`http://127.0.0.1:${port}/ok`)).text(), 'ok'); + + const kept = await withTimeout( + sink.waitFor(t => t.contexts?.trace?.data?.['http.response.status_code'] === 200), + 5000, + 'Deno.serve transaction for the 200 response', + ); + + abortController.abort(); + await server.finished; + + // `denoServeIntegration` owns this filtering via its own `ignoreStatusCodes`. + assertEquals(kept.contexts?.trace?.origin, 'auto.http.deno'); + assertEquals(kept.contexts?.response?.status_code, 200); + + const dropped = sink.transactions.filter(t => t.contexts?.trace?.data?.['http.response.status_code'] === 404); + assertEquals(dropped.length, 0, `expected the 404 Deno.serve transaction to be dropped, got ${dropped.length}`); + }, +}); + +Deno.test({ + name: 'denoHttpIntegration: ignoreStatusCodes does not cross-talk with Deno.serve transactions', + async fn() { + resetGlobals(); + const sink = transactionSink(); + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + beforeSendTransaction: sink.beforeSendTransaction, + traceLifecycle: 'static', + // Ask DenoHttp to drop 200s. DenoServe keeps its own defaults, so `Deno.serve` + // 200s must survive -- the two integrations no longer share a filter. + integrations: [denoHttpIntegration({ ignoreStatusCodes: [200] })], + }); + + const abortController = new AbortController(); + let onListen: ((_: unknown) => void) | undefined; + const listening = new Promise(resolve => (onListen = resolve)); + const server = Deno.serve( + { port: 0, signal: abortController.signal, onListen, hostname: '127.0.0.1' }, + () => new Response('ok'), + ); + await listening; + const port = server.addr.port; + + assertEquals(await (await fetch(`http://127.0.0.1:${port}/ok`)).text(), 'ok'); + + const kept = await withTimeout( + sink.waitFor(t => t.contexts?.trace?.origin === 'auto.http.deno'), + 5000, + 'Deno.serve transaction unaffected by DenoHttp ignoreStatusCodes', + ); + + abortController.abort(); + await server.finished; + + assertEquals(kept.contexts?.trace?.data?.['http.response.status_code'], 200); + }, +}); + +Deno.test({ + name: 'denoHttpIntegration: ignoreStatusCodes overrides the default list', + async fn() { + resetGlobals(); + const sink = transactionSink(); + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + beforeSendTransaction: sink.beforeSendTransaction, + traceLifecycle: 'static', + integrations: [denoHttpIntegration({ ignoreStatusCodes: [500] })], + }); + + const { port, close } = await startStatusServer(); + + assertEquals(await (await fetch(`http://127.0.0.1:${port}/500`)).text(), 'ok'); + assertEquals(await (await fetch(`http://127.0.0.1:${port}/404`)).text(), 'ok'); + + // 404 is no longer ignored once the default list is replaced. + const kept = await withTimeout( + sink.waitFor(t => t.contexts?.trace?.data?.['http.response.status_code'] === 404), + 5000, + 'http.server transaction for the 404 response', + ); + + await close(); + + assertEquals(kept.transaction, 'GET /404'); + + const dropped = sink.transactions.filter(t => t.contexts?.trace?.data?.['http.response.status_code'] === 500); + assertEquals(dropped.length, 0, `expected the 500 transaction to be dropped, got ${dropped.length}`); + }, +}); diff --git a/packages/node/src/integrations/http/httpServerSpansIntegration.ts b/packages/node/src/integrations/http/httpServerSpansIntegration.ts index 8633c6f23938..170eae1399e0 100644 --- a/packages/node/src/integrations/http/httpServerSpansIntegration.ts +++ b/packages/node/src/integrations/http/httpServerSpansIntegration.ts @@ -36,6 +36,8 @@ import type { } from '@sentry/core'; import { debug, + DEFAULT_IGNORE_STATUS_CODES, + processHttpServerTransactionEvent, getSpanStatusFromHttpCode, httpHeadersToSpanAttributes, getContentLengthFromHeaders, @@ -104,12 +106,7 @@ export interface HttpServerSpansIntegrationOptions { const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions = {}) => { const ignoreStaticAssets = options.ignoreStaticAssets ?? true; const ignoreIncomingRequests = options.ignoreIncomingRequests; - const ignoreStatusCodes = options.ignoreStatusCodes ?? [ - [401, 404], - // 300 and 304 are possibly valid status codes we do not want to filter - [301, 303], - [305, 399], - ]; + const ignoreStatusCodes = options.ignoreStatusCodes ?? DEFAULT_IGNORE_STATUS_CODES; const { onSpanCreated } = options; @@ -228,29 +225,7 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions }); }, processEvent(event) { - if (event.type === 'transaction') { - const statusCode = event.contexts?.trace?.data?.[HTTP_RESPONSE_STATUS_CODE]; - if (typeof statusCode === 'number') { - // Drop transaction if it has a status code that should be ignored - if (shouldFilterStatusCode(statusCode, ignoreStatusCodes)) { - DEBUG_BUILD && debug.log('Dropping transaction due to status code', statusCode); - return null; - } - - // Surface the HTTP status as the top-level `response` context. The OTel SDK span - // exporter already does this on its path; doing it here covers transactions produced - // by the `SentryTracerProvider`, which bypasses that exporter. - event.contexts = { - ...event.contexts, - response: { - ...event.contexts?.response, - status_code: statusCode, - }, - }; - } - } - - return event; + return processHttpServerTransactionEvent(event, ignoreStatusCodes); }, afterAllSetup(client) { if (!DEBUG_BUILD) { @@ -387,17 +362,3 @@ function getIncomingRequestAttributesOnResponse( return newAttributes; } - -/** - * If the given status code should be filtered for the given list of status codes/ranges. - */ -function shouldFilterStatusCode(statusCode: number, dropForStatusCodes: (number | [number, number])[]): boolean { - return dropForStatusCodes.some(code => { - if (typeof code === 'number') { - return code === statusCode; - } - - const [min, max] = code; - return statusCode >= min && statusCode <= max; - }); -}