diff --git a/dev-packages/bun-integration-tests/node-suites/excludes.ts b/dev-packages/bun-integration-tests/node-suites/excludes.ts index 65edcd311712..2e0cb03fa3f6 100644 --- a/dev-packages/bun-integration-tests/node-suites/excludes.ts +++ b/dev-packages/bun-integration-tests/node-suites/excludes.ts @@ -54,6 +54,7 @@ const NO_FETCH_INSTRUMENTATION = [ 'suites/tracing/requests/fetch-no-tracing-no-spans/test.ts', 'suites/tracing/requests/fetch-no-tracing/test.ts', 'suites/tracing/requests/fetch-sampled-no-active-span/test.ts', + 'suites/tracing/requests/fetch-sentry-requests/test.ts', 'suites/tracing/requests/fetch-unsampled/test.ts', 'suites/tracing/requests/traceparent/test.ts', ]; diff --git a/dev-packages/deno-integration-tests/node-suites/excludes.ts b/dev-packages/deno-integration-tests/node-suites/excludes.ts index a6ad2a290f16..b93a8ff63a65 100644 --- a/dev-packages/deno-integration-tests/node-suites/excludes.ts +++ b/dev-packages/deno-integration-tests/node-suites/excludes.ts @@ -26,6 +26,7 @@ const NO_FETCH_INSTRUMENTATION = [ 'suites/tracing/requests/fetch-no-tracing-no-spans/test.ts', 'suites/tracing/requests/fetch-no-tracing/test.ts', 'suites/tracing/requests/fetch-sampled-no-active-span/test.ts', + 'suites/tracing/requests/fetch-sentry-requests/test.ts', 'suites/tracing/requests/fetch-unsampled/test.ts', 'suites/tracing/requests/traceparent/test.ts', 'suites/tracing/sample-rand-propagation/test.ts', diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/instrument.mjs new file mode 100644 index 000000000000..83be6e35634b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@localhost/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/scenario.mjs new file mode 100644 index 000000000000..b6d68dda3e09 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/scenario.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; + +// Another SDK in this process (e.g. the Next.js Edge Runtime SDK) sends to Sentry without suppressing our tracing +const sentryUrl = `${process.env.SERVER_URL}/api/1337/envelope/?sentry_version=7&sentry_key=public&sentry_client=other`; + +Sentry.startSpan({ name: 'test_span' }, async () => { + await fetch(sentryUrl).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/v0`).then(res => res.text()); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/test.ts new file mode 100644 index 000000000000..afdd01bab691 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sentry-requests/test.ts @@ -0,0 +1,25 @@ +import { SENTRY_OP, URL_FULL } from '@sentry/conventions/attributes'; +import { HTTP_CLIENT } from '@sentry/conventions/op'; +import { createTestServer } from '@sentry-internal/test-utils'; +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../../utils/runner'; + +describe('outgoing fetch requests to Sentry', () => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('does not create spans for requests to Sentry', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer().start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + span: container => { + const requestSpans = container.items.filter(span => span.attributes[SENTRY_OP]?.value === HTTP_CLIENT); + expect(requestSpans.map(span => span.attributes[URL_FULL]?.value)).toEqual([`${SERVER_URL}/api/v0`]); + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/instrument.mjs new file mode 100644 index 000000000000..83be6e35634b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@localhost/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/scenario.mjs new file mode 100644 index 000000000000..99666e006794 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/scenario.mjs @@ -0,0 +1,25 @@ +import * as Sentry from '@sentry/node'; +import * as http from 'http'; + +// Another SDK in this process (e.g. the Next.js Edge Runtime SDK) sends to Sentry without suppressing our tracing +const sentryUrl = `${process.env.SERVER_URL}/api/1337/envelope/?sentry_version=7&sentry_key=public&sentry_client=other`; + +Sentry.startSpan({ name: 'test_span' }, async () => { + await makeHttpRequest(sentryUrl); + await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`); +}); + +function makeHttpRequest(url) { + return new Promise(resolve => { + http + .request(url, httpRes => { + httpRes.on('data', () => { + // we don't care about data + }); + httpRes.on('end', () => { + resolve(); + }); + }) + .end(); + }); +} diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/test.ts new file mode 100644 index 000000000000..f2c0c4d106ec --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sentry-requests/test.ts @@ -0,0 +1,28 @@ +import { SENTRY_OP, URL_FULL } from '@sentry/conventions/attributes'; +import { HTTP_CLIENT } from '@sentry/conventions/op'; +import { createTestServer } from '@sentry-internal/test-utils'; +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../../utils/runner'; +import { supports } from '../../../../utils'; + +// Bun before 1.4 does not instrument outgoing `node:http` requests. +// See https://github.com/getsentry/sentry-javascript/issues/23881 +describe.runIf(supports({ bunMin: '1.4.0' }))('outgoing http requests to Sentry', () => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('does not create spans for requests to Sentry', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer().start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .expect({ + span: container => { + const requestSpans = container.items.filter(span => span.attributes[SENTRY_OP]?.value === HTTP_CLIENT); + expect(requestSpans.map(span => span.attributes[URL_FULL]?.value)).toEqual([`${SERVER_URL}/api/v0`]); + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); +}); diff --git a/packages/core/src/integrations/http/client-subscriptions.ts b/packages/core/src/integrations/http/client-subscriptions.ts index 554445eb8053..0137fba2a9cb 100644 --- a/packages/core/src/integrations/http/client-subscriptions.ts +++ b/packages/core/src/integrations/http/client-subscriptions.ts @@ -35,6 +35,7 @@ import { DEBUG_BUILD } from '../../debug-build'; import { LOG_PREFIX, HTTP_ON_CLIENT_REQUEST } from './constants'; import type { ClientSubscriptionName } from './constants'; import { getClient } from '../../currentScopes'; +import { isSentryRequestUrl } from '../../utils/isSentryRequestUrl'; import { hasSpansEnabled } from '../../utils/hasSpansEnabled'; import { doubleWrapWarning } from './double-wrap-warning'; @@ -66,8 +67,10 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): const { request } = data as { request: HttpClientRequest }; - // check if request is ignored. if so, we do nothing at all. - if (options.ignoreOutgoingRequests?.(getRequestUrlFromClientRequest(request), request)) { + // check if request is ignored or goes to Sentry. if so, we do nothing at all. + // Our own transport suppresses tracing, but another SDK in this process (e.g. the Next.js Edge Runtime SDK) may not. + const url = getRequestUrlFromClientRequest(request); + if (isSentryRequestUrl(url, getClient()) || options.ignoreOutgoingRequests?.(url, request)) { return; } diff --git a/packages/nextjs/src/server/serverSpanHooks.ts b/packages/nextjs/src/server/serverSpanHooks.ts index 3d21bb094eae..f243813d7a45 100644 --- a/packages/nextjs/src/server/serverSpanHooks.ts +++ b/packages/nextjs/src/server/serverSpanHooks.ts @@ -3,10 +3,8 @@ import { HTTP_METHOD, HTTP_REQUEST_METHOD, HTTP_ROUTE, - HTTP_TARGET, SENTRY_OP, SENTRY_ORIGIN, - URL_QUERY, } from '@sentry/conventions/attributes'; import { MIDDLEWARE } from '@sentry/conventions/op'; import type { Client, Options, Span } from '@sentry/core'; @@ -57,26 +55,6 @@ export const NEXTJS_SERVER_IGNORE_SPANS: NonNullable = [ * Registers the hooks that name, enrich and filter the spans and events of the Next.js server on `client`. */ export function addNextjsServerSpanHooks(client: Client): void { - client.on('beforeSampling', ({ spanAttributes }, samplingDecision) => { - // There are situations where the Next.js Node.js server forwards requests for the Edge Runtime server (e.g. in - // middleware) and this causes spans for Sentry ingest requests to be created. These are not exempt from our tracing - // because we didn't get the chance to do `suppressTracing`, since this happens outside of userland. - // We need to drop these spans. - if ( - // eslint-disable-next-line typescript/no-deprecated - (typeof spanAttributes[HTTP_TARGET] === 'string' && - // eslint-disable-next-line typescript/no-deprecated - spanAttributes[HTTP_TARGET].includes('sentry_key') && - // eslint-disable-next-line typescript/no-deprecated - spanAttributes[HTTP_TARGET].includes('sentry_client')) || - (typeof spanAttributes[URL_QUERY] === 'string' && - spanAttributes[URL_QUERY].includes('sentry_key') && - spanAttributes[URL_QUERY].includes('sentry_client')) - ) { - samplingDecision.decision = false; - } - }); - client.on('spanStart', span => handleOnSpanStart(span, client)); // Normalize name/op/source/status on the request root span at span end, before it is serialized into diff --git a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index 9d1617bbac9e..83a8b31bbe39 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -22,6 +22,7 @@ import type { Span, SpanAttributes } from '@sentry/core'; import { debug, getClient, + isSentryRequestUrl, getSanitizedUrlString, getSpanStatusFromHttpCode, hasSpanStreamingEnabled, @@ -171,15 +172,19 @@ function parseRequestHeaders(request: UndiciRequest): Map !!config.ignoreOutgoingRequests?.(url), - () => false, - ); + // Requests to Sentry are only traced here when another SDK in this process (e.g. the Next.js Edge Runtime SDK) sends + // them, since our own transport suppresses tracing + const isIgnored = + isSentryRequestUrl(url, getClient()) || + safeCallback( + DEBUG_BUILD ? 'The `ignoreOutgoingRequests` callback threw an error, not ignoring the request:' : '', + () => !!config.ignoreOutgoingRequests?.(url), + () => false, + ); // Breadcrumbs & span-less trace propagation are additionally skipped when tracing is suppressed. // We store this here because the active context is no longer correct in later channel events. - const ignoreForBreadcrumbs = isTracingSuppressed() || !!ignoredByCallback; + const ignoreForBreadcrumbs = isTracingSuppressed() || isIgnored; ignoreRequestMap.set(request, ignoreForBreadcrumbs); // When spans are disabled we do not create a span, but we still inject trace propagation headers @@ -192,9 +197,9 @@ function onRequestCreated(config: NodeFetchOptions, { request }: RequestMessage) } // Ignore span creation if: - // - the outgoing request is ignored via config + // - the outgoing request is ignored via config or goes to Sentry // - method is 'CONNECT' - if (request.method === 'CONNECT' || ignoredByCallback) { + if (request.method === 'CONNECT' || isIgnored) { return; } diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index a387cec9b6f1..1ee1e6ac3251 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -81,7 +81,9 @@ describe('instrumentUndici', () => { }); it.each([true, false])('falls back to `HTTP` for an unknown method (span streaming: %s)', streaming => { - getClient.mockReturnValue(streaming ? { getOptions: () => ({ traceLifecycle: 'stream' }) } : undefined); + getClient.mockReturnValue( + streaming ? { getOptions: () => ({ traceLifecycle: 'stream' }), getDsn: () => undefined } : undefined, + ); const request = { method: 'FROBNICATE', @@ -96,7 +98,7 @@ describe('instrumentUndici', () => { }); it('drops the URL path but keeps the domain with span streaming enabled', () => { - getClient.mockReturnValue({ getOptions: () => ({ traceLifecycle: 'stream' }) }); + getClient.mockReturnValue({ getOptions: () => ({ traceLifecycle: 'stream' }), getDsn: () => undefined }); const request = { method: 'QUERY',