Skip to content

fix(nextjs): Remove Http integration from Next.js #11304

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 11 commits into from
Apr 9, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,15 @@ test('Should send a transaction with a fetch span', async ({ page }) => {
}),
);

// TODO: Uncomment the below when fixed. For whatever reason that we now have accepted, spans created with Node.js' http.get() will not attach themselves to transactions.
// More info: https://github.com/getsentry/sentry-javascript/pull/11016/files#diff-10fa195789425786c6e5e769380be18790768f0b76319ee41bbb488d9fe50405R4
// expect((await transactionPromise).spans).toContainEqual(
// expect.objectContaining({
// data: expect.objectContaining({
// 'http.method': 'GET',
// 'sentry.op': 'http.client',
// 'sentry.origin': 'auto.http.otel.http',
// }),
// description: 'GET http://example.com/',
// }),
// );
expect((await transactionPromise).spans).toContainEqual(
expect.objectContaining({
data: expect.objectContaining({
'http.method': 'GET',
'sentry.op': 'http.client',
// todo: without the HTTP integration in the Next.js SDK, this is set to 'manual' -> we could rename this to be more specific
'sentry.origin': 'manual',
}),
description: 'GET http://example.com/',
}),
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ test('Should record exceptions and transactions for faulty route handlers', asyn

expect(routehandlerTransaction.contexts?.trace?.status).toBe('unknown_error');
expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server');
expect(routehandlerTransaction.contexts?.trace?.origin).toBe('auto.function.nextjs');

expect(routehandlerError.exception?.values?.[0].value).toBe('route-handler-error');
// TODO: Uncomment once we update the scope transaction name on the server side
Expand Down
2 changes: 2 additions & 0 deletions packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@
"access": "public"
},
"dependencies": {
"@opentelemetry/api": "1.7.0",
"@rollup/plugin-commonjs": "24.0.0",
"@sentry/core": "8.0.0-alpha.9",
"@sentry/node": "8.0.0-alpha.9",
"@sentry/react": "8.0.0-alpha.9",
"@sentry/types": "8.0.0-alpha.9",
"@sentry/utils": "8.0.0-alpha.9",
"@sentry/opentelemetry": "8.0.0-alpha.9",
"@sentry/vercel-edge": "8.0.0-alpha.9",
"@sentry/webpack-plugin": "2.16.0",
"chalk": "3.0.0",
Expand Down
129 changes: 74 additions & 55 deletions packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,58 @@
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
SPAN_STATUS_ERROR,
addTracingExtensions,
captureException,
continueTrace,
getActiveSpan,
getRootSpan,
handleCallbackErrors,
setHttpStatus,
startSpan,
} from '@sentry/core';
import type { Span } from '@sentry/types';
import { winterCGHeadersToDict } from '@sentry/utils';

import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils';
import type { RouteHandlerContext } from './types';
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
import { flushQueue } from './utils/responseEnd';
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';

/** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js.
* In case there is no root span, we start a new span. */
function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise<Response>): Promise<Response> {
const activeSpan = getActiveSpan();
const rootSpan = activeSpan && getRootSpan(activeSpan);

if (rootSpan) {
rootSpan.updateName(spanName);
rootSpan.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
});

return cb(rootSpan);
} else {
return startSpan(
{
op: 'http.server',
name: spanName,
forceTransaction: true,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
},
},
(span: Span) => {
return cb(span);
},
);
}
}

/**
* Wraps a Next.js route handler with performance and error instrumentation.
*/
Expand All @@ -26,7 +62,9 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
context: RouteHandlerContext,
): (...args: Parameters<F>) => ReturnType<F> extends Promise<unknown> ? ReturnType<F> : Promise<ReturnType<F>> {
addTracingExtensions();

const { method, parameterizedRoute, headers } = context;

return new Proxy(routeHandler, {
apply: (originalFunction, thisArg, args) => {
return withIsolationScopeOrReuseFromRootSpan(async isolationScope => {
Expand All @@ -35,63 +73,44 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
headers: headers ? winterCGHeadersToDict(headers) : undefined,
},
});
return continueTrace(
{
// TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here
sentryTrace: headers?.get('sentry-trace') ?? undefined,
baggage: headers?.get('baggage'),
},
async () => {
try {
return await startSpan(
{
op: 'http.server',
name: `${method} ${parameterizedRoute}`,
forceTransaction: true,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
},
},
async span => {
const response: Response = await handleCallbackErrors(
() => originalFunction.apply(thisArg, args),
error => {
// Next.js throws errors when calling `redirect()`. We don't wanna report these.
if (isRedirectNavigationError(error)) {
// Don't do anything
} else if (isNotFoundNavigationError(error)) {
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' });
} else {
captureException(error, {
mechanism: {
handled: false,
},
});
}
},
);

try {
if (span && response.status) {
setHttpStatus(span, response.status);
}
} catch {
// best effort - response may be undefined?
}
try {
return await startOrUpdateSpan(`${method} ${parameterizedRoute}`, async (rootSpan: Span) => {
const response: Response = await handleCallbackErrors(
() => originalFunction.apply(thisArg, args),
error => {
// Next.js throws errors when calling `redirect()`. We don't wanna report these.
if (isRedirectNavigationError(error)) {
// Don't do anything
} else if (isNotFoundNavigationError(error) && rootSpan) {
rootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' });
} else {
captureException(error, {
mechanism: {
handled: false,
},
});
}
},
);

return response;
},
);
} finally {
if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') {
// 1. Edge transport requires manual flushing
// 2. Lambdas require manual flushing to prevent execution freeze before the event is sent
await flushQueue();
try {
if (rootSpan && response.status) {
setHttpStatus(rootSpan, response.status);
}
} catch {
// best effort - response may be undefined?
}
},
);

return response;
});
} finally {
if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') {
// 1. Edge transport requires manual flushing
// 2. Lambdas require manual flushing to prevent execution freeze before the event is sent
await flushQueue();
}
}
});
},
});
Expand Down
8 changes: 6 additions & 2 deletions packages/nextjs/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration

