Revert "fix(node): Ensure httpIntegration propagates traces (#15233)"

getsentry · Feb 10, 2025 · 8b5e39a · 8b5e39a
1 parent 8d3851c
commit 8b5e39a
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 276 deletions.
diff --git a/...kages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/scenario.ts b/...kages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/scenario.ts
diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/test.ts
diff --git a/packages/core/src/utils-hoist/baggage.ts b/packages/core/src/utils-hoist/baggage.ts
@@ -130,7 +130,7 @@ function baggageHeaderToObject(baggageHeader: string): Record<string, string> {
  * @returns a baggage header string, or `undefined` if the object didn't have any values, since an empty baggage header
  * is not spec compliant.
  */
-export function objectToBaggageHeader(object: Record<string, string>): string | undefined {
+function objectToBaggageHeader(object: Record<string, string>): string | undefined {
   if (Object.keys(object).length === 0) {
     // An empty baggage header is not spec compliant: We return undefined.
     return undefined;

diff --git a/packages/core/src/utils-hoist/index.ts b/packages/core/src/utils-hoist/index.ts
@@ -128,7 +128,6 @@ export {
   baggageHeaderToDynamicSamplingContext,
   dynamicSamplingContextToSentryBaggageHeader,
   parseBaggageHeader,
-  objectToBaggageHeader,
 } from './baggage';
 
 export { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from './url';

diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts
@@ -1,44 +1,33 @@
+/* eslint-disable max-lines */
 import type * as http from 'node:http';
 import type { IncomingMessage, RequestOptions } from 'node:http';
 import type * as https from 'node:https';
 import type { EventEmitter } from 'node:stream';
-/* eslint-disable max-lines */
 import { VERSION } from '@opentelemetry/core';
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
 import type { AggregationCounts, Client, RequestEventData, SanitizedRequestData, Scope } from '@sentry/core';
 import {
-  LRUMap,
   addBreadcrumb,
   generateSpanId,
   getBreadcrumbLogLevelFromHttpStatusCode,
   getClient,
   getIsolationScope,
   getSanitizedUrlString,
-  getTraceData,
   httpRequestToRequestData,
   logger,
-  objectToBaggageHeader,
-  parseBaggageHeader,
   parseUrl,
   stripUrlQueryAndFragment,
   withIsolationScope,
   withScope,
 } from '@sentry/core';
-import { shouldPropagateTraceForUrl } from '@sentry/opentelemetry';
 import { DEBUG_BUILD } from '../../debug-build';
 import { getRequestUrl } from '../../utils/getRequestUrl';
 import { getRequestInfo } from './vendor/getRequestInfo';
 
 type Http = typeof http;
 type Https = typeof https;
 
-type RequestArgs =
-  // eslint-disable-next-line @typescript-eslint/ban-types
-  | [url: string | URL, options?: RequestOptions, callback?: Function]
-  // eslint-disable-next-line @typescript-eslint/ban-types
-  | [options: RequestOptions, callback?: Function];
-
 type SentryHttpInstrumentationOptions = InstrumentationConfig & {
   /**
    * Whether breadcrumbs should be recorded for requests.
@@ -91,11 +80,8 @@ const MAX_BODY_BYTE_LENGTH = 1024 * 1024;
  * https://github.com/open-telemetry/opentelemetry-js/blob/f8ab5592ddea5cba0a3b33bf8d74f27872c0367f/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
  */
 export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpInstrumentationOptions> {
-  private _propagationDecisionMap: LRUMap<string, boolean>;
-
   public constructor(config: SentryHttpInstrumentationOptions = {}) {
     super('@sentry/instrumentation-http', VERSION, config);
-    this._propagationDecisionMap = new LRUMap<string, boolean>(100);
   }
 
   /** @inheritdoc */
@@ -222,32 +208,22 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
       return function outgoingRequest(this: unknown, ...args: unknown[]): http.ClientRequest {
         instrumentation._diag.debug('http instrumentation for outgoing requests');
 
+        // Making a copy to avoid mutating the original args array
         // We need to access and reconstruct the request options object passed to `ignoreOutgoingRequests`
         // so that it matches what Otel instrumentation passes to `ignoreOutgoingRequestHook`.
         // @see https://github.com/open-telemetry/opentelemetry-js/blob/7293e69c1e55ca62e15d0724d22605e61bd58952/experimental/packages/opentelemetry-instrumentation-http/src/http.ts#L756-L789
-        const requestArgs = [...args] as RequestArgs;
-
-        let options = requestArgs[0];
+        const argsCopy = [...args];
 
-        // Make sure correct fallback attributes are set on the options object for https before we pass them to the vendored getRequestInfo function.
-        // Ref: https://github.com/open-telemetry/opentelemetry-js/blob/887ff1cd6e3f795f703e40a9fbe89b3cba7e88c3/experimental/packages/opentelemetry-instrumentation-http/src/http.ts#L390
-        if (component === 'https' && typeof options === 'object' && options?.constructor?.name !== 'URL') {
-          options = Object.assign({}, options);
-          options.protocol = options.protocol || 'https:';
-          options.port = options.port || 443;
-        }
+        const options = argsCopy.shift() as URL | http.RequestOptions | string;
 
-        const extraOptions = typeof requestArgs[1] === 'object' ? requestArgs[1] : undefined;
+        const extraOptions =
+          typeof argsCopy[0] === 'object' && (typeof options === 'string' || options instanceof URL)
+            ? (argsCopy.shift() as http.RequestOptions)
+            : undefined;
 
-        const { optionsParsed, origin, pathname } = getRequestInfo(instrumentation._diag, options, extraOptions);
+        const { optionsParsed } = getRequestInfo(instrumentation._diag, options, extraOptions);
 
-        const url = getAbsoluteUrl(origin, pathname);
-
-        addSentryHeadersToRequestOptions(url, optionsParsed, instrumentation._propagationDecisionMap);
-
-        const request = original.apply(this, [optionsParsed, ...requestArgs.slice(1)]) as ReturnType<
-          typeof http.request
-        >;
+        const request = original.apply(this, args) as ReturnType<typeof http.request>;
 
         request.prependListener('response', (response: http.IncomingMessage) => {
           const _breadcrumbs = instrumentation.getConfig().breadcrumbs;
@@ -481,44 +457,6 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope):
   }
 }
 
-/**
- * Mutates the passed in `options` and adds `sentry-trace` / `baggage` headers, if they are not already set.
- */
-function addSentryHeadersToRequestOptions(
-  url: string,
-  options: RequestOptions,
-  propagationDecisionMap: LRUMap<string, boolean>,
-): void {
-  // Manually add the trace headers, if it applies
-  // Note: We do not use `propagation.inject()` here, because our propagator relies on an active span
-  // Which we do not have in this case
-  const tracePropagationTargets = getClient()?.getOptions().tracePropagationTargets;
-  const addedHeaders = shouldPropagateTraceForUrl(url, tracePropagationTargets, propagationDecisionMap)
-    ? getTraceData()
-    : undefined;
-
-  if (!addedHeaders) {
-    return;
-  }
-
-  if (!options.headers) {
-    options.headers = {};
-  }
-  const headers = options.headers;
-
-  const { 'sentry-trace': sentryTrace, baggage } = addedHeaders;
-
-  // We do not want to overwrite existing header here, if it was already set
-  if (sentryTrace && !headers['sentry-trace']) {
-    headers['sentry-trace'] = sentryTrace;
-  }
-
-  // For baggage, we make sure to merge this into a possibly existing header
-  if (baggage) {
-    headers['baggage'] = mergeBaggageHeaders(headers['baggage'], baggage);
-  }
-}
-
 /**
  * Starts a session and tracks it in the context of a given isolation scope.
  * When the passed response is finished, the session is put into a task and is
@@ -593,49 +531,3 @@ const clientToRequestSessionAggregatesMap = new Map<
   Client,
   { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } }
 >();
-
-function getAbsoluteUrl(origin: string, path: string = '/'): string {
-  try {
-    const url = new URL(path, origin);
-    return url.toString();
-  } catch {
-    // fallback: Construct it on our own
-    const url = `${origin}`;
-
-    if (url.endsWith('/') && path.startsWith('/')) {
-      return `${url}${path.slice(1)}`;
-    }
-
-    if (!url.endsWith('/') && !path.startsWith('/')) {
-      return `${url}/${path.slice(1)}`;
-    }
-
-    return `${url}${path}`;
-  }
-}
-
-function mergeBaggageHeaders(
-  existing: string | string[] | number | undefined,
-  baggage: string,
-): string | string[] | number | undefined {
-  if (!existing) {
-    return baggage;
-  }
-
-  const existingBaggageEntries = parseBaggageHeader(existing);
-  const newBaggageEntries = parseBaggageHeader(baggage);
-
-  if (!newBaggageEntries) {
-    return existing;
-  }
-
-  // Existing entries take precedence, ensuring order remains stable for minimal changes
-  const mergedBaggageEntries = { ...existingBaggageEntries };
-  Object.entries(newBaggageEntries).forEach(([key, value]) => {
-    if (!mergedBaggageEntries[key]) {
-      mergedBaggageEntries[key] = value;
-    }
-  });
-
-  return objectToBaggageHeader(mergedBaggageEntries);
-}