better support v3.21.0 and test it

Author: mydea

dev-packages/e2e-tests/test-applications/node-fastify-3/tests/transactions.test.ts

@@ -65,15 +65,19 @@ test('Sends an API route transaction', async ({ baseURL }) => {
 
   const spans = transactionEvent.spans || [];
 
+  /* TODO(v11): Uncomment this. This only works on fastify v3.21.0, not worth it to test this here */
+  /*
   expect(spans).toContainEqual({
     data: {
-      'plugin.name': 'sentry-fastify-error-handler',
-      'fastify.type': 'request_handler',
-      'http.route': '/test-transaction',
       'sentry.origin': 'auto.http.otel.fastify',
       'sentry.op': 'request_handler.fastify',
+      'fastify.root': '@sentry/instrumentation-fastify',
+      'http.request.method': 'GET',
+      'url.path': '/test-transaction',
+      'http.route': '/test-transaction',
+      'http.response.status_code': 200,
     },
-    description: 'sentry-fastify-error-handler',
+    description: 'GET /test-transaction',
     op: 'request_handler.fastify',
     parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
     span_id: expect.stringMatching(/[a-f0-9]{16}/),
@@ -83,16 +87,17 @@ test('Sends an API route transaction', async ({ baseURL }) => {
     trace_id: expect.stringMatching(/[a-f0-9]{32}/),
     origin: 'auto.http.otel.fastify',
   });
+  */
 
   expect(spans).toContainEqual({
-    data: {
-      'plugin.name': 'sentry-fastify-error-handler',
-      'fastify.type': 'request_handler',
-      'http.route': '/test-transaction',
-      'sentry.op': 'request_handler.fastify',
+    data: expect.objectContaining({
       'sentry.origin': 'auto.http.otel.fastify',
-    },
-    description: 'sentry-fastify-error-handler',
+      'sentry.op': 'request_handler.fastify',
+      // format is slightly different in v3.20.0 and v3.21.0
+      'fastify.type': expect.stringMatching(/request[-_]handler/),
+      'http.route': '/test-transaction',
+    }),
+    description: expect.stringContaining('sentry-fastify-error-handler'),
     op: 'request_handler.fastify',
     parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
     span_id: expect.stringMatching(/[a-f0-9]{16}/),

packages/node/src/integrations/tracing/fastify/index.ts

@@ -11,7 +11,7 @@ import {
 
 interface FastifyIntegration extends Integration {
   getShouldHandleError: () => (error: Error, request: FastifyRequest, reply: FastifyReply) => boolean;
-  // This will be removed in the next major version.
+  // todo(v11): Remove this
   setShouldHandleError: (
     shouldHandleError: (error: Error, request: FastifyRequest, reply: FastifyReply) => boolean,
   ) => void;

packages/server-utils/src/integrations/tracing-channel/fastify/instrumentation.ts

@@ -66,6 +66,31 @@ const kSetNotFoundOriginal = Symbol('sentry fastify setNotFoundHandler original'
 
 type AnyFn = (...args: any[]) => any;
 
+/**
+ * Read the matched route URL off a request. Fastify >=4 exposes it on `request.routeOptions.url`,
+ * while v3 only has the (since-removed-in-v5) `request.routerPath`.
+ */
+function getRequestRouteUrl(request: any): string | undefined {
+  return request.routeOptions?.url ?? request.routerPath;
+}
+
+/**
+ * Read the per-route config off a request. Fastify >=4 exposes it on `request.routeOptions.config`,
+ * while v3 uses `request.routeConfig`. Used to honor the `{ config: { otel: false } }` opt-out.
+ */
+function getRequestRouteConfig(request: any): { otel?: boolean } | undefined {
+  return request.routeOptions?.config ?? request.routeConfig;
+}
+
+/**
+ * Detect whether one of a wrapped handler's arguments is the Fastify request. We can't rely on a
+ * single property since the route metadata moved from `routerPath` (v3) to `routeOptions` (>=4),
+ * so we accept either shape.
+ */
+function isFastifyRequest(arg: any): boolean {
+  return !!arg && typeof arg === 'object' && !!arg.method && !!arg.url && (!!arg.routeOptions || 'routerPath' in arg);
+}
+
 /**
  * The Fastify plugin that wires up the request/hook/handler spans. It is registered on every Fastify
  * instance via the `fastify.initialization` diagnostics channel.
@@ -146,7 +171,7 @@ function appendRouteHook(existing: AnyFn | AnyFn[] | undefined, hook: AnyFn): An
 }
 
 function startRequestSpanHook(this: any, request: any, _reply: any, hookDone: () => void): void {
-  if (request.routeOptions.config?.otel === false) {
+  if (getRequestRouteConfig(request)?.otel === false) {
     return hookDone();
   }
 
@@ -157,7 +182,7 @@ function startRequestSpanHook(this: any, request: any, _reply: any, hookDone: ()
     [URL_PATH]: request.url,
   };
 
-  const route = request.routeOptions.url as string | undefined;
+  const route = getRequestRouteUrl(request);
   if (route != null) {
     attributes[HTTP_ROUTE] = route;
 
@@ -288,7 +313,7 @@ function setNotFoundHandlerPatched(this: any, hooks: any, handler?: any): void {
 
 function getRequestFromArgs(args: any[]): any | null {
   for (const arg of args) {
-    if (arg?.routeOptions && arg.url && arg.method) {
+    if (isFastifyRequest(arg)) {
       return arg;
     }
   }
@@ -299,7 +324,7 @@ function handlerWrapper(handler: AnyFn, hookName: string, spanAttributes: Record
   return function handlerWrapped(this: any, ...args: any[]) {
     const request = getRequestFromArgs(args);
 
-    if (request === null || request.routeOptions.config?.otel === false) {
+    if (request === null || getRequestRouteConfig(request)?.otel === false) {
       return handler.call(this, ...args);
     }
 
@@ -340,7 +365,7 @@ function stripFastifyPrefix(hookName = ''): string {
 
 function instrumentOnRequest(fastify: FastifyInstance): void {
   fastify.addHook('onRequest', async (request: FastifyRequest, _reply) => {
-    const routeName = request.routeOptions?.url;
+    const routeName = getRequestRouteUrl(request);
     const method = request.method || 'GET';
 
     getIsolationScope().setTransactionName(`${method} ${routeName}`);