feat(node): Ensure connect spans have better data (#12130)

Ensuring connect spans have correct op & origin. Also streamlining E2E test app for this.
getsentry · May 21, 2024 · 77ff5d9 · 77ff5d9
1 parent 85db0de
commit 77ff5d9
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 11 deletions.
diff --git a/...cations/node-connect/playwright.config.ts → ...ations/node-connect/playwright.config.mjs b/...cations/node-connect/playwright.config.ts → ...ations/node-connect/playwright.config.mjs
@@ -1,4 +1,3 @@
-import type { PlaywrightTestConfig } from '@playwright/test';
 import { devices } from '@playwright/test';
 
 const connectPort = 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-connect/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-connect/tests/transactions.test.ts
@@ -72,20 +72,22 @@ test('Sends an API route transaction', async ({ baseURL }) => {
         },
         {
           data: {
-            'sentry.origin': 'manual',
+            'sentry.origin': 'auto.http.otel.connect',
+            'sentry.op': 'request_handler.connect',
             'http.route': '/test-transaction',
             'connect.type': 'request_handler',
             'connect.name': '/test-transaction',
             'otel.kind': 'INTERNAL',
           },
-          description: 'request handler - /test-transaction',
+          op: 'request_handler.connect',
+          description: '/test-transaction',
           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',
+          origin: 'auto.http.otel.connect',
         },
       ],
       transaction: 'GET /test-transaction',

diff --git a/dev-packages/e2e-tests/test-applications/node-connect/tsconfig.json b/dev-packages/e2e-tests/test-applications/node-connect/tsconfig.json
@@ -6,5 +6,5 @@
     "strict": true,
     "noEmit": true
   },
-  "include": ["*.ts"]
+  "include": ["src/*.ts"]
 }
diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts
@@ -16,10 +16,12 @@ describe('connect auto-instrumentation', () => {
           'connect.type': 'request_handler',
           'http.route': '/',
           'otel.kind': 'INTERNAL',
-          'sentry.origin': 'manual',
+          'sentry.origin': 'auto.http.otel.connect',
+          'sentry.op': 'request_handler.connect',
         }),
-        description: 'request handler - /',
-        origin: 'manual',
+        description: '/',
+        origin: 'auto.http.otel.connect',
+        op: 'request_handler.connect',
         status: 'ok',
       }),
     ]),

diff --git a/packages/node/src/integrations/tracing/connect.ts b/packages/node/src/integrations/tracing/connect.ts
@@ -1,8 +1,16 @@
 import { isWrapped } from '@opentelemetry/core';
 import { ConnectInstrumentation } from '@opentelemetry/instrumentation-connect';
-import { captureException, defineIntegration, isEnabled } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  captureException,
+  defineIntegration,
+  getClient,
+  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';
 
 type ConnectApp = {
@@ -30,6 +38,16 @@ function connectErrorMiddleware(err: any, req: any, res: any, next: any): void {
 export const setupConnectErrorHandler = (app: ConnectApp): void => {
   app.use(connectErrorMiddleware);
 
+  // Sadly, ConnectInstrumentation has no requestHook, so we need to add the attributes 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 => {
+      addConnectSpanAttributes(span);
+    });
+  }
+
   if (!isWrapped(app.use) && isEnabled()) {
     consoleSandbox(() => {
       // eslint-disable-next-line no-console
@@ -39,3 +57,26 @@ export const setupConnectErrorHandler = (app: ConnectApp): void => {
     });
   }
 };
+
+function addConnectSpanAttributes(span: Span): void {
+  const attributes = spanToJSON(span).data || {};
+
+  // this is one of: middleware, request_handler
+  const type = attributes['connect.type'];
+
+  // If this is already set, or we have no connect span, no need to process again...
+  if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
+    return;
+  }
+
+  span.setAttributes({
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.connect',
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.connect`,
+  });
+
+  // Also update the name, we don't need to "middleware - " prefix
+  const name = attributes['connect.name'];
+  if (typeof name === 'string') {
+    span.updateName(name);
+  }
+}