Skip to content

Commit 960dcd1

Browse files
chargomechargome
committed
fix spans
1 parent 2a2613d commit 960dcd1

File tree

7 files changed

+170
-47
lines changed

7 files changed

+170
-47
lines changed

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/(nested-layout)/nested-layout/[dynamic]/layout.tsx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { PropsWithChildren } from 'react';
2+
3+
export const dynamic = 'force-dynamic';
4+
5+
export default function Layout({ children }: PropsWithChildren<{}>) {
6+
return (
7+
<div>
8+
<p>DynamicLayout</p>
9+
{children}
10+
</div>
11+
);
12+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/(nested-layout)/nested-layout/[dynamic]/page.tsx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
export const dynamic = 'force-dynamic';
2+
3+
export default async function Page() {
4+
return (
5+
<div>
6+
<p>Dynamic Page</p>
7+
</div>
8+
);
9+
}
10+
11+
export async function generateMetadata() {
12+
return {
13+
title: 'I am dynamic page generated metadata',
14+
};
15+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/connected-servercomponent-trace.test.ts‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,43 @@ test('Will create a transaction with spans for every server component and metada
1414
return span.description;
1515
});
1616

17-
expect(spanDescriptions).toContainEqual('resolve root layout module');
18-
expect(spanDescriptions).toContainEqual('resolve layout module "(nested-layout)"');
19-
expect(spanDescriptions).toContainEqual('resolve layout module "nested-layout"');
20-
expect(spanDescriptions).toContainEqual('resolve page module');
17+
expect(spanDescriptions).toContainEqual('resolve page components');
18+
expect(spanDescriptions).toContainEqual('render route (app) /nested-layout');
19+
expect(spanDescriptions).toContainEqual('build component tree');
20+
expect(spanDescriptions).toContainEqual('resolve root layout server component');
21+
expect(spanDescriptions).toContainEqual('resolve layout server component "(nested-layout)"');
22+
expect(spanDescriptions).toContainEqual('resolve layout server component "nested-layout"');
23+
expect(spanDescriptions).toContainEqual('resolve page server component "/nested-layout"');
24+
expect(spanDescriptions).toContainEqual('generateMetadata /(nested-layout)/nested-layout/page');
2125
expect(spanDescriptions).toContainEqual('Page.generateMetadata (/(nested-layout)/nested-layout)');
26+
expect(spanDescriptions).toContainEqual('start response');
27+
expect(spanDescriptions).toContainEqual('NextNodeServer.clientComponentLoading');
28+
});
29+
30+
test('Will create a transaction with spans for every server component and metadata generation functions when visiting a dynamic page', async ({
31+
page,
32+
}) => {
33+
const serverTransactionEventPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
34+
console.log(transactionEvent?.transaction);
35+
return transactionEvent?.transaction === 'GET /nested-layout/[dynamic]';
36+
});
37+
38+
await page.goto('/nested-layout/123');
39+
40+
const spanDescriptions = (await serverTransactionEventPromise).spans?.map(span => {
41+
return span.description;
42+
});
43+
44+
expect(spanDescriptions).toContainEqual('resolve page components');
45+
expect(spanDescriptions).toContainEqual('render route (app) /nested-layout/[dynamic]');
46+
expect(spanDescriptions).toContainEqual('build component tree');
47+
expect(spanDescriptions).toContainEqual('resolve root layout server component');
48+
expect(spanDescriptions).toContainEqual('resolve layout server component "(nested-layout)"');
49+
expect(spanDescriptions).toContainEqual('resolve layout server component "nested-layout"');
50+
expect(spanDescriptions).toContainEqual('resolve layout server component "[dynamic]"');
51+
expect(spanDescriptions).toContainEqual('resolve page server component "/nested-layout/[dynamic]"');
52+
expect(spanDescriptions).toContainEqual('generateMetadata /(nested-layout)/nested-layout/[dynamic]/page');
53+
expect(spanDescriptions).toContainEqual('Page.generateMetadata (/(nested-layout)/nested-layout/[dynamic])');
54+
expect(spanDescriptions).toContainEqual('start response');
55+
expect(spanDescriptions).toContainEqual('NextNodeServer.clientComponentLoading');
2256
});

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,20 @@ test('Should set a "not_found" status on a server component span when notFound()
7070

7171
const transactionEvent = await serverComponentTransactionPromise;
7272

73-
// Transaction should have status ok, because the http status is ok, but the server component span should be not_found
73+
// Transaction should have status ok, because the http status is ok, but the render component span should be not_found
7474
expect(transactionEvent.contexts?.trace?.status).toBe('ok');
7575
expect(transactionEvent.spans).toContainEqual(
7676
expect.objectContaining({
77-
description: 'Page Server Component (/server-component/not-found)',
78-
op: 'function.nextjs',
77+
description: 'render route (app) /server-component/not-found',
7978
status: 'not_found',
79+
}),
80+
);
81+
82+
// Page server component span should have the right name and attributes
83+
expect(transactionEvent.spans).toContainEqual(
84+
expect.objectContaining({
85+
description: 'resolve page server component "/server-component/not-found"',
86+
op: 'function.nextjs',
8087
data: expect.objectContaining({
8188
'sentry.nextjs.ssr.function.type': 'Page',
8289
'sentry.nextjs.ssr.function.route': '/server-component/not-found',
@@ -102,13 +109,20 @@ test('Should capture an error and transaction for a app router page', async ({ p
102109
// Error event should have the right transaction name
103110
expect(errorEvent.transaction).toBe(`Page Server Component (/server-component/faulty)`);
104111

105-
// Transaction should have status ok, because the http status is ok, but the server component span should be internal_error
112+
// Transaction should have status ok, because the http status is ok, but the render component span should be internal_error
106113
expect(transactionEvent.contexts?.trace?.status).toBe('ok');
107114
expect(transactionEvent.spans).toContainEqual(
108115
expect.objectContaining({
109-
description: 'Page Server Component (/server-component/faulty)',
110-
op: 'function.nextjs',
116+
description: 'render route (app) /server-component/faulty',
111117
status: 'internal_error',
118+
}),
119+
);
120+
121+
// The page server component span should have the right name and attributes
122+
expect(transactionEvent.spans).toContainEqual(
123+
expect.objectContaining({
124+
description: 'resolve page server component "/server-component/faulty"',
125+
op: 'function.nextjs',
112126
data: expect.objectContaining({
113127
'sentry.nextjs.ssr.function.type': 'Page',
114128
'sentry.nextjs.ssr.function.route': '/server-component/faulty',

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

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,23 @@
1-
import type { PropagationContext, SpanAttributes } from '@sentry/core';
2-
import { debug, getActiveSpan, getRootSpan, GLOBAL_OBJ, Scope, spanToJSON, startNewTrace } from '@sentry/core';
1+
import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
2+
import type { PropagationContext, Span, SpanAttributes } from '@sentry/core';
3+
import {
4+
debug,
5+
getActiveSpan,
6+
getRootSpan,
7+
GLOBAL_OBJ,
8+
Scope,
9+
SEMANTIC_ATTRIBUTE_SENTRY_OP,
10+
spanToJSON,
11+
startNewTrace,
12+
} from '@sentry/core';
313
import { DEBUG_BUILD } from '../debug-build';
414
import { ATTR_NEXT_SEGMENT, ATTR_NEXT_SPAN_NAME, ATTR_NEXT_SPAN_TYPE } from '../nextSpanAttributes';
515
import { TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION } from '../span-attributes-with-logic-attached';
616

717
const commonPropagationContextMap = new WeakMap<object, PropagationContext>();
818

19+
const PAGE_SEGMENT = '__PAGE__';
20+
921
/**
1022
* Takes a shared (garbage collectable) object between resources, e.g. a headers object shared between Next.js server components and returns a common propagation context.
1123
*
@@ -126,16 +138,44 @@ export function isResolveSegmentSpan(spanAttributes: SpanAttributes): boolean {
126138
/**
127139
* Returns the enhanced name for a resolve segment span.
128140
* @param segment The segment of the resolve segment span.
141+
* @param route The route of the resolve segment span.
129142
* @returns The enhanced name for the resolve segment span.
130143
*/
131-
export function getEnhancedResolveSegmentSpanName(segment: string): string {
132-
if (segment === '__PAGE__') {
133-
return 'resolve page module';
144+
export function getEnhancedResolveSegmentSpanName({ segment, route }: { segment: string; route: string }): string {
145+
if (segment === PAGE_SEGMENT) {
146+
return `resolve page server component "${route}"`;
134147
}
135148

136149
if (segment === '') {
137-
return 'resolve root layout module';
150+
return 'resolve root layout server component';
151+
}
152+
153+
return `resolve layout server component "${segment}"`;
154+
}
155+
156+
/**
157+
* Maybe enhances the span name for a resolve segment span.
158+
* If the span is not a resolve segment span, this function does nothing.
159+
* @param activeSpan The active span.
160+
* @param spanAttributes The attributes of the span to check.
161+
* @param rootSpanAttributes The attributes of the according root span.
162+
*/
163+
export function maybeEnhanceServerComponentSpanName(
164+
activeSpan: Span,
165+
spanAttributes: SpanAttributes,
166+
rootSpanAttributes: SpanAttributes,
167+
): void {
168+
if (!isResolveSegmentSpan(spanAttributes)) {
169+
return;
138170
}
139171

140-
return `resolve layout module "${segment}"`;
172+
const segment = spanAttributes[ATTR_NEXT_SEGMENT] as string;
173+
const route = rootSpanAttributes[ATTR_HTTP_ROUTE];
174+
const enhancedName = getEnhancedResolveSegmentSpanName({ segment, route: typeof route === 'string' ? route : '' });
175+
activeSpan.updateName(enhancedName);
176+
activeSpan.setAttributes({
177+
'sentry.nextjs.ssr.function.type': segment === PAGE_SEGMENT ? 'Page' : 'Layout',
178+
'sentry.nextjs.ssr.function.route': route,
179+
});
180+
activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'function.nextjs');
141181
}

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

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import type { RequestEventData } from '@sentry/core';
2-
import { captureException, handleCallbackErrors, winterCGHeadersToDict, withIsolationScope } from '@sentry/core';
2+
import {
3+
captureException,
4+
getActiveSpan,
5+
getIsolationScope,
6+
handleCallbackErrors,
7+
SPAN_STATUS_ERROR,
8+
SPAN_STATUS_OK,
9+
winterCGHeadersToDict,
10+
} from '@sentry/core';
311
import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils';
412
import type { ServerComponentContext } from '../common/types';
513
import { flushSafelyWithTimeout, waitUntil } from '../common/utils/responseEnd';
@@ -28,28 +36,37 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
2836
} satisfies RequestEventData,
2937
});
3038

31-
return withIsolationScope(isolationScope, () => {
32-
return handleCallbackErrors(
33-
() => originalFunction.apply(thisArg, args),
34-
error => {
39+
return handleCallbackErrors(
40+
() => originalFunction.apply(thisArg, args),
41+
error => {
42+
const isolationScope = getIsolationScope();
43+
const span = getActiveSpan();
44+
const { componentRoute, componentType } = context;
45+
isolationScope.setTransactionName(`${componentType} Server Component (${componentRoute})`);
46+
47+
if (span) {
3548
if (isNotFoundNavigationError(error)) {
3649
// We don't want to report "not-found"s
50+
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' });
3751
} else if (isRedirectNavigationError(error)) {
3852
// We don't want to report redirects
53+
span.setStatus({ code: SPAN_STATUS_OK });
3954
} else {
40-
captureException(error, {
41-
mechanism: {
42-
handled: false,
43-
type: 'auto.function.nextjs.server_component',
44-
},
45-
});
55+
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
4656
}
47-
},
48-
() => {
49-
waitUntil(flushSafelyWithTimeout());
50-
},
51-
);
52-
});
57+
}
58+
59+
captureException(error, {
60+
mechanism: {
61+
handled: false,
62+
type: 'auto.function.nextjs.server_component',
63+
},
64+
});
65+
},
66+
() => {
67+
waitUntil(flushSafelyWithTimeout());
68+
},
69+
);
5370
},
5471
});
5572
}

‎packages/nextjs/src/server/handleOnSpanStart.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,10 @@ import {
1111
spanToJSON,
1212
} from '@sentry/core';
1313
import { getScopesFromContext } from '@sentry/opentelemetry';
14-
import {
15-
ATTR_NEXT_ROUTE,
16-
ATTR_NEXT_SEGMENT,
17-
ATTR_NEXT_SPAN_NAME,
18-
ATTR_NEXT_SPAN_TYPE,
19-
} from '../common/nextSpanAttributes';
14+
import { ATTR_NEXT_ROUTE, ATTR_NEXT_SPAN_NAME, ATTR_NEXT_SPAN_TYPE } from '../common/nextSpanAttributes';
2015
import { addHeadersAsAttributes } from '../common/utils/addHeadersAsAttributes';
2116
import { dropMiddlewareTunnelRequests } from '../common/utils/dropMiddlewareTunnelRequests';
22-
import { getEnhancedResolveSegmentSpanName, isResolveSegmentSpan } from '../common/utils/tracingUtils';
17+
import { maybeEnhanceServerComponentSpanName } from '../common/utils/tracingUtils';
2318

2419
/**
2520
* Handles the on span start event for Next.js spans.
@@ -30,14 +25,14 @@ import { getEnhancedResolveSegmentSpanName, isResolveSegmentSpan } from '../comm
3025
export function handleOnSpanStart(span: Span): void {
3126
const spanAttributes = spanToJSON(span).data;
3227
const rootSpan = getRootSpan(span);
28+
const rootSpanAttributes = spanToJSON(rootSpan).data;
3329
const isRootSpan = span === rootSpan;
3430

3531
dropMiddlewareTunnelRequests(span, spanAttributes);
3632

3733
// What we do in this glorious piece of code, is hoist any information about parameterized routes from spans emitted
3834
// by Next.js via the `next.route` attribute, up to the transaction by setting the http.route attribute.
3935
if (typeof spanAttributes?.[ATTR_NEXT_ROUTE] === 'string') {
40-
const rootSpanAttributes = spanToJSON(rootSpan).data;
4136
// Only hoist the http.route attribute if the transaction doesn't already have it
4237
if (
4338
// eslint-disable-next-line deprecation/deprecation
@@ -88,9 +83,5 @@ export function handleOnSpanStart(span: Span): void {
8883
setCapturedScopesOnSpan(span, scope, isolationScope);
8984
}
9085

91-
// Enhancing server component span names
92-
if (isResolveSegmentSpan(spanAttributes)) {
93-
// type conversion is safe because we already know the attribute is a string
94-
span.updateName(getEnhancedResolveSegmentSpanName(spanAttributes[ATTR_NEXT_SEGMENT] as string));
95-
}
86+
maybeEnhanceServerComponentSpanName(span, spanAttributes, rootSpanAttributes);
9687
}

0 commit comments

Comments
 (0)