Skip to content
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

feat(next): Handle existing root spans for isolation scope #11479

Merged
merged 1 commit into from
Apr 8, 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
@@ -0,0 +1,39 @@
import {
getActiveSpan,
getCapturedScopesOnSpan,
getDefaultIsolationScope,
getRootSpan,
spanToJSON,
withIsolationScope,
} from '@sentry/core';
import type { Scope } from '@sentry/types';

/**
* Wrap a callback with a new isolation scope.
* However, if we have an active root span that was generated by next, we want to reuse the isolation scope from that span.
*/
export function withIsolationScopeOrReuseFromRootSpan<T>(cb: (isolationScope: Scope) => T): T {
const activeSpan = getActiveSpan();

if (!activeSpan) {
return withIsolationScope(cb);
}

const rootSpan = getRootSpan(activeSpan);

// Verify this is a next span
if (!spanToJSON(rootSpan).data?.['next.route']) {
Copy link
Member Author

Choose a reason for hiding this comment

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

this feels like a reasonable starting point here I guess? We should sync this with the (not yet existing) isolation scope integration!

Copy link
Member

Choose a reason for hiding this comment

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

We could also use next.span_name here as this attribute is added to (I think) almost all spans. Look here

return withIsolationScope(cb);
}

const scopes = getCapturedScopesOnSpan(rootSpan);

const isolationScope = scopes.isolationScope;

// If this is the default isolation scope, we still want to fork one
if (isolationScope === getDefaultIsolationScope()) {
return withIsolationScope(cb);
}

return withIsolationScope(isolationScope, cb);
}
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/utils/wrapperUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import {
startSpan,
startSpanManual,
withActiveSpan,
withIsolationScope,
} from '@sentry/core';
import type { Span } from '@sentry/types';
import { isString } from '@sentry/utils';

import { platformSupportsStreaming } from './platformSupportsStreaming';
import { autoEndSpanOnResponseEnd, flushQueue } from './responseEnd';
import { withIsolationScopeOrReuseFromRootSpan } from './withIsolationScopeOrReuseFromRootSpan';

