From 862b4121a805fc4ec8305538ecaad4858826890b Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Thu, 1 Sep 2022 14:41:14 +0000
Subject: [PATCH 01/13] feat(nextjs): Trace pageload transitions

---
 packages/nextjs/src/config/webpack.ts     |  21 +++++
 packages/nextjs/src/performance/client.ts | 104 ++++++----------------
 2 files changed, 50 insertions(+), 75 deletions(-)

diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts
index 84650b80e3c7..a727161840b1 100644
--- a/packages/nextjs/src/config/webpack.ts
+++ b/packages/nextjs/src/config/webpack.ts
@@ -57,6 +57,27 @@ export function constructWebpackConfigFunction(
       newConfig = userNextConfig.webpack(newConfig, buildContext);
     }
 
+    // TODO: Build folder is not always .next/
+    const routeManifestFileContents = fs.readFileSync(path.join(process.cwd(), './.next/routes-manifest.json'), {
+      encoding: 'utf8',
+    });
+
+    const routeManifest = JSON.parse(routeManifestFileContents);
+    const routeTable = {};
+
+    // @ts-ignore
+    routeManifest.dynamicRoutes.forEach(({ regex, page }) => {
+      // @ts-ignore
+      routeTable[page] = regex;
+    });
+
+    newConfig.plugins?.push(
+      // @ts-ignore asdf
+      new buildContext.webpack.DefinePlugin({
+        __INJECTED_ROUTE_TABLE__: JSON.stringify(routeTable),
+      }),
+    );
+
     if (isServer) {
       newConfig.module = {
         ...newConfig.module,
diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index 257c21c429bf..385e54842b21 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -1,10 +1,9 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
 import { getCurrentHub } from '@sentry/hub';
-import { Primitive, TraceparentData, Transaction, TransactionContext } from '@sentry/types';
+import { Primitive, TraceparentData, Transaction, TransactionContext, TransactionSource } from '@sentry/types';
 import {
   extractTraceparentData,
-  fill,
   getGlobalObject,
   logger,
   parseBaggageHeader,
@@ -18,6 +17,9 @@ const global = getGlobalObject<Window>();
 
 type StartTransactionCb = (context: TransactionContext) => Transaction | undefined;
 
+declare const __INJECTED_ROUTE_TABLE__: { [parameterizedRoute: string]: string } | undefined;
+const routeTable = typeof __INJECTED_ROUTE_TABLE__ === 'undefined' ? undefined : __INJECTED_ROUTE_TABLE__;
+
 /**
  * Describes data located in the __NEXT_DATA__ script tag. This tag is present on every page of a Next.js app.
  */
@@ -97,16 +99,6 @@ const DEFAULT_TAGS = {
 } as const;
 
 let activeTransaction: Transaction | undefined = undefined;
-let startTransaction: StartTransactionCb | undefined = undefined;
-
-// We keep track of the previous page location so we can avoid creating transactions when navigating to the same page.
-// This variable should always contain a pathname. (without query string or fragment)
-// We are making a tradeoff by not starting transactions when just the query string changes. One could argue that we
-// should in fact start transactions when the query changes, however, in some cases (for example when typing in a search
-// box) the query might change multiple times a second, resulting in way too many transactions.
-// Because we currently don't have a real way of preventing transactions to be created in this case (except for the
-// shotgun approach `startTransactionOnLocationChange: false`), we won't start transactions when *just* the query changes.
-let previousLocation: string | undefined = undefined;
 
 // We keep track of the previous transaction name so we can set the `from` field on navigation transactions.
 let prevTransactionName: string | undefined = undefined;
@@ -126,13 +118,10 @@ export function nextRouterInstrumentation(
   startTransactionOnPageLoad: boolean = true,
   startTransactionOnLocationChange: boolean = true,
 ): void {
-  startTransaction = startTransactionCb;
-
   if (startTransactionOnPageLoad) {
     const { route, traceParentData, baggage, params } = extractNextDataTagInformation();
 
     prevTransactionName = route || global.location.pathname;
-    previousLocation = global.location.pathname;
 
     const source = route ? 'route' : 'url';
 
@@ -149,55 +138,25 @@ export function nextRouterInstrumentation(
     });
   }
 
-  Router.ready(() => {
-    // Spans that aren't attached to any transaction are lost; so if transactions aren't
-    // created (besides potentially the onpageload transaction), no need to wrap the router.
-    if (!startTransactionOnLocationChange) return;
-
-    // `withRouter` uses `useRouter` underneath:
-    // https://github.com/vercel/next.js/blob/de42719619ae69fbd88e445100f15701f6e1e100/packages/next/client/with-router.tsx#L21
-    // Router events also use the router:
-    // https://github.com/vercel/next.js/blob/de42719619ae69fbd88e445100f15701f6e1e100/packages/next/client/router.ts#L92
-    // `Router.changeState` handles the router state changes, so it may be enough to only wrap it
-    // (instead of wrapping all of the Router's functions).
-    const routerPrototype = Object.getPrototypeOf(Router.router);
-    fill(routerPrototype, 'changeState', changeStateWrapper);
-  });
-}
-
-type RouterChangeState = (
-  method: string,
-  url: string,
-  as: string,
-  options: Record<string, any>,
-  ...args: any[]
-) => void;
-type WrappedRouterChangeState = RouterChangeState;
+  if (startTransactionOnLocationChange) {
+    Router.events.on('routeChangeStart', (pathname: string) => {
+      function getNavigationTargetName(): [string, TransactionSource] {
+        if (routeTable) {
+          const match = Object.entries(routeTable).find(([, routeRegExpr]) => {
+            return pathname.match(new RegExp(routeRegExpr));
+          });
+
+          if (match) {
+            return [match[0], 'route'];
+          } else {
+            return [stripUrlQueryAndFragment(pathname), 'route'];
+          }
+        } else {
+          return [stripUrlQueryAndFragment(pathname), 'url'];
+        }
+      }
 
-/**
- * Wraps Router.changeState()
- * https://github.com/vercel/next.js/blob/da97a18dafc7799e63aa7985adc95f213c2bf5f3/packages/next/next-server/lib/router/router.ts#L1204
- * Start a navigation transaction every time the router changes state.
- */
-function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): WrappedRouterChangeState {
-  return function wrapper(
-    this: any,
-    method: string,
-    // The parameterized url, ex. posts/[id]/[comment]
-    url: string,
-    // The actual url, ex. posts/85/my-comment
-    as: string,
-    options: Record<string, any>,
-    // At the moment there are no additional arguments (meaning the rest parameter is empty).
-    // This is meant to protect from future additions to Next.js API, especially since this is an
-    // internal API.
-    ...args: any[]
-  ): Promise<boolean> {
-    const newTransactionName = stripUrlQueryAndFragment(url);
-
-    // do not start a transaction if it's from the same page
-    if (startTransaction !== undefined && previousLocation !== as) {
-      previousLocation = as;
+      const [newTransactionName, source] = getNavigationTargetName();
 
       if (activeTransaction) {
         activeTransaction.finish();
@@ -205,22 +164,17 @@ function changeStateWrapper(originalChangeStateWrapper: RouterChangeState): Wrap
 
       const tags: Record<string, Primitive> = {
         ...DEFAULT_TAGS,
-        method,
-        ...options,
+        from: prevTransactionName,
       };
 
-      if (prevTransactionName) {
-        tags.from = prevTransactionName;
-      }
-
       prevTransactionName = newTransactionName;
-      activeTransaction = startTransaction({
-        name: prevTransactionName,
+
+      startTransactionCb({
+        name: newTransactionName,
         op: 'navigation',
         tags,
-        metadata: { source: 'route' },
+        metadata: { source },
       });
-    }
-    return originalChangeStateWrapper.call(this, method, url, as, options, ...args);
-  };
+    });
+  }
 }

From 1713ab18581d43325a9751bd87af973191f46ddf Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 08:07:59 +0000
Subject: [PATCH 02/13] Implement solution with way smaller bundlesize
 footprint using `__BUILD_MANIFEST`

---
 packages/browser/src/transports/fetch.ts  |  5 ++
 packages/nextjs/src/config/webpack.ts     | 21 -------
 packages/nextjs/src/performance/client.ts | 77 ++++++++++++++---------
 3 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts
index 2d7bea31ae6a..3a8c0230b7a4 100644
--- a/packages/browser/src/transports/fetch.ts
+++ b/packages/browser/src/transports/fetch.ts
@@ -17,6 +17,11 @@ export function makeFetchTransport(
       method: 'POST',
       referrerPolicy: 'origin',
       headers: options.headers,
+      // Outgoing requests are usually cancelled when navigating to a different page, causing a "TypeError: Failed to
+      // fetch" error and sending a "network_error" client-outcome. In chrome the request status shows "(cancelled)".
+      // The `keepalive` flag keeps outgoing requests alive, even when switching pages. We want this since we're
+      // frequently sending events (i.e.navigation transactions) right before the user is switching pages.
+      keepalive: true,
       ...options.fetchOptions,
     };
 
diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts
index a727161840b1..84650b80e3c7 100644
--- a/packages/nextjs/src/config/webpack.ts
+++ b/packages/nextjs/src/config/webpack.ts
@@ -57,27 +57,6 @@ export function constructWebpackConfigFunction(
       newConfig = userNextConfig.webpack(newConfig, buildContext);
     }
 
-    // TODO: Build folder is not always .next/
-    const routeManifestFileContents = fs.readFileSync(path.join(process.cwd(), './.next/routes-manifest.json'), {
-      encoding: 'utf8',
-    });
-
-    const routeManifest = JSON.parse(routeManifestFileContents);
-    const routeTable = {};
-
-    // @ts-ignore
-    routeManifest.dynamicRoutes.forEach(({ regex, page }) => {
-      // @ts-ignore
-      routeTable[page] = regex;
-    });
-
-    newConfig.plugins?.push(
-      // @ts-ignore asdf
-      new buildContext.webpack.DefinePlugin({
-        __INJECTED_ROUTE_TABLE__: JSON.stringify(routeTable),
-      }),
-    );
-
     if (isServer) {
       newConfig.module = {
         ...newConfig.module,
diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index 385e54842b21..52d3d491c175 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -1,5 +1,3 @@
-/* eslint-disable @typescript-eslint/no-explicit-any */
-
 import { getCurrentHub } from '@sentry/hub';
 import { Primitive, TraceparentData, Transaction, TransactionContext, TransactionSource } from '@sentry/types';
 import {
@@ -13,13 +11,16 @@ import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
 import { default as Router } from 'next/router';
 import type { ParsedUrlQuery } from 'querystring';
 
-const global = getGlobalObject<Window>();
+const global = getGlobalObject<
+  Window & {
+    __BUILD_MANIFEST?: {
+      sortedPages?: string[];
+    };
+  }
+>();
 
 type StartTransactionCb = (context: TransactionContext) => Transaction | undefined;
 
-declare const __INJECTED_ROUTE_TABLE__: { [parameterizedRoute: string]: string } | undefined;
-const routeTable = typeof __INJECTED_ROUTE_TABLE__ === 'undefined' ? undefined : __INJECTED_ROUTE_TABLE__;
-
 /**
  * Describes data located in the __NEXT_DATA__ script tag. This tag is present on every page of a Next.js app.
  */
@@ -139,27 +140,18 @@ export function nextRouterInstrumentation(
   }
 
   if (startTransactionOnLocationChange) {
-    Router.events.on('routeChangeStart', (pathname: string) => {
-      function getNavigationTargetName(): [string, TransactionSource] {
-        if (routeTable) {
-          const match = Object.entries(routeTable).find(([, routeRegExpr]) => {
-            return pathname.match(new RegExp(routeRegExpr));
-          });
-
-          if (match) {
-            return [match[0], 'route'];
-          } else {
-            return [stripUrlQueryAndFragment(pathname), 'route'];
-          }
-        } else {
-          return [stripUrlQueryAndFragment(pathname), 'url'];
-        }
-      }
-
-      const [newTransactionName, source] = getNavigationTargetName();
-
-      if (activeTransaction) {
-        activeTransaction.finish();
+    Router.events.on('routeChangeStart', (navigationTarget: string) => {
+      const matchedRoute = getNextRouteFromPathname(stripUrlQueryAndFragment(navigationTarget));
+
+      let transactionName: string;
+      let transactionSource: TransactionSource;
+
+      if (matchedRoute) {
+        transactionName = matchedRoute;
+        transactionSource = 'route';
+      } else {
+        transactionName = navigationTarget;
+        transactionSource = 'url';
       }
 
       const tags: Record<string, Primitive> = {
@@ -167,14 +159,39 @@ export function nextRouterInstrumentation(
         from: prevTransactionName,
       };
 
-      prevTransactionName = newTransactionName;
+      prevTransactionName = transactionName;
+
+      if (activeTransaction) {
+        activeTransaction.finish();
+      }
 
       startTransactionCb({
-        name: newTransactionName,
+        name: transactionName,
         op: 'navigation',
         tags,
-        metadata: { source },
+        metadata: { source: transactionSource },
       });
     });
   }
 }
+
+function getNextRouteFromPathname(pathname: string): string | void {
+  const pageRoutes = (global.__BUILD_MANIFEST || {}).sortedPages;
+
+  // Page route should in 99.999% of the cases be defined by now but just to be sure we make a check here
+  if (pageRoutes) {
+    return pageRoutes.find(route => {
+      const routeRegExp = convertNextRouteToRegExp(route);
+      return pathname.match(routeRegExp);
+    });
+  }
+}
+
+function convertNextRouteToRegExp(route: string): RegExp {
+  return new RegExp(
+    `^${route
+      .split('/')
+      .map(routePart => routePart.replace(/^\[.*\]$/, '.*'))
+      .join('/')}$`,
+  );
+}

From 89eea47a2e0287922614cbd0276577f25f0a8445 Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 08:16:58 +0000
Subject: [PATCH 03/13] Add a TODO

---
 packages/nextjs/src/performance/client.ts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index 52d3d491c175..b1c207718aed 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -79,6 +79,8 @@ function extractNextDataTagInformation(): NextDataTagInfo {
   // `nextData.page` always contains the parameterized route - except for when an error occurs in a data fetching
   // function, then it is "/_error", but that isn't a problem since users know which route threw by looking at the
   // parent transaction
+  // TODO: Actually this is a (even though it is not that big), because the DSC and the transaction payload will contain
+  // a different transaction name.Maybe we can fix this.Idea: Also send transaction name via pageProps when available.
   nextDataTagInfo.route = page;
   nextDataTagInfo.params = query;
 

From 3f8374dd5f05e14f74fafb5df8a9369f602a1795 Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 12:52:09 +0000
Subject: [PATCH 04/13] Fix route matching and add tests

---
 packages/nextjs/src/performance/client.ts     |  44 ++-
 .../nextjs/test/performance/client.test.ts    | 295 +++++++++---------
 2 files changed, 178 insertions(+), 161 deletions(-)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index b1c207718aed..2c2c56b4093e 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -101,10 +101,12 @@ const DEFAULT_TAGS = {
   'routing.instrumentation': 'next-router',
 } as const;
 
+// We keep track of the active transaction so we can finish it when we start a navigation transaction.
 let activeTransaction: Transaction | undefined = undefined;
 
-// We keep track of the previous transaction name so we can set the `from` field on navigation transactions.
-let prevTransactionName: string | undefined = undefined;
+// We keep track of the previous location name so we can set the `from` field on navigation transactions.
+// This is either a route or a pathname.
+let prevLocationName: string | undefined = undefined;
 
 const client = getCurrentHub().getClient();
 
@@ -121,15 +123,14 @@ export function nextRouterInstrumentation(
   startTransactionOnPageLoad: boolean = true,
   startTransactionOnLocationChange: boolean = true,
 ): void {
-  if (startTransactionOnPageLoad) {
-    const { route, traceParentData, baggage, params } = extractNextDataTagInformation();
-
-    prevTransactionName = route || global.location.pathname;
+  const { route, traceParentData, baggage, params } = extractNextDataTagInformation();
+  prevLocationName = route || global.location.pathname;
 
+  if (startTransactionOnPageLoad) {
     const source = route ? 'route' : 'url';
 
     activeTransaction = startTransactionCb({
-      name: prevTransactionName,
+      name: prevLocationName,
       op: 'pageload',
       tags: DEFAULT_TAGS,
       ...(params && client && client.getOptions().sendDefaultPii && { data: params }),
@@ -158,10 +159,10 @@ export function nextRouterInstrumentation(
 
       const tags: Record<string, Primitive> = {
         ...DEFAULT_TAGS,
-        from: prevTransactionName,
+        from: prevLocationName,
       };
 
-      prevTransactionName = transactionName;
+      prevLocationName = transactionName;
 
       if (activeTransaction) {
         activeTransaction.finish();
@@ -190,10 +191,27 @@ function getNextRouteFromPathname(pathname: string): string | void {
 }
 
 function convertNextRouteToRegExp(route: string): RegExp {
+  // We can assume a route is at least "/".
+  const routeParts = route.split('/');
+
+  let optionalCatchallWildcardRegex = '';
+  if (routeParts[routeParts.length - 1].match(/^\[\[\.\.\..+\]\]$/)) {
+    // If last route part has pattern "[[...xyz]]"
+    // We pop the latest route part to get rid of the required trailing slash
+    routeParts.pop();
+    optionalCatchallWildcardRegex = '(?:/(.+?))?';
+  }
+
+  const rejoinedRouteParts = routeParts
+    .map(
+      routePart =>
+        routePart
+          .replace(/^\[\.\.\..+\]$/, '(.+?)') // Replace catch all wildcard with regex wildcard
+          .replace(/^\[.*\]$/, '([^/]+?)'), // Replace route wildcards with lazy regex wildcards
+    )
+    .join('/');
+
   return new RegExp(
-    `^${route
-      .split('/')
-      .map(routePart => routePart.replace(/^\[.*\]$/, '.*'))
-      .join('/')}$`,
+    `^${rejoinedRouteParts}${optionalCatchallWildcardRegex}(?:/)?$`, // optional slash at the end
   );
 }
diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts
index 279c39ed90fc..5e108dfb4e74 100644
--- a/packages/nextjs/test/performance/client.test.ts
+++ b/packages/nextjs/test/performance/client.test.ts
@@ -5,84 +5,98 @@ import { default as Router } from 'next/router';
 
 import { nextRouterInstrumentation } from '../../src/performance/client';
 
-let readyCalled = false;
+const globalObject = getGlobalObject<
+  Window & {
+    __BUILD_MANIFEST?: {
+      sortedPages?: string[];
+    };
+  }
+>();
+
+const originalBuildManifest = globalObject.__BUILD_MANIFEST;
+const originalBuildManifestRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
+
 jest.mock('next/router', () => {
-  const router = {};
-  Object.setPrototypeOf(router, { changeState: () => undefined });
+  const eventHandlers: { [eventName: string]: ((...args: any[]) => void)[] } = {};
   return {
     default: {
-      router,
-      route: '/[user]/posts/[id]',
-      readyCallbacks: [],
-      ready(cb: () => void) {
-        readyCalled = true;
-        return cb();
+      events: {
+        on(type: string, handler: (...args: any[]) => void) {
+          if (eventHandlers[type]) {
+            eventHandlers[type].push(handler);
+          } else {
+            eventHandlers[type] = [handler];
+          }
+        },
+        emit(type: string, ...eventArgs: any[]) {
+          if (eventHandlers[type]) {
+            eventHandlers[type].forEach(eventHandler => {
+              eventHandler(...eventArgs);
+            });
+          }
+        },
       },
     },
   };
 });
 
-describe('client', () => {
-  beforeEach(() => {
-    readyCalled = false;
-    if (Router.router) {
-      Router.router.changeState('pushState', '/[user]/posts/[id]', '/abhi/posts/123', {});
-    }
-  });
+describe('nextRouterInstrumentation', () => {
+  const originalGlobalDocument = getGlobalObject<Window>().document;
+  const originalGlobalLocation = getGlobalObject<Window>().location;
 
-  describe('nextRouterInstrumentation', () => {
-    const originalGlobalDocument = getGlobalObject<Window>().document;
-    const originalGlobalLocation = getGlobalObject<Window>().location;
+  function setUpNextPage(pageProperties: {
+    url: string;
+    route: string;
+    query?: any;
+    props?: any;
+    navigatableRoutes?: string[];
+    hasNextData: boolean;
+  }) {
+    const nextDataContent: NextData = {
+      props: pageProperties.props,
+      page: pageProperties.route,
+      query: pageProperties.query,
+      buildId: 'y76hvndNJBAithejdVGLW',
+      isFallback: false,
+      gssp: true,
+      appGip: true,
+      scriptLoader: [],
+    };
 
-    function setUpNextPage(pageProperties: {
-      url: string;
-      route: string;
-      query: any;
-      props: any;
-      hasNextData: boolean;
-    }) {
-      const nextDataContent: NextData = {
-        props: pageProperties.props,
-        page: pageProperties.route,
-        query: pageProperties.query,
-        buildId: 'y76hvndNJBAithejdVGLW',
-        isFallback: false,
-        gssp: true,
-        appGip: true,
-        scriptLoader: [],
-      };
+    const dom = new JSDOM(
+      // Just an example of what a __NEXT_DATA__ tag might look like
+      pageProperties.hasNextData
+        ? `<body><script id="__NEXT_DATA__" type="application/json">${JSON.stringify(nextDataContent)}</script></body>`
+        : '<body><h1>No next data :(</h1></body>',
+      { url: pageProperties.url },
+    );
 
-      const dom = new JSDOM(
-        // Just an example of what a __NEXT_DATA__ tag might look like
-        pageProperties.hasNextData
-          ? `<body><script id="__NEXT_DATA__" type="application/json">${JSON.stringify(
-              nextDataContent,
-            )}</script></body>`
-          : '<body><h1>No next data :(</h1></body>',
-        { url: pageProperties.url },
-      );
+    // The Next.js routing instrumentations requires a few things to be present on pageload:
+    // 1. Access to window.document API for `window.document.getElementById`
+    // 2. Access to window.location API for `window.location.pathname`
+    Object.defineProperty(global, 'document', { value: dom.window.document, writable: true });
+    Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });
 
-      // The Next.js routing instrumentations requires a few things to be present on pageload:
-      // 1. Access to window.document API for `window.document.getElementById`
-      // 2. Access to window.location API for `window.location.pathname`
-      Object.defineProperty(global, 'document', { value: dom.window.document, writable: true });
-      Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });
-    }
+    // Define Next.js clientside build manifest with navigatable routes
+    (global as any).__BUILD_MANIFEST = {
+      ...(global as any).__BUILD_MANIFEST,
+      sortedPages: pageProperties.navigatableRoutes,
+    };
+  }
 
-    afterEach(() => {
-      // Clean up JSDom
-      Object.defineProperty(global, 'document', { value: originalGlobalDocument });
-      Object.defineProperty(global, 'location', { value: originalGlobalLocation });
-    });
+  afterEach(() => {
+    // Clean up JSDom
+    Object.defineProperty(global, 'document', { value: originalGlobalDocument });
+    Object.defineProperty(global, 'location', { value: originalGlobalLocation });
 
-    it('waits for Router.ready()', () => {
-      setUpNextPage({ url: 'https://example.com/', route: '/', query: {}, props: {}, hasNextData: false });
-      const mockStartTransaction = jest.fn();
-      expect(readyCalled).toBe(false);
-      nextRouterInstrumentation(mockStartTransaction);
-      expect(readyCalled).toBe(true);
-    });
+    // Reset Next.js' __BUILD_MANIFEST
+    (global as any).__BUILD_MANIFEST = originalBuildManifest;
+    if ((global as any).__BUILD_MANIFEST) {
+      (global as any).__BUILD_MANIFEST.sortedPages = originalBuildManifestRoutes;
+    }
+  });
 
+  describe('pageload transactions', () => {
     it.each([
       [
         'https://example.com/lforst/posts/1337?q=42',
@@ -183,103 +197,88 @@ describe('client', () => {
 
     it('does not create a pageload transaction if option not given', () => {
       const mockStartTransaction = jest.fn();
+      setUpNextPage({ url: 'https://example.com/', route: '/', hasNextData: false });
       nextRouterInstrumentation(mockStartTransaction, false);
       expect(mockStartTransaction).toHaveBeenCalledTimes(0);
     });
+  });
 
-    describe('navigation transactions', () => {
-      // [name, in, out]
-      const table: Array<[string, [string, string, string, Record<string, unknown>], Record<string, unknown>]> = [
-        [
-          'creates parameterized transaction',
-          ['pushState', '/posts/[id]', '/posts/32', {}],
-          {
-            name: '/posts/[id]',
-            op: 'navigation',
-            tags: {
-              from: '/[user]/posts/[id]',
-              method: 'pushState',
-              'routing.instrumentation': 'next-router',
-            },
-            metadata: {
-              source: 'route',
-            },
-          },
-        ],
-        [
-          'strips query parameters',
-          ['replaceState', '/posts/[id]?name=cat', '/posts/32?name=cat', {}],
-          {
-            name: '/posts/[id]',
-            op: 'navigation',
-            tags: {
-              from: '/[user]/posts/[id]',
-              method: 'replaceState',
-              'routing.instrumentation': 'next-router',
-            },
-            metadata: {
-              source: 'route',
-            },
-          },
-        ],
-        [
-          'creates regular transactions',
-          ['pushState', '/about', '/about', {}],
-          {
-            name: '/about',
-            op: 'navigation',
-            tags: {
-              from: '/[user]/posts/[id]',
-              method: 'pushState',
-              'routing.instrumentation': 'next-router',
-            },
-            metadata: {
-              source: 'route',
-            },
-          },
-        ],
-      ];
-
-      it.each(table)('%s', (...test) => {
+  describe('new navigation transactions', () => {
+    it.each([
+      ['/news', '/news', 'route'],
+      ['/news/', '/news', 'route'],
+      ['/some-route-that-is-not-defined-12332', '/some-route-that-is-not-defined-12332', 'url'], // unknown route
+      ['/posts/42', '/posts/[id]', 'route'],
+      ['/posts/42/', '/posts/[id]', 'route'],
+      ['/posts/42?someParam=1', '/posts/[id]', 'route'], // query params are ignored
+      ['/posts/42/details', '/posts/[id]/details', 'route'],
+      ['/users/1337/friends/closeby/good', '/users/[id]/friends/[...filters]', 'route'],
+      ['/users/1337/friends', '/users/1337/friends', 'url'],
+      ['/statistics/page-visits', '/statistics/[[...parameters]]', 'route'],
+      ['/statistics', '/statistics/[[...parameters]]', 'route'],
+      ['/a/b/c/d', '/[a]/b/[c]/[...d]', 'route'],
+      ['/a/b/c/d/e', '/[a]/b/[c]/[...d]', 'route'],
+      ['/a/b/c', '/a/b/c', 'url'],
+      ['/e/f/g/h', '/e/[f]/[g]/[[...h]]', 'route'],
+      ['/e/f/g/h/i', '/e/[f]/[g]/[[...h]]', 'route'],
+      ['/e/f/g', '/e/[f]/[g]/[[...h]]', 'route'],
+    ])(
+      'should create a parameterized transaction on route change (%s)',
+      (targetLocation, expectedTransactionName, expectedTransactionSource) => {
         const mockStartTransaction = jest.fn();
-        nextRouterInstrumentation(mockStartTransaction, false);
-        expect(mockStartTransaction).toHaveBeenCalledTimes(0);
 
-        // @ts-ignore we can spread into test
-        Router.router?.changeState(...test[1]);
-        expect(mockStartTransaction).toHaveBeenLastCalledWith(test[2]);
-      });
-    });
+        setUpNextPage({
+          url: 'https://example.com/home',
+          route: '/home',
+          hasNextData: true,
+          navigatableRoutes: [
+            '/home',
+            '/news',
+            '/posts/[id]',
+            '/posts/[id]/details',
+            '/users/[id]/friends/[...filters]',
+            '/statistics/[[...parameters]]',
+            // just some complicated routes to see if we get the matching right
+            '/[a]/b/[c]/[...d]',
+            '/e/[f]/[g]/[[...h]]',
+          ],
+        });
+
+        nextRouterInstrumentation(mockStartTransaction, false, true);
+
+        Router.events.emit('routeChangeStart', targetLocation);
 
-    it('does not create navigation transaction with the same name', () => {
+        expect(mockStartTransaction).toHaveBeenCalledTimes(1);
+        expect(mockStartTransaction).toHaveBeenCalledWith(
+          expect.objectContaining({
+            name: expectedTransactionName,
+            op: 'navigation',
+            tags: expect.objectContaining({
+              'routing.instrumentation': 'next-router',
+            }),
+            metadata: expect.objectContaining({
+              source: expectedTransactionSource,
+            }),
+          }),
+        );
+      },
+    );
+
+    it('should not create transaction when navigation transactions are disabled', () => {
       const mockStartTransaction = jest.fn();
-      nextRouterInstrumentation(mockStartTransaction, false);
-      expect(mockStartTransaction).toHaveBeenCalledTimes(0);
 
-      Router.router?.changeState('pushState', '/login', '/login', {});
-      expect(mockStartTransaction).toHaveBeenCalledTimes(1);
-      expect(mockStartTransaction).toHaveBeenLastCalledWith({
-        name: '/login',
-        op: 'navigation',
-        tags: { from: '/[user]/posts/[id]', method: 'pushState', 'routing.instrumentation': 'next-router' },
-        metadata: {
-          source: 'route',
-        },
+      setUpNextPage({
+        url: 'https://example.com/home',
+        route: '/home',
+        hasNextData: true,
+        navigatableRoutes: ['/home', '/posts/[id]'],
       });
 
-      Router.router?.changeState('pushState', '/login', '/login', {});
-      expect(mockStartTransaction).toHaveBeenCalledTimes(1);
+      nextRouterInstrumentation(mockStartTransaction, false, false);
 
-      Router.router?.changeState('pushState', '/posts/[id]', '/posts/123', {});
-      expect(mockStartTransaction).toHaveBeenCalledTimes(2);
-      expect(mockStartTransaction).toHaveBeenLastCalledWith({
-        name: '/posts/[id]',
-        op: 'navigation',
-        tags: { from: '/login', method: 'pushState', 'routing.instrumentation': 'next-router' },
-        metadata: {
-          source: 'route',
-        },
-      });
+      Router.events.emit('routeChangeStart', '/posts/42');
+
+      expect(mockStartTransaction).not.toHaveBeenCalled();
     });
   });
 });

From f7eb469af1f2d545878f1553217f296482bdd90e Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 13:08:15 +0000
Subject: [PATCH 05/13] Update integration tests

---
 .../integration/pages/[id]/withInitialProps.tsx  | 11 ++++++++++-
 .../pages/[id]/withServerSideProps.tsx           | 11 ++++++++++-
 .../integration/test/client/tracingNavigate.js   | 16 ++++++++--------
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/packages/nextjs/test/integration/pages/[id]/withInitialProps.tsx b/packages/nextjs/test/integration/pages/[id]/withInitialProps.tsx
index 01b557bdd09f..12558228b23c 100644
--- a/packages/nextjs/test/integration/pages/[id]/withInitialProps.tsx
+++ b/packages/nextjs/test/integration/pages/[id]/withInitialProps.tsx
@@ -1,4 +1,13 @@
-const WithInitialPropsPage = ({ data }: { data: string }) => <h1>WithInitialPropsPage {data}</h1>;
+import Link from 'next/link';
+
+const WithInitialPropsPage = ({ data }: { data: string }) => (
+  <>
+    <h1>WithInitialPropsPage {data}</h1>
+    <Link href="/1337/withServerSideProps">
+      <a id="server-side-props-page">Go to withServerSideProps</a>
+    </Link>
+  </>
+);
 
 WithInitialPropsPage.getInitialProps = () => {
   return { data: '[some getInitialProps data]' };
diff --git a/packages/nextjs/test/integration/pages/[id]/withServerSideProps.tsx b/packages/nextjs/test/integration/pages/[id]/withServerSideProps.tsx
index 486313a9b9c3..f5b5a3b3465c 100644
--- a/packages/nextjs/test/integration/pages/[id]/withServerSideProps.tsx
+++ b/packages/nextjs/test/integration/pages/[id]/withServerSideProps.tsx
@@ -1,4 +1,13 @@
-const WithServerSidePropsPage = ({ data }: { data: string }) => <h1>WithServerSidePropsPage {data}</h1>;
+import Link from 'next/link';
+
+const WithServerSidePropsPage = ({ data }: { data: string }) => (
+  <>
+    <h1>WithServerSidePropsPage {data}</h1>
+    <Link href="/3c2e87573d/withInitialProps">
+      <a id="initial-props-page">Go to withInitialProps</a>
+    </Link>
+  </>
+);
 
 export async function getServerSideProps() {
   return { props: { data: '[some getServerSideProps data]' } };
diff --git a/packages/nextjs/test/integration/test/client/tracingNavigate.js b/packages/nextjs/test/integration/test/client/tracingNavigate.js
index 65894f857177..327d03de58ae 100644
--- a/packages/nextjs/test/integration/test/client/tracingNavigate.js
+++ b/packages/nextjs/test/integration/test/client/tracingNavigate.js
@@ -2,11 +2,11 @@ const { sleep } = require('../utils/common');
 const { expectRequestCount, isTransactionRequest, expectTransaction } = require('../utils/client');
 
 module.exports = async ({ page, url, requests }) => {
-  await page.goto(`${url}/healthy`);
+  await page.goto(`${url}/42/withInitialProps/`);
   await page.waitForRequest(isTransactionRequest);
 
   expectTransaction(requests.transactions[0], {
-    transaction: '/healthy',
+    transaction: '/[id]/withInitialProps',
     type: 'transaction',
     contexts: {
       trace: {
@@ -17,17 +17,17 @@ module.exports = async ({ page, url, requests }) => {
 
   await sleep(250);
 
-  await page.click('a#alsoHealthy');
+  await page.click('a#server-side-props-page');
   await page.waitForRequest(isTransactionRequest);
 
   expectTransaction(requests.transactions[1], {
-    transaction: '/alsoHealthy',
+    transaction: '/[id]/withServerSideProps',
     type: 'transaction',
     contexts: {
       trace: {
         op: 'navigation',
         tags: {
-          from: '/healthy',
+          from: '/[id]/withInitialProps',
         },
       },
     },
@@ -35,17 +35,17 @@ module.exports = async ({ page, url, requests }) => {
 
   await sleep(250);
 
-  await page.click('a#healthy');
+  await page.click('a#initial-props-page');
   await page.waitForRequest(isTransactionRequest);
 
   expectTransaction(requests.transactions[2], {
-    transaction: '/healthy',
+    transaction: '/[id]/withInitialProps',
     type: 'transaction',
     contexts: {
       trace: {
         op: 'navigation',
         tags: {
-          from: '/alsoHealthy',
+          from: '/[id]/withServerSideProps',
         },
       },
     },

From da2bf3bacd40acedc654a811838ef2f299b67775 Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 13:21:37 +0000
Subject: [PATCH 06/13] Remove keepalive changes

---
 packages/browser/src/transports/fetch.ts | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts
index 3a8c0230b7a4..2d7bea31ae6a 100644
--- a/packages/browser/src/transports/fetch.ts
+++ b/packages/browser/src/transports/fetch.ts
@@ -17,11 +17,6 @@ export function makeFetchTransport(
       method: 'POST',
       referrerPolicy: 'origin',
       headers: options.headers,
-      // Outgoing requests are usually cancelled when navigating to a different page, causing a "TypeError: Failed to
-      // fetch" error and sending a "network_error" client-outcome. In chrome the request status shows "(cancelled)".
-      // The `keepalive` flag keeps outgoing requests alive, even when switching pages. We want this since we're
-      // frequently sending events (i.e.navigation transactions) right before the user is switching pages.
-      keepalive: true,
       ...options.fetchOptions,
     };
 

From a50c0d95d9863fceb10c1d5e2836424f17805367 Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 14:50:42 +0000
Subject: [PATCH 07/13] typo

---
 packages/nextjs/src/performance/client.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index 2c2c56b4093e..f6d8a9b13761 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -79,7 +79,7 @@ function extractNextDataTagInformation(): NextDataTagInfo {
   // `nextData.page` always contains the parameterized route - except for when an error occurs in a data fetching
   // function, then it is "/_error", but that isn't a problem since users know which route threw by looking at the
   // parent transaction
-  // TODO: Actually this is a (even though it is not that big), because the DSC and the transaction payload will contain
+  // TODO: Actually this is a problem (even though it is not that big), because the DSC and the transaction payload will contain
   // a different transaction name.Maybe we can fix this.Idea: Also send transaction name via pageProps when available.
   nextDataTagInfo.route = page;
   nextDataTagInfo.params = query;

From cc117306c937cb5a05497e3aec017408898545cc Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 14:50:59 +0000
Subject: [PATCH 08/13] more typo

---
 packages/nextjs/src/performance/client.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index f6d8a9b13761..455d21a0b610 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -80,7 +80,7 @@ function extractNextDataTagInformation(): NextDataTagInfo {
   // function, then it is "/_error", but that isn't a problem since users know which route threw by looking at the
   // parent transaction
   // TODO: Actually this is a problem (even though it is not that big), because the DSC and the transaction payload will contain
-  // a different transaction name.Maybe we can fix this.Idea: Also send transaction name via pageProps when available.
+  // a different transaction name.Maybe we can fix this. Idea: Also send transaction name via pageProps when available.
   nextDataTagInfo.route = page;
   nextDataTagInfo.params = query;
 

From 39f6fe5c6b95a483599989404bc9c5ae6995dd8c Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Mon, 5 Sep 2022 14:51:23 +0000
Subject: [PATCH 09/13] more typo

---
 packages/nextjs/src/performance/client.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index 455d21a0b610..dff09f220946 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -80,7 +80,7 @@ function extractNextDataTagInformation(): NextDataTagInfo {
   // function, then it is "/_error", but that isn't a problem since users know which route threw by looking at the
   // parent transaction
   // TODO: Actually this is a problem (even though it is not that big), because the DSC and the transaction payload will contain
-  // a different transaction name.Maybe we can fix this. Idea: Also send transaction name via pageProps when available.
+  // a different transaction name. Maybe we can fix this. Idea: Also send transaction name via pageProps when available.
   nextDataTagInfo.route = page;
   nextDataTagInfo.params = query;
 

From 398ad2af1a818eeeb86294484139d5b6a459bc4a Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Tue, 6 Sep 2022 11:17:00 +0000
Subject: [PATCH 10/13] Add some explaining comments

---
 packages/nextjs/src/performance/client.ts | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index dff09f220946..15e6a3fe9c05 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -190,14 +190,28 @@ function getNextRouteFromPathname(pathname: string): string | void {
   }
 }
 
+/**
+ * Converts a Next.js style route to a regular expression that matches on pathnames (no query params or URL fragments).
+ *
+ * In general this involves replacing any instances of square brackets in a route with a wildcard:
+ * e.g. "/users/[id]/info" becomes /\/users\/([^/]+?)\/info/
+ *
+ * Some additional edgecases need to be considered:
+ * - All routes have an optional slash at the end, meaning users can navigate to "/users/[id]/info" or
+ *   "/users/[id]/info/" - both will be resolved to "/users/[id]/info".
+ * - Non-optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[...params]").
+ * - Optional "catchall"s at the end of a route must be considered when matching (e.g. "/users/[[...params]]").
+ *
+ * @param route A Next.js style route as it is found in `global.__BUILD_MANIFEST.sortedPages`
+ */
 function convertNextRouteToRegExp(route: string): RegExp {
   // We can assume a route is at least "/".
   const routeParts = route.split('/');
 
   let optionalCatchallWildcardRegex = '';
   if (routeParts[routeParts.length - 1].match(/^\[\[\.\.\..+\]\]$/)) {
-    // If last route part has pattern "[[...xyz]]"
-    // We pop the latest route part to get rid of the required trailing slash
+    // If last route part has pattern "[[...xyz]]" we pop the latest route part to get rid of the required trailing
+    // slash that would come before it if we didn't pop it.
     routeParts.pop();
     optionalCatchallWildcardRegex = '(?:/(.+?))?';
   }

From cda4b9fb412c077f7dc589978e5302a1136988bb Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Tue, 6 Sep 2022 11:40:07 +0000
Subject: [PATCH 11/13] Create span to signify when route change is complete

---
 packages/nextjs/src/performance/client.ts | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index 15e6a3fe9c05..9c2bc61f0d36 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -168,12 +168,31 @@ export function nextRouterInstrumentation(
         activeTransaction.finish();
       }
 
-      startTransactionCb({
+      const navigationTransaction = startTransactionCb({
         name: transactionName,
         op: 'navigation',
         tags,
         metadata: { source: transactionSource },
       });
+
+      if (navigationTransaction) {
+        // In addition to the navigation transaction we're also starting a span to mark Next.js's `routeChangeStart`
+        // and `routeChangeComplete` events.
+        // We don't want to finish the navigation transaction on `routeChangeComplete`, since users might want to attach
+        // spans to that transaction even after `routeChangeComplete` is fired (eg. HTTP requests in some useEffect
+        // hooks). Instead, we'll simply let the navigation transaction finish itself (it's an `IdleTransaction`).
+        const nextRouteChangeSpan = navigationTransaction.startChild({
+          op: 'ui.nextjs.route-change',
+          description: 'Next.js Route Change',
+        });
+
+        const finishRouteChangeSpan = (): void => {
+          nextRouteChangeSpan.finish();
+          Router.events.off('routeChangeComplete', finishRouteChangeSpan);
+        };
+
+        Router.events.on('routeChangeComplete', finishRouteChangeSpan);
+      }
     });
   }
 }

From 53b5021eb321553f48ca805f4c1e7c6d0bf11b20 Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Tue, 6 Sep 2022 11:48:43 +0000
Subject: [PATCH 12/13] avoid the void

---
 packages/nextjs/src/performance/client.ts | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/performance/client.ts
index 9c2bc61f0d36..764d11e27fe8 100644
--- a/packages/nextjs/src/performance/client.ts
+++ b/packages/nextjs/src/performance/client.ts
@@ -197,16 +197,18 @@ export function nextRouterInstrumentation(
   }
 }
 
-function getNextRouteFromPathname(pathname: string): string | void {
+function getNextRouteFromPathname(pathname: string): string | undefined {
   const pageRoutes = (global.__BUILD_MANIFEST || {}).sortedPages;
 
   // Page route should in 99.999% of the cases be defined by now but just to be sure we make a check here
-  if (pageRoutes) {
-    return pageRoutes.find(route => {
-      const routeRegExp = convertNextRouteToRegExp(route);
-      return pathname.match(routeRegExp);
-    });
+  if (!pageRoutes) {
+    return;
   }
+
+  return pageRoutes.find(route => {
+    const routeRegExp = convertNextRouteToRegExp(route);
+    return pathname.match(routeRegExp);
+  });
 }
 
 /**

From a340b5c3ce242e28df309bf958f4f49d17597576 Mon Sep 17 00:00:00 2001
From: Luca Forstner <luca.forstner@sentry.io>
Date: Tue, 6 Sep 2022 13:35:57 +0000
Subject: [PATCH 13/13] Add test to check if Router.events.off was called

---
 .../nextjs/test/performance/client.test.ts    | 49 +++++++++++++++----
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts
index 5e108dfb4e74..01494fd7c519 100644
--- a/packages/nextjs/test/performance/client.test.ts
+++ b/packages/nextjs/test/performance/client.test.ts
@@ -1,3 +1,4 @@
+import { Transaction } from '@sentry/types';
 import { getGlobalObject } from '@sentry/utils';
 import { JSDOM } from 'jsdom';
 import { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
@@ -16,18 +17,24 @@ const globalObject = getGlobalObject<
 const originalBuildManifest = globalObject.__BUILD_MANIFEST;
 const originalBuildManifestRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
 
+let eventHandlers: { [eventName: string]: Set<(...args: any[]) => void> } = {};
+
 jest.mock('next/router', () => {
-  const eventHandlers: { [eventName: string]: ((...args: any[]) => void)[] } = {};
   return {
     default: {
       events: {
         on(type: string, handler: (...args: any[]) => void) {
-          if (eventHandlers[type]) {
-            eventHandlers[type].push(handler);
-          } else {
-            eventHandlers[type] = [handler];
+          if (!eventHandlers[type]) {
+            eventHandlers[type] = new Set();
           }
+
+          eventHandlers[type].add(handler);
         },
+        off: jest.fn((type: string, handler: (...args: any[]) => void) => {
+          if (eventHandlers[type]) {
+            eventHandlers[type].delete(handler);
+          }
+        }),
         emit(type: string, ...eventArgs: any[]) {
           if (eventHandlers[type]) {
             eventHandlers[type].forEach(eventHandler => {
@@ -40,6 +47,18 @@ jest.mock('next/router', () => {
   };
 });
 
+function createMockStartTransaction() {
+  return jest.fn(
+    () =>
+      ({
+        startChild: () => ({
+          finish: () => undefined,
+        }),
+        finish: () => undefined,
+      } as Transaction),
+  );
+}
+
 describe('nextRouterInstrumentation', () => {
   const originalGlobalDocument = getGlobalObject<Window>().document;
   const originalGlobalLocation = getGlobalObject<Window>().location;
@@ -94,6 +113,12 @@ describe('nextRouterInstrumentation', () => {
     if ((global as any).__BUILD_MANIFEST) {
       (global as any).__BUILD_MANIFEST.sortedPages = originalBuildManifestRoutes;
     }
+
+    // Clear all event handlers
+    eventHandlers = {};
+
+    // Necessary to clear all Router.events.off() mock call numbers
+    jest.clearAllMocks();
   });
 
   describe('pageload transactions', () => {
@@ -187,7 +212,7 @@ describe('nextRouterInstrumentation', () => {
     ])(
       'creates a pageload transaction (#%#)',
       (url, route, query, props, hasNextData, expectedStartTransactionArgument) => {
-        const mockStartTransaction = jest.fn();
+        const mockStartTransaction = createMockStartTransaction();
         setUpNextPage({ url, route, query, props, hasNextData });
         nextRouterInstrumentation(mockStartTransaction);
         expect(mockStartTransaction).toHaveBeenCalledTimes(1);
@@ -196,7 +221,7 @@ describe('nextRouterInstrumentation', () => {
     );
 
     it('does not create a pageload transaction if option not given', () => {
-      const mockStartTransaction = jest.fn();
+      const mockStartTransaction = createMockStartTransaction();
       setUpNextPage({ url: 'https://example.com/', route: '/', hasNextData: false });
       nextRouterInstrumentation(mockStartTransaction, false);
       expect(mockStartTransaction).toHaveBeenCalledTimes(0);
@@ -225,7 +250,7 @@ describe('nextRouterInstrumentation', () => {
     ])(
       'should create a parameterized transaction on route change (%s)',
       (targetLocation, expectedTransactionName, expectedTransactionSource) => {
-        const mockStartTransaction = jest.fn();
+        const mockStartTransaction = createMockStartTransaction();
 
         setUpNextPage({
           url: 'https://example.com/home',
@@ -261,11 +286,17 @@ describe('nextRouterInstrumentation', () => {
             }),
           }),
         );
+
+        Router.events.emit('routeChangeComplete', targetLocation);
+        // eslint-disable-next-line @typescript-eslint/unbound-method
+        expect(Router.events.off).toHaveBeenCalledWith('routeChangeComplete', expect.anything());
+        // eslint-disable-next-line @typescript-eslint/unbound-method
+        expect(Router.events.off).toHaveBeenCalledTimes(1);
       },
     );
 
     it('should not create transaction when navigation transactions are disabled', () => {
-      const mockStartTransaction = jest.fn();
+      const mockStartTransaction = createMockStartTransaction();
 
       setUpNextPage({
         url: 'https://example.com/home',