-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(nextjs): Add spans and route parameterization in data fetching wrappers #5564
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
Merged
lobsterkatie
merged 4 commits into
master
from
kmclb-nextjs-add-spans-and-routes-in-datafetch-wrappers
Aug 16, 2022
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
11721ab
inject route as global variable
lobsterkatie e03682c
pass parameterized route to wrapper functions
lobsterkatie 20abb1f
add span creation and transaction name replacement to wrapper core fu…
lobsterkatie 84cd837
add basic error handling
lobsterkatie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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) | ||
lforst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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; | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.