Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
594e5ca
feat(nextjs): Add server-side request transactions (#3533)
lobsterkatie May 17, 2021
17105aa
Merge branch 'master' into kmclb-next-js-performance
lobsterkatie May 17, 2021
b6b922a
add missing tracing dependency back in
lobsterkatie May 17, 2021
135b1bf
Merge branch 'master' into kmclb-next-js-performance
lobsterkatie May 18, 2021
1d90053
Merge branch 'master' into kmclb-next-js-performance
lobsterkatie May 18, 2021
0e46ae4
Merge branch 'master' into kmclb-next-js-performance
lobsterkatie May 20, 2021
5d4dcab
feat(nextjs): Client performance monitoring (#3552)
iker-barriocanal May 20, 2021
da07ea7
fix(nextjs): Use domains to prevent scope bleed on backend (#3574)
lobsterkatie May 20, 2021
4c2a385
Merge branch 'master' into kmclb-next-js-performance
lobsterkatie May 20, 2021
415d04e
Merge branch 'master' into kmclb-next-js-performance
lobsterkatie May 22, 2021
a0bab2f
feat(nextjs): Parameterize server-side transaction names and harvest …
lobsterkatie May 25, 2021
596f83a
chore(nextjs): Generalized server-side performance cleanup (#3581)
lobsterkatie May 25, 2021
1bee1f3
feat(nextjs): Continue trace using incoming `sentry-trace` header (#3…
lobsterkatie May 25, 2021
65aea8a
ref(nextjs): Improve webpack config merging (#3596)
lobsterkatie May 25, 2021
4d37cd1
feat(nextjs): Automatic performance monitoring (#3586)
iker-barriocanal May 25, 2021
07f6851
clarify names, add comments, rework function adding integration to ar…
lobsterkatie May 25, 2021
4e453f7
address PR feedback
lobsterkatie May 26, 2021
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
1 change: 1 addition & 0 deletions packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@sentry/integrations": "6.4.1",
"@sentry/node": "6.4.1",
"@sentry/react": "6.4.1",
"@sentry/tracing": "6.4.1",
"@sentry/utils": "6.4.1",
"@sentry/webpack-plugin": "1.15.0",
"tslib": "^1.9.3"
Expand Down
32 changes: 31 additions & 1 deletion packages/nextjs/src/index.client.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,47 @@
import { configureScope, init as reactInit } from '@sentry/react';
import { Integrations } from '@sentry/tracing';

import { nextRouterInstrumentation } from './performance/client';
import { MetadataBuilder } from './utils/metadataBuilder';
import { NextjsOptions } from './utils/nextjsOptions';
import { addIntegration, UserIntegrations } from './utils/userIntegrations';

export * from '@sentry/react';
export { nextRouterInstrumentation } from './performance/client';

const { BrowserTracing } = Integrations;

/** Inits the Sentry NextJS SDK on the browser with the React SDK. */
export function init(options: NextjsOptions): void {
const metadataBuilder = new MetadataBuilder(options, ['nextjs', 'react']);
metadataBuilder.addSdkMetadata();
options.environment = options.environment || process.env.NODE_ENV;
reactInit(options);

// Only add BrowserTracing if a tracesSampleRate or tracesSampler is set
const integrations =
options.tracesSampleRate === undefined && options.tracesSampler === undefined
? options.integrations
: createClientIntegrations(options.integrations);

reactInit({
...options,
integrations,
});
configureScope(scope => {
scope.setTag('runtime', 'browser');
});
}

const defaultBrowserTracingIntegration = new BrowserTracing({
routingInstrumentation: nextRouterInstrumentation,
});

function createClientIntegrations(integrations?: UserIntegrations): UserIntegrations {
if (integrations) {
return addIntegration(defaultBrowserTracingIntegration, integrations, [
['options.routingInstrumentation', nextRouterInstrumentation],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not pass in the integration name here? I think that would simplify the logic when trying the find the integration to update.

Copy link
Member Author

@lobsterkatie lobsterkatie May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought of that, but I was trying not to completely redo everything, LOL. Happy to do it, though.

Damn, didn't read closely enough. You mean instead of passing in a fully-formed instance? I'm not sure how that makes things simpler.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this (like the logic I had above), so we know exactly what integration to update.

Suggested change
['options.routingInstrumentation', nextRouterInstrumentation],
['BrowserTracing', 'options.routingInstrumentation', nextRouterInstrumentation],

then we can just

for (const integration of allIntegrations) {
  if integration.name === options[0] {
    update(integration, options[1], options[2]);
  }
}

]);
} else {
return [defaultBrowserTracingIntegration];
}
}
3 changes: 2 additions & 1 deletion packages/nextjs/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export function init(options: NextjsOptions): void {
const metadataBuilder = new MetadataBuilder(options, ['nextjs', 'node']);
metadataBuilder.addSdkMetadata();
options.environment = options.environment || process.env.NODE_ENV;
// TODO capture project root and store in an env var for RewriteFrames?
addServerIntegrations(options);
// Right now we only capture frontend sessions for Next.js
options.autoSessionTracking = false;
Expand Down Expand Up @@ -47,5 +48,5 @@ function addServerIntegrations(options: NextjsOptions): void {
export { withSentryConfig } from './utils/config';
export { withSentry } from './utils/handlers';

// TODO capture project root (which this returns) for RewriteFrames?
// wrap various server methods to enable error monitoring and tracing
instrumentServer();
115 changes: 115 additions & 0 deletions packages/nextjs/src/performance/client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import { Primitive, Transaction, TransactionContext } from '@sentry/types';
import { fill, getGlobalObject } from '@sentry/utils';
import { default as Router } from 'next/router';

const global = getGlobalObject<Window>();

type StartTransactionCb = (context: TransactionContext) => Transaction | undefined;

const DEFAULT_TAGS = Object.freeze({
'routing.instrumentation': 'next-router',
});

const QUERY_PARAM_REGEX = /\?(.*)/;

let activeTransaction: Transaction | undefined = undefined;
let prevTransactionName: string | undefined = undefined;
let startTransaction: StartTransactionCb | undefined = undefined;

/**
* Creates routing instrumention for Next Router. Only supported for
* client side routing. Works for Next >= 10.
*
* Leverages the SingletonRouter from the `next/router` to
* generate pageload/navigation transactions and parameterize
* transaction names.
*/
export function nextRouterInstrumentation(
startTransactionCb: StartTransactionCb,
startTransactionOnPageLoad: boolean = true,
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) {
prevTransactionName = Router.route !== null ? removeQueryParams(Router.route) : global.location.pathname;
activeTransaction = startTransactionCb({
name: prevTransactionName,
op: 'pageload',
tags: DEFAULT_TAGS,
});
}

// 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;

// `withRouter` uses `useRouter` underneath:
// https://github.com/vercel/next.js/blob/de42719619ae69fbd88e445100f15701f6e1e100/packages/next/client/with-router.tsx#L21
// Router events also use the router:
// https://github.com/vercel/next.js/blob/de42719619ae69fbd88e445100f15701f6e1e100/packages/next/client/router.ts#L92
// `Router.changeState` handles the router state changes, so it may be enough to only wrap it
// (instead of wrapping all of the Router's functions).
const routerPrototype = Object.getPrototypeOf(Router.router);
fill(routerPrototype, 'changeState', changeStateWrapper);
});
}

type RouterChangeState = (
method: string,
url: string,
as: string,
options: Record<string, any>,
...args: any[]
) => void;
type WrappedRouterChangeState = RouterChangeState;

/**
* Wraps Router.changeState()
* https://github.com/vercel/next.js/blob/da97a18dafc7799e63aa7985adc95f213c2bf5f3/packages/next/next-server/lib/router/router.ts#L1204
* Start a navigation transaction every time the router changes state.
*/
function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): WrappedRouterChangeState {
const wrapper = function(
this: any,
method: string,
// The parameterized url, ex. posts/[id]/[comment]
url: string,
// The actual url, ex. posts/85/my-comment
as: string,
options: Record<string, any>,
// At the moment there are no additional arguments (meaning the rest parameter is empty).
// This is meant to protect from future additions to Next.js API, especially since this is an
// internal API.
...args: any[]
): Promise<boolean> {
if (startTransaction !== undefined) {
if (activeTransaction) {
activeTransaction.finish();
}
const tags: Record<string, Primitive> = {
...DEFAULT_TAGS,
method,
...options,
};
if (prevTransactionName) {
tags.from = prevTransactionName;
}
prevTransactionName = removeQueryParams(url);
activeTransaction = startTransaction({
name: prevTransactionName,
op: 'navigation',
tags,
});
}
return originalChangeStateWrapper.call(this, method, url, as, options, ...args);
};
return wrapper;
}

export function removeQueryParams(route: string): string {
return route.replace(QUERY_PARAM_REGEX, '');
}
Loading