ref(node): Use Sentry span APIs in vendored connect instrumentation

dev-packages/node-integration-tests/suites/tracing/connect/test.ts

@@ -97,6 +97,12 @@ describe('connect auto-instrumentation', () => {
                   origin: 'auto.http.otel.connect',
                   op: 'middleware.connect',
                 }),
+
+                expect.objectContaining({
+                  description: '/error',
+                  op: 'request_handler.connect',
+                  status: 'internal_error',
+                }),
               ]),
             },
           })

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

@@ -1,13 +1,6 @@
 import { ConnectInstrumentation } from './vendored/instrumentation';
-import type { IntegrationFn, Span } from '@sentry/core';
-import {
-  captureException,
-  defineIntegration,
-  getClient,
-  SEMANTIC_ATTRIBUTE_SENTRY_OP,
-  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
-  spanToJSON,
-} from '@sentry/core';
+import type { IntegrationFn } from '@sentry/core';
+import { captureException, defineIntegration } from '@sentry/core';
 import { ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core';
 
 type ConnectApp = {
@@ -78,39 +71,5 @@ 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 connect
-  const client = getClient();
-  if (client) {
-    client.on('spanStart', span => {
-      addConnectSpanAttributes(span);
-    });
-  }
-
   ensureIsWrapped(app.use, 'connect');
 };
-
-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 the "middleware - " prefix
-  const name = attributes['connect.name'];
-  if (typeof name === 'string') {
-    span.updateName(name);
-  }
-}

packages/node/src/integrations/tracing/connect/vendored/enums/AttributeNames.ts

@@ -27,8 +27,3 @@ export enum ConnectTypes {
   MIDDLEWARE = 'middleware',
   REQUEST_HANDLER = 'request_handler',
 }
-
-export enum ConnectNames {
-  MIDDLEWARE = 'middleware',
-  REQUEST_HANDLER = 'request handler',
-}

packages/node/src/integrations/tracing/connect/vendored/instrumentation.ts

@@ -19,11 +19,11 @@
  * - Minor TypeScript strictness adjustments for this repository's compiler settings
  */
 
-import type { Span, SpanOptions } from '@opentelemetry/api';
 import type { ServerResponse } from 'http';
-import { AttributeNames, ConnectNames, ConnectTypes } from './enums/AttributeNames';
+import { AttributeNames, ConnectTypes } from './enums/AttributeNames';
 import type { HandleFunction, NextFunction, PatchedRequest, Server, Use, UseArgs, UseArgs2 } from './internal-types';
-import { SDK_VERSION } from '@sentry/core';
+import type { Span } from '@sentry/core';
+import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startInactiveSpan } from '@sentry/core';
 import { setHttpServerSpanRouteAttribute } from '../../../../utils/setHttpServerSpanRouteAttribute';
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation';
@@ -68,37 +68,30 @@ export class ConnectInstrumentation extends InstrumentationBase {
     };
   }
 
-  public _patchNext(next: NextFunction, finishSpan: () => void): NextFunction {
+  public _patchNext(next: NextFunction, span: Span, finishSpan: () => void): NextFunction {
     return function nextFunction(this: NextFunction, err?: unknown): void {
+      if (err) {
+        span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
+      }
       const result = next.apply(this, [err]);
       finishSpan();
       return result;
     };
   }
 
   public _startSpan(routeName: string, middleWare: HandleFunction): Span {
-    let connectType: ConnectTypes;
-    let connectName: string;
-    let connectTypeName: string;
-    if (routeName) {
-      connectType = ConnectTypes.REQUEST_HANDLER;
-      connectTypeName = ConnectNames.REQUEST_HANDLER;
-      connectName = routeName;
-    } else {
-      connectType = ConnectTypes.MIDDLEWARE;
-      connectTypeName = ConnectNames.MIDDLEWARE;
-      connectName = middleWare.name || ANONYMOUS_NAME;
-    }
-    const spanName = `${connectTypeName} - ${connectName}`;
-    const options: SpanOptions = {
+    const connectType = routeName ? ConnectTypes.REQUEST_HANDLER : ConnectTypes.MIDDLEWARE;
+    const connectName = routeName || middleWare.name || ANONYMOUS_NAME;
+    return startInactiveSpan({
+      name: connectName,
+      op: `${connectType}.connect`,
       attributes: {
+        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.connect',
         [ATTR_HTTP_ROUTE]: routeName.length > 0 ? routeName : '/',
         [AttributeNames.CONNECT_TYPE]: connectType,
         [AttributeNames.CONNECT_NAME]: connectName,
       },
-    };
-
-    return this.tracer.startSpan(spanName, options);
+    });
   }
 
   public _patchMiddleware(routeName: string, middleWare: HandleFunction): HandleFunction {
@@ -134,9 +127,15 @@ export class ConnectInstrumentation extends InstrumentationBase {
       }
 
       res.addListener('close', finishSpan);
-      arguments[nextArgIdx] = patchNext(next, finishSpan);
+      arguments[nextArgIdx] = patchNext(next, span, finishSpan);
 
-      return Reflect.apply(middleWare, this, arguments);
+      try {
+        return Reflect.apply(middleWare, this, arguments);
+      } catch (e) {
+        span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
+        finishSpan();
+        throw e;
+      }
     }
 
     Object.defineProperty(patchedMiddleware, 'length', {

packages/node/src/integrations/tracing/connect/vendored/utils.ts

@@ -18,9 +18,10 @@
  * - Upstream version: @opentelemetry/instrumentation-connect@0.61.0
  */
 
-import { diag } from '@opentelemetry/api';
+import { debug } from '@sentry/core';
 import type { PatchedRequest } from './internal-types';
 import { _LAYERS_STORE_PROPERTY } from './internal-types';
+import { DEBUG_BUILD } from '../../../../debug-build';
 
 export const addNewStackLayer = (request: PatchedRequest) => {
   if (Array.isArray(request[_LAYERS_STORE_PROPERTY]) === false) {
@@ -37,7 +38,7 @@ export const addNewStackLayer = (request: PatchedRequest) => {
     if (stackLength === request[_LAYERS_STORE_PROPERTY].length) {
       request[_LAYERS_STORE_PROPERTY].pop();
     } else {
-      diag.warn('Connect: Trying to pop the stack multiple time');
+      DEBUG_BUILD && debug.warn('Connect: Trying to pop the stack multiple time');
     }
   };
 };