forked from getsentry/sentry-javascript
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(nextjs): Add spans and route parameterization in data fetching w…
…rappers (getsentry#5564) This adds span creation and route parameterization to the experimental `getStaticProps` and `getServerSideProps` wrappers. In order to do the parameterization, we store the parameterized route( which is the same as the filepath of the page file) as a global variable in the code we add using the `dataFetchersLoader` and then pass it to the wrappers, which then replace the name of the active transaction with the parameterized route we've stored. It also adds basic error-capturing to the wrappers. Open issues/questions (not solved by this PR): - Because of prefetching, revalidation of static pages, and the like, the data fetching functions can run in the background, during handling of a route different from their own. (For example, if page A has a nextjs `Link` component pointing to page B, next will prefetch the data for B (and therefore run its data fetching functions) while handling the request for A. We need to make sure that we don't accidentally overwrite the transaction name in that case to B. - There's more we should do in terms of error handling. Specifically, we should port much of the logic in `withSentry` into these wrappers also. - Porting the error-handling logic will mean we need to deal with the domain question - how to keep reactivating the correct domain as we go into and out of data fetching and rendering functions. (There are other items listed in the meta issue linked below, but the above are the ones directly relevant to the work in this PR.) Ref: getsentry#5505
- Loading branch information
1 parent
c2cdf92
commit e861cc4
Showing
6 changed files
with
84 additions
and
23 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 changes: 4 additions & 3 deletions
7
packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| import { GSSP } from './types'; | ||
| import { callOriginal } from './wrapperUtils'; | ||
| import { wrapperCore } from './wrapperUtils'; | ||
|
|
||
| /** | ||
| * Create a wrapped version of the user's exported `getServerSideProps` function | ||
| * | ||
| * @param origGetServerSideProps: The user's `getServerSideProps` function | ||
| * @param route: The page's parameterized route | ||
| * @returns A wrapped version of the function | ||
| */ | ||
| export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn']): GSSP['wrappedFn'] { | ||
| export function withSentryGetServerSideProps(origGetServerSideProps: GSSP['fn'], route: string): GSSP['wrappedFn'] { | ||
| return async function (context: GSSP['context']): Promise<GSSP['result']> { | ||
| return callOriginal<GSSP>(origGetServerSideProps, context); | ||
| return wrapperCore<GSSP>(origGetServerSideProps, context, route); | ||
| }; | ||
| } |
7 changes: 4 additions & 3 deletions
7
packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| import { GSProps } from './types'; | ||
| import { callOriginal } from './wrapperUtils'; | ||
| import { wrapperCore } from './wrapperUtils'; | ||
|
|
||
| /** | ||
| * Create a wrapped version of the user's exported `getStaticProps` function | ||
| * | ||
| * @param origGetStaticProps: The user's `getStaticProps` function | ||
| * @param route: The page's parameterized route | ||
| * @returns A wrapped version of the function | ||
| */ | ||
| export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn']): GSProps['wrappedFn'] { | ||
| export function withSentryGetStaticProps(origGetStaticProps: GSProps['fn'], route: string): GSProps['wrappedFn'] { | ||
| return async function (context: GSProps['context']): Promise<GSProps['result']> { | ||
| return callOriginal<GSProps>(origGetStaticProps, context); | ||
| return wrapperCore<GSProps>(origGetStaticProps, context, route); | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,65 @@ | ||
| import { captureException } from '@sentry/core'; | ||
| import { getActiveTransaction } from '@sentry/tracing'; | ||
| import { Span } from '@sentry/types'; | ||
|
|
||
| import { DataFetchingFunction } from './types'; | ||
|
|
||
| /** | ||
| * Pass-through wrapper for the original function, used as a first step in eventually wrapping the data-fetching | ||
| * functions with code for tracing. | ||
| * Create a span to track the wrapped function and update transaction name with parameterized route. | ||
| * | ||
| * @template T Types for `getInitialProps`, `getStaticProps`, and `getServerSideProps` | ||
| * @param origFunction The user's exported `getInitialProps`, `getStaticProps`, or `getServerSideProps` function | ||
| * @param context The context object passed by nextjs to the function | ||
| * @param route The route currently being served | ||
| * @returns The result of calling the user's function | ||
| */ | ||
| export async function callOriginal<T extends DataFetchingFunction>( | ||
| export async function wrapperCore<T extends DataFetchingFunction>( | ||
| origFunction: T['fn'], | ||
| context: T['context'], | ||
| route: string, | ||
| ): Promise<T['result']> { | ||
| let props; | ||
| const transaction = getActiveTransaction(); | ||
|
|
||
| if (transaction) { | ||
| // Pull off any leading underscores we've added in the process of wrapping the function | ||
| const wrappedFunctionName = origFunction.name.replace(/^_*/, ''); | ||
|
|
||
| // TODO: Make sure that the given route matches the name of the active transaction (to prevent background data | ||
| // fetching from switching the name to a completely other route) | ||
| transaction.name = route; | ||
| transaction.metadata.source = 'route'; | ||
|
|
||
| // 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', description: `${wrappedFunctionName} (${route})` }); | ||
|
|
||
| const props = await callOriginal(origFunction, context, span); | ||
|
|
||
| span.finish(); | ||
|
|
||
| // TODO: Can't figure out how to tell TS that the types are correlated - that a `GSPropsFunction` will only get passed | ||
| // `GSPropsContext` and never, say, `GSSPContext`. That's what wrapping everything in objects and using the generic | ||
| // and pulling the types from the generic rather than specifying them directly was supposed to do, but... no luck. | ||
| // eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any | ||
| props = await (origFunction as any)(context); | ||
| return props; | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return callOriginal(origFunction, context); | ||
| } | ||
|
|
||
| /** Call the original function, capturing any errors and finishing the span (if any) in case of error */ | ||
| async function callOriginal<T extends DataFetchingFunction>( | ||
| origFunction: T['fn'], | ||
| context: T['context'], | ||
| span?: Span, | ||
| ): Promise<T['result']> { | ||
| try { | ||
| // eslint-disable-next-line prefer-const, @typescript-eslint/no-explicit-any | ||
| return (origFunction as any)(context); | ||
| } catch (err) { | ||
| if (span) { | ||
| span.finish(); | ||
| } | ||
|
|
||
| return props; | ||
| // TODO Copy more robust error handling over from `withSentry` | ||
| captureException(err); | ||
| throw err; | ||
| } | ||
| } |