-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(nextjs): Instrument server-side getInitialProps
of _app
, _document
and _error
#5604
Changes from all commits
913ca74
4a8a477
ce9b9a5
d3f0311
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,6 @@ | ||
export { withSentryGetStaticProps } from './withSentryGetStaticProps'; | ||
export { withSentryGetServerSideProps } from './withSentryGetServerSideProps'; | ||
export { withSentryServerSideGetInitialProps } from './withSentryServerSideGetInitialProps'; | ||
export { withSentryServerSideAppGetInitialProps } from './withSentryServerSideAppGetInitialProps'; | ||
export { withSentryServerSideDocumentGetInitialProps } from './withSentryServerSideDocumentGetInitialProps'; | ||
export { withSentryServerSideErrorGetInitialProps } from './withSentryServerSideErrorGetInitialProps'; | ||
export { withSentryGetServerSideProps } from './withSentryGetServerSideProps'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
import { hasTracingEnabled } from '@sentry/tracing'; | ||
import App from 'next/app'; | ||
|
||
import { isBuild } from '../../utils/isBuild'; | ||
import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; | ||
|
||
type AppGetInitialProps = typeof App['getInitialProps']; | ||
|
||
/** | ||
* Create a wrapped version of the user's exported `getInitialProps` function in | ||
* a custom app ("_app.js"). | ||
* | ||
* @param origAppGetInitialProps The user's `getInitialProps` function | ||
* @param parameterizedRoute The page's parameterized route | ||
* @returns A wrapped version of the function | ||
*/ | ||
export function withSentryServerSideAppGetInitialProps(origAppGetInitialProps: AppGetInitialProps): AppGetInitialProps { | ||
return async function ( | ||
...appGetInitialPropsArguments: Parameters<AppGetInitialProps> | ||
): ReturnType<AppGetInitialProps> { | ||
if (isBuild()) { | ||
return origAppGetInitialProps(...appGetInitialPropsArguments); | ||
} | ||
|
||
const [context] = appGetInitialPropsArguments; | ||
const { req, res } = context.ctx; | ||
|
||
const errorWrappedAppGetInitialProps = withErrorInstrumentation(origAppGetInitialProps); | ||
|
||
if (hasTracingEnabled()) { | ||
// Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and | ||
// `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
return callTracedServerSideDataFetcher(errorWrappedAppGetInitialProps, appGetInitialPropsArguments, req!, res!, { | ||
dataFetcherRouteName: '/_app', | ||
requestedRouteName: context.ctx.pathname, | ||
dataFetchingMethodName: 'getInitialProps', | ||
}); | ||
} else { | ||
return errorWrappedAppGetInitialProps(...appGetInitialPropsArguments); | ||
} | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import { hasTracingEnabled } from '@sentry/tracing'; | ||
import Document from 'next/document'; | ||
|
||
import { isBuild } from '../../utils/isBuild'; | ||
import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; | ||
|
||
type DocumentGetInitialProps = typeof Document.getInitialProps; | ||
|
||
/** | ||
* Create a wrapped version of the user's exported `getInitialProps` function in | ||
* a custom document ("_document.js"). | ||
* | ||
* @param origDocumentGetInitialProps The user's `getInitialProps` function | ||
* @param parameterizedRoute The page's parameterized route | ||
* @returns A wrapped version of the function | ||
*/ | ||
export function withSentryServerSideDocumentGetInitialProps( | ||
origDocumentGetInitialProps: DocumentGetInitialProps, | ||
): DocumentGetInitialProps { | ||
return async function ( | ||
...documentGetInitialPropsArguments: Parameters<DocumentGetInitialProps> | ||
): ReturnType<DocumentGetInitialProps> { | ||
if (isBuild()) { | ||
return origDocumentGetInitialProps(...documentGetInitialPropsArguments); | ||
} | ||
|
||
const [context] = documentGetInitialPropsArguments; | ||
const { req, res } = context; | ||
|
||
const errorWrappedGetInitialProps = withErrorInstrumentation(origDocumentGetInitialProps); | ||
|
||
if (hasTracingEnabled()) { | ||
// Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and | ||
// `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object | ||
return callTracedServerSideDataFetcher( | ||
errorWrappedGetInitialProps, | ||
documentGetInitialPropsArguments, | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
req!, | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
res!, | ||
{ | ||
dataFetcherRouteName: '/_document', | ||
requestedRouteName: context.pathname, | ||
dataFetchingMethodName: 'getInitialProps', | ||
}, | ||
); | ||
} else { | ||
return errorWrappedGetInitialProps(...documentGetInitialPropsArguments); | ||
} | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import { hasTracingEnabled } from '@sentry/tracing'; | ||
import { NextPageContext } from 'next'; | ||
import { ErrorProps } from 'next/error'; | ||
|
||
import { isBuild } from '../../utils/isBuild'; | ||
import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; | ||
|
||
type ErrorGetInitialProps = (context: NextPageContext) => Promise<ErrorProps>; | ||
|
||
/** | ||
* Create a wrapped version of the user's exported `getInitialProps` function in | ||
* a custom error page ("_error.js"). | ||
* | ||
* @param origErrorGetInitialProps The user's `getInitialProps` function | ||
* @param parameterizedRoute The page's parameterized route | ||
* @returns A wrapped version of the function | ||
*/ | ||
export function withSentryServerSideErrorGetInitialProps( | ||
origErrorGetInitialProps: ErrorGetInitialProps, | ||
): ErrorGetInitialProps { | ||
return async function ( | ||
...errorGetInitialPropsArguments: Parameters<ErrorGetInitialProps> | ||
): ReturnType<ErrorGetInitialProps> { | ||
if (isBuild()) { | ||
return origErrorGetInitialProps(...errorGetInitialPropsArguments); | ||
} | ||
|
||
const [context] = errorGetInitialPropsArguments; | ||
const { req, res } = context; | ||
|
||
const errorWrappedGetInitialProps = withErrorInstrumentation(origErrorGetInitialProps); | ||
|
||
if (hasTracingEnabled()) { | ||
// Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and | ||
// `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, errorGetInitialPropsArguments, req!, res!, { | ||
dataFetcherRouteName: '/_error', | ||
requestedRouteName: context.pathname, | ||
dataFetchingMethodName: 'getInitialProps', | ||
}); | ||
} else { | ||
return errorWrappedGetInitialProps(...errorGetInitialPropsArguments); | ||
} | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,7 @@ type GetInitialProps = Required<NextPage>['getInitialProps']; | |
* @param parameterizedRoute The page's parameterized route | ||
* @returns A wrapped version of the function | ||
*/ | ||
export function withSentryServerSideGetInitialProps( | ||
origGetInitialProps: GetInitialProps, | ||
parameterizedRoute: string, | ||
): GetInitialProps { | ||
export function withSentryServerSideGetInitialProps(origGetInitialProps: GetInitialProps): GetInitialProps { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, maybe this will solve the problem I was having here, which forced me to do the typecast. I spent entirely too much time trying to bend TS to my will on that one and failed entirely. |
||
return async function ( | ||
...getInitialPropsArguments: Parameters<GetInitialProps> | ||
): Promise<ReturnType<GetInitialProps>> { | ||
|
@@ -34,8 +31,9 @@ export function withSentryServerSideGetInitialProps( | |
// `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req!, res!, { | ||
parameterizedRoute, | ||
functionName: 'getInitialProps', | ||
dataFetcherRouteName: context.pathname, | ||
requestedRouteName: context.pathname, | ||
dataFetchingMethodName: 'getInitialProps', | ||
}); | ||
} else { | ||
return errorWrappedGetInitialProps(...getInitialPropsArguments); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,8 +73,12 @@ export function callTracedServerSideDataFetcher<F extends (...args: any[]) => Pr | |
req: IncomingMessage, | ||
res: ServerResponse, | ||
options: { | ||
parameterizedRoute: string; | ||
functionName: string; | ||
/** Parameterized route of the request - will be used for naming the transaction. */ | ||
requestedRouteName: string; | ||
/** Name of the route the data fetcher was defined in - will be used for describing the data fetcher's span. */ | ||
dataFetcherRouteName: string; | ||
/** Name of the data fetching method - will be used for describing the data fetcher's span. */ | ||
dataFetchingMethodName: string; | ||
Comment on lines
+76
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice - this makes it really clear! |
||
}, | ||
): Promise<ReturnType<F>> { | ||
return domain.create().bind(async () => { | ||
|
@@ -84,8 +88,8 @@ export function callTracedServerSideDataFetcher<F extends (...args: any[]) => Pr | |
// TODO: Extract trace data from `req` object (trace and baggage headers) and attach it to transaction | ||
|
||
const newTransaction = startTransaction({ | ||
op: 'nextjs.data', | ||
name: options.parameterizedRoute, | ||
op: 'nextjs.data.server', | ||
lobsterkatie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name: options.requestedRouteName, | ||
metadata: { | ||
source: 'route', | ||
}, | ||
|
@@ -97,8 +101,8 @@ export function callTracedServerSideDataFetcher<F extends (...args: any[]) => Pr | |
} | ||
|
||
const dataFetcherSpan = requestTransaction.startChild({ | ||
op: 'nextjs.data', | ||
description: `${options.functionName} (${options.parameterizedRoute})`, | ||
op: 'nextjs.data.server', | ||
description: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`, | ||
}); | ||
|
||
const currentScope = getCurrentHub().getScope(); | ||
|
@@ -158,7 +162,7 @@ export async function callDataFetcherTraced<F extends (...args: any[]) => Promis | |
// Capture the route, since pre-loading, revalidation, etc might mean that this span may happen during another | ||
// route's transaction | ||
const span = transaction.startChild({ | ||
op: 'nextjs.data', | ||
op: 'nextjs.data.server', | ||
description: `${dataFetchingMethodName} (${parameterizedRoute})`, | ||
}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this is moved over to the proxy template, what would think of using a map for all of these checks? Something like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we merge #5602 I will rebase this PR and also #5593 onto it so that the wrappers use the new proxy loader. This seems like a good approach! 👍