feat(nextjs): Parameterize server-side transaction names and harvest request data (#3577)

This PR does two things:

1) It wraps two more methods on `next`'s `Server` class, one of which is called on every API request, and the other of which is called on every page request. Both methods are passed a parameterized version of the path, which our wrappers grab and use to modify the name of the currently active transaction.

2) It adds an event processor which pulls data off of the request and adds it to the event. This runs whether or not tracing is enabled, in order to add data to both error and transaction events.

The only other thing of minor interest is that the type for `Event.request.query_string` is expanded to include objects, which is okay according to the dev docs: https://develop.sentry.dev/sdk/event-payloads/request/#attribute).

Author: lobsterkatie

packages/nextjs/src/utils/instrumentServer.ts

@@ -1,10 +1,11 @@
 import { deepReadDirSync } from '@sentry/node';
-import { hasTracingEnabled } from '@sentry/tracing';
-import { Transaction } from '@sentry/types';
+import { getActiveTransaction, hasTracingEnabled } from '@sentry/tracing';
+import { Event as SentryEvent, Transaction } from '@sentry/types';
 import { fill, logger } from '@sentry/utils';
 import * as domain from 'domain';
 import * as http from 'http';
 import { default as createNextServer } from 'next';
+import * as querystring from 'querystring';
 import * as url from 'url';
 
 import * as Sentry from '../index.server';
