Skip to content

Commit

Permalink
feat(nextjs): Improve Next.js serverside span data quality (#13652)
Browse files Browse the repository at this point in the history
Sometimes we were sending faulty `default` ops for Next.js transactions.
This seemed to happen when the HTTP integration couldn't get a handle of
the incoming requests (yet to figure out why).

This PR:
- Backfills the op for faulty transactions
- Sets the origin of spans generated by Next.js to `auto` (I couldn't
come up with a more specific origin because the second part of it is an
op and none are satisfactory)
- Remove the `sentry.skip_span_data_inference` which is only used
internally.

This change is hard to test because it seems to happen flakily that the
http integration isn't working.

Fixes #13598
  • Loading branch information
lforst committed Sep 10, 2024
1 parent ae4451d commit 02290d1
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test('Should emit a span for a generateMetadata() function invokation', async ({
expect(transaction.spans).toContainEqual(
expect.objectContaining({
description: 'generateMetadata /generation-functions/page',
origin: 'manual',
origin: 'auto',
parent_span_id: expect.any(String),
span_id: expect.any(String),
status: 'ok',
Expand Down Expand Up @@ -74,7 +74,7 @@ test('Should send a transaction event for a generateViewport() function invokati
expect((await transactionPromise).spans).toContainEqual(
expect.objectContaining({
description: 'generateViewport /generation-functions/page',
origin: 'manual',
origin: 'auto',
parent_span_id: expect.any(String),
span_id: expect.any(String),
status: 'ok',
Expand Down
24 changes: 24 additions & 0 deletions packages/nextjs/src/server/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
applySdkMetadata,
getClient,
Expand Down Expand Up @@ -189,6 +190,7 @@ export function init(options: NodeOptions): NodeClient | undefined {
// with patterns (e.g. http.server spans) that will produce confusing data.
if (spanAttributes?.['next.span_type'] !== undefined) {
span.setAttribute('sentry.skip_span_data_inference', true);
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto');
}

// We want to rename these spans because they look like "GET /path/to/route" and we already emit spans that look
Expand Down Expand Up @@ -286,6 +288,28 @@ export function init(options: NodeOptions): NodeClient | undefined {
),
);

getGlobalScope().addEventProcessor(
Object.assign(
(event => {
// Sometimes, the HTTP integration will not work, causing us not to properly set an op for spans generated by
// Next.js that are actually more or less correct server HTTP spans, so we are backfilling the op here.
if (
event.type === 'transaction' &&
event.transaction?.match(/^(RSC )?GET /) &&
event.contexts?.trace?.data?.['sentry.rsc'] === true &&
!event.contexts.trace.op
) {
event.contexts.trace.data = event.contexts.trace.data || {};
event.contexts.trace.data[SEMANTIC_ATTRIBUTE_SENTRY_OP] = 'http.server';
event.contexts.trace.op = 'http.server';
}

return event;
}) satisfies EventProcessor,
{ id: 'NextjsTransactionEnhancer' },
),
);

if (process.env.NODE_ENV === 'development') {
getGlobalScope().addEventProcessor(devErrorSymbolicationEventProcessor);
}
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry/src/spanExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ function removeSentryAttributes(data: Record<string, unknown>): Record<string, u
/* eslint-disable @typescript-eslint/no-dynamic-delete */
delete cleanedData[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE];
delete cleanedData[SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE];
delete cleanedData['sentry.skip_span_data_inference'];
/* eslint-enable @typescript-eslint/no-dynamic-delete */

return cleanedData;
Expand Down

0 comments on commit 02290d1

Please sign in to comment.