Skip to content

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion packages/nextjs/src/config/loaders/dataFetchersLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const DATA_FETCHING_FUNCTIONS = {

type LoaderOptions = {
projectDir: string;
pagesDir: string;
};

/**
Expand Down Expand Up @@ -108,7 +109,7 @@ export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>,
}

// We know one or the other will be defined, depending on the version of webpack being used
const { projectDir } = 'getOptions' in this ? this.getOptions() : this.query;
const { projectDir, pagesDir } = 'getOptions' in this ? this.getOptions() : this.query;

// In the following branch we will proxy the user's file. This means we return code (basically an entirely new file)
// that re - exports all the user file's originial export, but with a "sentry-proxy-loader" query in the module
Expand Down Expand Up @@ -172,6 +173,20 @@ export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>,

// Fill in template placeholders
let injectedCode = modifiedTemplateCode;
const route = path
// Get the path of the file insde of the pages directory
.relative(pagesDir, this.resourcePath)
// Add a slash at the beginning
.replace(/(.*)/, '/$1')
// Pull off the file extension
.replace(/\.(jsx?|tsx?)/, '')
// Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into
// just `/xyz`
.replace(/\/index$/, '')
// In case all of the above have left us with an empty string (which will happen if we're dealing with the
// homepage), sub back in the root route
.replace(/^$/, '/');
injectedCode = injectedCode.replace('__FILEPATH__', route);
for (const { placeholder, alias } of Object.values(DATA_FETCHING_FUNCTIONS)) {
injectedCode = injectedCode.replace(new RegExp(placeholder, 'g'), alias);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@ declare const __ORIG_GSPROPS__: GetStaticPropsFunction;
// eslint-disable-next-line import/no-extraneous-dependencies, import/no-unresolved
import * as ServerSideSentryNextjsSDK from '@sentry/nextjs';

const PARAMETERIZED_ROUTE = '__FILEPATH__';

export const getServerSideProps =
typeof __ORIG_GSSP__ === 'function'
? ServerSideSentryNextjsSDK.withSentryGetServerSideProps(__ORIG_GSSP__)
? ServerSideSentryNextjsSDK.withSentryGetServerSideProps(__ORIG_GSSP__, PARAMETERIZED_ROUTE)
: __ORIG_GSSP__;

export const getStaticProps =
typeof __ORIG_GSPROPS__ === 'function'
? ServerSideSentryNextjsSDK.withSentryGetStaticProps(__ORIG_GSPROPS__)
? ServerSideSentryNextjsSDK.withSentryGetStaticProps(__ORIG_GSPROPS__, PARAMETERIZED_ROUTE)
: __ORIG_GSPROPS__;
9 changes: 5 additions & 4 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ export function constructWebpackConfigFunction(
newConfig = userNextConfig.webpack(newConfig, buildContext);
}

const pageRegex = new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages(/.+)\\.(jsx?|tsx?)`);

if (isServer) {
newConfig.module = {
...newConfig.module,
Expand All @@ -81,12 +79,15 @@ export function constructWebpackConfigFunction(
};

if (userSentryOptions.experiments?.autoWrapDataFetchers) {
const pagesDir = newConfig.resolve?.alias?.['private-next-pages'] as string;

newConfig.module.rules.push({
test: pageRegex,
// Nextjs allows the `pages` folder to optionally live inside a `src` folder
test: new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages/.*\\.(jsx?|tsx?)`),
use: [
{
loader: path.resolve(__dirname, 'loaders/dataFetchersLoader.js'),
options: { projectDir },
options: { projectDir, pagesDir },
},
],
});
Expand Down
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);
};
}
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);
};
}
60 changes: 50 additions & 10 deletions packages/nextjs/src/config/wrappers/wrapperUtils.ts
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;
}
}