Skip to content

Commit 92bc6c6

Browse files
author
Luca Forstner
authored
feat(nexjs): Sample out low-quality spans on older Next.js versions (#11722)
1 parent edfea16 commit 92bc6c6

13 files changed

+147
-51
lines changed

packages/core/src/baseclient.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import type {
2222
SessionAggregates,
2323
SeverityLevel,
2424
Span,
25+
SpanAttributes,
26+
SpanContextData,
2527
StartSpanOptions,
2628
TransactionEvent,
2729
Transport,
@@ -416,6 +418,20 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
416418
callback: (feedback: FeedbackEvent, options?: { includeReplay: boolean }) => void,
417419
): void;
418420

421+
/** @inheritdoc */
422+
public on(
423+
hook: 'beforeSampling',
424+
callback: (
425+
samplingData: {
426+
spanAttributes: SpanAttributes;
427+
spanName: string;
428+
parentSampled?: boolean;
429+
parentContext?: SpanContextData;
430+
},
431+
samplingDecision: { decision: boolean },
432+
) => void,
433+
): void;
434+
419435
/** @inheritdoc */
420436
public on(
421437
hook: 'startPageLoadSpan',
@@ -442,6 +458,18 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
442458
this._hooks[hook].push(callback);
443459
}
444460

461+
/** @inheritdoc */
462+
public emit(
463+
hook: 'beforeSampling',
464+
samplingData: {
465+
spanAttributes: SpanAttributes;
466+
spanName: string;
467+
parentSampled?: boolean;
468+
parentContext?: SpanContextData;
469+
},
470+
samplingDecision: { decision: boolean },
471+
): void;
472+
445473
/** @inheritdoc */
446474
public emit(hook: 'spanStart', span: Span): void;
447475

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

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { isString } from '@sentry/utils';
1717

1818
import { platformSupportsStreaming } from './platformSupportsStreaming';
1919
import { autoEndSpanOnResponseEnd, flushQueue } from './responseEnd';
20-
import { commonObjectToIsolationScope } from './tracingUtils';
20+
import { commonObjectToIsolationScope, escapeNextjsTracing } from './tracingUtils';
2121

2222
declare module 'http' {
2323
interface IncomingMessage {
@@ -90,44 +90,46 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
9090
},
9191
): (...params: Parameters<F>) => Promise<ReturnType<F>> {
9292
return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
93-
const isolationScope = commonObjectToIsolationScope(req);
94-
return withIsolationScope(isolationScope, () => {
95-
isolationScope.setSDKProcessingMetadata({
96-
request: req,
97-
});
93+
return escapeNextjsTracing(() => {
94+
const isolationScope = commonObjectToIsolationScope(req);
95+
return withIsolationScope(isolationScope, () => {
96+
isolationScope.setSDKProcessingMetadata({
97+
request: req,
98+
});
9899

99-
const sentryTrace =
100-
req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined;
101-
const baggage = req.headers?.baggage;
102-
103-
return continueTrace({ sentryTrace, baggage }, () => {
104-
const requestSpan = getOrStartRequestSpan(req, res, options.requestedRouteName);
105-
return withActiveSpan(requestSpan, () => {
106-
return startSpanManual(
107-
{
108-
op: 'function.nextjs',
109-
name: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`,
110-
attributes: {
111-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
112-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
100+
const sentryTrace =
101+
req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined;
102+
const baggage = req.headers?.baggage;
103+
104+
return continueTrace({ sentryTrace, baggage }, () => {
105+
const requestSpan = getOrStartRequestSpan(req, res, options.requestedRouteName);
106+
return withActiveSpan(requestSpan, () => {
107+
return startSpanManual(
108+
{
109+
op: 'function.nextjs',
110+
name: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`,
111+
attributes: {
112+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
113+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
114+
},
113115
},
114-
},
115-
async dataFetcherSpan => {
116-
dataFetcherSpan.setStatus({ code: SPAN_STATUS_OK });
117-
try {
118-
return await origDataFetcher.apply(this, args);
119-
} catch (e) {
120-
dataFetcherSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
121-
requestSpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
122-
throw e;
123-
} finally {
124-
dataFetcherSpan.end();
125-
if (!platformSupportsStreaming()) {
126-
await flushQueue();
116+
async dataFetcherSpan => {
117+
dataFetcherSpan.setStatus({ code: SPAN_STATUS_OK });
118+
try {
119+
return await origDataFetcher.apply(this, args);
120+
} catch (e) {
121+
dataFetcherSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
122+
requestSpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
123+
throw e;
124+
} finally {
125+
dataFetcherSpan.end();
126+
if (!platformSupportsStreaming()) {
127+
await flushQueue();
128+
}
127129
}
128-
}
129-
},
130-
);
130+
},
131+
);
132+
});
131133
});
132134
});
133135
});

packages/nextjs/src/server/index.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,22 @@ const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
2222
__sentryRewritesTunnelPath__?: string;
2323
};
2424

25+
// https://github.com/lforst/nextjs-fork/blob/9051bc44d969a6e0ab65a955a2fc0af522a83911/packages/next/src/server/lib/trace/constants.ts#L11
26+
const NEXTJS_SPAN_NAME_PREFIXES = [
27+
'BaseServer.',
28+
'LoadComponents.',
29+
'NextServer.',
30+
'createServer.',
31+
'startServer.',
32+
'NextNodeServer.',
33+
'Render.',
34+
'AppRender.',
35+
'Router.',
36+
'Node.',
37+
'AppRouteRouteHandlers.',
38+
'ResolveMetadata.',
39+
];
40+
2541
/**
2642
* A passthrough error boundary for the server that doesn't depend on any react. Error boundaries don't catch SSR errors
2743
* so they should simply be a passthrough.
@@ -90,7 +106,7 @@ export function init(options: NodeOptions): void {
90106
customDefaultIntegrations.push(distDirRewriteFramesIntegration({ distDirName }));
91107
}
92108

93-
const opts = {
109+
const opts: NodeOptions = {
94110
environment: process.env.SENTRY_ENVIRONMENT || getVercelEnv(false) || process.env.NODE_ENV,
95111
defaultIntegrations: customDefaultIntegrations,
96112
...options,
@@ -113,6 +129,20 @@ export function init(options: NodeOptions): void {
113129

114130
nodeInit(opts);
115131

132+
const client = getClient();
133+
client?.on('beforeSampling', ({ spanAttributes, spanName, parentSampled, parentContext }, samplingDecision) => {
134+
// If we encounter a span emitted by Next.js, we do not want to sample it
135+
// The reason for this is that the data quality of the spans varies, it is different per version of Next,
136+
// and we need to keep our manual instrumentation around for the edge runtime anyhow.
137+
// BUT we only do this if we don't have a parent span with a sampling decision yet (or if the parent is remote)
138+
if (
139+
(spanAttributes['next.span_type'] || NEXTJS_SPAN_NAME_PREFIXES.some(prefix => spanName.startsWith(prefix))) &&
140+
(parentSampled === undefined || parentContext?.isRemote)
141+
) {
142+
samplingDecision.decision = false;
143+
}
144+
});
145+
116146
addEventProcessor(
117147
Object.assign(
118148
(event => {

packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ describe('Error API Endpoints', () => {
3434
const envelopes = await env.getMultipleEnvelopeRequest({
3535
url,
3636
envelopeType: 'transaction',
37-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
37+
count: 1,
3838
});
3939

4040
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('Error Server-side Props', () => {
3333
const envelopes = await env.getMultipleEnvelopeRequest({
3434
url,
3535
envelopeType: 'transaction',
36-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
36+
count: 1,
3737
});
3838

3939
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/nextjs/test/integration/test/server/tracing200.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('Tracing 200', () => {
88
const envelopes = await env.getMultipleEnvelopeRequest({
99
url,
1010
envelopeType: 'transaction',
11-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
11+
count: 1,
1212
});
1313

1414
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/nextjs/test/integration/test/server/tracing500.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('Tracing 500', () => {
88
const envelopes = await env.getMultipleEnvelopeRequest({
99
url,
1010
envelopeType: 'transaction',
11-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
11+
count: 1,
1212
});
1313

1414
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/nextjs/test/integration/test/server/tracingHttp.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('Tracing HTTP', () => {
88
const envelopes = await env.getMultipleEnvelopeRequest({
99
url,
1010
envelopeType: 'transaction',
11-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
11+
count: 1,
1212
});
1313

1414
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('getInitialProps', () => {
88
const envelopes = await env.getMultipleEnvelopeRequest({
99
url,
1010
envelopeType: 'transaction',
11-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
11+
count: 1,
1212
});
1313

1414
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('getServerSideProps', () => {
88
const envelopes = await env.getMultipleEnvelopeRequest({
99
url,
1010
envelopeType: 'transaction',
11-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
11+
count: 1,
1212
});
1313

1414
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('tracingServerGetServerSidePropsCustomPageExtension', () => {
88
const envelopes = await env.getMultipleEnvelopeRequest({
99
url,
1010
envelopeType: 'transaction',
11-
count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
11+
count: 1,
1212
});
1313

1414
const sentryTransactionEnvelope = envelopes.find(envelope => {

packages/opentelemetry/src/sampler.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,18 @@ export class SentrySampler implements Sampler {
6464

6565
const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined;
6666

67-
// If we encounter a span emitted by Next.js, we do not want to sample it
68-
// The reason for this is that the data quality of the spans varies, it is different per version of Next,
69-
// and we need to keep our manual instrumentation around for the edge runtime anyhow.
70-
// BUT we only do this if we don't have a parent span with a sampling decision yet (or if the parent is remote)
71-
if (spanAttributes['next.span_type'] && (typeof parentSampled !== 'boolean' || parentContext?.isRemote)) {
67+
const mutableSamplingDecision = { decision: true };
68+
this._client.emit(
69+
'beforeSampling',
70+
{
71+
spanAttributes: spanAttributes,
72+
spanName: spanName,
73+
parentSampled: parentSampled,
74+
parentContext: parentContext,
75+
},
76+
mutableSamplingDecision,
77+
);
78+
if (!mutableSamplingDecision.decision) {
7279
return { decision: SamplingDecision.NOT_RECORD, traceState: traceState };
7380
}
7481

packages/types/src/client.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import type { Scope } from './scope';
1414
import type { SdkMetadata } from './sdkmetadata';
1515
import type { Session, SessionAggregates } from './session';
1616
import type { SeverityLevel } from './severity';
17-
import type { Span } from './span';
17+
import type { Span, SpanAttributes, SpanContextData } from './span';
1818
import type { StartSpanOptions } from './startSpanOptions';
1919
import type { Transport, TransportMakeRequestResponse } from './transport';
2020

@@ -178,6 +178,23 @@ export interface Client<O extends ClientOptions = ClientOptions> {
178178
*/
179179
on(hook: 'spanStart', callback: (span: Span) => void): void;
180180

181+
/**
182+
* Register a callback before span sampling runs. Receives a `samplingDecision` object argument with a `decision`
183+
* property that can be used to make a sampling decision that will be enforced, before any span sampling runs.
184+
*/
185+
on(
186+
hook: 'beforeSampling',
187+
callback: (
188+
samplingData: {
189+
spanAttributes: SpanAttributes;
190+
spanName: string;
191+
parentSampled?: boolean;
192+
parentContext?: SpanContextData;
193+
},
194+
samplingDecision: { decision: boolean },
195+
) => void,
196+
): void;
197+
181198
/**
182199
* Register a callback for whenever a span is ended.
183200
* Receives the span as argument.
@@ -262,6 +279,18 @@ export interface Client<O extends ClientOptions = ClientOptions> {
262279
/** Fire a hook whener a span starts. */
263280
emit(hook: 'spanStart', span: Span): void;
264281

282+
/** A hook that is called every time before a span is sampled. */
283+
emit(
284+
hook: 'beforeSampling',
285+
samplingData: {
286+
spanAttributes: SpanAttributes;
287+
spanName: string;
288+
parentSampled?: boolean;
289+
parentContext?: SpanContextData;
290+
},
291+
samplingDecision: { decision: boolean },
292+
): void;
293+
265294
/** Fire a hook whener a span ends. */
266295
emit(hook: 'spanEnd', span: Span): void;
267296

0 commit comments

Comments
 (0)