diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts index ab3c40a21471..d855e4918ce5 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts @@ -1,3 +1,4 @@ +import os from 'os'; import type { PlaywrightTestConfig } from '@playwright/test'; import { devices } from '@playwright/test'; @@ -31,6 +32,8 @@ const config: PlaywrightTestConfig = { }, /* Run tests in files in parallel */ fullyParallel: true, + /* Defaults to half the number of CPUs. The tests are not really CPU-bound but rather I/O-bound with all the polling we do so we increase the concurrency to the CPU count. */ + workers: os.cpus().length, /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, /* `next dev` is incredibly buggy with the app dir */ diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts index ab3c40a21471..599afc629b87 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.ts @@ -1,3 +1,4 @@ +import os from 'os'; import type { PlaywrightTestConfig } from '@playwright/test'; import { devices } from '@playwright/test'; @@ -29,6 +30,8 @@ const config: PlaywrightTestConfig = { */ timeout: 10000, }, + /* Defaults to half the number of CPUs. The tests are not really CPU-bound but rather I/O-bound with all the polling we do so we increase the concurrency to the CPU count. */ + workers: os.cpus().length, /* Run tests in files in parallel */ fullyParallel: true, /* Fail the build on CI if you accidentally left test.only in the source code. */ diff --git a/packages/nextjs/src/client/browserTracingIntegration.ts b/packages/nextjs/src/client/browserTracingIntegration.ts index c3eb18887301..bf62725e105b 100644 --- a/packages/nextjs/src/client/browserTracingIntegration.ts +++ b/packages/nextjs/src/client/browserTracingIntegration.ts @@ -1,8 +1,17 @@ -import { BrowserTracing as OriginalBrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/react'; +import { + BrowserTracing as OriginalBrowserTracing, + browserTracingIntegration as originalBrowserTracingIntegration, + defaultRequestInstrumentationOptions, + startBrowserTracingNavigationSpan, + startBrowserTracingPageLoadSpan, +} from '@sentry/react'; +import type { Integration, StartSpanOptions } from '@sentry/types'; import { nextRouterInstrumentation } from '../index.client'; /** * A custom BrowserTracing integration for Next.js. + * + * @deprecated Use `browserTracingIntegration` instead. */ export class BrowserTracing extends OriginalBrowserTracing { public constructor(options?: ConstructorParameters[0]) { @@ -19,8 +28,71 @@ export class BrowserTracing extends OriginalBrowserTracing { ] : // eslint-disable-next-line deprecation/deprecation [...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/], + // eslint-disable-next-line deprecation/deprecation routingInstrumentation: nextRouterInstrumentation, ...options, }); } } + +/** + * A custom BrowserTracing integration for Next.js. + */ +export function browserTracingIntegration( + options?: Parameters[0], +): Integration { + const browserTracingIntegrationInstance = originalBrowserTracingIntegration({ + // eslint-disable-next-line deprecation/deprecation + tracingOrigins: + process.env.NODE_ENV === 'development' + ? [ + // Will match any URL that contains "localhost" but not "webpack.hot-update.json" - The webpack dev-server + // has cors and it doesn't like extra headers when it's accessed from a different URL. + // TODO(v8): Ideally we rework our tracePropagationTargets logic so this hack won't be necessary anymore (see issue #9764) + /^(?=.*localhost)(?!.*webpack\.hot-update\.json).*/, + /^\/(?!\/)/, + ] + : // eslint-disable-next-line deprecation/deprecation + [...defaultRequestInstrumentationOptions.tracingOrigins, /^(api\/)/], + ...options, + instrumentNavigation: false, + instrumentPageLoad: false, + }); + + return { + ...browserTracingIntegrationInstance, + afterAllSetup(client) { + const startPageloadCallback = (startSpanOptions: StartSpanOptions): void => { + startBrowserTracingPageLoadSpan(client, startSpanOptions); + }; + + const startNavigationCallback = (startSpanOptions: StartSpanOptions): void => { + startBrowserTracingNavigationSpan(client, startSpanOptions); + }; + + // We need to run the navigation span instrumentation before the `afterAllSetup` hook on the normal browser + // tracing integration because we need to ensure the order of execution is as follows: + // Instrumentation to start span on RSC fetch request runs -> Instrumentation to put tracing headers from active span on fetch runs + // If it were the other way around, the RSC fetch request would not receive the tracing headers from the navigation transaction. + // eslint-disable-next-line deprecation/deprecation + nextRouterInstrumentation( + () => undefined, + false, + options?.instrumentNavigation, + startPageloadCallback, + startNavigationCallback, + ); + + browserTracingIntegrationInstance.afterAllSetup(client); + + // eslint-disable-next-line deprecation/deprecation + nextRouterInstrumentation( + () => undefined, + options?.instrumentPageLoad, + false, + startPageloadCallback, + startNavigationCallback, + ); + }, + }; +} diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index a1c20937f578..e0d22445a3a1 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -1,5 +1,5 @@ import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; -import type { BrowserOptions, browserTracingIntegration } from '@sentry/react'; +import type { BrowserOptions } from '@sentry/react'; import { Integrations as OriginalIntegrations, getCurrentScope, @@ -10,11 +10,13 @@ import type { EventProcessor, Integration } from '@sentry/types'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; +import { browserTracingIntegration } from './browserTracingIntegration'; import { BrowserTracing } from './browserTracingIntegration'; import { rewriteFramesIntegration } from './rewriteFramesIntegration'; import { applyTunnelRouteOption } from './tunnelRoute'; export * from '@sentry/react'; +// eslint-disable-next-line deprecation/deprecation export { nextRouterInstrumentation } from './routing/nextRoutingInstrumentation'; export { captureUnderscoreErrorException } from '../common/_error'; @@ -35,6 +37,7 @@ export const Integrations = { // // import { BrowserTracing } from '@sentry/nextjs'; // const instance = new BrowserTracing(); +// eslint-disable-next-line deprecation/deprecation export { BrowserTracing, rewriteFramesIntegration }; // Treeshakable guard to remove all code related to tracing @@ -68,7 +71,7 @@ export function init(options: BrowserOptions): void { } // TODO v8: Remove this again -// We need to handle BrowserTracing passed to `integrations` that comes from `@sentry/tracing`, not `@sentry/sveltekit` :( +// We need to handle BrowserTracing passed to `integrations` that comes from `@sentry/tracing`, not `@sentry/nextjs` :( function fixBrowserTracingIntegration(options: BrowserOptions): void { const { integrations } = options; if (!integrations) { @@ -89,6 +92,7 @@ function fixBrowserTracingIntegration(options: BrowserOptions): void { function isNewBrowserTracingIntegration( integration: Integration, ): integration is Integration & { options?: Parameters[0] } { + // eslint-disable-next-line deprecation/deprecation return !!integration.afterAllSetup && !!(integration as BrowserTracing).options; } @@ -102,17 +106,21 @@ function maybeUpdateBrowserTracingIntegration(integrations: Integration[]): Inte // If `browserTracingIntegration()` was added, we need to force-convert it to our custom one if (isNewBrowserTracingIntegration(browserTracing)) { const { options } = browserTracing; + // eslint-disable-next-line deprecation/deprecation integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options); } // If BrowserTracing was added, but it is not our forked version, // replace it with our forked version with the same options + // eslint-disable-next-line deprecation/deprecation if (!(browserTracing instanceof BrowserTracing)) { + // eslint-disable-next-line deprecation/deprecation const options: ConstructorParameters[0] = (browserTracing as BrowserTracing).options; // This option is overwritten by the custom integration delete options.routingInstrumentation; // eslint-disable-next-line deprecation/deprecation delete options.tracingOrigins; + // eslint-disable-next-line deprecation/deprecation integrations[integrations.indexOf(browserTracing)] = new BrowserTracing(options); } @@ -126,7 +134,7 @@ function getDefaultIntegrations(options: BrowserOptions): Integration[] { // will get treeshaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { if (hasTracingEnabled(options)) { - customDefaultIntegrations.push(new BrowserTracing()); + customDefaultIntegrations.push(browserTracingIntegration()); } } @@ -140,4 +148,6 @@ export function withSentryConfig(exportedUserNextConfig: T): T { return exportedUserNextConfig; } +export { browserTracingIntegration } from './browserTracingIntegration'; + export * from '../common'; diff --git a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts index 3083013e084a..25ec697a2161 100644 --- a/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts @@ -1,30 +1,34 @@ import { WINDOW } from '@sentry/react'; -import type { Primitive, Transaction, TransactionContext } from '@sentry/types'; +import type { Primitive, Span, StartSpanOptions, Transaction, TransactionContext } from '@sentry/types'; import { addFetchInstrumentationHandler, browserPerformanceTimeOrigin } from '@sentry/utils'; type StartTransactionCb = (context: TransactionContext) => Transaction | undefined; +type StartSpanCb = (context: StartSpanOptions) => void; const DEFAULT_TAGS = { 'routing.instrumentation': 'next-app-router', } as const; /** - * Instruments the Next.js Clientside App Router. + * Instruments the Next.js Client App Router. */ +// TODO(v8): Clean this function up by splitting into pageload and navigation instrumentation respectively. Also remove startTransactionCb in the process. export function appRouterInstrumentation( startTransactionCb: StartTransactionCb, startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, + startPageloadSpanCallback: StartSpanCb, + startNavigationSpanCallback: StartSpanCb, ): void { // We keep track of the active transaction so we can finish it when we start a navigation transaction. - let activeTransaction: Transaction | undefined = undefined; + let activeTransaction: Span | 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({ + const transactionContext = { name: prevLocationName, op: 'pageload', origin: 'auto.pageload.nextjs.app_router_instrumentation', @@ -32,7 +36,9 @@ export function appRouterInstrumentation( // pageload should always start at timeOrigin (and needs to be in s, not ms) startTimestamp: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined, metadata: { source: 'url' }, - }); + } as const; + activeTransaction = startTransactionCb(transactionContext); + startPageloadSpanCallback(transactionContext); } if (startTransactionOnLocationChange) { @@ -66,13 +72,16 @@ export function appRouterInstrumentation( activeTransaction.end(); } - startTransactionCb({ + const transactionContext = { name: transactionName, op: 'navigation', origin: 'auto.navigation.nextjs.app_router_instrumentation', tags, metadata: { source: 'url' }, - }); + } as const; + + startTransactionCb(transactionContext); + startNavigationSpanCallback(transactionContext); }); } } diff --git a/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts index 3010faad4183..4706fb8a32f2 100644 --- a/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/nextRoutingInstrumentation.ts @@ -1,23 +1,40 @@ import { WINDOW } from '@sentry/react'; -import type { Transaction, TransactionContext } from '@sentry/types'; +import type { StartSpanOptions, Transaction, TransactionContext } from '@sentry/types'; import { appRouterInstrumentation } from './appRouterRoutingInstrumentation'; import { pagesRouterInstrumentation } from './pagesRouterRoutingInstrumentation'; type StartTransactionCb = (context: TransactionContext) => Transaction | undefined; +type StartSpanCb = (context: StartSpanOptions) => void; /** - * Instruments the Next.js Clientside Router. + * Instruments the Next.js Client Router. + * + * @deprecated Use `browserTracingIntegration()` as exported from `@sentry/nextjs` instead. */ export function nextRouterInstrumentation( startTransactionCb: StartTransactionCb, startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, + startPageloadSpanCallback?: StartSpanCb, + startNavigationSpanCallback?: StartSpanCb, ): void { const isAppRouter = !WINDOW.document.getElementById('__NEXT_DATA__'); if (isAppRouter) { - appRouterInstrumentation(startTransactionCb, startTransactionOnPageLoad, startTransactionOnLocationChange); + appRouterInstrumentation( + startTransactionCb, + startTransactionOnPageLoad, + startTransactionOnLocationChange, + startPageloadSpanCallback || (() => undefined), + startNavigationSpanCallback || (() => undefined), + ); } else { - pagesRouterInstrumentation(startTransactionCb, startTransactionOnPageLoad, startTransactionOnLocationChange); + pagesRouterInstrumentation( + startTransactionCb, + startTransactionOnPageLoad, + startTransactionOnLocationChange, + startPageloadSpanCallback || (() => undefined), + startNavigationSpanCallback || (() => undefined), + ); } } diff --git a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts index e360e51df56b..c3f466a566ea 100644 --- a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts @@ -1,7 +1,7 @@ import type { ParsedUrlQuery } from 'querystring'; import { getClient, getCurrentScope } from '@sentry/core'; import { WINDOW } from '@sentry/react'; -import type { Primitive, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; +import type { Primitive, StartSpanOptions, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { browserPerformanceTimeOrigin, logger, @@ -20,6 +20,7 @@ const globalObject = WINDOW as typeof WINDOW & { }; type StartTransactionCb = (context: TransactionContext) => Transaction | undefined; +type StartSpanCb = (context: StartSpanOptions) => void; /** * Describes data located in the __NEXT_DATA__ script tag. This tag is present on every page of a Next.js app. @@ -117,6 +118,8 @@ export function pagesRouterInstrumentation( startTransactionCb: StartTransactionCb, startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, + startPageloadSpanCallback: StartSpanCb, + startNavigationSpanCallback: StartSpanCb, ): void { const { route, params, sentryTrace, baggage } = extractNextDataTagInformation(); // eslint-disable-next-line deprecation/deprecation @@ -130,7 +133,7 @@ export function pagesRouterInstrumentation( if (startTransactionOnPageLoad) { const source = route ? 'route' : 'url'; - activeTransaction = startTransactionCb({ + const transactionContext = { name: prevLocationName, op: 'pageload', origin: 'auto.pageload.nextjs.pages_router_instrumentation', @@ -143,7 +146,9 @@ export function pagesRouterInstrumentation( source, dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, - }); + } as const; + activeTransaction = startTransactionCb(transactionContext); + startPageloadSpanCallback(transactionContext); } if (startTransactionOnLocationChange) { @@ -173,13 +178,15 @@ export function pagesRouterInstrumentation( activeTransaction.end(); } - const navigationTransaction = startTransactionCb({ + const transactionContext = { name: transactionName, op: 'navigation', origin: 'auto.navigation.nextjs.pages_router_instrumentation', tags, metadata: { source: transactionSource }, - }); + } as const; + const navigationTransaction = startTransactionCb(transactionContext); + startNavigationSpanCallback(transactionContext); if (navigationTransaction) { // In addition to the navigation transaction we're also starting a span to mark Next.js's `routeChangeStart` diff --git a/packages/nextjs/test/clientSdk.test.ts b/packages/nextjs/test/clientSdk.test.ts index f4ec99c3cc71..0ce7733dc137 100644 --- a/packages/nextjs/test/clientSdk.test.ts +++ b/packages/nextjs/test/clientSdk.test.ts @@ -117,111 +117,107 @@ describe('Client init()', () => { expect(installedBreadcrumbsIntegration).toBeDefined(); }); - describe('`BrowserTracing` integration', () => { - it('adds `BrowserTracing` integration if `tracesSampleRate` is set', () => { - init({ - dsn: TEST_DSN, - tracesSampleRate: 1.0, - }); + it('forces correct router instrumentation if user provides `BrowserTracing` in an array', () => { + init({ + dsn: TEST_DSN, + tracesSampleRate: 1.0, + // eslint-disable-next-line deprecation/deprecation + integrations: [new BrowserTracing({ finalTimeout: 10 })], + }); - const client = getClient()!; - const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + const client = getClient()!; + // eslint-disable-next-line deprecation/deprecation + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + + expect(browserTracingIntegration).toBeDefined(); + expect(browserTracingIntegration?.options).toEqual( + expect.objectContaining({ + // eslint-disable-next-line deprecation/deprecation + routingInstrumentation: nextRouterInstrumentation, + // This proves it's still the user's copy + finalTimeout: 10, + }), + ); + }); - expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration?.options).toEqual( - expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - }), - ); + it('forces correct router instrumentation if user provides `browserTracingIntegration`', () => { + init({ + dsn: TEST_DSN, + integrations: [browserTracingIntegration({ finalTimeout: 10 })], + enableTracing: true, }); - it('adds `BrowserTracing` integration if `tracesSampler` is set', () => { - init({ - dsn: TEST_DSN, - tracesSampler: () => true, - }); - - const client = getClient()!; - const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + const client = getClient()!; + // eslint-disable-next-line deprecation/deprecation + const integration = client.getIntegrationByName('BrowserTracing'); + + expect(integration).toBeDefined(); + expect(integration?.options).toEqual( + expect.objectContaining({ + // eslint-disable-next-line deprecation/deprecation + routingInstrumentation: nextRouterInstrumentation, + // This proves it's still the user's copy + finalTimeout: 10, + }), + ); + }); - expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration?.options).toEqual( - expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - }), - ); + it('forces correct router instrumentation if user provides `BrowserTracing` in a function', () => { + init({ + dsn: TEST_DSN, + tracesSampleRate: 1.0, + // eslint-disable-next-line deprecation/deprecation + integrations: defaults => [...defaults, new BrowserTracing({ startTransactionOnLocationChange: false })], }); - it('does not add `BrowserTracing` integration if tracing not enabled in SDK', () => { - init({ - dsn: TEST_DSN, - }); + const client = getClient()!; - const client = getClient()!; - const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + // eslint-disable-next-line deprecation/deprecation + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); - expect(browserTracingIntegration).toBeUndefined(); - }); + expect(browserTracingIntegration).toBeDefined(); + expect(browserTracingIntegration?.options).toEqual( + expect.objectContaining({ + // eslint-disable-next-line deprecation/deprecation + routingInstrumentation: nextRouterInstrumentation, + // This proves it's still the user's copy + startTransactionOnLocationChange: false, + }), + ); + }); - it('forces correct router instrumentation if user provides `BrowserTracing` in an array', () => { + describe('browserTracingIntegration()', () => { + it('adds `browserTracingIntegration()` integration if `tracesSampleRate` is set', () => { init({ dsn: TEST_DSN, tracesSampleRate: 1.0, - integrations: [new BrowserTracing({ finalTimeout: 10 })], }); const client = getClient()!; - const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); - - expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration?.options).toEqual( - expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - // This proves it's still the user's copy - finalTimeout: 10, - }), - ); + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + expect(browserTracingIntegration?.name).toBe('BrowserTracing'); }); - it('forces correct router instrumentation if user provides `browserTracingIntegration`', () => { + it('adds `browserTracingIntegration()` integration if `tracesSampler` is set', () => { init({ dsn: TEST_DSN, - integrations: [browserTracingIntegration({ finalTimeout: 10 })], - enableTracing: true, + tracesSampler: () => true, }); const client = getClient()!; - const integration = client.getIntegrationByName('BrowserTracing'); - - expect(integration).toBeDefined(); - expect(integration?.options).toEqual( - expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - // This proves it's still the user's copy - finalTimeout: 10, - }), - ); + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + expect(browserTracingIntegration?.name).toBe('BrowserTracing'); }); - it('forces correct router instrumentation if user provides `BrowserTracing` in a function', () => { + it('does not add `browserTracingIntegration()` integration if tracing not enabled in SDK', () => { init({ dsn: TEST_DSN, - tracesSampleRate: 1.0, - integrations: defaults => [...defaults, new BrowserTracing({ startTransactionOnLocationChange: false })], }); const client = getClient()!; - const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); - - expect(browserTracingIntegration).toBeDefined(); - expect(browserTracingIntegration?.options).toEqual( - expect.objectContaining({ - routingInstrumentation: nextRouterInstrumentation, - // This proves it's still the user's copy - startTransactionOnLocationChange: false, - }), - ); + const browserTracingIntegration = client.getIntegrationByName('BrowserTracing'); + expect(browserTracingIntegration).toBeUndefined(); }); }); }); diff --git a/packages/nextjs/test/performance/appRouterInstrumentation.test.ts b/packages/nextjs/test/performance/appRouterInstrumentation.test.ts index 3337b99ab9a9..34a6b31fc60f 100644 --- a/packages/nextjs/test/performance/appRouterInstrumentation.test.ts +++ b/packages/nextjs/test/performance/appRouterInstrumentation.test.ts @@ -30,7 +30,10 @@ describe('appRouterInstrumentation', () => { 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); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); + + appRouterInstrumentation(startTransactionCallbackFn, true, false, mockStartPageloadSpan, mockStartNavigationSpan); expect(startTransactionCallbackFn).toHaveBeenCalledWith( expect.objectContaining({ name: '/some/page', @@ -42,12 +45,26 @@ describe('appRouterInstrumentation', () => { metadata: { source: 'url' }, }), ); + expect(mockStartPageloadSpan).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); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); + + appRouterInstrumentation(startTransactionCallbackFn, false, false, mockStartPageloadSpan, mockStartNavigationSpan); expect(startTransactionCallbackFn).not.toHaveBeenCalled(); }); @@ -60,7 +77,10 @@ describe('appRouterInstrumentation', () => { }); const startTransactionCallbackFn = jest.fn(); - appRouterInstrumentation(startTransactionCallbackFn, false, true); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); + + appRouterInstrumentation(startTransactionCallbackFn, false, true, mockStartPageloadSpan, mockStartNavigationSpan); fetchInstrumentationHandlerCallback!({ args: [ @@ -85,6 +105,16 @@ describe('appRouterInstrumentation', () => { 'routing.instrumentation': 'next-app-router', }, }); + expect(mockStartNavigationSpan).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([ @@ -133,7 +163,7 @@ describe('appRouterInstrumentation', () => { }, ], ])( - 'should not create naviagtion transactions for fetch requests that are not navigating RSC requests (%s)', + 'should not create navigation 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; @@ -143,9 +173,13 @@ describe('appRouterInstrumentation', () => { }); const startTransactionCallbackFn = jest.fn(); - appRouterInstrumentation(startTransactionCallbackFn, false, true); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); + + appRouterInstrumentation(startTransactionCallbackFn, false, true, mockStartPageloadSpan, mockStartNavigationSpan); fetchInstrumentationHandlerCallback!(fetchCallbackData); expect(startTransactionCallbackFn).not.toHaveBeenCalled(); + expect(mockStartNavigationSpan).not.toHaveBeenCalled(); }, ); @@ -153,9 +187,12 @@ describe('appRouterInstrumentation', () => { setUpPage('https://example.com/some/page?someParam=foobar'); const addFetchInstrumentationHandlerImpl = jest.fn(); const startTransactionCallbackFn = jest.fn(); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); addFetchInstrumentationHandlerSpy.mockImplementationOnce(addFetchInstrumentationHandlerImpl); - appRouterInstrumentation(startTransactionCallbackFn, false, false); + appRouterInstrumentation(startTransactionCallbackFn, false, false, mockStartPageloadSpan, mockStartNavigationSpan); expect(addFetchInstrumentationHandlerImpl).not.toHaveBeenCalled(); + expect(mockStartNavigationSpan).not.toHaveBeenCalled(); }); }); diff --git a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts index 592df911bde2..3e032c1f01d1 100644 --- a/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts +++ b/packages/nextjs/test/performance/pagesRouterInstrumentation.test.ts @@ -211,20 +211,41 @@ describe('pagesRouterInstrumentation', () => { 'creates a pageload transaction (#%#)', (url, route, query, props, hasNextData, expectedStartTransactionArgument) => { const mockStartTransaction = createMockStartTransaction(); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); + setUpNextPage({ url, route, query, props, hasNextData }); - pagesRouterInstrumentation(mockStartTransaction); + pagesRouterInstrumentation( + mockStartTransaction, + undefined, + undefined, + mockStartPageloadSpan, + mockStartNavigationSpan, + ); + expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockStartTransaction).toHaveBeenLastCalledWith( expect.objectContaining(expectedStartTransactionArgument), ); + expect(mockStartPageloadSpan).toHaveBeenCalledWith(expect.objectContaining(expectedStartTransactionArgument)); }, ); it('does not create a pageload transaction if option not given', () => { const mockStartTransaction = createMockStartTransaction(); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); + setUpNextPage({ url: 'https://example.com/', route: '/', hasNextData: false }); - pagesRouterInstrumentation(mockStartTransaction, false); - expect(mockStartTransaction).toHaveBeenCalledTimes(0); + pagesRouterInstrumentation( + mockStartTransaction, + false, + undefined, + mockStartPageloadSpan, + mockStartNavigationSpan, + ); + expect(mockStartTransaction).not.toHaveBeenCalled(); + expect(mockStartPageloadSpan).not.toHaveBeenCalled(); }); }); @@ -252,6 +273,8 @@ describe('pagesRouterInstrumentation', () => { 'should create a parameterized transaction on route change (%s)', (targetLocation, expectedTransactionName, expectedTransactionSource) => { const mockStartTransaction = createMockStartTransaction(); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); setUpNextPage({ url: 'https://example.com/home', @@ -270,7 +293,7 @@ describe('pagesRouterInstrumentation', () => { ], }); - pagesRouterInstrumentation(mockStartTransaction, false, true); + pagesRouterInstrumentation(mockStartTransaction, false, true, mockStartPageloadSpan, mockStartNavigationSpan); Router.events.emit('routeChangeStart', targetLocation); @@ -287,6 +310,18 @@ describe('pagesRouterInstrumentation', () => { }), }), ); + expect(mockStartNavigationSpan).toHaveBeenCalledWith( + expect.objectContaining({ + name: expectedTransactionName, + op: 'navigation', + tags: expect.objectContaining({ + 'routing.instrumentation': 'next-pages-router', + }), + metadata: expect.objectContaining({ + source: expectedTransactionSource, + }), + }), + ); Router.events.emit('routeChangeComplete', targetLocation); // eslint-disable-next-line @typescript-eslint/unbound-method @@ -298,6 +333,8 @@ describe('pagesRouterInstrumentation', () => { it('should not create transaction when navigation transactions are disabled', () => { const mockStartTransaction = createMockStartTransaction(); + const mockStartPageloadSpan = jest.fn(); + const mockStartNavigationSpan = jest.fn(); setUpNextPage({ url: 'https://example.com/home', @@ -306,11 +343,12 @@ describe('pagesRouterInstrumentation', () => { navigatableRoutes: ['/home', '/posts/[id]'], }); - pagesRouterInstrumentation(mockStartTransaction, false, false); + pagesRouterInstrumentation(mockStartTransaction, false, false, mockStartPageloadSpan, mockStartNavigationSpan); Router.events.emit('routeChangeStart', '/posts/42'); expect(mockStartTransaction).not.toHaveBeenCalled(); + expect(mockStartNavigationSpan).not.toHaveBeenCalled(); }); }); });