export * from '@sentry/node';
import type { EventProcessor } from '@sentry/types';
import { requestIsolationScopeIntegration } from './requestIsolationScopeIntegration';

export { captureUnderscoreErrorException } from '../common/_error';
export { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration';
Expand Down Expand Up @@ -75,10 +76,13 @@ export function init(options: NodeOptions): void {
...getDefaultIntegrations(options).filter(
integration =>
integration.name !== 'OnUncaughtException' &&
// Next.js comes with its own Node-Fetch instrumentation so we shouldn't add ours on-top
integration.name !== 'NodeFetch',
// Next.js comes with its own Node-Fetch instrumentation, so we shouldn't add ours on-top
integration.name !== 'NodeFetch' &&
// Next.js comes with its own Http instrumentation for OTel which lead to double spans for route handler requests
integration.name !== 'Http',
),
onUncaughtExceptionIntegration(),
requestIsolationScopeIntegration(),
];

// This value is injected at build time, based on the output directory specified in the build config. Though a default
Expand Down
44 changes: 44 additions & 0 deletions packages/nextjs/src/server/requestIsolationScopeIntegration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { SpanKind } from '@opentelemetry/api';
import {
defineIntegration,
getCapturedScopesOnSpan,
getCurrentScope,
getIsolationScope,
getRootSpan,
setCapturedScopesOnSpan,
spanToJSON,
} from '@sentry/core';
import { getSpanKind } from '@sentry/opentelemetry';

/**
* This integration is responsible for creating isolation scopes for incoming Http requests.
* We do so by waiting for http spans to be created and then forking the isolation scope.
*
* Normally the isolation scopes would be created by our Http instrumentation, however Next.js brings it's own Http
* instrumentation so we had to disable ours.
*/
export const requestIsolationScopeIntegration = defineIntegration(() => {
return {
name: 'RequestIsolationScope',
setup(client) {
client.on('spanStart', span => {
const spanJson = spanToJSON(span);
const data = spanJson.data || {};

// The following check is a heuristic to determine whether the started span is a span that tracks an incoming HTTP request
if (
(getSpanKind(span) === SpanKind.SERVER && data['http.method']) ||
(span === getRootSpan(span) && data['next.route'])
) {
const scopes = getCapturedScopesOnSpan(span);

// Update the isolation scope, isolate this request
const isolationScope = (scopes.isolationScope || getIsolationScope()).clone();
const scope = scopes.scope || getCurrentScope();

setCapturedScopesOnSpan(span, scope, isolationScope);
}
});
},
};
});
10 changes: 9 additions & 1 deletion packages/opentelemetry/src/utils/mapStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ const canonicalGrpcErrorCodesMap: Record<string, SpanStatus['message']> = {
'16': 'unauthenticated',
} as const;

const isStatusErrorMessageValid = (message: string): boolean => {
return Object.values(canonicalGrpcErrorCodesMap).includes(message as SpanStatus['message']);
};

/**
* Get a Sentry span status from an otel span.
*/
Expand All @@ -39,7 +43,11 @@ export function mapStatus(span: AbstractSpan): SpanStatus {
return { code: SPAN_STATUS_OK };
// If the span is already marked as erroneous we return that exact status
} else if (status.code === SpanStatusCode.ERROR) {
return { code: SPAN_STATUS_ERROR, message: status.message };
if (typeof status.message === 'undefined' || isStatusErrorMessageValid(status.message)) {
return { code: SPAN_STATUS_ERROR, message: status.message };
} else {
return { code: SPAN_STATUS_ERROR, message: 'unknown_error' };
}
}
}

Expand Down