Skip to content

ref(nextjs): Switch edge wrapping to OTEL #13937

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 26 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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 @@ -20,5 +20,5 @@ export async function middleware(request: NextRequest) {

// See "Matching Paths" below to learn more
export const config = {
matcher: ['/api/endpoint-behind-middleware'],
matcher: ['/api/endpoint-behind-middleware', '/api/endpoint-behind-faulty-middleware'],
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
/// <reference types="next/navigation-types/compat/navigation" />

// NOTE: This file should not be edited
// see https://nextjs.org/docs/basic-features/typescript for more information.
// see https://nextjs.org/docs/app/building-your-application/configuring/typescript for more information.
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,22 @@ export const config = {
export default async function handler() {
// Without a working async context strategy the two spans created by `Sentry.startSpan()` would be nested.

const outerSpanPromise = Sentry.withIsolationScope(() => {
return Sentry.startSpan({ name: 'outer-span' }, () => {
return new Promise<void>(resolve => setTimeout(resolve, 300));
});
const outerSpanPromise = Sentry.startSpan({ name: 'outer-span' }, () => {
return new Promise<void>(resolve => setTimeout(resolve, 300));
});

setTimeout(() => {
Sentry.withIsolationScope(() => {
return Sentry.startSpan({ name: 'inner-span' }, () => {
const innerSpanPromise = new Promise<void>(resolve => {
setTimeout(() => {
Sentry.startSpan({ name: 'inner-span' }, () => {
return new Promise<void>(resolve => setTimeout(resolve, 100));
}).then(() => {
resolve();
});
});
}, 100);
}, 100);
});

await outerSpanPromise;
await innerSpanPromise;

return new Response('ok', { status: 200 });
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { NextApiRequest, NextApiResponse } from 'next';

type Data = {
name: string;
};

export default function handler(req: NextApiRequest, res: NextApiResponse<Data>) {
res.status(200).json({ name: 'John Doe' });
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { waitForTransaction } from '@sentry-internal/test-utils';

test('Should allow for async context isolation in the edge SDK', async ({ request }) => {
const edgerouteTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return transactionEvent?.transaction === 'GET /api/async-context-edge-endpoint';
return (
transactionEvent?.transaction === 'GET /api/async-context-edge-endpoint' &&
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
);
});

await request.get('/api/async-context-edge-endpoint');
Expand All @@ -13,8 +16,5 @@ test('Should allow for async context isolation in the edge SDK', async ({ reques
const outerSpan = asyncContextEdgerouteTransaction.spans?.find(span => span.description === 'outer-span');
const innerSpan = asyncContextEdgerouteTransaction.spans?.find(span => span.description === 'inner-span');

// @ts-expect-error parent_span_id exists
expect(outerSpan?.parent_span_id).toStrictEqual(asyncContextEdgerouteTransaction.contexts?.trace?.span_id);
// @ts-expect-error parent_span_id exists
expect(innerSpan?.parent_span_id).toStrictEqual(asyncContextEdgerouteTransaction.contexts?.trace?.span_id);
expect(outerSpan?.parent_span_id).toStrictEqual(innerSpan?.parent_span_id);
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ test('Should create a transaction for edge routes', async ({ request }) => {
const edgerouteTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'GET /api/edge-endpoint' &&
transactionEvent?.contexts?.trace?.status === 'ok' &&
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
);
});
Expand All @@ -24,31 +23,11 @@ test('Should create a transaction for edge routes', async ({ request }) => {
expect(edgerouteTransaction.request?.headers?.['x-yeet']).toBe('test-value');
});

test('Should create a transaction with error status for faulty edge routes', async ({ request }) => {
test('Faulty edge routes', async ({ request }) => {
const edgerouteTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'GET /api/error-edge-endpoint' &&
transactionEvent?.contexts?.trace?.status === 'unknown_error'
);
return transactionEvent?.transaction === 'GET /api/error-edge-endpoint';
});

request.get('/api/error-edge-endpoint').catch(() => {
// Noop
});

const edgerouteTransaction = await edgerouteTransactionPromise;

expect(edgerouteTransaction.contexts?.trace?.status).toBe('unknown_error');
expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server');
expect(edgerouteTransaction.contexts?.runtime?.name).toBe('vercel-edge');

// Assert that isolation scope works properly
expect(edgerouteTransaction.tags?.['my-isolated-tag']).toBe(true);
expect(edgerouteTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
});

// TODO(lforst): This cannot make it into production - Make sure to fix this test
test.skip('Should record exceptions for faulty edge routes', async ({ request }) => {
const errorEventPromise = waitForError('nextjs-app-dir', errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'Edge Route Error';
});
Expand All @@ -57,11 +36,21 @@ test.skip('Should record exceptions for faulty edge routes', async ({ request })
// Noop
});

const errorEvent = await errorEventPromise;
const [edgerouteTransaction, errorEvent] = await Promise.all([
test.step('should create a transaction', () => edgerouteTransactionPromise),
test.step('should create an error event', () => errorEventPromise),
]);

// Assert that isolation scope works properly
expect(errorEvent.tags?.['my-isolated-tag']).toBe(true);
expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
test.step('should create transactions with the right fields', () => {
expect(edgerouteTransaction.contexts?.trace?.status).toBe('unknown_error');
expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server');
expect(edgerouteTransaction.contexts?.runtime?.name).toBe('vercel-edge');
});

expect(errorEvent.transaction).toBe('GET /api/error-edge-endpoint');
test.step('should have scope isolation', () => {
expect(edgerouteTransaction.tags?.['my-isolated-tag']).toBe(true);
expect(edgerouteTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
expect(errorEvent.tags?.['my-isolated-tag']).toBe(true);
expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@ test('Should record exceptions for faulty edge server components', async ({ page
expect(errorEvent.transaction).toBe(`Page Server Component (/edge-server-components/error)`);
});

// TODO(lforst): This test skip cannot make it into production - make sure to fix this test before merging into develop branch
// TODO(lforst): Fix this test
test.skip('Should record transaction for edge server components', async ({ page }) => {
const serverComponentTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return transactionEvent?.transaction === 'GET /edge-server-components';
return (
transactionEvent?.transaction === 'GET /edge-server-components' &&
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
);
});

await page.goto('/edge-server-components');

const serverComponentTransaction = await serverComponentTransactionPromise;

expect(serverComponentTransaction).toBe(1);
expect(serverComponentTransaction).toBeDefined();
expect(serverComponentTransaction.request?.headers).toBeDefined();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';

test('Should create a transaction for middleware', async ({ request }) => {
const middlewareTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return transactionEvent?.transaction === 'middleware' && transactionEvent?.contexts?.trace?.status === 'ok';
return transactionEvent?.transaction === 'middleware GET /api/endpoint-behind-middleware';
});

const response = await request.get('/api/endpoint-behind-middleware');
Expand All @@ -12,54 +12,48 @@ test('Should create a transaction for middleware', async ({ request }) => {
const middlewareTransaction = await middlewareTransactionPromise;

expect(middlewareTransaction.contexts?.trace?.status).toBe('ok');
expect(middlewareTransaction.contexts?.trace?.op).toBe('middleware.nextjs');
expect(middlewareTransaction.contexts?.trace?.op).toBe('http.server.middleware');
expect(middlewareTransaction.contexts?.runtime?.name).toBe('vercel-edge');

// Assert that isolation scope works properly
expect(middlewareTransaction.tags?.['my-isolated-tag']).toBe(true);
expect(middlewareTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
});

test('Should create a transaction with error status for faulty middleware', async ({ request }) => {
test('Faulty middlewares', async ({ request }) => {
const middlewareTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'middleware' && transactionEvent?.contexts?.trace?.status === 'unknown_error'
);
return transactionEvent?.transaction === 'middleware GET /api/endpoint-behind-faulty-middleware';
});

request.get('/api/endpoint-behind-middleware', { headers: { 'x-should-throw': '1' } }).catch(() => {
// Noop
});

const middlewareTransaction = await middlewareTransactionPromise;

expect(middlewareTransaction.contexts?.trace?.status).toBe('unknown_error');
expect(middlewareTransaction.contexts?.trace?.op).toBe('middleware.nextjs');
expect(middlewareTransaction.contexts?.runtime?.name).toBe('vercel-edge');
});

// TODO(lforst): This cannot make it into production - Make sure to fix this test
test.skip('Records exceptions happening in middleware', async ({ request }) => {
const errorEventPromise = waitForError('nextjs-app-dir', errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'Middleware Error';
});

request.get('/api/endpoint-behind-middleware', { headers: { 'x-should-throw': '1' } }).catch(() => {
request.get('/api/endpoint-behind-faulty-middleware', { headers: { 'x-should-throw': '1' } }).catch(() => {
// Noop
});

const errorEvent = await errorEventPromise;
await test.step('should record transactions', async () => {
const middlewareTransaction = await middlewareTransactionPromise;
expect(middlewareTransaction.contexts?.trace?.status).toBe('unknown_error');
expect(middlewareTransaction.contexts?.trace?.op).toBe('http.server.middleware');
expect(middlewareTransaction.contexts?.runtime?.name).toBe('vercel-edge');
});

// Assert that isolation scope works properly
expect(errorEvent.tags?.['my-isolated-tag']).toBe(true);
expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
expect(errorEvent.transaction).toBe('middleware');
await test.step('should record exceptions', async () => {
const errorEvent = await errorEventPromise;

// Assert that isolation scope works properly
expect(errorEvent.tags?.['my-isolated-tag']).toBe(true);
expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
expect(errorEvent.transaction).toBe('middleware GET /api/endpoint-behind-faulty-middleware');
});
});

test('Should trace outgoing fetch requests inside middleware and create breadcrumbs for it', async ({ request }) => {
const middlewareTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'middleware' &&
transactionEvent?.transaction === 'middleware GET /api/endpoint-behind-middleware' &&
!!transactionEvent.spans?.find(span => span.op === 'http.client')
);
});
Expand Down
91 changes: 0 additions & 91 deletions packages/nextjs/src/common/utils/edgeWrapperUtils.ts

This file was deleted.

Loading
Loading