From 3bb8d17b2e332b3cd9d9020277af5232903f1466 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Aug 2022 11:57:14 +0200 Subject: [PATCH] feat(nextjs): Improve pageload transaction creation (#5574) Co-authored-by: Abhijeet Prasad --- packages/nextjs/src/performance/client.ts | 146 ++++++++++++-- packages/nextjs/test/index.client.test.ts | 16 ++ .../nextjs/test/performance/client.test.ts | 183 ++++++++++++++++-- packages/types/src/options.ts | 9 +- 4 files changed, 316 insertions(+), 38 deletions(-) diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts index 1b89d4255dd6..5d2ccb3cb04f 100644 --- a/packages/nextjs/src/performance/client.ts +++ b/packages/nextjs/src/performance/client.ts @@ -1,13 +1,113 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { Primitive, Transaction, TransactionContext } from '@sentry/types'; -import { fill, getGlobalObject, stripUrlQueryAndFragment } from '@sentry/utils'; +import { getCurrentHub } from '@sentry/hub'; +import { Primitive, TraceparentData, Transaction, TransactionContext } from '@sentry/types'; +import { + extractTraceparentData, + fill, + getGlobalObject, + logger, + parseBaggageHeader, + stripUrlQueryAndFragment, +} 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'; const global = getGlobalObject(); type StartTransactionCb = (context: TransactionContext) => Transaction | undefined; +/** + * Describes data located in the __NEXT_DATA__ script tag. This tag is present on every page of a Next.js app. + */ +interface SentryEnhancedNextData extends NextData { + // contains props returned by `getInitialProps` - except for `pageProps`, these are the props that got returned by `getServerSideProps` or `getStaticProps` + props: { + _sentryGetInitialPropsTraceData?: string; // trace parent info, if injected by server-side `getInitialProps` + _sentryGetInitialPropsBaggage?: string; // baggage, if injected by server-side `getInitialProps` + pageProps?: { + _sentryGetServerSidePropsTraceData?: string; // trace parent info, if injected by server-side `getServerSideProps` + _sentryGetServerSidePropsBaggage?: string; // baggage, if injected by server-side `getServerSideProps` + + // The following two values are only injected in a very special case with the following conditions: + // 1. The page's `getStaticPaths` method must have returned `fallback: 'blocking'`. + // 2. The requested page must be a "miss" in terms of "Incremental Static Regeneration", meaning the requested page has not been generated before. + // In this case, a page is requested and only served when `getStaticProps` is done. There is not even a fallback page or similar. + _sentryGetStaticPropsTraceData?: string; // trace parent info, if injected by server-side `getStaticProps` + _sentryGetStaticPropsBaggage?: string; // baggage, if injected by server-side `getStaticProps` + }; + }; +} + +interface NextDataTagInfo { + route?: string; + traceParentData?: TraceparentData; + baggage?: string; + params?: ParsedUrlQuery; +} + +/** + * Every Next.js page (static and dynamic ones) comes with a script tag with the id "__NEXT_DATA__". This script tag + * contains a JSON object with data that was either generated at build time for static pages (`getStaticProps`), or at + * runtime with data fetchers like `getServerSideProps.`. + * + * We can use this information to: + * - Always get the parameterized route we're in when loading a page. + * - Send trace information (trace-id, baggage) from the server to the client. + * + * This function extracts this information. + */ +function extractNextDataTagInformation(): NextDataTagInfo { + let nextData: SentryEnhancedNextData | undefined; + // Let's be on the safe side and actually check first if there is really a __NEXT_DATA__ script tag on the page. + // Theoretically this should always be the case though. + const nextDataTag = global.document.getElementById('__NEXT_DATA__'); + if (nextDataTag && nextDataTag.innerHTML) { + try { + nextData = JSON.parse(nextDataTag.innerHTML); + } catch (e) { + __DEBUG_BUILD__ && logger.warn('Could not extract __NEXT_DATA__'); + } + } + + if (!nextData) { + return {}; + } + + const nextDataTagInfo: NextDataTagInfo = {}; + + const { page, query, props } = nextData; + + // `nextData.page` always contains the parameterized route + nextDataTagInfo.route = page; + nextDataTagInfo.params = query; + + if (props) { + const { pageProps } = props; + + const getInitialPropsBaggage = props._sentryGetInitialPropsBaggage; + const getServerSidePropsBaggage = pageProps && pageProps._sentryGetServerSidePropsBaggage; + const getStaticPropsBaggage = pageProps && pageProps._sentryGetStaticPropsBaggage; + // Ordering of the following shouldn't matter but `getInitialProps` generally runs before `getServerSideProps` or `getStaticProps` so we give it priority. + const baggage = getInitialPropsBaggage || getServerSidePropsBaggage || getStaticPropsBaggage; + if (baggage) { + nextDataTagInfo.baggage = baggage; + } + + const getInitialPropsTraceData = props._sentryGetInitialPropsTraceData; + const getServerSidePropsTraceData = pageProps && pageProps._sentryGetServerSidePropsTraceData; + const getStaticPropsTraceData = pageProps && pageProps._sentryGetStaticPropsTraceData; + // Ordering of the following shouldn't matter but `getInitialProps` generally runs before `getServerSideProps` or `getStaticProps` so we give it priority. + const traceData = getInitialPropsTraceData || getServerSidePropsTraceData || getStaticPropsTraceData; + if (traceData) { + nextDataTagInfo.traceParentData = extractTraceparentData(traceData); + } + } + + return nextDataTagInfo; +} + const DEFAULT_TAGS = { 'routing.instrumentation': 'next-router', } as const; @@ -16,6 +116,8 @@ let activeTransaction: Transaction | undefined = undefined; let prevTransactionName: string | undefined = undefined; let startTransaction: StartTransactionCb | undefined = undefined; +const client = getCurrentHub().getClient(); + /** * Creates routing instrumention for Next Router. Only supported for * client side routing. Works for Next >= 10. @@ -30,24 +132,27 @@ export function nextRouterInstrumentation( startTransactionOnLocationChange: boolean = true, ): void { startTransaction = startTransactionCb; - Router.ready(() => { - // We can only start the pageload transaction when we have access to the parameterized - // route name. Setting the transaction name after the transaction is started could lead - // to possible race conditions with the router, so this approach was taken. - if (startTransactionOnPageLoad) { - const pathIsRoute = Router.route !== null; - - prevTransactionName = pathIsRoute ? stripUrlQueryAndFragment(Router.route) : global.location.pathname; - activeTransaction = startTransactionCb({ - name: prevTransactionName, - op: 'pageload', - tags: DEFAULT_TAGS, - metadata: { - source: pathIsRoute ? 'route' : 'url', - }, - }); - } + if (startTransactionOnPageLoad) { + const { route, traceParentData, baggage, params } = extractNextDataTagInformation(); + + prevTransactionName = route || global.location.pathname; + const source = route ? 'route' : 'url'; + + activeTransaction = startTransactionCb({ + name: prevTransactionName, + op: 'pageload', + tags: DEFAULT_TAGS, + ...(params && client && client.getOptions().sendDefaultPii && { data: params }), + ...traceParentData, + metadata: { + source, + ...(baggage && { baggage: parseBaggageHeader(baggage) }), + }, + }); + } + + Router.ready(() => { // Spans that aren't attached to any transaction are lost; so if transactions aren't // created (besides potentially the onpageload transaction), no need to wrap the router. if (!startTransactionOnLocationChange) return; @@ -78,7 +183,7 @@ type WrappedRouterChangeState = RouterChangeState; * Start a navigation transaction every time the router changes state. */ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): WrappedRouterChangeState { - const wrapper = function ( + return function wrapper( this: any, method: string, // The parameterized url, ex. posts/[id]/[comment] @@ -115,5 +220,4 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap } return originalChangeStateWrapper.call(this, method, url, as, options, ...args); }; - return wrapper; } diff --git a/packages/nextjs/test/index.client.test.ts b/packages/nextjs/test/index.client.test.ts index a4f2972537be..cf60c27df0e3 100644 --- a/packages/nextjs/test/index.client.test.ts +++ b/packages/nextjs/test/index.client.test.ts @@ -4,6 +4,7 @@ import * as SentryReact from '@sentry/react'; import { Integrations as TracingIntegrations } from '@sentry/tracing'; import { Integration } from '@sentry/types'; import { getGlobalObject, logger } from '@sentry/utils'; +import { JSDOM } from 'jsdom'; import { init, Integrations, nextRouterInstrumentation } from '../src/index.client'; import { NextjsOptions } from '../src/utils/nextjsOptions'; @@ -16,6 +17,21 @@ const reactInit = jest.spyOn(SentryReact, 'init'); const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent'); const loggerLogSpy = jest.spyOn(logger, 'log'); +// We're setting up JSDom here because 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` +const dom = new JSDOM(undefined, { url: 'https://example.com/' }); +Object.defineProperty(global, 'document', { value: dom.window.document, writable: true }); +Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true }); + +const originalGlobalDocument = getGlobalObject().document; +const originalGlobalLocation = getGlobalObject().location; +afterAll(() => { + // Clean up JSDom + Object.defineProperty(global, 'document', { value: originalGlobalDocument }); + Object.defineProperty(global, 'location', { value: originalGlobalLocation }); +}); + describe('Client init()', () => { afterEach(() => { jest.clearAllMocks(); diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index bf8f118cd8c4..2a90b7db45a1 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -1,3 +1,6 @@ +import { getGlobalObject } from '@sentry/utils'; +import { JSDOM } from 'jsdom'; +import { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils'; import { default as Router } from 'next/router'; import { nextRouterInstrumentation } from '../../src/performance/client'; @@ -28,28 +31,182 @@ describe('client', () => { }); describe('nextRouterInstrumentation', () => { + const originalGlobalDocument = getGlobalObject().document; + const originalGlobalLocation = getGlobalObject().location; + + function setUpNextPage(pageProperties: { + url: string; + route: string; + query: any; + props: any; + hasNextData: boolean; + }) { + const nextDataContent: NextData = { + props: pageProperties.props, + page: pageProperties.route, + query: pageProperties.query, + buildId: 'y76hvndNJBAithejdVGLW', + isFallback: false, + gssp: true, + appGip: true, + scriptLoader: [], + }; + + const dom = new JSDOM( + // Just an example of what a __NEXT_DATA__ tag might look like + pageProperties.hasNextData + ? `` + : '

No next data :(

', + { url: pageProperties.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(global, 'document', { value: dom.window.document, writable: true }); + Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true }); + } + + afterEach(() => { + // Clean up JSDom + Object.defineProperty(global, 'document', { value: originalGlobalDocument }); + Object.defineProperty(global, 'location', { value: originalGlobalLocation }); + }); + it('waits for Router.ready()', () => { + setUpNextPage({ url: 'https://example.com/', route: '/', query: {}, props: {}, hasNextData: false }); const mockStartTransaction = jest.fn(); expect(readyCalled).toBe(false); nextRouterInstrumentation(mockStartTransaction); expect(readyCalled).toBe(true); }); - it('creates a pageload transaction', () => { - const mockStartTransaction = jest.fn(); - nextRouterInstrumentation(mockStartTransaction); - expect(mockStartTransaction).toHaveBeenCalledTimes(1); - expect(mockStartTransaction).toHaveBeenLastCalledWith({ - name: '/[user]/posts/[id]', - op: 'pageload', - tags: { - 'routing.instrumentation': 'next-router', + it.each([ + [ + 'https://example.com/lforst/posts/1337?q=42', + '/[user]/posts/[id]', + { user: 'lforst', id: '1337', q: '42' }, + { + _sentryGetInitialPropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', + _sentryGetInitialPropsBaggage: + 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', }, - metadata: { - source: 'route', + true, + { + name: '/[user]/posts/[id]', + op: 'pageload', + tags: { + 'routing.instrumentation': 'next-router', + }, + metadata: { + source: 'route', + baggage: [{ environment: 'myEnv', release: '2.1.0' }, '', true], + }, + traceId: 'c82b8554881b4d28ad977de04a4fb40a', + parentSpanId: 'a755953cd3394d5f', + parentSampled: true, }, - }); - }); + ], + [ + 'https://example.com/dynamic', + '/dynamic', + {}, + { + pageProps: { + _sentryGetServerSidePropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', + _sentryGetServerSidePropsBaggage: + 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', + }, + }, + true, + { + name: '/dynamic', + op: 'pageload', + tags: { + 'routing.instrumentation': 'next-router', + }, + metadata: { + source: 'route', + baggage: [{ environment: 'myEnv', release: '2.1.0' }, '', true], + }, + traceId: 'c82b8554881b4d28ad977de04a4fb40a', + parentSpanId: 'a755953cd3394d5f', + parentSampled: true, + }, + ], + [ + 'https://example.com/static', + '/static', + {}, + { + pageProps: { + _sentryGetStaticPropsTraceData: 'c82b8554881b4d28ad977de04a4fb40a-a755953cd3394d5f-1', + _sentryGetStaticPropsBaggage: + 'other=vendor,foo=bar,third=party,last=item,sentry-release=2.1.0,sentry-environment=myEnv', + }, + }, + true, + { + name: '/static', + op: 'pageload', + tags: { + 'routing.instrumentation': 'next-router', + }, + metadata: { + source: 'route', + baggage: [{ environment: 'myEnv', release: '2.1.0' }, '', true], + }, + traceId: 'c82b8554881b4d28ad977de04a4fb40a', + parentSpanId: 'a755953cd3394d5f', + parentSampled: true, + }, + ], + [ + 'https://example.com/', + '/', + {}, + {}, + true, + { + name: '/', + op: 'pageload', + tags: { + 'routing.instrumentation': 'next-router', + }, + metadata: { + source: 'route', + }, + }, + ], + [ + 'https://example.com/lforst/posts/1337?q=42', + '/', + {}, + {}, + false, // no __NEXT_DATA__ tag + { + name: '/lforst/posts/1337', + op: 'pageload', + tags: { + 'routing.instrumentation': 'next-router', + }, + metadata: { + source: 'url', + }, + }, + ], + ])( + 'creates a pageload transaction (#%#)', + (url, route, query, props, hasNextData, expectedStartTransactionArgument) => { + const mockStartTransaction = jest.fn(); + setUpNextPage({ url, route, query, props, hasNextData }); + nextRouterInstrumentation(mockStartTransaction); + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenLastCalledWith(expectedStartTransactionArgument); + }, + ); it('does not create a pageload transaction if option not given', () => { const mockStartTransaction = jest.fn(); diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 077b49e4105f..9b8d26d1c7f8 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -153,16 +153,17 @@ export interface ClientOptions