Realign spans between versions

Author: onurtemizkan

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

@@ -53,36 +53,40 @@ test('Sends an API route transaction', async ({ baseURL }) => {
           span_id: expect.stringMatching(/[a-f0-9]{16}/),
           trace_id: expect.stringMatching(/[a-f0-9]{32}/),
           data: {
-            'sentry.origin': 'manual',
+            'sentry.origin': 'auto.http.otel.fastify',
+            'sentry.op': 'hook.fastify',
             'service.name': 'fastify',
             'hook.name': 'fastify -> @fastify/otel -> @fastify/middie - onRequest',
             'fastify.type': 'hook',
             'hook.callback.name': 'runMiddie',
           },
-          description: 'handler - runMiddie',
+          description: '@fastify/middie - onRequest',
+          op: 'hook.fastify',
           parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
           start_timestamp: expect.any(Number),
           timestamp: expect.any(Number),
           status: 'ok',
-          origin: 'manual',
+          origin: 'auto.http.otel.fastify',
         },
         {
           span_id: expect.stringMatching(/[a-f0-9]{16}/),
           trace_id: expect.stringMatching(/[a-f0-9]{32}/),
           data: {
-            'sentry.origin': 'manual',
+            'sentry.origin': 'auto.http.otel.fastify',
+            'sentry.op': 'request-handler.fastify',
             'service.name': 'fastify',
             'hook.name': 'fastify -> @fastify/otel -> @fastify/middie - route-handler',
             'fastify.type': 'request-handler',
             'http.route': '/test-transaction',
             'hook.callback.name': 'anonymous',
           },
-          description: 'handler - fastify -> @fastify/otel -> @fastify/middie',
+          description: '@fastify/middie - route-handler',
+          op: 'request-handler.fastify',
           parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
           start_timestamp: expect.any(Number),
           timestamp: expect.any(Number),
           status: 'ok',
-          origin: 'manual',
+          origin: 'auto.http.otel.fastify',
         },
         {
           span_id: expect.stringMatching(/[a-f0-9]{16}/),

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

@@ -61,13 +61,13 @@ test('Sends an API route transaction', async ({ baseURL }) => {
   expect(spans).toContainEqual({
     data: {
       'plugin.name': 'sentry-fastify-error-handler',
-      'fastify.type': 'middleware',
-      'hook.name': 'onRequest',
+      'fastify.type': 'request_handler',
+      'http.route': '/test-transaction',
       'sentry.origin': 'auto.http.otel.fastify',
-      'sentry.op': 'middleware.fastify',
+      'sentry.op': 'request_handler.fastify',
     },
     description: 'sentry-fastify-error-handler',
-    op: 'middleware.fastify',
+    op: 'request_handler.fastify',
     parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
     span_id: expect.stringMatching(/[a-f0-9]{16}/),
     start_timestamp: expect.any(Number),

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

@@ -62,12 +62,12 @@ test('Sends an API route transaction', async ({ baseURL }) => {
     data: {
       'fastify.type': 'hook',
       'hook.callback.name': 'anonymous',
-      'hook.name': 'fastify -> @fastify/otel -> sentry-fastify-error-handler - onRequest',
+      'hook.name': 'fastify -> @fastify/otel - onRequest',
       'sentry.op': 'hook.fastify',
       'sentry.origin': 'auto.http.otel.fastify',
       'service.name': 'fastify',
     },
-    description: 'sentry-fastify-error-handler - onRequest',
+    description: '@fastify/otel - onRequest',
     op: 'hook.fastify',
     parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
     span_id: expect.stringMatching(/[a-f0-9]{16}/),
@@ -78,7 +78,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
     origin: 'auto.http.otel.fastify',
   });
 
-
   expect(spans).toContainEqual({
     data: {
       'sentry.origin': 'manual',

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

@@ -62,12 +62,12 @@ test('Sends an API route transaction', async ({ baseURL }) => {
     data: {
       'fastify.type': 'hook',
       'hook.callback.name': 'anonymous',
-      'hook.name': 'fastify -> @fastify/otel -> sentry-fastify-error-handler - onRequest',
+      'hook.name': 'fastify -> @fastify/otel - onRequest',
       'sentry.op': 'hook.fastify',
       'sentry.origin': 'auto.http.otel.fastify',
       'service.name': 'fastify',
     },
-    description: 'sentry-fastify-error-handler - onRequest',
+    description: '@fastify/otel - onRequest',
     op: 'hook.fastify',
     parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
     span_id: expect.stringMatching(/[a-f0-9]{16}/),
@@ -78,7 +78,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
     origin: 'auto.http.otel.fastify',
   });
 
-
   expect(spans).toContainEqual({
     data: {
       'sentry.origin': 'manual',

packages/node/package.json

@@ -65,7 +65,7 @@
     "access": "public"
   },
   "dependencies": {
-    "@fastify/otel": "0.5.2",
+    "@fastify/otel": "fastify/otel#main",
     "@opentelemetry/api": "^1.9.0",
     "@opentelemetry/context-async-hooks": "^1.30.1",
     "@opentelemetry/core": "^1.30.1",
@@ -101,7 +101,8 @@
     "import-in-the-middle": "^1.13.0"
   },
   "devDependencies": {
-    "@types/node": "^18.19.1"
+    "@types/node": "^18.19.1",
+    "fastify": "^5.3.0"
   },
   "scripts": {
     "build": "run-p build:transpile build:types",

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

@@ -1,4 +1,5 @@
 import FastifyOtelInstrumentation from '@fastify/otel';
+import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify';
 import {
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -14,7 +15,6 @@ import { generateInstrumentOnce } from '../../../otel/instrument';
 import { FastifyInstrumentationV3 } from './v3/instrumentation';
 import * as diagnosticsChannel from 'node:diagnostics_channel';
 import { DEBUG_BUILD } from '../../../debug-build';
-import type { FastifyInstance, FastifyReply, FastifyRequest } from './types';
 
 interface FastifyHandlerOptions {
   /**
@@ -54,28 +54,25 @@ interface FastifyHandlerOptions {
 const INTEGRATION_NAME = 'Fastify';
 const INTEGRATION_NAME_V3 = 'Fastify-V3';
 
-export const instrumentFastifyV3 = generateInstrumentOnce(
-  INTEGRATION_NAME_V3,
-  () =>
-    new FastifyInstrumentationV3({
-      requestHook(span) {
-        addFastifySpanAttributes(span);
-      },
-    }),
-);
+export const instrumentFastifyV3 = generateInstrumentOnce(INTEGRATION_NAME_V3, () => new FastifyInstrumentationV3());
 
 export const instrumentFastify = generateInstrumentOnce(INTEGRATION_NAME, () => {
-  // FastifyOtelInstrumentation does not have a `requestHook`
-  // so we can't use `addFastifySpanAttributes` here for now
   const fastifyOtelInstrumentationInstance = new FastifyOtelInstrumentation();
   const plugin = fastifyOtelInstrumentationInstance.plugin();
 
+  // This message handler works for Fastify versions 3, 4 and 5
   diagnosticsChannel.subscribe('fastify.initialization', message => {
     const fastifyInstance = (message as { fastify?: FastifyInstance }).fastify;
 
     fastifyInstance?.register(plugin).after(err => {
       if (err) {
         DEBUG_BUILD && logger.error('Failed to setup Fastify instrumentation', err);
+      } else {
+        instrumentClient();
+
+        if (fastifyInstance) {
+          instrumentOnRequest(fastifyInstance);
+        }
       }
     });
   });
@@ -153,18 +150,6 @@ export function setupFastifyErrorHandler(fastify: FastifyInstance, options?: Par
         }
       });
 
-      // registering `onRequest` hook here instead of using Otel `onRequest` callback b/c `onRequest` hook
-      // is ironically called in the fastify `preHandler` hook which is called later in the lifecycle:
-      // https://fastify.dev/docs/latest/Reference/Lifecycle/
-      fastify.addHook('onRequest', async (request, _reply) => {
-        // Taken from Otel Fastify instrumentation:
-        // https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts#L94-L96
-        const routeName = request.routeOptions?.url || request.routerPath;
-        const method = request.method || 'GET';
-
-        getIsolationScope().setTransactionName(`${method} ${routeName}`);
-      });
-
       done();
     },
     {
@@ -173,43 +158,67 @@ export function setupFastifyErrorHandler(fastify: FastifyInstance, options?: Par
     },
   );
 
-  // Sadly, middleware spans do not go through `requestHook`, so we handle those here
-  // We register this hook in this method, because if we register it in the integration `setup`,
-  // it would always run even for users that are not even using fastify
-  const client = getClient();
-  if (client) {
-    client.on('spanStart', span => {
-      addFastifySpanAttributes(span);
-    });
-  }
-
+  // eslint-disable-next-line @typescript-eslint/no-floating-promises
   fastify.register(plugin);
 }
 
 function addFastifySpanAttributes(span: Span): void {
-  const attributes = spanToJSON(span).data;
+  const spanJSON = spanToJSON(span);
+  const spanName = spanJSON.description;
+  const attributes = spanJSON.data;
 
-  // this is one of: middleware, request_handler
   const type = attributes['fastify.type'];
 
+  const isHook = type === 'hook';
+  const isHandler = type === spanName?.startsWith('handler -');
+  // In @fastify/otel `request-handler` is separated by dash, not underscore
+  const isRequestHandler = spanName === 'request' || type === 'request-handler';
+
   // If this is already set, or we have no fastify span, no need to process again...
-  if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
+  if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || (!isHandler && !isRequestHandler && !isHook)) {
     return;
   }
 
+  const opPrefix = isHook ? 'hook' : isHandler ? 'middleware' : isRequestHandler ? 'request-handler' : '<unknown>';
+
   span.setAttributes({
     [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.fastify',
-    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.fastify`,
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${opPrefix}.fastify`,
   });
 
-  // Also update the name, we don't need to "middleware - " prefix
-  const name = attributes['fastify.name'] || attributes['plugin.name'] || attributes['hook.name'];
-  if (typeof name === 'string') {
+  const attrName = attributes['fastify.name'] || attributes['plugin.name'] || attributes['hook.name'];
+  if (typeof attrName === 'string') {
     // Try removing `fastify -> ` and `@fastify/otel -> ` prefixes
     // This is a bit of a hack, and not always working for all spans
     // But it's the best we can do without a proper API
-    const updatedName = name.replace(/^fastify -> /, '').replace(/^@fastify\/otel -> /, '');
+    const updatedName = attrName.replace(/^fastify -> /, '').replace(/^@fastify\/otel -> /, '');
 
     span.updateName(updatedName);
   }
 }
+
+function instrumentClient(): void {
+  const client = getClient();
+  if (client) {
+    client.on('spanStart', (span: Span) => {
+      addFastifySpanAttributes(span);
+    });
+  }
+}
+
+function instrumentOnRequest(fastify: FastifyInstance): void {
+  fastify.addHook('onRequest', async (request: FastifyRequest, _reply) => {
+    if (request.opentelemetry) {
+      const { span } = request.opentelemetry();
+
+      if (span) {
+        addFastifySpanAttributes(span);
+      }
+    }
+
+    const routeName = request.routeOptions?.url;
+    const method = request.method || 'GET';
+
+    getIsolationScope().setTransactionName(`${method} ${routeName}`);
+  });
+}

packages/node/src/integrations/tracing/fastify/v3/instrumentation.ts

@@ -28,6 +28,14 @@ import {
   safeExecuteInTheMiddle,
 } from '@opentelemetry/instrumentation';
 import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
+import type { Span } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  getClient,
+  getIsolationScope,
+  spanToJSON,
+} from '@sentry/core';
 
 import { AttributeNames, FastifyNames, FastifyTypes } from './enums/AttributeNames';
 
@@ -101,6 +109,10 @@ export class FastifyInstrumentationV3 extends InstrumentationBase<FastifyInstrum
       if (routeName && rpcMetadata?.type === RPCType.HTTP) {
         rpcMetadata.route = routeName;
       }
+
+      const method = request.method || 'GET';
+
+      getIsolationScope().setTransactionName(`${method} ${routeName}`);
       done();
     };
   }
@@ -197,6 +209,8 @@ export class FastifyInstrumentationV3 extends InstrumentationBase<FastifyInstrum
       app.addHook('onRequest', instrumentation._hookOnRequest());
       app.addHook('preHandler', instrumentation._hookPreHandler());
 
+      instrumentClient();
+
       instrumentation._wrap(app, 'addHook', instrumentation._wrapAddHook());
 
       return app;
@@ -249,7 +263,6 @@ export class FastifyInstrumentationV3 extends InstrumentationBase<FastifyInstrum
       const anyRequest = request as any;
 
       const handler = anyRequest.routeOptions?.handler || anyRequest.context?.handler;
-
       const handlerName = handler?.name.startsWith('bound ') ? handler.name.substring(6) : handler?.name;
       const spanName = `${FastifyNames.REQUEST_HANDLER} - ${handlerName || this.pluginName || ANONYMOUS_NAME}`;
 
@@ -266,6 +279,8 @@ export class FastifyInstrumentationV3 extends InstrumentationBase<FastifyInstrum
       }
       const span = startSpan(reply, instrumentation.tracer, spanName, spanAttributes);
 
+      addFastifyV3SpanAttributes(span);
+
       const { requestHook } = instrumentation.getConfig();
       if (requestHook) {
         safeExecuteInTheMiddle(
@@ -285,3 +300,40 @@ export class FastifyInstrumentationV3 extends InstrumentationBase<FastifyInstrum
     };
   }
 }
+
+function instrumentClient(): void {
+  const client = getClient();
+  if (client) {
+    client.on('spanStart', (span: Span) => {
+      addFastifyV3SpanAttributes(span);
+    });
+  }
+}
+
+function addFastifyV3SpanAttributes(span: Span): void {
+  const attributes = spanToJSON(span).data;
+
+  // this is one of: middleware, request_handler
+  const type = attributes['fastify.type'];
+
+  // If this is already set, or we have no fastify span, no need to process again...
+  if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
+    return;
+  }
+
+  span.setAttributes({
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.fastify',
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.fastify`,
+  });
+
+  // Also update the name, we don't need to "middleware - " prefix
+  const name = attributes['fastify.name'] || attributes['plugin.name'] || attributes['hook.name'];
+  if (typeof name === 'string') {
+    // Try removing `fastify -> ` and `@fastify/otel -> ` prefixes
+    // This is a bit of a hack, and not always working for all spans
+    // But it's the best we can do without a proper API
+    const updatedName = name.replace(/^fastify -> /, '').replace(/^@fastify\/otel -> /, '');
+
+    span.updateName(updatedName);
+  }
+}