Skip to content

Commit f24fc68

Browse files
author
Luca Forstner
authored
ref(nextjs): Make individual features opt into OTEL tracing (#13910)
1 parent 92c0117 commit f24fc68

File tree

17 files changed

+71
-152
lines changed

17 files changed

+71
-152
lines changed

dev-packages/e2e-tests/publish-packages.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,19 @@ const packageTarballPaths = glob.sync('packages/*/sentry-*.tgz', {
1212

1313
// Publish built packages to the fake registry
1414
packageTarballPaths.forEach(tarballPath => {
15+
// eslint-disable-next-line no-console
16+
console.log(`Publishing tarball ${tarballPath} ...`);
1517
// `--userconfig` flag needs to be before `publish`
1618
childProcess.exec(
1719
`npm --userconfig ${__dirname}/test-registry.npmrc publish ${tarballPath}`,
1820
{
1921
cwd: repositoryRoot, // Can't use __dirname here because npm would try to publish `@sentry-internal/e2e-tests`
2022
encoding: 'utf8',
2123
},
22-
(err, stdout, stderr) => {
23-
// eslint-disable-next-line no-console
24-
console.log(stdout);
25-
// eslint-disable-next-line no-console
26-
console.log(stderr);
24+
err => {
2725
if (err) {
2826
// eslint-disable-next-line no-console
29-
console.error(err);
27+
console.error(`Error publishing tarball ${tarballPath}`, err);
3028
process.exit(1);
3129
}
3230
},

dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/excluded-api-endpoints.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { expect, test } from '@playwright/test';
22
import { waitForTransaction } from '@sentry-internal/test-utils';
33

4-
test('should not automatically create transactions for routes that were excluded from auto wrapping (string)', async ({
4+
// TODO(lforst): This cannot make it into production - Make sure to fix this test
5+
// The problem is that if we are not applying build time instrumentation Next.js will still emit spans (which is fine, but we need to find a different way of testing that build time instrumentation is successfully disabled - maybe with an attribute or something if build-time instrumentation is applied)
6+
test.skip('should not automatically create transactions for routes that were excluded from auto wrapping (string)', async ({
57
request,
68
}) => {
79
const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
@@ -23,7 +25,9 @@ test('should not automatically create transactions for routes that were excluded
2325
expect(transactionPromiseReceived).toBe(false);
2426
});
2527

26-
test('should not automatically create transactions for routes that were excluded from auto wrapping (regex)', async ({
28+
// TODO(lforst): This cannot make it into production - Make sure to fix this test
29+
// The problem is that if we are not applying build time instrumentation Next.js will still emit spans (which is fine, but we need to find a different way of testing that build time instrumentation is successfully disabled - maybe with an attribute or something if build-time instrumentation is applied)
30+
test.skip('should not automatically create transactions for routes that were excluded from auto wrapping (regex)', async ({
2731
request,
2832
}) => {
2933
const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {

dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ export default function Layout({ children }: { children: React.ReactNode }) {
2727
<Link href="/server-component/parameter/42">/server-component/parameter/42</Link>
2828
</li>
2929
<li>
30-
<Link href="/server-component/parameter/foo/bar/baz">/server-component/parameter/foo/bar/baz</Link>
30+
<Link href="/server-component/parameter/foo/bar/baz" prefetch={false}>
31+
/server-component/parameter/foo/bar/baz
32+
</Link>
3133
</li>
3234
<li>
3335
<Link href="/not-found">/not-found</Link>

packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,17 @@ import {
33
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
44
captureException,
55
continueTrace,
6-
getActiveSpan,
7-
getRootSpan,
86
setHttpStatus,
97
startSpanManual,
108
withIsolationScope,
119
} from '@sentry/core';
12-
import { isString, logger, objectify, vercelWaitUntil } from '@sentry/utils';
10+
import { isString, logger, objectify } from '@sentry/utils';
11+
12+
import { vercelWaitUntil } from '@sentry/utils';
1313
import type { NextApiRequest } from 'next';
14-
import { TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION } from '../span-attributes-with-logic-attached';
1514
import type { AugmentedNextApiResponse, NextApiHandler } from '../types';
1615
import { flushSafelyWithTimeout } from '../utils/responseEnd';
17-
import { escapeNextjsTracing } from '../utils/tracingUtils';
16+
import { dropNextjsRootContext, escapeNextjsTracing } from '../utils/tracingUtils';
1817

1918
export type AugmentedNextApiRequest = NextApiRequest & {
2019
__withSentry_applied__?: boolean;
@@ -29,21 +28,13 @@ export type AugmentedNextApiRequest = NextApiRequest & {
2928
* @returns The wrapped handler which will always return a Promise.
3029
*/
3130
export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameterizedRoute: string): NextApiHandler {
32-
// Since the API route handler spans emitted by Next.js are super buggy with completely wrong timestamps
33-
// (fix pending at the time of writing this: https://github.com/vercel/next.js/pull/70908) we want to intentionally
34-
// drop them. In the future, when Next.js' OTEL instrumentation is in a high-quality place we can potentially think
35-
// about keeping them.
36-
const nextJsOwnedSpan = getActiveSpan();
37-
if (nextJsOwnedSpan) {
38-
getRootSpan(nextJsOwnedSpan)?.setAttribute(TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION, true);
39-
}
40-
4131
return new Proxy(apiHandler, {
4232
apply: (
4333
wrappingTarget,
4434
thisArg,
4535
args: [AugmentedNextApiRequest | undefined, AugmentedNextApiResponse | undefined],
4636
) => {
37+
dropNextjsRootContext();
4738
return escapeNextjsTracing(() => {
4839
const [req, res] = args;
4940

packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentryVercelCrons.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ type EdgeRequest = {
99
};
1010

1111
/**
12-
* Wraps a function with Sentry crons instrumentation by automaticaly sending check-ins for the given Vercel crons config.
12+
* Wraps a function with Sentry crons instrumentation by automatically sending check-ins for the given Vercel crons config.
1313
*/
1414
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1515
export function wrapApiHandlerWithSentryVercelCrons<F extends (...args: any[]) => any>(

packages/nextjs/src/common/pages-router-instrumentation/wrapPageComponentWithSentry.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { captureException, getCurrentScope, withIsolationScope } from '@sentry/core';
22
import { extractTraceparentData } from '@sentry/utils';
3-
import { escapeNextjsTracing } from '../utils/tracingUtils';
3+
import { dropNextjsRootContext, escapeNextjsTracing } from '../utils/tracingUtils';
44

55
interface FunctionComponent {
66
(...args: unknown[]): unknown;
@@ -25,6 +25,7 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C
2525
if (isReactClassComponent(pageComponent)) {
2626
return class SentryWrappedPageComponent extends pageComponent {
2727
public render(...args: unknown[]): unknown {
28+
dropNextjsRootContext();
2829
return escapeNextjsTracing(() => {
2930
return withIsolationScope(() => {
3031
const scope = getCurrentScope();
@@ -62,6 +63,7 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C
6263
} else if (typeof pageComponent === 'function') {
6364
return new Proxy(pageComponent, {
6465
apply(target, thisArg, argArray: [{ _sentryTraceData?: string } | undefined]) {
66+
dropNextjsRootContext();
6567
return escapeNextjsTracing(() => {
6668
return withIsolationScope(() => {
6769
const scope = getCurrentScope();

packages/nextjs/src/common/utils/tracingUtils.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { Scope, startNewTrace } from '@sentry/core';
1+
import { Scope, getActiveSpan, getRootSpan, spanToJSON, startNewTrace } from '@sentry/core';
22
import type { PropagationContext } from '@sentry/types';
33
import { GLOBAL_OBJ, logger } from '@sentry/utils';
44
import { DEBUG_BUILD } from '../debug-build';
5+
import { TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION } from '../span-attributes-with-logic-attached';
56

67
const commonPropagationContextMap = new WeakMap<object, PropagationContext>();
78

@@ -92,3 +93,19 @@ export function escapeNextjsTracing<T>(cb: () => T): T {
9293
});
9394
}
9495
}
96+
97+
/**
98+
* Ideally this function never lands in the develop branch.
99+
*
100+
* Drops the entire span tree this function was called in, if it was a span tree created by Next.js.
101+
*/
102+
export function dropNextjsRootContext(): void {
103+
const nextJsOwnedSpan = getActiveSpan();
104+
if (nextJsOwnedSpan) {
105+
const rootSpan = getRootSpan(nextJsOwnedSpan);
106+
const rootSpanAttributes = spanToJSON(rootSpan).data;
107+
if (rootSpanAttributes?.['next.span_type']) {
108+
getRootSpan(nextJsOwnedSpan)?.setAttribute(TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION, true);
109+
}
110+
}
111+
}

packages/nextjs/src/common/utils/wrapperUtils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import type { Span } from '@sentry/types';
1717
import { isString, vercelWaitUntil } from '@sentry/utils';
1818

1919
import { autoEndSpanOnResponseEnd, flushSafelyWithTimeout } from './responseEnd';
20-
import { commonObjectToIsolationScope, escapeNextjsTracing } from './tracingUtils';
20+
import { commonObjectToIsolationScope, dropNextjsRootContext, escapeNextjsTracing } from './tracingUtils';
2121

2222
declare module 'http' {
2323
interface IncomingMessage {
@@ -93,6 +93,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
9393
this: unknown,
9494
...args: Parameters<F>
9595
): Promise<{ data: ReturnType<F>; sentryTrace?: string; baggage?: string }> {
96+
dropNextjsRootContext();
9697
return escapeNextjsTracing(() => {
9798
const isolationScope = commonObjectToIsolationScope(req);
9899
return withIsolationScope(isolationScope, () => {

packages/nextjs/src/common/withServerActionInstrumentation.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
import {
22
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
33
SPAN_STATUS_ERROR,
4+
captureException,
5+
continueTrace,
6+
getClient,
47
getIsolationScope,
8+
handleCallbackErrors,
9+
startSpan,
510
withIsolationScope,
611
} from '@sentry/core';
7-
import { captureException, continueTrace, getClient, handleCallbackErrors, startSpan } from '@sentry/core';
812
import { logger, vercelWaitUntil } from '@sentry/utils';
913

1014
import { DEBUG_BUILD } from './debug-build';
1115
import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils';
1216
import { flushSafelyWithTimeout } from './utils/responseEnd';
13-
import { escapeNextjsTracing } from './utils/tracingUtils';
17+
import { dropNextjsRootContext, escapeNextjsTracing } from './utils/tracingUtils';
1418

1519
interface Options {
1620
formData?: FormData;
@@ -64,6 +68,7 @@ async function withServerActionInstrumentationImplementation<A extends (...args:
6468
options: Options,
6569
callback: A,
6670
): Promise<ReturnType<A>> {
71+
dropNextjsRootContext();
6772
return escapeNextjsTracing(() => {
6873
return withIsolationScope(async isolationScope => {
6974
const sendDefaultPii = getClient()?.getOptions().sendDefaultPii;

packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
5050
const rootSpan = getRootSpan(activeSpan);
5151
const { scope } = getCapturedScopesOnSpan(rootSpan);
5252
setCapturedScopesOnSpan(rootSpan, scope ?? new Scope(), isolationScope);
53-
54-
// We mark the root span as an app router span so we can allow-list it in our span processor that would normally filter out all Next.js transactions/spans
55-
rootSpan.setAttribute('sentry.rsc', true);
5653
}
5754

5855
let data: Record<string, unknown> | undefined = undefined;

0 commit comments

Comments
 (0)