@@ -29,6 +30,8 @@ interface Server {
 interface NextRequest extends http.IncomingMessage {
   cookies: Record<string, string>;
   url: string;
+  query: { [key: string]: string };
+  headers: { [key: string]: string };
 }
 
 interface NextResponse extends http.ServerResponse {
@@ -40,11 +43,19 @@ interface NextResponse extends http.ServerResponse {
 type HandlerGetter = () => Promise<ReqHandler>;
 type ReqHandler = (req: NextRequest, res: NextResponse, parsedUrl?: url.UrlWithParsedQuery) => Promise<void>;
 type ErrorLogger = (err: Error) => void;
+type ApiPageEnsurer = (path: string) => Promise<void>;
+type PageComponentFinder = (
+  pathname: string,
+  query: querystring.ParsedUrlQuery,
+  params: { [key: string]: any } | null,
+) => Promise<{ [key: string]: any } | null>;
 
 // these aliases are purely to make the function signatures more easily understandable
 type WrappedHandlerGetter = HandlerGetter;
 type WrappedErrorLogger = ErrorLogger;
 type WrappedReqHandler = ReqHandler;
+type WrappedApiPageEnsurer = ApiPageEnsurer;
+type WrappedPageComponentFinder = PageComponentFinder;
 
 // TODO is it necessary for this to be an object?
 const closure: PlainObject = {};
@@ -125,6 +136,12 @@ function makeWrappedHandlerGetter(origHandlerGetter: HandlerGetter): WrappedHand
       // to the appropriate handlers)
       fill(serverPrototype, 'handleRequest', makeWrappedReqHandler);
 
+      // Wrap as a way to grab the parameterized request URL to use as the transaction name for API requests and page
+      // requests, respectively. These methods are chosen because they're the first spot in the request-handling process
+      // where the parameterized path is provided as an argument, so it's easy to grab.
+      fill(serverPrototype, 'ensureApiPage', makeWrappedMethodForGettingParameterizedPath);
+      fill(serverPrototype, 'findPageComponents', makeWrappedMethodForGettingParameterizedPath);
+
       sdkSetupComplete = true;
     }
 
@@ -182,40 +199,80 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
     // local.on('error', Sentry.captureException);
 
     local.run(() => {
-      // We only want to record page and API requests
-      if (hasTracingEnabled() && shouldTraceRequest(req.url, publicDirFiles)) {
-        const transaction = Sentry.startTransaction({
-          name: `${(req.method || 'GET').toUpperCase()} ${req.url}`,
-          op: 'http.server',
-        });
-        Sentry.getCurrentHub()
-          .getScope()
-          ?.setSpan(transaction);
-
-        res.__sentry__ = { transaction };
-
-        res.once('finish', () => {
-          const transaction = res.__sentry__?.transaction;
-          if (transaction) {
-            // Push `transaction.finish` to the next event loop so open spans have a chance to finish before the transaction
-            // closes
-            setImmediate(() => {
-              // TODO
-              // addExpressReqToTransaction(transaction, req);
+      const currentScope = Sentry.getCurrentHub().getScope();
+
+      if (currentScope) {
+        currentScope.addEventProcessor(event => addRequestDataToEvent(event, req));
+
+        // We only want to record page and API requests
+        if (hasTracingEnabled() && shouldTraceRequest(req.url, publicDirFiles)) {
+          // pull off query string, if any
+          const reqPath = req.url.split('?')[0];
+
+          // requests for pages will only ever be GET requests, so don't bother to include the method in the transaction
+          // name; requests to API routes could be GET, POST, PUT, etc, so do include it there
+          const namePrefix = req.url.startsWith('/api') ? `${(req.method || 'GET').toUpperCase()} ` : '';
+
+          const transaction = Sentry.startTransaction({
+            name: `${namePrefix}${reqPath}`,
+            op: 'http.server',
+            metadata: { requestPath: req.url.split('?')[0] },
+          });
+
+          currentScope.setSpan(transaction);
+
+          res.once('finish', () => {
+            const transaction = getActiveTransaction();
+            if (transaction) {
               transaction.setHttpStatus(res.statusCode);
-              transaction.finish();
-            });
-          }
-        });
 
-        return origReqHandler.call(this, req, res, parsedUrl);
+              // we'll collect this data in a more targeted way in the event processor we added above,
+              // `addRequestDataToEvent`
+              delete transaction.metadata.requestPath;
+
+              // Push `transaction.finish` to the next event loop so open spans have a chance to finish before the
+              // transaction closes
+              setImmediate(() => {
+                transaction.finish();
+              });
+            }
+          });
+        }
       }
+
+      return origReqHandler.call(this, req, res, parsedUrl);
     });
   };
 
   return wrappedReqHandler;
 }
 
+/**
+ * Wrap the given method in order to use the parameterized path passed to it in the transaction name.
+ *
+ * @param origMethod Either `ensureApiPage` (called for every API request) or `findPageComponents` (called for every
+ * page request), both from the `Server` class
+ * @returns A wrapped version of the given method
+ */
+function makeWrappedMethodForGettingParameterizedPath(
+  origMethod: ApiPageEnsurer | PageComponentFinder,
+): WrappedApiPageEnsurer | WrappedPageComponentFinder {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const wrappedMethod = async function(this: Server, parameterizedPath: string, ...args: any[]): Promise<any> {
+    const transaction = getActiveTransaction();
+
+    // replace specific URL with parameterized version
+    if (transaction && transaction.metadata.requestPath) {
+      const origPath = transaction.metadata.requestPath;
+      transaction.name = transaction.name.replace(origPath, parameterizedPath);
+    }
+
+    return origMethod.call(this, parameterizedPath, ...args);
+  };
+
+  return wrappedMethod;
+}
+
 /**
  * Determine if the request should be traced, by filtering out requests for internal next files and static resources.
  *
@@ -228,3 +285,17 @@ function shouldTraceRequest(url: string, publicDirFiles: Set<string>): boolean {
   // `static` is a deprecated but still-functional location for static resources
   return !url.startsWith('/_next/') && !url.startsWith('/static/') && !publicDirFiles.has(url);
 }
+
+function addRequestDataToEvent(event: SentryEvent, req: NextRequest): SentryEvent {
+  event.request = {
+    ...event.request,
+    // TODO body/data
+    url: req.url.split('?')[0],
+    cookies: req.cookies,
+    headers: req.headers,
+    method: req.method,
+    query_string: req.query,
+  };
+
+  return event;
+}

packages/types/src/index.ts

@@ -15,7 +15,7 @@ export { Mechanism } from './mechanism';
 export { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc';
 export { Options } from './options';
 export { Package } from './package';
-export { Request, SentryRequest, SentryRequestType } from './request';
+export { QueryParams, Request, SentryRequest, SentryRequestType } from './request';
 export { Response } from './response';
 export { Runtime } from './runtime';
 export { CaptureContext, Scope, ScopeContext } from './scope';

packages/types/src/request.ts

@@ -13,8 +13,10 @@ export interface Request {
   url?: string;
   method?: string;
   data?: any;
-  query_string?: string;
+  query_string?: QueryParams;
   cookies?: { [key: string]: string };
   env?: { [key: string]: string };
   headers?: { [key: string]: string };
 }
+
+export type QueryParams = string | { [key: string]: string } | Array<[string, string]>;

packages/types/src/transaction.ts

@@ -132,4 +132,7 @@ export interface TransactionMetadata {
     sentry?: string;
     thirdparty?: string;
   };
+
+  /** For transactions tracing server-side request handling, the path of the request being tracked. */
+  requestPath?: string;
 }