ref(node): Streamline undici (node-fetch) instrumentation

dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.ts

@@ -0,0 +1,35 @@
+import { createServer } from 'http';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+});
+
+// Bind and immediately release a port so we have an address that reliably refuses the connection.
+// A refused outgoing request fires the `undici:request:error` channel, exercising the error path.
+function getRefusedPort(): Promise<number> {
+  return new Promise(resolve => {
+    const server = createServer();
+    server.listen(0, () => {
+      const { port } = server.address() as { port: number };
+      server.close(() => resolve(port));
+    });
+  });
+}
+
+async function run(): Promise<void> {
+  const port = await getRefusedPort();
+
+  await Sentry.startSpan({ name: 'test_transaction' }, async () => {
+    await fetch(`http://localhost:${port}/api/v0`).catch(() => {
+      // Ignore the expected connection error
+    });
+  });
+}
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+run();

dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts

@@ -0,0 +1,21 @@
+import { expect, test } from 'vitest';
+import { createRunner } from '../../../../utils/runner';
+
+test('captures an errored span for a failed outgoing fetch request', async () => {
+  await createRunner(__dirname, 'scenario.ts')
+    .expect({
+      transaction: {
+        transaction: 'test_transaction',
+        spans: expect.arrayContaining([
+          expect.objectContaining({
+            description: expect.stringMatching(/GET http:\/\/localhost:\d+\//),
+            op: 'http.client',
+            origin: 'auto.http.otel.node_fetch',
+            status: 'internal_error',
+          }),
+        ]),
+      },
+    })
+    .start()
+    .completed();
+});

dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts

@@ -0,0 +1,22 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+  integrations: [
+    Sentry.nativeNodeFetchIntegration({
+      headersToSpanAttributes: {
+        requestHeaders: ['x-test-header'],
+        responseHeaders: ['x-powered-by'],
+      },
+    }),
+  ],
+});
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+Sentry.startSpan({ name: 'test_transaction' }, async () => {
+  await fetch(`${process.env.SERVER_URL}/api/v0`, { headers: { 'x-test-header': 'test-value' } });
+});

dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts

@@ -0,0 +1,35 @@
+import { createTestServer } from '@sentry-internal/test-utils';
+import { expect, test } from 'vitest';
+import { createRunner } from '../../../../utils/runner';
+
+test('maps configured request & response headers to span attributes', async () => {
+  expect.assertions(2);
+
+  const [SERVER_URL, closeTestServer] = await createTestServer()
+    .get('/api/v0', headers => {
+      expect(headers['x-test-header']).toBe('test-value');
+    })
+    .start();
+
+  await createRunner(__dirname, 'scenario.ts')
+    .withEnv({ SERVER_URL })
+    .expect({
+      transaction: {
+        transaction: 'test_transaction',
+        spans: expect.arrayContaining([
+          expect.objectContaining({
+            description: expect.stringMatching(/GET .*\/api\/v0/),
+            op: 'http.client',
+            origin: 'auto.http.otel.node_fetch',
+            data: expect.objectContaining({
+              'http.request.header.x-test-header': ['test-value'],
+              'http.response.header.x-powered-by': ['Express'],
+            }),
+          }),
+        ]),
+      },
+    })
+    .start()
+    .completed();
+  closeTestServer();
+});

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

