Skip to content

Commit a8531b5

Browse files
mydeamydea
committed
WIP: skip next emitted spans??
1 parent 39c8290 commit a8531b5

File tree

2 files changed

+65
-85
lines changed

2 files changed

+65
-85
lines changed

‎packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts

Lines changed: 36 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@ import {
44
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
55
SPAN_STATUS_ERROR,
66
captureException,
7-
getActiveSpan,
8-
getRootSpan,
97
handleCallbackErrors,
108
setHttpStatus,
11-
spanToJSON,
129
startSpan,
1310
} from '@sentry/core';
1411
import type { Span } from '@sentry/types';
@@ -19,49 +16,6 @@ import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
1916
import { flushQueue } from './utils/responseEnd';
2017
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';
2118

22-
/** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js.
23-
* In case there is no root span, we start a new span. */
24-
function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise<Response>): Promise<Response> {
25-
const activeSpan = getActiveSpan();
26-
const rootSpan = activeSpan && getRootSpan(activeSpan);
27-
28-
// We have different possible scenarios here:
29-
// 1. If we have no root span, we just create a new span
30-
// 2. We have a root span that that we want to update here
31-
// 3. We have a root span that was already updated (e.g. if this is a nested call)
32-
33-
const attributes = {
34-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
35-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
36-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
37-
} as const;
38-
39-
if (!rootSpan) {
40-
return startSpan(
41-
{
42-
name: spanName,
43-
forceTransaction: true,
44-
attributes,
45-
},
46-
cb,
47-
);
48-
}
49-
50-
// If `op` is set, we assume this was already processed before
51-
// Probably this is a nested call, no need to update anything anymore
52-
// OR, if we don't have next.span_type, we don't know where this comes from and don't want to mess with it
53-
const existingAttributes = spanToJSON(rootSpan).data || {};
54-
if (existingAttributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !existingAttributes['next.span_type']) {
55-
return cb(rootSpan);
56-
}
57-
58-
// Finally, we want to update the root span, as the ones generated by next are often not good enough for us
59-
rootSpan.updateName(spanName);
60-
rootSpan.setAttributes(attributes);
61-
62-
return cb(rootSpan);
63-
}
64-
6519
/**
6620
* Wraps a Next.js route handler with performance and error instrumentation.
6721
*/
@@ -82,35 +36,46 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
8236
});
8337

8438
try {
85-
return await startOrUpdateSpan(`${method} ${parameterizedRoute}`, async (rootSpan: Span) => {
86-
const response: Response = await handleCallbackErrors(
87-
() => originalFunction.apply(thisArg, args),
88-
error => {
89-
// Next.js throws errors when calling `redirect()`. We don't wanna report these.
90-
if (isRedirectNavigationError(error)) {
91-
// Don't do anything
92-
} else if (isNotFoundNavigationError(error) && rootSpan) {
93-
rootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' });
94-
} else {
95-
captureException(error, {
96-
mechanism: {
97-
handled: false,
98-
},
99-
});
100-
}
39+
return await startSpan(
40+
{
41+
name: `${method} ${parameterizedRoute}`,
42+
forceTransaction: true,
43+
attributes: {
44+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
45+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
46+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
10147
},
102-
);
48+
},
49+
async (span: Span) => {
50+
const response: Response = await handleCallbackErrors(
51+
() => originalFunction.apply(thisArg, args),
52+
error => {
53+
// Next.js throws errors when calling `redirect()`. We don't wanna report these.
54+
if (isRedirectNavigationError(error)) {
55+
// Don't do anything
56+
} else if (isNotFoundNavigationError(error)) {
57+
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' });
58+
} else {
59+
captureException(error, {
60+
mechanism: {
61+
handled: false,
62+
},
63+
});
64+
}
65+
},
66+
);
10367

104-
try {
105-
if (rootSpan && response.status) {
106-
setHttpStatus(rootSpan, response.status);
68+
try {
69+
if (response.status) {
70+
setHttpStatus(span, response.status);
71+
}
72+
} catch {
73+
// best effort - response may be undefined?
10774
}
108-
} catch {
109-
// best effort - response may be undefined?
110-
}
11175

112-
return response;
113-
});
76+
return response;
77+
},
78+
);
11479
} finally {
11580
if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') {
11681
// 1. Edge transport requires manual flushing

‎packages/opentelemetry/src/sampler.ts

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ export class SentrySampler implements Sampler {
5252
return { decision: SamplingDecision.NOT_RECORD, traceState };
5353
}
5454

55+
// If we encounter a span emitted by Next.js, we do not want to sample it
56+
// The reason for this is that the data quality of the spans varies, it is different per version of Next,
57+
// and we need to keep our manual instrumentation around for the edge runtime anyhow.
58+
if (spanAttributes['next.span_type']) {
59+
return { decision: SamplingDecision.NOT_RECORD, traceState: traceState };
60+
}
61+
5562
// If we have a http.client span that has no local parent, we never want to sample it
5663
// but we want to leave downstream sampling decisions up to the server
5764
if (
@@ -62,20 +69,7 @@ export class SentrySampler implements Sampler {
6269
return { decision: SamplingDecision.NOT_RECORD, traceState };
6370
}
6471

65-
let parentSampled: boolean | undefined = undefined;
66-
67-
// Only inherit sample rate if `traceId` is the same
68-
// Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones
69-
if (parentSpan && parentContext && isSpanContextValid(parentContext) && parentContext.traceId === traceId) {
70-
if (parentContext.isRemote) {
71-
parentSampled = getParentRemoteSampled(parentSpan);
72-
DEBUG_BUILD &&
73-
logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`);
74-
} else {
75-
parentSampled = getSamplingDecision(parentContext);
76-
DEBUG_BUILD && logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`);
77-
}
78-
}
72+
const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined;
7973

8074
const [sampled, sampleRate] = sampleSpan(options, {
8175
name: spanName,
@@ -129,3 +123,24 @@ function getParentRemoteSampled(parentSpan: Span): boolean | undefined {
129123
// Only inherit sampled if `traceId` is the same
130124
return traceparentData && traceId === traceparentData.traceId ? traceparentData.sampled : undefined;
131125
}
126+
127+
function getParentSampled(parentSpan: Span, traceId: string, spanName: string): boolean | undefined {
128+
const parentContext = parentSpan.spanContext();
129+
130+
// Only inherit sample rate if `traceId` is the same
131+
// Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones
132+
if (isSpanContextValid(parentContext) && parentContext.traceId === traceId) {
133+
if (parentContext.isRemote) {
134+
const parentSampled = getParentRemoteSampled(parentSpan);
135+
DEBUG_BUILD &&
136+
logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`);
137+
return parentSampled;
138+
}
139+
140+
const parentSampled = getSamplingDecision(parentContext);
141+
DEBUG_BUILD && logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`);
142+
return parentSampled;
143+
}
144+
145+
return undefined;
146+
}

0 commit comments

Comments
 (0)