feat(nextjs/vercel-edge/cloudflare): Switch to OTEL for performance m…

…onitoring (#13889) Co-authored-by: Charly Gomez <charly.gomez@sentry.io>
getsentry · Oct 24, 2024 · fe639f4 · fe639f4
1 parent c56d84d
commit fe639f4
Show file tree

Hide file tree

Showing 71 changed files with 1,565 additions and 1,094 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -1037,6 +1037,7 @@ jobs:
           retention-days: 7
 
       - name: Pre-process E2E Test Dumps
+        if: always()
         run: |
           node ./scripts/normalize-e2e-test-dump-transaction-events.js
 
@@ -1193,6 +1194,7 @@ jobs:
         run: pnpm ${{ matrix.assert-command || 'test:assert' }}
 
       - name: Pre-process E2E Test Dumps
+        if: always()
         run: |
           node ./scripts/normalize-e2e-test-dump-transaction-events.js
 

diff --git a/.size-limit.js b/.size-limit.js
@@ -224,7 +224,7 @@ module.exports = [
     import: createImport('init'),
     ignore: ['next/router', 'next/constants'],
     gzip: true,
-    limit: '39 KB',
+    limit: '39.1 KB',
   },
   // SvelteKit SDK (ESM)
   {

diff --git a/dev-packages/e2e-tests/publish-packages.ts b/dev-packages/e2e-tests/publish-packages.ts
@@ -12,21 +12,19 @@ const packageTarballPaths = glob.sync('packages/*/sentry-*.tgz', {
 
 // Publish built packages to the fake registry
 packageTarballPaths.forEach(tarballPath => {
+  // eslint-disable-next-line no-console
+  console.log(`Publishing tarball ${tarballPath} ...`);
   // `--userconfig` flag needs to be before `publish`
   childProcess.exec(
     `npm --userconfig ${__dirname}/test-registry.npmrc publish ${tarballPath}`,
     {
       cwd: repositoryRoot, // Can't use __dirname here because npm would try to publish `@sentry-internal/e2e-tests`
       encoding: 'utf8',
     },
-    (err, stdout, stderr) => {
-      // eslint-disable-next-line no-console
-      console.log(stdout);
-      // eslint-disable-next-line no-console
-      console.log(stderr);
+    err => {
       if (err) {
         // eslint-disable-next-line no-console
-        console.error(err);
+        console.error(`Error publishing tarball ${tarballPath}`, err);
         process.exit(1);
       }
     },

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/package.json b/dev-packages/e2e-tests/test-applications/nextjs-13/package.json
@@ -17,7 +17,7 @@
     "@types/node": "18.11.17",
     "@types/react": "18.0.26",
     "@types/react-dom": "18.0.9",
-    "next": "13.2.0",
+    "next": "13.5.7",
     "react": "18.2.0",
     "react-dom": "18.2.0",
     "typescript": "4.9.5"

diff --git a/...tjs-13/pages/customPageExtension.page.tsx → ...ages/[param]/customPageExtension.page.tsx b/...tjs-13/pages/customPageExtension.page.tsx → ...ages/[param]/customPageExtension.page.tsx
diff --git a/...tjs-13/pages/error-getServerSideProps.tsx → ...ages/[param]/error-getServerSideProps.tsx b/...tjs-13/pages/error-getServerSideProps.tsx → ...ages/[param]/error-getServerSideProps.tsx
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/pages-dir-pageload.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/pages-dir-pageload.test.ts
@@ -46,3 +46,38 @@ test('should create a pageload transaction when the `pages` directory is used',
     type: 'transaction',
   });
 });
+
+test('should create a pageload transaction with correct name when an error occurs in getServerSideProps', async ({
+  page,
+}) => {
+  const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
+    return (
+      transactionEvent.transaction === '/[param]/error-getServerSideProps' &&
+      transactionEvent.contexts?.trace?.op === 'pageload'
+    );
+  });
+
+  await page.goto(`/something/error-getServerSideProps`, { waitUntil: 'networkidle' });
+
+  const transaction = await transactionPromise;
+
+  expect(transaction).toMatchObject({
+    contexts: {
+      trace: {
+        data: {
+          'sentry.op': 'pageload',
+          'sentry.origin': 'auto.pageload.nextjs.pages_router_instrumentation',
+          'sentry.source': 'route',
+        },
+        op: 'pageload',
+        origin: 'auto.pageload.nextjs.pages_router_instrumentation',
+      },
+    },
+    transaction: '/[param]/error-getServerSideProps',
+    transaction_info: { source: 'route' },
+    type: 'transaction',
+  });
+
+  // Ensure the transaction name is not '/_error'
+  expect(transaction.transaction).not.toBe('/_error');
+});
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getInitialProps.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getInitialProps.test.ts
@@ -11,7 +11,7 @@ test('should propagate serverside `getInitialProps` trace to client', async ({ p
 
   const serverTransactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
     return (
-      transactionEvent.transaction === '/[param]/withInitialProps' &&
+      transactionEvent.transaction === 'GET /[param]/withInitialProps' &&
       transactionEvent.contexts?.trace?.op === 'http.server'
     );
   });
@@ -47,7 +47,7 @@ test('should propagate serverside `getInitialProps` trace to client', async ({ p
           status: 'ok',
         },
       },
-      transaction: '/[param]/withInitialProps',
+      transaction: 'GET /[param]/withInitialProps',
       transaction_info: {
         source: 'route',
       },

diff --git a/...ackages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getServerSideProps.test.ts b/...ackages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getServerSideProps.test.ts
@@ -11,7 +11,7 @@ test('Should record performance for getServerSideProps', async ({ page }) => {
 
   const serverTransactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
     return (
-      transactionEvent.transaction === '/[param]/withServerSideProps' &&
+      transactionEvent.transaction === 'GET /[param]/withServerSideProps' &&
       transactionEvent.contexts?.trace?.op === 'http.server'
     );
   });
@@ -47,7 +47,7 @@ test('Should record performance for getServerSideProps', async ({ page }) => {
           status: 'ok',
         },
       },
-      transaction: '/[param]/withServerSideProps',
+      transaction: 'GET /[param]/withServerSideProps',
       transaction_info: {
         source: 'route',
       },

diff --git a/...ackages/e2e-tests/test-applications/nextjs-13/tests/server/excluded-api-endpoints.test.ts b/...ackages/e2e-tests/test-applications/nextjs-13/tests/server/excluded-api-endpoints.test.ts
@@ -1,7 +1,7 @@
 import { expect, test } from '@playwright/test';
 import { waitForTransaction } from '@sentry-internal/test-utils';
 
-test('should not automatically create transactions for routes that were excluded from auto wrapping (string)', async ({
+test('should not apply build-time instrumentation for routes that were excluded from auto wrapping (string)', async ({
   request,
 }) => {
   const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
@@ -13,17 +13,13 @@ test('should not automatically create transactions for routes that were excluded
 
   expect(await (await request.get(`/api/endpoint-excluded-with-string`)).text()).toBe('{"success":true}');
 
-  let transactionPromiseReceived = false;
-  transactionPromise.then(() => {
-    transactionPromiseReceived = true;
-  });
-
-  await new Promise(resolve => setTimeout(resolve, 5_000));
+  const transaction = await transactionPromise;
 
-  expect(transactionPromiseReceived).toBe(false);
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).toBeDefined();
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).not.toBe('auto.http.nextjs'); // This is the origin set by the build time instrumentation
 });
 
-test('should not automatically create transactions for routes that were excluded from auto wrapping (regex)', async ({
+test('should not apply build-time instrumentation for routes that were excluded from auto wrapping (regex)', async ({
   request,
 }) => {
   const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
@@ -35,12 +31,8 @@ test('should not automatically create transactions for routes that were excluded
 
   expect(await (await request.get(`/api/endpoint-excluded-with-regex`)).text()).toBe('{"success":true}');
 
-  let transactionPromiseReceived = false;
-  transactionPromise.then(() => {
-    transactionPromiseReceived = true;
-  });
-
-  await new Promise(resolve => setTimeout(resolve, 5_000));
+  const transaction = await transactionPromise;
 
-  expect(transactionPromiseReceived).toBe(false);
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).toBeDefined();
+  expect(transaction.contexts?.trace?.data?.['sentry.origin']).not.toBe('auto.http.nextjs'); // This is the origin set by the build time instrumentation
 });
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.ts
@@ -8,12 +8,12 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
 
   const transactionEventPromise = waitForTransaction('nextjs-13', transactionEvent => {
     return (
-      transactionEvent.transaction === '/error-getServerSideProps' &&
+      transactionEvent.transaction === 'GET /[param]/error-getServerSideProps' &&
       transactionEvent.contexts?.trace?.op === 'http.server'
     );
   });
 
-  await page.goto('/error-getServerSideProps');
+  await page.goto('/dogsaregreat/error-getServerSideProps');
 
   expect(await errorEventPromise).toMatchObject({
     contexts: {
@@ -40,7 +40,7 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
       url: expect.stringMatching(/^http.*\/error-getServerSideProps/),
     },
     timestamp: expect.any(Number),
-    transaction: 'getServerSideProps (/error-getServerSideProps)',
+    transaction: 'getServerSideProps (/[param]/error-getServerSideProps)',
   });
 
   expect(await transactionEventPromise).toMatchObject({
@@ -60,11 +60,11 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
         data: {
           'http.response.status_code': 500,
           'sentry.op': 'http.server',
-          'sentry.origin': 'auto.function.nextjs',
+          'sentry.origin': 'auto',
           'sentry.source': 'route',
         },
         op: 'http.server',
-        origin: 'auto.function.nextjs',
+        origin: 'auto',
         span_id: expect.any(String),
         status: 'internal_error',
         trace_id: expect.any(String),
@@ -80,7 +80,7 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
     },
     start_timestamp: expect.any(Number),
     timestamp: expect.any(Number),
-    transaction: '/error-getServerSideProps',
+    transaction: 'GET /[param]/error-getServerSideProps',
     transaction_info: { source: 'route' },
     type: 'transaction',
   });
@@ -95,11 +95,12 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
 
   const transactionEventPromise = waitForTransaction('nextjs-13', transactionEvent => {
     return (
-      transactionEvent.transaction === '/customPageExtension' && transactionEvent.contexts?.trace?.op === 'http.server'
+      transactionEvent.transaction === 'GET /[param]/customPageExtension' &&
+      transactionEvent.contexts?.trace?.op === 'http.server'
     );
   });
 
-  await page.goto('/customPageExtension');
+  await page.goto('/123/customPageExtension');
 
   expect(await errorEventPromise).toMatchObject({
     contexts: {
@@ -126,7 +127,7 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
       url: expect.stringMatching(/^http.*\/customPageExtension/),
     },
     timestamp: expect.any(Number),
-    transaction: 'getServerSideProps (/customPageExtension)',
+    transaction: 'getServerSideProps (/[param]/customPageExtension)',
   });
 
   expect(await transactionEventPromise).toMatchObject({
@@ -146,11 +147,11 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
         data: {
           'http.response.status_code': 500,
           'sentry.op': 'http.server',
-          'sentry.origin': 'auto.function.nextjs',
+          'sentry.origin': 'auto',
           'sentry.source': 'route',
         },
         op: 'http.server',
-        origin: 'auto.function.nextjs',
+        origin: 'auto',
         span_id: expect.any(String),
         status: 'internal_error',
         trace_id: expect.any(String),
@@ -166,7 +167,7 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
     },
     start_timestamp: expect.any(Number),
     timestamp: expect.any(Number),
-    transaction: '/customPageExtension',
+    transaction: 'GET /[param]/customPageExtension',
     transaction_info: { source: 'route' },
     type: 'transaction',
   });

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx
@@ -9,31 +9,49 @@ export default function Layout({ children }: { children: React.ReactNode }) {
           <h1>Layout (/)</h1>
           <ul>
             <li>
-              <Link href="/">/</Link>
+              <Link href="/" prefetch={false}>
+                /
+              </Link>
             </li>
             <li>
-              <Link href="/client-component">/client-component</Link>
+              <Link href="/client-component" prefetch={false}>
+                /client-component
+              </Link>
             </li>
             <li>
-              <Link href="/client-component/parameter/42">/client-component/parameter/42</Link>
+              <Link href="/client-component/parameter/42" prefetch={false}>
+                /client-component/parameter/42
+              </Link>
             </li>
             <li>
-              <Link href="/client-component/parameter/foo/bar/baz">/client-component/parameter/foo/bar/baz</Link>
+              <Link href="/client-component/parameter/foo/bar/baz" prefetch={false}>
+                /client-component/parameter/foo/bar/baz
+              </Link>
             </li>
             <li>
-              <Link href="/server-component">/server-component</Link>
+              <Link href="/server-component" prefetch={false}>
+                /server-component
+              </Link>
             </li>
             <li>
-              <Link href="/server-component/parameter/42">/server-component/parameter/42</Link>
+              <Link href="/server-component/parameter/42" prefetch={false}>
+                /server-component/parameter/42
+              </Link>
             </li>
             <li>
-              <Link href="/server-component/parameter/foo/bar/baz">/server-component/parameter/foo/bar/baz</Link>
+              <Link href="/server-component/parameter/foo/bar/baz" prefetch={false}>
+                /server-component/parameter/foo/bar/baz
+              </Link>
             </li>
             <li>
-              <Link href="/not-found">/not-found</Link>
+              <Link href="/not-found" prefetch={false}>
+                /not-found
+              </Link>
             </li>
             <li>
-              <Link href="/redirect">/redirect</Link>
+              <Link href="/redirect" prefetch={false}>
+                /redirect
+              </Link>
             </li>
           </ul>
           <SpanContextProvider>{children}</SpanContextProvider>

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts
@@ -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'],
 };
diff --git a/...kages/e2e-tests/test-applications/nextjs-app-dir/pages/api/async-context-edge-endpoint.ts b/...kages/e2e-tests/test-applications/nextjs-app-dir/pages/api/async-context-edge-endpoint.ts
@@ -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 });
 }
diff --git a/...e2e-tests/test-applications/nextjs-app-dir/pages/api/endpoint-behind-faulty-middleware.ts b/...e2e-tests/test-applications/nextjs-app-dir/pages/api/endpoint-behind-faulty-middleware.ts
@@ -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' });
+}