feat(node): Ensure fastify spans have better data

dev-packages/e2e-tests/test-applications/node-fastify/package.json

@@ -23,7 +23,7 @@
   },
   "devDependencies": {
     "@sentry-internal/event-proxy-server": "link:../../../event-proxy-server",
-    "@playwright/test": "^1.38.1"
+    "@playwright/test": "^1.44.0"
   },
   "volta": {
     "extends": "../../package.json"

dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.ts → dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.mjs

@@ -1,4 +1,3 @@
-import type { PlaywrightTestConfig } from '@playwright/test';
 import { devices } from '@playwright/test';
 
 const fastifyPort = 3030;
@@ -7,7 +6,7 @@ const eventProxyPort = 3031;
 /**
  * See https://playwright.dev/docs/test-configuration.
  */
-const config: PlaywrightTestConfig = {
+const config = {
   testDir: './tests',
   /* Maximum time one test can run for. */
   timeout: 150_000,

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

@@ -55,70 +55,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
 
   expect(transactionEvent).toEqual(
     expect.objectContaining({
-      spans: [
-        {
-          data: {
-            'plugin.name': 'fastify -> sentry-fastify-error-handler',
-            'fastify.type': 'middleware',
-            'hook.name': 'onRequest',
-            'otel.kind': 'INTERNAL',
-            'sentry.origin': 'manual',
-          },
-          description: 'middleware - fastify -> sentry-fastify-error-handler',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-          origin: 'manual',
-        },
-        {
-          data: {
-            'plugin.name': 'fastify -> sentry-fastify-error-handler',
-            'fastify.type': 'request_handler',
-            'http.route': '/test-transaction',
-            'otel.kind': 'INTERNAL',
-            'sentry.origin': 'auto.http.otel.fastify',
-          },
-          description: 'request handler - fastify -> sentry-fastify-error-handler',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-          origin: 'auto.http.otel.fastify',
-        },
-        {
-          data: {
-            'otel.kind': 'INTERNAL',
-            'sentry.origin': 'manual',
-          },
-          description: 'test-span',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-          origin: 'manual',
-        },
-        {
-          data: {
-            'otel.kind': 'INTERNAL',
-            'sentry.origin': 'manual',
-          },
-          description: 'child-span',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-          origin: 'manual',
-        },
-      ],
       transaction: 'GET /test-transaction',
       type: 'transaction',
       transaction_info: {
@@ -127,6 +63,78 @@ test('Sends an API route transaction', async ({ baseURL }) => {
     }),
   );
 
+  const spans = transactionEvent.spans || [];
+
+  expect(spans).toContainEqual({
+    data: {
+      'plugin.name': 'fastify -> sentry-fastify-error-handler',
+      'fastify.type': 'middleware',
+      'hook.name': 'onRequest',
+      'otel.kind': 'INTERNAL',
+      'sentry.origin': 'auto.http.otel.fastify',
+      'sentry.op': 'middleware.fastify',
+    },
+    description: 'sentry-fastify-error-handler',
+    op: 'middleware.fastify',
+    parent_span_id: expect.any(String),
+    span_id: expect.any(String),
+    start_timestamp: expect.any(Number),
+    status: 'ok',
+    timestamp: expect.any(Number),
+    trace_id: expect.any(String),
+    origin: 'auto.http.otel.fastify',
+  });
+
+  expect(spans).toContainEqual({
+    data: {
+      'plugin.name': 'fastify -> sentry-fastify-error-handler',
+      'fastify.type': 'request_handler',
+      'http.route': '/test-transaction',
+      'otel.kind': 'INTERNAL',
+      'sentry.op': 'request_handler.fastify',
+      'sentry.origin': 'auto.http.otel.fastify',
+    },
+    description: 'sentry-fastify-error-handler',
+    op: 'request_handler.fastify',
+    parent_span_id: expect.any(String),
+    span_id: expect.any(String),
+    start_timestamp: expect.any(Number),
+    status: 'ok',
+    timestamp: expect.any(Number),
+    trace_id: expect.any(String),
+    origin: 'auto.http.otel.fastify',
+  });
+
+  expect(spans).toContainEqual({
+    data: {
+      'otel.kind': 'INTERNAL',
+      'sentry.origin': 'manual',
+    },
+    description: 'test-span',
+    parent_span_id: expect.any(String),
+    span_id: expect.any(String),
+    start_timestamp: expect.any(Number),
+    status: 'ok',
+    timestamp: expect.any(Number),
+    trace_id: expect.any(String),
+    origin: 'manual',
+  });
+
+  expect(spans).toContainEqual({
+    data: {
+      'otel.kind': 'INTERNAL',
+      'sentry.origin': 'manual',
+    },
+    description: 'child-span',
+    parent_span_id: expect.any(String),
+    span_id: expect.any(String),
+    start_timestamp: expect.any(Number),
+    status: 'ok',
+    timestamp: expect.any(Number),
+    trace_id: expect.any(String),
+    origin: 'manual',
+  });
+
   await expect
     .poll(
       async () => {

dev-packages/e2e-tests/test-applications/node-fastify/tsconfig.json

@@ -6,5 +6,5 @@
     "strict": true,
     "outDir": "dist"
   },
-  "include": ["*.ts"]
+  "include": ["./src/*.ts"]
 }

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

@@ -1,11 +1,39 @@
 import { isWrapped } from '@opentelemetry/core';
 import { FastifyInstrumentation } from '@opentelemetry/instrumentation-fastify';
-import { captureException, defineIntegration, getIsolationScope, isEnabled } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  captureException,
+  defineIntegration,
+  getClient,
+  getIsolationScope,
+  isEnabled,
+  spanToJSON,
+} from '@sentry/core';
 import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry';
-import type { IntegrationFn } from '@sentry/types';
+import type { IntegrationFn, Span } from '@sentry/types';
 import { consoleSandbox } from '@sentry/utils';
 
-import { addOriginToSpan } from '../../utils/addOriginToSpan';
+// We inline the types we care about here
+interface Fastify {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  register: (plugin: any) => void;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  addHook: (hook: string, handler: (request: any, reply: any, error: Error) => void) => void;
+}
+
+/**
+ * Minimal request type containing properties around route information.
+ * Works for Fastify 3, 4 and presumably 5.
+ */
+interface FastifyRequestRouteInfo {
+  // since fastify@4.10.0
+  routeOptions?: {
+    url?: string;
+    method?: string;
+  };
+  routerPath?: string;
+}
 
 const _fastifyIntegration = (() => {
   return {
@@ -14,7 +42,7 @@ const _fastifyIntegration = (() => {
       addOpenTelemetryInstrumentation(
         new FastifyInstrumentation({
           requestHook(span) {
-            addOriginToSpan(span, 'auto.http.otel.fastify');
+            addFastifySpanAttributes(span);
           },
         }),
       );
@@ -29,27 +57,6 @@ const _fastifyIntegration = (() => {
  */
 export const fastifyIntegration = defineIntegration(_fastifyIntegration);
 
-// We inline the types we care about here
-interface Fastify {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  register: (plugin: any) => void;
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  addHook: (hook: string, handler: (request: any, reply: any, error: Error) => void) => void;
-}
-
-/**
- * Minimal request type containing properties around route information.
- * Works for Fastify 3, 4 and presumably 5.
- */
-interface FastifyRequestRouteInfo {
-  // since fastify@4.10.0
-  routeOptions?: {
-    url?: string;
-    method?: string;
-  };
-  routerPath?: string;
-}
-
 /**
  * Setup an error handler for Fastify.
  */
@@ -84,6 +91,16 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
 
   fastify.register(plugin);
 
+  // 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);
+    });
+  }
+
   if (!isWrapped(fastify.addHook) && isEnabled()) {
     consoleSandbox(() => {
       // eslint-disable-next-line no-console
@@ -93,3 +110,27 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
     });
   }
 }
+
+function addFastifySpanAttributes(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') {
+    // Also remove `fastify -> ` prefix
+    span.updateName(name.replace(/^fastify -> /, ''));
+  }
+}