diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16/app/page-behind-proxy/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-16/app/page-behind-proxy/page.tsx new file mode 100644 index 000000000000..7d2ee050d65f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-16/app/page-behind-proxy/page.tsx @@ -0,0 +1,5 @@ +export const dynamic = 'force-dynamic'; + +export default function Page() { + return

Page behind proxy

; +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16/package.json b/dev-packages/e2e-tests/test-applications/nextjs-16/package.json index 0415c053a37e..b90612c6b6c1 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-16/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-16/package.json @@ -48,6 +48,7 @@ }, "devDependencies": { "@playwright/test": "~1.63.0", + "@sentry/conventions": "0.27.0", "@sentry-internal/test-utils": "link:../../../test-utils", "@types/node": "^20", "@types/pg": "^8.11.10", diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16/proxy.ts b/dev-packages/e2e-tests/test-applications/nextjs-16/proxy.ts index 2daf05c0c50d..dd42ea499092 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-16/proxy.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-16/proxy.ts @@ -33,5 +33,5 @@ export async function proxy(request: NextRequest) { // See "Matching Paths" below to learn more export const config = { - matcher: ['/api/endpoint-behind-middleware', '/api/endpoint-behind-faulty-middleware'], + matcher: ['/api/endpoint-behind-middleware', '/api/endpoint-behind-faulty-middleware', '/page-behind-proxy'], }; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts index b4063c5be7dd..f90e8383d856 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-16/tests/middleware.test.ts @@ -1,4 +1,5 @@ -import { expect, test } from '@playwright/test'; +import { type APIRequestContext, expect, test } from '@playwright/test'; +import { HTTP_SERVER } from '@sentry/conventions/op'; import { collectStreamedSpans, collectStreamedSpansUntilSegment, @@ -48,6 +49,35 @@ test('Should create a span for middleware', async ({ request }) => { expect(routeSpan.attributes['isolation_scope.has_proxy_marker']).toEqual({ value: false, type: 'boolean' }); }); +// Next.js runs the proxy in a `BaseServer.handleRequest` span of its own, before the one that renders the route. That +// first span has neither a route nor the middleware span as a child, so it must not become a second `http.server` +// segment. +async function expectOneHttpServerSegment(request: APIRequestContext, path: string): Promise { + const spansPromise = collectStreamedSpans( + 'nextjs-16', + spans => + spans.some(span => span.is_segment && span.name === 'middleware GET') && + spans.some(span => span.is_segment && span.name === `GET ${path}`), + ); + + const response = await request.get(path); + expect(response.status()).toBe(200); + + const segmentSpans = (await spansPromise).filter(span => span.is_segment); + expect(segmentSpans.map(span => span.name).sort()).toEqual([`GET ${path}`, 'middleware GET']); + expect(segmentSpans.filter(span => getSpanOp(span) === HTTP_SERVER).map(span => span.name)).toEqual([`GET ${path}`]); +} + +test('Should create one http.server segment for a route handler behind the proxy', async ({ request }) => { + test.skip(getRuntime() === 'cloudflare', 'On Workers the middleware span is a child of the request span'); + await expectOneHttpServerSegment(request, '/api/endpoint-behind-middleware'); +}); + +test('Should create one http.server segment for a page behind the proxy', async ({ request }) => { + test.skip(getRuntime() === 'cloudflare', 'On Workers the middleware span is a child of the request span'); + await expectOneHttpServerSegment(request, '/page-behind-proxy'); +}); + // `withSentry` from `@sentry/nextjs/cloudflare` wraps the Worker entry, so middleware runs inside the request's // `http.server` segment and its span is a child of it rather than a segment of its own. test('Should create a span for middleware inside the request segment of the Worker', async ({ request }) => { diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index c61b3975d41b..a012cd20aaba 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -677,6 +677,16 @@ export abstract class Client { callback: (spanScope: { scope: Scope; parentSpan: Span | undefined }) => void, ): () => void; + /** + * Register a callback right before a span is created, before `ignoreSpans` is checked. Listeners can change the + * name and the attributes of the span by mutating the passed `spanStartData` object. + * @returns {() => void} A function that, when executed, removes the registered callback. + */ + public on( + hook: 'beforeSpanStart', + callback: (spanStartData: { name: string | undefined; attributes: SpanAttributes }) => void, + ): () => void; + /** * Register a callback before span sampling runs. Receives a `samplingDecision` object argument with a `decision` * property that can be used to make a sampling decision that will be enforced, before any span sampling runs. @@ -1038,6 +1048,9 @@ export abstract class Client { /** A hook that is called right before a span is created; listeners mutate the passed object. */ public emit(hook: 'prepareSpanScope', spanScope: { scope: Scope; parentSpan: Span | undefined }): void; + /** A hook that is called right before a span is created, before `ignoreSpans` is checked. */ + public emit(hook: 'beforeSpanStart', spanStartData: { name: string | undefined; attributes: SpanAttributes }): void; + /** A hook that is called every time before a span is sampled. */ public emit( hook: 'beforeSampling', diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index cb6f9f853184..6ef61fe099f8 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -362,6 +362,12 @@ function createChildOrRootSpan({ } const client = getClient(); + // Copy the attributes, so listeners do not mutate an object the caller passed in + const spanStartData = { name: spanArguments.name, attributes: { ...spanArguments.attributes } }; + client?.emit('beforeSpanStart', spanStartData); + spanArguments.name = spanStartData.name; + spanArguments.attributes = spanStartData.attributes; + if (_shouldIgnoreStreamedSpan(client, spanArguments)) { if (!isTracingSuppressed(scope)) { // if tracing is actively suppressed (Sentry.suppressTracing(...)), diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index f59946d50c10..055e8dbb969a 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -2717,6 +2717,51 @@ describe('ignoreSpans (core path, streaming)', () => { }); }); + it('ignores a span that a beforeSpanStart listener marks for ignoreSpans', () => { + const options = getDefaultTestClientOptions({ + tracesSampleRate: 1, + traceLifecycle: 'stream', + ignoreSpans: [{ attributes: { 'test.ignore': true } }], + }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + const spyOnDroppedEvent = vi.spyOn(client, 'recordDroppedEvent'); + client.on('beforeSpanStart', ({ name, attributes }) => { + if (name === 'GET') { + attributes['test.ignore'] = true; + } + }); + + startSpan({ name: 'GET' }, span => { + expect(span).toBeInstanceOf(SentryNonRecordingSpan); + }); + startSpan({ name: 'GET /users' }, span => { + expect(span).toBeInstanceOf(SentrySpan); + }); + + expect(spyOnDroppedEvent).toHaveBeenCalledTimes(1); + expect(spyOnDroppedEvent).toHaveBeenCalledWith('ignored', 'span'); + }); + + it('applies the changes of beforeSpanStart listeners without mutating the attributes passed in', () => { + const options = getDefaultTestClientOptions({ tracesSampleRate: 1 }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + client.on('beforeSpanStart', spanStartData => { + spanStartData.name = 'renamed'; + spanStartData.attributes['test.added'] = 'yes'; + }); + + const attributes = { 'test.original': 'yes' }; + const span = startInactiveSpan({ name: 'original', attributes }); + + expect(spanToJSON(span).name).toBe('renamed'); + expect(spanToJSON(span).attributes).toMatchObject({ 'test.original': 'yes', 'test.added': 'yes' }); + expect(attributes).toEqual({ 'test.original': 'yes' }); + }); + it('returns SentryNonRecordingSpan for startInactiveSpan matching ignoreSpans', () => { const options = getDefaultTestClientOptions({ tracesSampleRate: 1, diff --git a/packages/nextjs/src/server/serverSpanHooks.ts b/packages/nextjs/src/server/serverSpanHooks.ts index 3d21bb094eae..244aadf6ee9f 100644 --- a/packages/nextjs/src/server/serverSpanHooks.ts +++ b/packages/nextjs/src/server/serverSpanHooks.ts @@ -9,12 +9,14 @@ import { URL_QUERY, } from '@sentry/conventions/attributes'; import { MIDDLEWARE } from '@sentry/conventions/op'; -import type { Client, Options, Span } from '@sentry/core'; +import type { Client, Options, Scope, Span } from '@sentry/core'; import { getIsolationScope, getRootSpan, + GLOBAL_OBJ, hasSpanStreamingEnabled, HTTP_SPAN_NAME_FALLBACK, + parseSemver, spanToJSON, } from '@sentry/core'; import { enhanceMiddlewareRootSpan } from '../common/enhanceMiddlewareRootSpan'; @@ -36,6 +38,15 @@ import { } from './vercelQueuesMonitoring'; const NEXTJS_ERROR_PAGE_ROUTES = ['/500', '/_error']; +const NEXT_REQUEST_META = Symbol.for('NextInternalRequestMeta'); + +// Spans of Next.js start without access to the request they belong to, so the request is looked up through the +// isolation scope of the request +const requestsByIsolationScope = new WeakMap(); + +const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + _sentryNextJsVersion?: string; +}; export const NEXTJS_SERVER_IGNORE_SPANS: NonNullable = [ // Static assets (matches `_next/static` anywhere in the name to handle custom basePath) @@ -77,6 +88,18 @@ export function addNextjsServerSpanHooks(client: Client): void { } }); + client.on('httpServerRequest', request => { + if (typeof request === 'object' && request) { + requestsByIsolationScope.set(getIsolationScope(), request); + } + }); + + client.on('beforeSpanStart', ({ attributes }) => { + if (isProxyInvocationSpan(attributes)) { + attributes[TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION] = true; + } + }); + 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 @@ -103,6 +126,30 @@ export function addNextjsServerSpanHooks(client: Client): void { type SpanAttributes = ReturnType['attributes']; +/** + * Whether a span is the `BaseServer.handleRequest` span that Next.js starts only to run the proxy (middleware). + * + * Next.js handles a request behind a proxy with two `handleRequest` calls: one that only runs the proxy, and one that + * renders the route. Since Next.js 16.3 (vercel/next.js#95306), the proxy span is not a child of the first span anymore, + * so that span would end up as an empty `http.server` segment next to the one of the route. On older versions it holds + * the proxy span and becomes the `middleware` segment, so it must be kept there. + */ +function isProxyInvocationSpan(spanAttributes: Record): boolean { + if (spanAttributes[ATTR_NEXT_SPAN_TYPE] !== 'BaseServer.handleRequest') { + return false; + } + + const { major, minor } = parseSemver(globalWithInjectedValues._sentryNextJsVersion ?? ''); + if (major === undefined || minor === undefined || major < 16 || (major === 16 && minor < 3)) { + return false; + } + + // Next.js sets this flag on the request before the call that only runs the proxy, and resets it before rendering + const request: { [NEXT_REQUEST_META]?: { middlewareInvoke?: boolean } } | undefined = + requestsByIsolationScope.get(getIsolationScope()); + return request?.[NEXT_REQUEST_META]?.middlewareInvoke === true; +} + /** * Hoists the parameterized route that a Next.js span carries in `next.route` up to the root span, as its name and * `http.route`. diff --git a/packages/nextjs/test/server/serverSpanHooks.test.ts b/packages/nextjs/test/server/serverSpanHooks.test.ts index aef1a76dd6d2..f2b237dca092 100644 --- a/packages/nextjs/test/server/serverSpanHooks.test.ts +++ b/packages/nextjs/test/server/serverSpanHooks.test.ts @@ -7,10 +7,21 @@ import { } from '@sentry/conventions/attributes'; import { MIDDLEWARE } from '@sentry/conventions/op'; import type { Client } from '@sentry/core'; -import { addChildSpanToSpan, SentrySpan, spanToStaticSpanJSON } from '@sentry/core'; -import { describe, expect, it } from 'vitest'; +import { + addChildSpanToSpan, + GLOBAL_OBJ, + SentrySpan, + shouldIgnoreSpan, + spanToStaticSpanJSON, + withIsolationScope, +} from '@sentry/core'; +import { afterEach, describe, expect, it } from 'vitest'; import { ATTR_NEXT_SPAN_NAME, ATTR_NEXT_SPAN_TYPE } from '../../src/common/nextSpanAttributes'; -import { handleOnSpanStart } from '../../src/server/serverSpanHooks'; +import { + addNextjsServerSpanHooks, + handleOnSpanStart, + NEXTJS_SERVER_IGNORE_SPANS, +} from '../../src/server/serverSpanHooks'; const client = { getOptions: () => ({}) } as unknown as Client; @@ -171,3 +182,50 @@ describe('handleOnSpanStart', () => { expect(spanToStaticSpanJSON(rootSpan).description).toBe('GET /api/endpoint'); }); }); + +describe('addNextjsServerSpanHooks beforeSpanStart', () => { + const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & { _sentryNextJsVersion?: string }; + + afterEach(() => { + delete globalWithInjectedValues._sentryNextJsVersion; + }); + + function isRequestSpanIgnored(nextVersion: string | undefined, middlewareInvoke: boolean | undefined): boolean { + globalWithInjectedValues._sentryNextJsVersion = nextVersion; + + const hooks: Record void> = {}; + addNextjsServerSpanHooks({ + on: (hook: string, callback: (...args: unknown[]) => void) => { + hooks[hook] = callback; + }, + getOptions: () => ({}), + } as unknown as Client); + + const request = { [Symbol.for('NextInternalRequestMeta')]: { middlewareInvoke } }; + const spanStartData = { name: 'GET', attributes: { [ATTR_NEXT_SPAN_TYPE]: 'BaseServer.handleRequest' } }; + withIsolationScope(() => { + hooks['httpServerRequest']?.(request, {}, {}); + hooks['beforeSpanStart']?.(spanStartData); + }); + return shouldIgnoreSpan( + { description: spanStartData.name, attributes: spanStartData.attributes }, + NEXTJS_SERVER_IGNORE_SPANS, + ); + } + + it('ignores the request span that Next.js only starts to run the proxy', () => { + expect(isRequestSpanIgnored('16.3.0', true)).toBe(true); + }); + + it('keeps the request span that renders the route', () => { + expect(isRequestSpanIgnored('16.3.0', false)).toBe(false); + }); + + it('keeps the request span that runs the proxy before Next.js 16.3, since the proxy span is its child there', () => { + expect(isRequestSpanIgnored('16.2.4', true)).toBe(false); + }); + + it('keeps the request span that runs the proxy if the Next.js version is unknown', () => { + expect(isRequestSpanIgnored(undefined, true)).toBe(false); + }); +});