diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts new file mode 100644 index 000000000000..3fa3691e0637 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts @@ -0,0 +1,52 @@ +import { test, expect } from '@playwright/test'; +import { waitForTransaction } from '../event-proxy-server'; + +test('Creates a pageload transaction for app router routes', async ({ page }) => { + const randomRoute = String(Math.random()); + + const clientPageloadTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { + return ( + transactionEvent?.transaction === `/server-component/parameter/${randomRoute}` && + transactionEvent.contexts?.trace?.op === 'pageload' + ); + }); + + await page.goto(`/server-component/parameter/${randomRoute}`); + + expect(await clientPageloadTransactionPromise).toBeDefined(); +}); + +test('Creates a navigation transaction for app router routes', async ({ page }) => { + const randomRoute = String(Math.random()); + + const clientPageloadTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { + return ( + transactionEvent?.transaction === `/server-component/parameter/${randomRoute}` && + transactionEvent.contexts?.trace?.op === 'pageload' + ); + }); + + await page.goto(`/server-component/parameter/${randomRoute}`); + await clientPageloadTransactionPromise; + await page.getByText('Page (/server-component/parameter/[parameter])').isVisible(); + + const clientNavigationTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { + return ( + transactionEvent?.transaction === '/server-component/parameter/foo/bar/baz' && + transactionEvent.contexts?.trace?.op === 'navigation' + ); + }); + + const servercomponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'Page Server Component (/server-component/parameter/[...parameters])' && + (await clientNavigationTransactionPromise).contexts?.trace?.trace_id === + transactionEvent.contexts?.trace?.trace_id + ); + }); + + await page.getByText('/server-component/parameter/foo/bar/baz').click(); + + expect(await clientNavigationTransactionPromise).toBeDefined(); + expect(await servercomponentTransactionPromise).toBeDefined(); +}); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts deleted file mode 100644 index ed9a68513a19..000000000000 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/trace.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { test } from '@playwright/test'; -import { waitForTransaction } from '../event-proxy-server'; - -if (process.env.TEST_ENV === 'production') { - // TODO: Fix that this is flakey on dev server - might be an SDK bug - test('Sends connected traces for server components', async ({ page }, testInfo) => { - await page.goto('/client-component'); - - const clientTransactionName = `e2e-next-js-app-dir: ${testInfo.title}`; - - const serverComponentTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { - return ( - transactionEvent?.transaction === 'Page Server Component (/server-component)' && - (await clientTransactionPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id - ); - }); - - const clientTransactionPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => { - return transactionEvent?.transaction === clientTransactionName; - }); - - await page.getByPlaceholder('Transaction name').fill(clientTransactionName); - await page.getByText('Start transaction').click(); - await page.getByRole('link', { name: /^\/server-component$/ }).click(); - await page.getByText('Page (/server-component)').isVisible(); - await page.getByText('Stop transaction').click(); - - await serverComponentTransaction; - }); -} diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 9de007bd2e95..b46da56cf98f 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -14,11 +14,11 @@ import { addOrUpdateIntegration } from '@sentry/utils'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; import { buildMetadata } from '../common/metadata'; -import { nextRouterInstrumentation } from './performance'; +import { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation'; import { applyTunnelRouteOption } from './tunnelRoute'; export * from '@sentry/react'; -export { nextRouterInstrumentation } from './performance'; +export { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation'; export { captureUnderscoreErrorException } from '../common/_error'; export { Integrations }; diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts new file mode 100644 index 000000000000..5d7ab3596c8e --- /dev/null +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -0,0 +1,113 @@ +import { WINDOW } from '@sentry/react'; +import type { HandlerDataFetch, Primitive, Transaction, TransactionContext } from '@sentry/types'; +import { addInstrumentationHandler, browserPerformanceTimeOrigin } from '@sentry/utils'; + +type StartTransactionCb = (context: TransactionContext) => Transaction | undefined; + +const DEFAULT_TAGS = { + 'routing.instrumentation': 'next-app-router', +} as const; + +/** + * Instruments the Next.js Clientside App Router. + */ +export function appRouterInstrumentation( + startTransactionCb: StartTransactionCb, + startTransactionOnPageLoad: boolean = true, + startTransactionOnLocationChange: boolean = true, +): void { + // We keep track of the active transaction so we can finish it when we start a navigation transaction. + let activeTransaction: Transaction | undefined = undefined; + + // We keep track of the previous location name so we can set the `from` field on navigation transactions. + // This is either a route or a pathname. + let prevLocationName = WINDOW.location.pathname; + + if (startTransactionOnPageLoad) { + activeTransaction = startTransactionCb({ + name: prevLocationName, + op: 'pageload', + origin: 'auto.pageload.nextjs.app_router_instrumentation', + tags: DEFAULT_TAGS, + // pageload should always start at timeOrigin (and needs to be in s, not ms) + startTimestamp: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined, + metadata: { source: 'url' }, + }); + } + + if (startTransactionOnLocationChange) { + addInstrumentationHandler('fetch', (handlerData: HandlerDataFetch) => { + // The instrumentation handler is invoked twice - once for starting a request and once when the req finishes + // We can use the existence of the end-timestamp to filter out "finishing"-events. + if (handlerData.endTimestamp !== undefined) { + return; + } + + // Only GET requests can be navigating RSC requests + if (handlerData.fetchData.method !== 'GET') { + return; + } + + const parsedNavigatingRscFetchArgs = parseNavigatingRscFetchArgs(handlerData.args); + + if (parsedNavigatingRscFetchArgs === null) { + return; + } + + const transactionName = parsedNavigatingRscFetchArgs.targetPathname; + const tags: Record = { + ...DEFAULT_TAGS, + from: prevLocationName, + }; + + prevLocationName = transactionName; + + if (activeTransaction) { + activeTransaction.finish(); + } + + startTransactionCb({ + name: transactionName, + op: 'navigation', + origin: 'auto.navigation.nextjs.app_router_instrumentation', + tags, + metadata: { source: 'url' }, + }); + }); + } +} + +function parseNavigatingRscFetchArgs(fetchArgs: unknown[]): null | { + targetPathname: string; +} { + // Make sure the first arg is a URL object + if (!fetchArgs[0] || typeof fetchArgs[0] !== 'object' || (fetchArgs[0] as URL).searchParams === undefined) { + return null; + } + + // Make sure the second argument is some kind of fetch config obj that contains headers + if (!fetchArgs[1] || typeof fetchArgs[1] !== 'object' || !('headers' in fetchArgs[1])) { + return null; + } + + try { + const url = fetchArgs[0] as URL; + const headers = fetchArgs[1].headers as Record; + + // Not an RSC request + if (headers['RSC'] !== '1') { + return null; + } + + // Prefetch requests are not navigating RSC requests + if (headers['Next-Router-Prefetch'] === '1') { + return null; + } + + return { + targetPathname: url.pathname, + }; + } catch { + return null; + } +} diff --git a/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts new file mode 100644 index 000000000000..3010faad4183 --- /dev/null +++ b/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts @@ -0,0 +1,23 @@ +import { WINDOW } from '@sentry/react'; +import type { Transaction, TransactionContext } from '@sentry/types'; + +import { appRouterInstrumentation } from './appRouterRoutingInstrumentation'; +import { pagesRouterInstrumentation } from './pagesRouterRoutingInstrumentation'; + +type StartTransactionCb = (context: TransactionContext) => Transaction | undefined; + +/** + * Instruments the Next.js Clientside Router. + */ +export function nextRouterInstrumentation( + startTransactionCb: StartTransactionCb, + startTransactionOnPageLoad: boolean = true, + startTransactionOnLocationChange: boolean = true, +): void { + const isAppRouter = !WINDOW.document.getElementById('__NEXT_DATA__'); + if (isAppRouter) { + appRouterInstrumentation(startTransactionCb, startTransactionOnPageLoad, startTransactionOnLocationChange); + } else { + pagesRouterInstrumentation(startTransactionCb, startTransactionOnPageLoad, startTransactionOnLocationChange); + } +} diff --git a/packages/nextjs/src/client/performance.ts b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts similarity index 93% rename from packages/nextjs/src/client/performance.ts rename to packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts index bdf1509d9ec1..a633e1cec6dc 100644 --- a/packages/nextjs/src/client/performance.ts +++ b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts @@ -1,7 +1,12 @@ import { getCurrentHub } from '@sentry/core'; import { WINDOW } from '@sentry/react'; import type { Primitive, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { logger, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils'; +import { + browserPerformanceTimeOrigin, + logger, + stripUrlQueryAndFragment, + tracingContextFromHeaders, +} from '@sentry/utils'; import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils'; import { default as Router } from 'next/router'; import type { ParsedUrlQuery } from 'querystring'; @@ -86,7 +91,7 @@ function extractNextDataTagInformation(): NextDataTagInfo { } const DEFAULT_TAGS = { - 'routing.instrumentation': 'next-router', + 'routing.instrumentation': 'next-pages-router', } as const; // We keep track of the active transaction so we can finish it when we start a navigation transaction. @@ -99,14 +104,14 @@ let prevLocationName: string | undefined = undefined; const client = getCurrentHub().getClient(); /** - * Creates routing instrumention for Next Router. Only supported for + * Instruments the Next.js pages router. Only supported for * client side routing. Works for Next >= 10. * * Leverages the SingletonRouter from the `next/router` to * generate pageload/navigation transactions and parameterize * transaction names. */ -export function nextRouterInstrumentation( +export function pagesRouterInstrumentation( startTransactionCb: StartTransactionCb, startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, @@ -125,7 +130,10 @@ export function nextRouterInstrumentation( activeTransaction = startTransactionCb({ name: prevLocationName, op: 'pageload', + origin: 'auto.pageload.nextjs.pages_router_instrumentation', tags: DEFAULT_TAGS, + // pageload should always start at timeOrigin (and needs to be in s, not ms) + startTimestamp: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined, ...(params && client && client.getOptions().sendDefaultPii && { data: params }), ...traceparentData, metadata: { @@ -165,6 +173,7 @@ export function nextRouterInstrumentation( const navigationTransaction = startTransactionCb({ name: transactionName, op: 'navigation', + origin: 'auto.navigation.nextjs.pages_router_instrumentation', tags, metadata: { source: transactionSource }, }); @@ -177,8 +186,8 @@ export function nextRouterInstrumentation( // hooks). Instead, we'll simply let the navigation transaction finish itself (it's an `IdleTransaction`). const nextRouteChangeSpan = navigationTransaction.startChild({ op: 'ui.nextjs.route-change', + origin: 'auto.ui.nextjs.pages_router_instrumentation', description: 'Next.js Route Change', - origin: 'auto.navigation.nextjs', }); const finishRouteChangeSpan = (): void => { diff --git a/packages/nextjs/test/performance/appRouterInstrumentation.test.ts b/packages/nextjs/test/performance/appRouterInstrumentation.test.ts new file mode 100644 index 000000000000..bebb0cbf8ab6 --- /dev/null +++ b/packages/nextjs/test/performance/appRouterInstrumentation.test.ts @@ -0,0 +1,161 @@ +import { WINDOW } from '@sentry/react'; +import type { HandlerDataFetch } from '@sentry/types'; +import * as sentryUtils from '@sentry/utils'; +import { JSDOM } from 'jsdom'; + +import { appRouterInstrumentation } from '../../src/client/routing/appRouterRoutingInstrumentation'; + +const addInstrumentationHandlerSpy = jest.spyOn(sentryUtils, 'addInstrumentationHandler'); + +function setUpPage(url: string) { + const dom = new JSDOM('

nothingness

', { url }); + + // The Next.js routing instrumentations requires a few things to be present on pageload: + // 1. Access to window.document API for `window.document.getElementById` + // 2. Access to window.location API for `window.location.pathname` + Object.defineProperty(WINDOW, 'document', { value: dom.window.document, writable: true }); + Object.defineProperty(WINDOW, 'location', { value: dom.window.document.location, writable: true }); +} + +describe('appRouterInstrumentation', () => { + const originalGlobalDocument = WINDOW.document; + const originalGlobalLocation = WINDOW.location; + + afterEach(() => { + // Clean up JSDom + Object.defineProperty(WINDOW, 'document', { value: originalGlobalDocument }); + Object.defineProperty(WINDOW, 'location', { value: originalGlobalLocation }); + }); + + it('should create a pageload transactions with the current location name', () => { + setUpPage('https://example.com/some/page?someParam=foobar'); + const startTransactionCallbackFn = jest.fn(); + appRouterInstrumentation(startTransactionCallbackFn, true, false); + expect(startTransactionCallbackFn).toHaveBeenCalledWith( + expect.objectContaining({ + name: '/some/page', + op: 'pageload', + origin: 'auto.pageload.nextjs.app_router_instrumentation', + tags: { + 'routing.instrumentation': 'next-app-router', + }, + metadata: { source: 'url' }, + }), + ); + }); + + it('should not create a pageload transaction when `startTransactionOnPageLoad` is false', () => { + setUpPage('https://example.com/some/page?someParam=foobar'); + const startTransactionCallbackFn = jest.fn(); + appRouterInstrumentation(startTransactionCallbackFn, false, false); + expect(startTransactionCallbackFn).not.toHaveBeenCalled(); + }); + + it('should create a navigation transactions when a navigation RSC request is sent', () => { + setUpPage('https://example.com/some/page?someParam=foobar'); + let fetchInstrumentationHandlerCallback: (arg: HandlerDataFetch) => void; + + addInstrumentationHandlerSpy.mockImplementationOnce((_type, callback) => { + fetchInstrumentationHandlerCallback = callback; + }); + + const startTransactionCallbackFn = jest.fn(); + appRouterInstrumentation(startTransactionCallbackFn, false, true); + + fetchInstrumentationHandlerCallback!({ + args: [ + new URL('https://example.com/some/server/component/page?_rsc=2rs8t'), + { + headers: { + RSC: '1', + }, + }, + ], + fetchData: { method: 'GET', url: 'https://example.com/some/server/component/page?_rsc=2rs8t' }, + startTimestamp: 1337, + }); + + expect(startTransactionCallbackFn).toHaveBeenCalledWith({ + name: '/some/server/component/page', + op: 'navigation', + origin: 'auto.navigation.nextjs.app_router_instrumentation', + metadata: { source: 'url' }, + tags: { + from: '/some/page', + 'routing.instrumentation': 'next-app-router', + }, + }); + }); + + it.each([ + [ + 'no RSC header', + { + args: [ + new URL('https://example.com/some/server/component/page?_rsc=2rs8t'), + { + headers: {}, + }, + ], + fetchData: { method: 'GET', url: 'https://example.com/some/server/component/page?_rsc=2rs8t' }, + startTimestamp: 1337, + }, + ], + [ + 'no GET request', + { + args: [ + new URL('https://example.com/some/server/component/page?_rsc=2rs8t'), + { + headers: { + RSC: '1', + }, + }, + ], + fetchData: { method: 'POST', url: 'https://example.com/some/server/component/page?_rsc=2rs8t' }, + startTimestamp: 1337, + }, + ], + [ + 'prefetch request', + { + args: [ + new URL('https://example.com/some/server/component/page?_rsc=2rs8t'), + { + headers: { + RSC: '1', + 'Next-Router-Prefetch': '1', + }, + }, + ], + fetchData: { method: 'GET', url: 'https://example.com/some/server/component/page?_rsc=2rs8t' }, + startTimestamp: 1337, + }, + ], + ])( + 'should not create naviagtion transactions for fetch requests that are not navigating RSC requests (%s)', + (_, fetchCallbackData) => { + setUpPage('https://example.com/some/page?someParam=foobar'); + let fetchInstrumentationHandlerCallback: (arg: HandlerDataFetch) => void; + + addInstrumentationHandlerSpy.mockImplementationOnce((_type, callback) => { + fetchInstrumentationHandlerCallback = callback; + }); + + const startTransactionCallbackFn = jest.fn(); + appRouterInstrumentation(startTransactionCallbackFn, false, true); + fetchInstrumentationHandlerCallback!(fetchCallbackData); + expect(startTransactionCallbackFn).not.toHaveBeenCalled(); + }, + ); + + it('should not create navigation transactions when `startTransactionOnLocationChange` is false', () => { + setUpPage('https://example.com/some/page?someParam=foobar'); + const addInstrumentationHandlerImpl = jest.fn(); + const startTransactionCallbackFn = jest.fn(); + + addInstrumentationHandlerSpy.mockImplementationOnce(addInstrumentationHandlerImpl); + appRouterInstrumentation(startTransactionCallbackFn, false, false); + expect(addInstrumentationHandlerImpl).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts similarity index 91% rename from packages/nextjs/test/performance/client.test.ts rename to packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts index cc0f212cbf18..6db342fb426c 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts @@ -4,7 +4,7 @@ import { JSDOM } from 'jsdom'; import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils'; import { default as Router } from 'next/router'; -import { nextRouterInstrumentation } from '../../src/client/performance'; +import { pagesRouterInstrumentation } from '../../src/client/routing/pagesRouterRoutingInstrumentation'; const globalObject = WINDOW as typeof WINDOW & { __BUILD_MANIFEST?: { @@ -57,7 +57,7 @@ function createMockStartTransaction() { ); } -describe('nextRouterInstrumentation', () => { +describe('pagesRouterInstrumentation', () => { const originalGlobalDocument = WINDOW.document; const originalGlobalLocation = WINDOW.location; @@ -136,7 +136,7 @@ describe('nextRouterInstrumentation', () => { name: '/[user]/posts/[id]', op: 'pageload', tags: { - 'routing.instrumentation': 'next-router', + 'routing.instrumentation': 'next-pages-router', }, metadata: { source: 'route', @@ -162,7 +162,7 @@ describe('nextRouterInstrumentation', () => { name: '/some-page', op: 'pageload', tags: { - 'routing.instrumentation': 'next-router', + 'routing.instrumentation': 'next-pages-router', }, metadata: { source: 'route', @@ -183,7 +183,7 @@ describe('nextRouterInstrumentation', () => { name: '/', op: 'pageload', tags: { - 'routing.instrumentation': 'next-router', + 'routing.instrumentation': 'next-pages-router', }, metadata: { source: 'route', @@ -200,7 +200,7 @@ describe('nextRouterInstrumentation', () => { name: '/lforst/posts/1337', op: 'pageload', tags: { - 'routing.instrumentation': 'next-router', + 'routing.instrumentation': 'next-pages-router', }, metadata: { source: 'url', @@ -212,16 +212,18 @@ describe('nextRouterInstrumentation', () => { (url, route, query, props, hasNextData, expectedStartTransactionArgument) => { const mockStartTransaction = createMockStartTransaction(); setUpNextPage({ url, route, query, props, hasNextData }); - nextRouterInstrumentation(mockStartTransaction); + pagesRouterInstrumentation(mockStartTransaction); expect(mockStartTransaction).toHaveBeenCalledTimes(1); - expect(mockStartTransaction).toHaveBeenLastCalledWith(expectedStartTransactionArgument); + expect(mockStartTransaction).toHaveBeenLastCalledWith( + expect.objectContaining(expectedStartTransactionArgument), + ); }, ); it('does not create a pageload transaction if option not given', () => { const mockStartTransaction = createMockStartTransaction(); setUpNextPage({ url: 'https://example.com/', route: '/', hasNextData: false }); - nextRouterInstrumentation(mockStartTransaction, false); + pagesRouterInstrumentation(mockStartTransaction, false); expect(mockStartTransaction).toHaveBeenCalledTimes(0); }); }); @@ -268,7 +270,7 @@ describe('nextRouterInstrumentation', () => { ], }); - nextRouterInstrumentation(mockStartTransaction, false, true); + pagesRouterInstrumentation(mockStartTransaction, false, true); Router.events.emit('routeChangeStart', targetLocation); @@ -278,7 +280,7 @@ describe('nextRouterInstrumentation', () => { name: expectedTransactionName, op: 'navigation', tags: expect.objectContaining({ - 'routing.instrumentation': 'next-router', + 'routing.instrumentation': 'next-pages-router', }), metadata: expect.objectContaining({ source: expectedTransactionSource, @@ -304,7 +306,7 @@ describe('nextRouterInstrumentation', () => { navigatableRoutes: ['/home', '/posts/[id]'], }); - nextRouterInstrumentation(mockStartTransaction, false, false); + pagesRouterInstrumentation(mockStartTransaction, false, false); Router.events.emit('routeChangeStart', '/posts/42');