@@ -5,10 +5,13 @@ import { createEsmAndCjsTests } from '../../../../utils/runner';
 describe('outgoing traceparent', () => {
   createEsmAndCjsTests(__dirname, 'scenario-fetch.mjs', 'instrument.mjs', (createRunner, test) => {
     test('outgoing fetch requests should get traceparent headers', async () => {
-      expect.assertions(5);
+      expect.assertions(7);
+
+      let outgoingSentryTrace: string | undefined;
 
       const [SERVER_URL, closeTestServer] = await createTestServer()
         .get('/api/v1', headers => {
+          outgoingSentryTrace = headers['sentry-trace'] as string;
           expect(headers['baggage']).toEqual(expect.any(String));
           expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})-1$/));
           expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0');
@@ -19,8 +22,14 @@ describe('outgoing traceparent', () => {
       await createRunner()
         .withEnv({ SERVER_URL })
         .expect({
-          transaction: {
-            // we're not too concerned with the actual transaction here since this is tested elsewhere
+          // The propagated `sentry-trace` must reference the `http.client` span, not the surrounding transaction span.
+          transaction: event => {
+            const propagatedSpanId = outgoingSentryTrace?.split('-')[1];
+            const httpClientSpan = event.spans?.find(span => span.op === 'http.client');
+
+            expect(httpClientSpan).toBeDefined();
+            expect(propagatedSpanId).toBe(httpClientSpan?.span_id);
+            expect(propagatedSpanId).not.toBe(event.contexts?.trace?.span_id);
           },
         })
         .start()

packages/node/src/integrations/node-fetch/index.ts

@@ -53,13 +53,16 @@ interface NodeFetchOptions extends Pick<
   ignoreOutgoingRequests?: (url: string) => boolean;
 }
 
-const instrumentOtelNodeFetch = generateInstrumentOnce(
-  INTEGRATION_NAME,
-  UndiciInstrumentation,
-  (options: NodeFetchOptions) => {
-    return _getConfigWithDefaults(options);
-  },
-);
+let _undiciInstrumentation: UndiciInstrumentation | undefined;
+
+// Sets up the vendored undici instrumentation (emits `http.client` spans & propagates traces).
+// The module-level singleton mirrors `generateInstrumentOnce`'s "instrument once per process" behavior.
+function instrumentNodeFetchSpans(options: NodeFetchOptions): void {
+  if (!_undiciInstrumentation) {
+    _undiciInstrumentation = new UndiciInstrumentation(_getConfigWithDefaults(options));
+  }
+  _undiciInstrumentation.enable();
+}
 
 const instrumentSentryNodeFetch = generateInstrumentOnce(
   `${INTEGRATION_NAME}.sentry`,
@@ -75,14 +78,14 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {
     setupOnce() {
       const instrumentSpans = _shouldInstrumentSpans(options, getClient<NodeClient>()?.getOptions());
 
-      // This is the "regular" OTEL instrumentation that emits spans
+      // This is the instrumentation that emits spans & propagates traces for outgoing fetch requests
       if (instrumentSpans) {
-        instrumentOtelNodeFetch(options);
+        instrumentNodeFetchSpans(options);
       }
 
-      // This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces
-      // This must be registered after the OTEL one, to ensure that the core trace propagation logic takes presedence
-      // Otherwise, the sentry-trace header may be set multiple times
+      // This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces.
+      // It must subscribe to the diagnostics channels after the span instrumentation above, so the core
+      // trace propagation logic takes precedence. Otherwise, the sentry-trace header may be set multiple times.
       instrumentSentryNodeFetch(options);
     },
   };
@@ -116,7 +119,6 @@ function _shouldInstrumentSpans(options: NodeFetchOptions, clientOptions: Partia
 /** Exported only for tests. */
 export function _getConfigWithDefaults(options: Partial<NodeFetchOptions> = {}): UndiciInstrumentationConfig {
   const instrumentationConfig = {
-    requireParentforSpans: false,
     ignoreRequestHook: request => {
       const url = getAbsoluteUrl(request.origin, request.path);
       const _ignoreOutgoingRequests = options.ignoreOutgoingRequests;

packages/node/src/integrations/node-fetch/vendored/internal-types.ts

@@ -7,7 +7,6 @@
  * - Upstream version: @opentelemetry/instrumentation-undici@0.24.0
  * - Tracking issue: https://github.com/getsentry/sentry-javascript/issues/20165
  */
-/* eslint-disable -- vendored @opentelemetry/instrumentation-undici (#20165) */
 
 import type { UndiciRequest, UndiciResponse } from './types';
 

packages/node/src/integrations/node-fetch/vendored/semconv.ts

@@ -0,0 +1,23 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * NOTICE from the Sentry authors:
+ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/ed97091c9890dd18e52759f2ea98e9d7593b3ae4/packages/instrumentation-undici
+ * - Upstream version: @opentelemetry/instrumentation-undici@0.24.0
+ * - The semantic-convention constants this package emits, inlined from
+ *   `@opentelemetry/semantic-conventions` (matching the sibling vendored dirs).
+ */
+
+export const ATTR_HTTP_REQUEST_METHOD = 'http.request.method' as const;
+export const ATTR_HTTP_REQUEST_METHOD_ORIGINAL = 'http.request.method_original' as const;
+export const ATTR_HTTP_RESPONSE_STATUS_CODE = 'http.response.status_code' as const;
+export const ATTR_NETWORK_PEER_ADDRESS = 'network.peer.address' as const;
+export const ATTR_NETWORK_PEER_PORT = 'network.peer.port' as const;
+export const ATTR_SERVER_ADDRESS = 'server.address' as const;
+export const ATTR_SERVER_PORT = 'server.port' as const;
+export const ATTR_URL_FULL = 'url.full' as const;
+export const ATTR_URL_PATH = 'url.path' as const;
+export const ATTR_URL_QUERY = 'url.query' as const;
+export const ATTR_URL_SCHEME = 'url.scheme' as const;
+export const ATTR_USER_AGENT_ORIGINAL = 'user_agent.original' as const;

packages/node/src/integrations/node-fetch/vendored/types.ts

@@ -7,10 +7,9 @@
  * - Upstream version: @opentelemetry/instrumentation-undici@0.24.0
  * - Tracking issue: https://github.com/getsentry/sentry-javascript/issues/20165
  */
-/* eslint-disable -- vendored @opentelemetry/instrumentation-undici (#20165) */
 
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
-import type { Attributes, Span } from '@opentelemetry/api';
+import type { Span, SpanAttributes } from '@sentry/core';
 
 export interface UndiciRequest {
   origin: string;
@@ -54,7 +53,7 @@ export interface ResponseHookFunction<RequestType = UndiciRequest, ResponseType
 }
 
 export interface StartSpanHookFunction<T = UndiciRequest> {
-  (request: T): Attributes;
+  (request: T): SpanAttributes;
 }
 
 // This package will instrument HTTP requests made through `undici` or  `fetch` global API
@@ -71,8 +70,6 @@ export interface UndiciInstrumentationConfig<
   responseHook?: ResponseHookFunction<RequestType, ResponseType>;
   /** Function for adding custom attributes before a span is started */
   startSpanHook?: StartSpanHookFunction<RequestType>;
-  /** Require parent to create span for outgoing requests */
-  requireParentforSpans?: boolean;
   /** Map the following HTTP headers to span attributes. */
   headersToSpanAttributes?: {
     requestHeaders?: string[];