declare module 'http' {
interface IncomingMessage {
Expand Down Expand Up @@ -89,7 +89,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
},
): (...params: Parameters<F>) => Promise<ReturnType<F>> {
return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
return withIsolationScope(async isolationScope => {
return withIsolationScopeOrReuseFromRootSpan(async isolationScope => {
isolationScope.setSDKProcessingMetadata({
request: req,
});
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/withServerActionInstrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import {
getClient,
handleCallbackErrors,
startSpan,
withIsolationScope,
} from '@sentry/core';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from './debug-build';
import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils';
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
import { flushQueue } from './utils/responseEnd';
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';

interface Options {
formData?: FormData;
Expand Down Expand Up @@ -58,7 +58,7 @@ async function withServerActionInstrumentationImplementation<A extends (...args:
callback: A,
): Promise<ReturnType<A>> {
addTracingExtensions();
return withIsolationScope(isolationScope => {
return withIsolationScopeOrReuseFromRootSpan(isolationScope => {
const sendDefaultPii = getClient()?.getOptions().sendDefaultPii;

let sentryTraceHeader;
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import {
continueTrace,
setHttpStatus,
startSpanManual,
withIsolationScope,
} from '@sentry/core';
import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils';

import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types';
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
import { flushQueue } from './utils/responseEnd';
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';

/**
* Wrap the given API route handler for tracing and error capturing. Thin wrapper around `withSentry`, which only
Expand Down Expand Up @@ -54,7 +54,7 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz

addTracingExtensions();

return withIsolationScope(isolationScope => {
return withIsolationScopeOrReuseFromRootSpan(isolationScope => {
return continueTrace(
{
// TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { addTracingExtensions, captureCheckIn, withIsolationScope } from '@sentry/core';
import { addTracingExtensions, captureCheckIn } from '@sentry/core';
import type { NextApiRequest } from 'next';

import type { VercelCronsConfig } from './types';
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';

type EdgeRequest = {
nextUrl: URL;
Expand All @@ -19,7 +20,7 @@ export function wrapApiHandlerWithSentryVercelCrons<F extends (...args: any[]) =
return new Proxy(handler, {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
apply: (originalFunction, thisArg, args: any[]) => {
return withIsolationScope(() => {
return withIsolationScopeOrReuseFromRootSpan(() => {
if (!args || !args[0]) {
return originalFunction.apply(thisArg, args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
getCurrentScope,
handleCallbackErrors,
startSpanManual,
withIsolationScope,
} from '@sentry/core';
import type { WebFetchHeaders } from '@sentry/types';
import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils';
Expand All @@ -17,6 +16,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import type { GenerationFunctionContext } from '../common/types';
import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils';
import { commonObjectToPropagationContext } from './utils/commonObjectTracing';
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';

/**
* Wraps a generation function (e.g. generateMetadata) with Sentry error and performance instrumentation.
Expand Down Expand Up @@ -47,7 +47,7 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
data = { params, searchParams };
}

return withIsolationScope(isolationScope => {
return withIsolationScopeOrReuseFromRootSpan(isolationScope => {
isolationScope.setSDKProcessingMetadata({
request: {
headers: headers ? winterCGHeadersToDict(headers) : undefined,
Expand Down
7 changes: 4 additions & 3 deletions packages/nextjs/src/common/wrapPageComponentWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { addTracingExtensions, captureException, getCurrentScope, withIsolationScope } from '@sentry/core';
import { addTracingExtensions, captureException, getCurrentScope } from '@sentry/core';
import { extractTraceparentData } from '@sentry/utils';
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';

interface FunctionComponent {
(...args: unknown[]): unknown;
Expand All @@ -25,7 +26,7 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C
if (isReactClassComponent(pageComponent)) {
return class SentryWrappedPageComponent extends pageComponent {
public render(...args: unknown[]): unknown {
return withIsolationScope(() => {
return withIsolationScopeOrReuseFromRootSpan(() => {
const scope = getCurrentScope();
// We extract the sentry trace data that is put in the component props by datafetcher wrappers
const sentryTraceData =
Expand Down Expand Up @@ -60,7 +61,7 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C
} else if (typeof pageComponent === 'function') {
return new Proxy(pageComponent, {
apply(target, thisArg, argArray: [{ _sentryTraceData?: string } | undefined]) {
return withIsolationScope(() => {
return withIsolationScopeOrReuseFromRootSpan(() => {
const scope = getCurrentScope();
// We extract the sentry trace data that is put in the component props by datafetcher wrappers
const sentryTraceData = argArray?.[0]?._sentryTraceData;
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import {
handleCallbackErrors,
setHttpStatus,
startSpan,
withIsolationScope,
} from '@sentry/core';
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';

/**
* Wraps a Next.js route handler with performance and error instrumentation.
Expand All @@ -29,7 +29,7 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
const { method, parameterizedRoute, headers } = context;
return new Proxy(routeHandler, {
apply: (originalFunction, thisArg, args) => {
return withIsolationScope(async isolationScope => {
return withIsolationScopeOrReuseFromRootSpan(async isolationScope => {
isolationScope.setSDKProcessingMetadata({
request: {
headers: headers ? winterCGHeadersToDict(headers) : undefined,
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/wrapServerComponentWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
getCurrentScope,
handleCallbackErrors,
startSpanManual,
withIsolationScope,
} from '@sentry/core';
import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils';

Expand All @@ -16,6 +15,7 @@ import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/
import type { ServerComponentContext } from '../common/types';
import { commonObjectToPropagationContext } from './utils/commonObjectTracing';
import { flushQueue } from './utils/responseEnd';
import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan';

/**
* Wraps an `app` directory server component with Sentry error instrumentation.
Expand All @@ -34,7 +34,7 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
return new Proxy(appDirComponent, {
apply: (originalFunction, thisArg, args) => {
// TODO: If we ever allow withIsolationScope to take a scope, we should pass a scope here that is shared between all of the server components, similar to what `commonObjectToPropagationContext` does.
return withIsolationScope(isolationScope => {
return withIsolationScopeOrReuseFromRootSpan(isolationScope => {
const completeHeadersDict: Record<string, string> = context.headers
? winterCGHeadersToDict(context.headers)
: {};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import {
Scope,
getCurrentScope,
getGlobalScope,
getIsolationScope,
setCapturedScopesOnSpan,
startSpan,
} from '@sentry/core';
import { GLOBAL_OBJ } from '@sentry/utils';
import { init } from '@sentry/vercel-edge';
import { AsyncLocalStorage } from 'async_hooks';

import { withIsolationScopeOrReuseFromRootSpan } from '../../src/common/utils/withIsolationScopeOrReuseFromRootSpan';

describe('withIsolationScopeOrReuseFromRootSpan', () => {
beforeEach(() => {
getIsolationScope().clear();
getCurrentScope().clear();
getGlobalScope().clear();
(GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage;

init({
enableTracing: true,
});
});

it('works without any span', () => {
const initialIsolationScope = getIsolationScope();
initialIsolationScope.setTag('aa', 'aa');

withIsolationScopeOrReuseFromRootSpan(isolationScope => {
isolationScope.setTag('bb', 'bb');
expect(isolationScope).not.toBe(initialIsolationScope);
expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
});
});

it('works with a non-next.js span', () => {
const initialIsolationScope = getIsolationScope();
initialIsolationScope.setTag('aa', 'aa');

const customScope = new Scope();

startSpan({ name: 'other' }, span => {
setCapturedScopesOnSpan(span, getCurrentScope(), customScope);

withIsolationScopeOrReuseFromRootSpan(isolationScope => {
isolationScope.setTag('bb', 'bb');
expect(isolationScope).not.toBe(initialIsolationScope);
expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
});
});
});

it('works with a next.js span', () => {
const initialIsolationScope = getIsolationScope();
initialIsolationScope.setTag('aa', 'aa');

const customScope = new Scope();

startSpan(
{
name: 'other',
attributes: { 'next.route': 'aha' },
},
span => {
setCapturedScopesOnSpan(span, getCurrentScope(), customScope);

withIsolationScopeOrReuseFromRootSpan(isolationScope => {
isolationScope.setTag('bb', 'bb');
expect(isolationScope).toBe(customScope);
expect(isolationScope.getScopeData().tags).toEqual({ bb: 'bb' });
});
},
);
});

it('works with a next.js span that has default isolation scope', () => {
const initialIsolationScope = getIsolationScope();
initialIsolationScope.setTag('aa', 'aa');

startSpan(
{
name: 'other',
attributes: { 'next.route': 'aha' },
},
() => {
withIsolationScopeOrReuseFromRootSpan(isolationScope => {
isolationScope.setTag('bb', 'bb');
expect(isolationScope).not.toBe(initialIsolationScope);
expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' });
});
},
);
});
});
Loading
Loading