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(nextjs): Connect server component transactions if there is no incoming trace #9845

Merged
merged 12 commits into from
Dec 18, 2023
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { PropsWithChildren } from 'react';

export const dynamic = 'force-dynamic';

export default function Layout({ children }: PropsWithChildren<{}>) {
return (
<div>
<p>Layout</p>
{children}
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { PropsWithChildren } from 'react';

export const dynamic = 'force-dynamic';

export default function Layout({ children }: PropsWithChildren<{}>) {
return (
<div>
<p>Layout</p>
{children}
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export const dynamic = 'force-dynamic';

export default function Page() {
return <p>Hello World!</p>;
}

export async function generateMetadata() {
return {
title: 'I am generated metadata',
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '../event-proxy-server';

test('Will capture a connected trace for all server components and generation functions when visiting a page', async ({
page,
}) => {
const someConnectedEvent = waitForTransaction('nextjs-13-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'Layout Server Component (/(nested-layout)/nested-layout)' ||
transactionEvent?.transaction === 'Layout Server Component (/(nested-layout))' ||
transactionEvent?.transaction === 'Page Server Component (/(nested-layout)/nested-layout)' ||
transactionEvent?.transaction === 'Page.generateMetadata (/(nested-layout)/nested-layout)'
);
});

const layout1Transaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'Layout Server Component (/(nested-layout)/nested-layout)' &&
(await someConnectedEvent).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id
);
});

const layout2Transaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'Layout Server Component (/(nested-layout))' &&
(await someConnectedEvent).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id
);
});

const pageTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page Server Component (/(nested-layout)/nested-layout)' &&
(await someConnectedEvent).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id
);
});

const generateMetadataTransaction = waitForTransaction('nextjs-13-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page.generateMetadata (/(nested-layout)/nested-layout)' &&
(await someConnectedEvent).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id
);
});

await page.goto('/nested-layout');

expect(await layout1Transaction).toBeDefined();
expect(await layout2Transaction).toBeDefined();
expect(await pageTransaction).toBeDefined();
expect(await generateMetadataTransaction).toBeDefined();
});
23 changes: 23 additions & 0 deletions packages/nextjs/src/common/utils/commonObjectTracing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type { PropagationContext } from '@sentry/types';

const commonMap = new WeakMap<object, PropagationContext>();

/**
* 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.
*/
export function commonObjectToPropagationContext(
commonObject: unknown,
propagationContext: PropagationContext,
): PropagationContext {
if (typeof commonObject === 'object' && commonObject) {
const memoTraceId = commonMap.get(commonObject);
if (memoTraceId) {
return memoTraceId;
lforst marked this conversation as resolved.
Show resolved Hide resolved
} else {
commonMap.set(commonObject, propagationContext);
return propagationContext;
}
} else {
return propagationContext;
}
}
15 changes: 15 additions & 0 deletions packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import {
captureException,
continueTrace,
getCurrentHub,
getCurrentScope,
runWithAsyncContext,
trace,
} from '@sentry/core';
import type { WebFetchHeaders } from '@sentry/types';
import { winterCGHeadersToDict } from '@sentry/utils';

import type { GenerationFunctionContext } from '../common/types';
import { commonObjectToPropagationContext } from './utils/commonObjectTracing';

/**
* Wraps a generation function (e.g. generateMetadata) with Sentry error and performance instrumentation.
Expand Down Expand Up @@ -45,6 +47,19 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
baggage: headers?.get('baggage'),
sentryTrace: headers?.get('sentry-trace') ?? undefined,
});

// If there is no incoming trace, we are setting the transaction context to one that is shared between all other
// transactions for this request. We do this based on the `headers` object, which is the same for all components.
const propagationContext = getCurrentScope().getPropagationContext();
if (!transactionContext.traceId && !transactionContext.parentSpanId) {
const { traceId: commonTraceId, spanId: commonSpanId } = commonObjectToPropagationContext(
headers,
propagationContext,
);
transactionContext.traceId = commonTraceId;
transactionContext.parentSpanId = commonSpanId;
}

return trace(
{
op: 'function.nextjs',
Expand Down
22 changes: 21 additions & 1 deletion packages/nextjs/src/common/wrapServerComponentWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import { addTracingExtensions, captureException, continueTrace, runWithAsyncContext, trace } from '@sentry/core';
import {
addTracingExtensions,
captureException,
continueTrace,
getCurrentScope,
runWithAsyncContext,
trace,
} from '@sentry/core';
import { winterCGHeadersToDict } from '@sentry/utils';

import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils';
import type { ServerComponentContext } from '../common/types';
import { commonObjectToPropagationContext } from './utils/commonObjectTracing';
import { flushQueue } from './utils/responseEnd';

/**
Expand Down Expand Up @@ -33,6 +41,18 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
baggage: context.baggageHeader ?? completeHeadersDict['baggage'],
});

// If there is no incoming trace, we are setting the transaction context to one that is shared between all other
// transactions for this request. We do this based on the `headers` object, which is the same for all components.
const propagationContext = getCurrentScope().getPropagationContext();
if (!transactionContext.traceId && !transactionContext.parentSpanId) {
const { traceId: commonTraceId, spanId: commonSpanId } = commonObjectToPropagationContext(
context.headers,
propagationContext,
);
transactionContext.traceId = commonTraceId;
transactionContext.parentSpanId = commonSpanId;
}

const res = trace(
{
...transactionContext,
Expand Down
2 changes: 0 additions & 2 deletions packages/nextjs/src/config/loaders/wrappingLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,6 @@ export default function wrappingLoader(
.replace(/(.*)/, '/$1')
// Pull off the file name
.replace(/\/[^/]+\.(js|ts|jsx|tsx)$/, '')
// Remove routing groups: https://beta.nextjs.org/docs/routing/defining-routes#example-creating-multiple-root-layouts
.replace(/\/(\(.*?\)\/)+/g, '/')
Comment on lines -160 to -161
Copy link
Member Author

Choose a reason for hiding this comment

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

Sneaking this in. This is actually a bug because routing groups should part of the txn name. I added this when I didn't fully understand the app router yet.

// In case all of the above have left us with an empty string (which will happen if we're dealing with the
// homepage), sub back in the root route
.replace(/^$/, '/');
Expand Down