diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/package.json b/dev-packages/e2e-tests/test-applications/node-fastify/package.json index 0293412ba76b..1a35474e1bba 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/package.json +++ b/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" diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.ts b/dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.mjs similarity index 94% rename from dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.ts rename to dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.mjs index 56a882460551..858e6aec7ef2 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.ts +++ b/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, diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-fastify/tests/transactions.test.ts index 8324a9913d1b..cfc320f3d4dd 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/tests/transactions.test.ts +++ b/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 () => { diff --git a/dev-packages/e2e-tests/test-applications/node-fastify/tsconfig.json b/dev-packages/e2e-tests/test-applications/node-fastify/tsconfig.json index 17bd2c1f4c00..6b69bfaa593b 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify/tsconfig.json +++ b/dev-packages/e2e-tests/test-applications/node-fastify/tsconfig.json @@ -6,5 +6,5 @@ "strict": true, "outDir": "dist" }, - "include": ["*.ts"] + "include": ["./src/*.ts"] } diff --git a/packages/node/src/integrations/tracing/fastify.ts b/packages/node/src/integrations/tracing/fastify.ts index 55ccc7b4b72d..33f2af3084bd 100644 --- a/packages/node/src/integrations/tracing/fastify.ts +++ b/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 -> /, '')); + } +}