fix(node-experimental): Ensure we only create HTTP spans when outgoing (#8966)

This is a fork of
#8937, with only the
"uncontroversial" stuff, mainly fixing that we only create HTTP
breadcrumbs for _outgoing_ requests.

In addition, this also migrates to using `requestHook` and
`responseHook` instead of `applyCustomAttributesOnSpan`. We may have to
revisit this later, but these hooks seem to have a better context
awareness (=they are called in a more reasonable OTEL context, which
gives the callbacks there better access to scope data etc). However that
means we cannot (easily) pass both request and response as breadcrumb
hints - not sure how important that is to us... For now I'd say that's
OK.

Note that also `requestHook` is only called when the request finishes,
so we already have all the response OTEL span attributes correctly set
there.

Author: mydea

packages/node-experimental/src/integrations/http.ts

@@ -95,8 +95,11 @@ export class Http implements Integration {
         new HttpInstrumentation({
           requireParentforOutgoingSpans: true,
           requireParentforIncomingSpans: false,
-          applyCustomAttributesOnSpan: (span, req, res) => {
-            this._onSpan(span as unknown as OtelSpan, req, res);
+          requestHook: (span, req) => {
+            this._updateSentrySpan(span as unknown as OtelSpan, req);
+          },
+          responseHook: (span, res) => {
+            this._addRequestBreadcrumb(span as unknown as OtelSpan, res);
           },
         }),
       ],
@@ -136,64 +139,70 @@ export class Http implements Integration {
     return;
   };
 
-  /** Handle an emitted span from the HTTP instrumentation. */
-  private _onSpan(
-    span: OtelSpan,
-    request: ClientRequest | IncomingMessage,
-    response: IncomingMessage | ServerResponse,
-  ): void {
-    const data = getRequestSpanData(span, request, response);
+  /** Update the Sentry span data based on the OTEL span. */
+  private _updateSentrySpan(span: OtelSpan, request: ClientRequest | IncomingMessage): void {
+    const data = getRequestSpanData(span);
     const { attributes } = span;
 
     const sentrySpan = _INTERNAL_getSentrySpan(span.spanContext().spanId);
-    if (sentrySpan) {
-      sentrySpan.origin = 'auto.http.otel.http';
+    if (!sentrySpan) {
+      return;
+    }
 
-      const additionalData: Record<string, unknown> = {
-        url: data.url,
-      };
+    sentrySpan.origin = 'auto.http.otel.http';
 
-      if (sentrySpan instanceof Transaction && span.kind === SpanKind.SERVER) {
-        sentrySpan.setMetadata({ request });
-      }
+    const additionalData: Record<string, unknown> = {
+      url: data.url,
+    };
 
-      if (attributes[SemanticAttributes.HTTP_STATUS_CODE]) {
-        const statusCode = attributes[SemanticAttributes.HTTP_STATUS_CODE] as string;
-        additionalData['http.response.status_code'] = statusCode;
+    if (sentrySpan instanceof Transaction && span.kind === SpanKind.SERVER) {
+      sentrySpan.setMetadata({ request });
+    }
 
-        sentrySpan.setTag('http.status_code', statusCode);
-      }
+    if (attributes[SemanticAttributes.HTTP_STATUS_CODE]) {
+      const statusCode = attributes[SemanticAttributes.HTTP_STATUS_CODE] as string;
+      additionalData['http.response.status_code'] = statusCode;
 
-      if (data['http.query']) {
-        additionalData['http.query'] = data['http.query'].slice(1);
-      }
-      if (data['http.fragment']) {
-        additionalData['http.fragment'] = data['http.fragment'].slice(1);
-      }
+      sentrySpan.setTag('http.status_code', statusCode);
+    }
 
-      Object.keys(additionalData).forEach(prop => {
-        const value = additionalData[prop];
-        sentrySpan.setData(prop, value);
-      });
+    if (data['http.query']) {
+      additionalData['http.query'] = data['http.query'].slice(1);
+    }
+    if (data['http.fragment']) {
+      additionalData['http.fragment'] = data['http.fragment'].slice(1);
     }
 
-    if (this._breadcrumbs) {
-      getCurrentHub().addBreadcrumb(
-        {
-          category: 'http',
-          data: {
-            status_code: response.statusCode,
-            ...data,
-          },
-          type: 'http',
-        },
-        {
-          event: 'response',
-          request,
-          response,
-        },
-      );
+    Object.keys(additionalData).forEach(prop => {
+      const value = additionalData[prop];
+      sentrySpan.setData(prop, value);
+    });
+  }
+
+  /** Add a breadcrumb for outgoing requests. */
+  private _addRequestBreadcrumb(span: OtelSpan, response: IncomingMessage | ServerResponse): void {
+    if (!this._breadcrumbs || span.kind !== SpanKind.CLIENT) {
+      return;
     }
+
+    const data = getRequestSpanData(span);
+    getCurrentHub().addBreadcrumb(
+      {
+        category: 'http',
+        data: {
+          status_code: response.statusCode,
+          ...data,
+        },
+        type: 'http',
+      },
+      {
+        event: 'response',
+        // TODO FN: Do we need access to `request` here?
+        // If we do, we'll have to use the `applyCustomAttributesOnSpan` hook instead,
+        // but this has worse context semantics than request/responseHook.
+        response,
+      },
+    );
   }
 }
 

packages/node-experimental/src/utils/getRequestSpanData.ts

@@ -2,16 +2,11 @@ import type { Span as OtelSpan } from '@opentelemetry/sdk-trace-base';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import type { SanitizedRequestData } from '@sentry/types';
 import { getSanitizedUrlString, parseUrl } from '@sentry/utils';
-import type { ClientRequest, IncomingMessage, ServerResponse } from 'http';
 
 /**
  * Get sanitizied request data from an OTEL span.
  */
-export function getRequestSpanData(
-  span: OtelSpan,
-  _request: ClientRequest | IncomingMessage,
-  _response: IncomingMessage | ServerResponse,
-): SanitizedRequestData {
+export function getRequestSpanData(span: OtelSpan): SanitizedRequestData {
   const data: SanitizedRequestData = {
     url: span.attributes[SemanticAttributes.HTTP_URL] as string,
     'http.method': (span.attributes[SemanticAttributes.HTTP_METHOD] as string) || 'GET',