fix fetch

dev-packages/node-integration-tests/suites/tracing/requests/fetch-noSampleRate/scenario.ts

@@ -0,0 +1,24 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  tracePropagationTargets: [/\/v0/, 'v1'],
+  integrations: [],
+  transport: loggingTransport,
+});
+
+async function run(): Promise<void> {
+  // Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented
+  await new Promise(resolve => setTimeout(resolve, 100));
+  await fetch(`${process.env.SERVER_URL}/api/v0`);
+  await fetch(`${process.env.SERVER_URL}/api/v1`);
+  await fetch(`${process.env.SERVER_URL}/api/v2`);
+  await fetch(`${process.env.SERVER_URL}/api/v3`);
+
+  Sentry.captureException(new Error('foo'));
+}
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+run();

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

@@ -0,0 +1,53 @@
+import { conditionalTest } from '../../../../utils';
+import { createRunner } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
+
+conditionalTest({ min: 18 })('outgoing fetch', () => {
+  test('outgoing fetch requests are correctly instrumented without tracesSampleRate', done => {
+    expect.assertions(15);
+
+    createTestServer(done)
+      .get('/api/v0', headers => {
+        expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
+        expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
+        expect(headers['baggage']).toEqual(expect.any(String));
+        expect(headers['__requestUrl']).toBeUndefined();
+      })
+      .get('/api/v1', headers => {
+        expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
+        expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
+        expect(headers['baggage']).toEqual(expect.any(String));
+        expect(headers['__requestUrl']).toBeUndefined();
+      })
+      .get('/api/v2', headers => {
+        expect(headers['baggage']).toBeUndefined();
+        expect(headers['sentry-trace']).toBeUndefined();
+        expect(headers['__requestUrl']).toBeUndefined();
+      })
+      .get('/api/v3', headers => {
+        expect(headers['baggage']).toBeUndefined();
+        expect(headers['sentry-trace']).toBeUndefined();
+        expect(headers['__requestUrl']).toBeUndefined();
+      })
+      .start()
+      .then(SERVER_URL => {
+        createRunner(__dirname, 'scenario.ts')
+          .withEnv({ SERVER_URL })
+          .ensureNoErrorOutput()
+          .ignore('session', 'sessions')
+          .expect({
+            event: {
+              exception: {
+                values: [
+                  {
+                    type: 'Error',
+                    value: 'foo',
+                  },
+                ],
+              },
+            },
+          })
+          .start(done);
+      });
+  });
+});

dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled/scenario.ts

@@ -0,0 +1,25 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  tracePropagationTargets: [/\/v0/, 'v1'],
+  integrations: [],
+  transport: loggingTransport,
+});
+
+async function run(): Promise<void> {
+  await Sentry.startSpan({ name: 'test_span' }, async () => {
+    // Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented
+    await new Promise(resolve => setTimeout(resolve, 100));
+    await fetch(`${process.env.SERVER_URL}/api/v0`);
+    await fetch(`${process.env.SERVER_URL}/api/v1`);
+    await fetch(`${process.env.SERVER_URL}/api/v2`);
+    await fetch(`${process.env.SERVER_URL}/api/v3`);
+  });
+}
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+run();

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

@@ -0,0 +1,40 @@
+import { conditionalTest } from '../../../../utils';
+import { createRunner } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
+
+conditionalTest({ min: 18 })('outgoing fetch', () => {
+  test('outgoing sampled fetch requests are correctly instrumented xxx', done => {
+    expect.assertions(11);
+
+    createTestServer(done)
+      .get('/api/v0', headers => {
+        expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
+        expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
+        expect(headers['baggage']).toEqual(expect.any(String));
+      })
+      .get('/api/v1', headers => {
+        expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
+        expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
+        expect(headers['baggage']).toEqual(expect.any(String));
+      })
+      .get('/api/v2', headers => {
+        expect(headers['baggage']).toBeUndefined();
+        expect(headers['sentry-trace']).toBeUndefined();
+      })
+      .get('/api/v3', headers => {
+        expect(headers['baggage']).toBeUndefined();
+        expect(headers['sentry-trace']).toBeUndefined();
+      })
+      .start()
+      .then(SERVER_URL => {
+        createRunner(__dirname, 'scenario.ts')
+          .withEnv({ SERVER_URL })
+          .expect({
+            transaction: {
+              // we're not too concerned with the actual transaction here since this is tested elsewhere
+            },
+          })
+          .start(done);
+      });
+  });
+});

dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/scenario.ts

@@ -0,0 +1,28 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  tracePropagationTargets: [/\/v0/, 'v1'],
+  tracesSampleRate: 0,
+  integrations: [],
+  transport: loggingTransport,
+});
+
+async function run(): Promise<void> {
+  // Wrap in span that is not sampled
+  await Sentry.startSpan({ name: 'outer' }, async () => {
+    // Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented
+    await new Promise(resolve => setTimeout(resolve, 100));
+    await fetch(`${process.env.SERVER_URL}/api/v0`);
+    await fetch(`${process.env.SERVER_URL}/api/v1`);
+    await fetch(`${process.env.SERVER_URL}/api/v2`);
+    await fetch(`${process.env.SERVER_URL}/api/v3`);
+  });
+
+  Sentry.captureException(new Error('foo'));
+}
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+run();

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

@@ -0,0 +1,48 @@
+import { conditionalTest } from '../../../../utils';
+import { createRunner } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
+
+conditionalTest({ min: 18 })('outgoing fetch', () => {
+  test('outgoing fetch requests are correctly instrumented when not sampled', done => {
+    expect.assertions(11);
+
+    createTestServer(done)
+      .get('/api/v0', headers => {
+        expect(headers['baggage']).toEqual(expect.any(String));
+        expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/));
+        expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0');
+      })
+      .get('/api/v1', headers => {
+        expect(headers['baggage']).toEqual(expect.any(String));
+        expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/));
+        expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0');
+      })
+      .get('/api/v2', headers => {
+        expect(headers['baggage']).toBeUndefined();
+        expect(headers['sentry-trace']).toBeUndefined();
+      })
+      .get('/api/v3', headers => {
+        expect(headers['baggage']).toBeUndefined();
+        expect(headers['sentry-trace']).toBeUndefined();
+      })
+      .start()
+      .then(SERVER_URL => {
+        createRunner(__dirname, 'scenario.ts')
+          .withEnv({ SERVER_URL })
+          .ignore('session', 'sessions')
+          .expect({
+            event: {
+              exception: {
+                values: [
+                  {
+                    type: 'Error',
+                    value: 'foo',
+                  },
+                ],
+              },
+            },
+          })
+          .start(done);
+      });
+  });
+});

dev-packages/node-integration-tests/suites/tracing/tracingNoSampleRate/test.ts → dev-packages/node-integration-tests/suites/tracing/requests/http-noSampleRate/test.ts

@@ -1,10 +1,10 @@
-import { createRunner } from '../../../utils/runner';
-import { createTestServer } from '../../../utils/server';
+import { createRunner } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
 
-test('HttpIntegration should instrument correct requests even without tracesSampleRate', done => {
+test('outgoing http requests are correctly instrumented without tracesSampleRate', done => {
   expect.assertions(15);
 
-  createTestServer(() => {})
+  createTestServer(done)
     .get('/api/v0', headers => {
       expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
       expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');

dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/scenario.ts

@@ -0,0 +1,20 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  tracePropagationTargets: [/\/v0/, 'v1'],
+  integrations: [],
+  transport: loggingTransport,
+});
+
+import * as http from 'http';
+
+Sentry.startSpan({ name: 'test_span' }, () => {
+  http.get(`${process.env.SERVER_URL}/api/v0`);
+  http.get(`${process.env.SERVER_URL}/api/v1`);
+  http.get(`${process.env.SERVER_URL}/api/v2`);
+  http.get(`${process.env.SERVER_URL}/api/v3`);
+});

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

@@ -0,0 +1,37 @@
+import { createRunner } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
+
+test('outgoing sampled http requests are correctly instrumented', done => {
+  expect.assertions(11);
+
+  createTestServer(done)
+    .get('/api/v0', headers => {
+      expect(headers['baggage']).toEqual(expect.any(String));
+      expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
+      expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
+    })
+    .get('/api/v1', headers => {
+      expect(headers['baggage']).toEqual(expect.any(String));
+      expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
+      expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
+    })
+    .get('/api/v2', headers => {
+      expect(headers['baggage']).toBeUndefined();
+      expect(headers['sentry-trace']).toBeUndefined();
+    })
+    .get('/api/v3', headers => {
+      expect(headers['baggage']).toBeUndefined();
+      expect(headers['sentry-trace']).toBeUndefined();
+    })
+    .start()
+    .then(SERVER_URL => {
+      createRunner(__dirname, 'scenario.ts')
+        .withEnv({ SERVER_URL })
+        .expect({
+          transaction: {
+            // we're not too concerned with the actual transaction here since this is tested elsewhere
+          },
+        })
+        .start(done);
+    });
+});

dev-packages/node-integration-tests/suites/tracing/tracingUnsampled/test.ts → dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts

@@ -1,7 +1,7 @@
-import { createRunner } from '../../../utils/runner';
-import { createTestServer } from '../../../utils/server';
+import { createRunner } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
 
-test('HttpIntegration should instrument correct requests even when not sampled', done => {
+test('outgoing http requests are correctly instrumented when not sampled', done => {
   expect.assertions(11);
 
   createTestServer(done)

packages/node/src/integrations/http.ts

@@ -3,6 +3,7 @@ import type { Span } from '@opentelemetry/api';
 import { SpanKind } from '@opentelemetry/api';
 import { registerInstrumentations } from '@opentelemetry/instrumentation';
 import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
+import { storeRequestUrlOnPropagationCarrier } from '@sentry/opentelemetry';
 
 import {
   addBreadcrumb,
@@ -17,7 +18,7 @@ import {
 import { getClient, getRequestSpanData, getSpanKind } from '@sentry/opentelemetry';
 import type { IntegrationFn } from '@sentry/types';
 
-import { addNonEnumerableProperty, stripUrlQueryAndFragment } from '@sentry/utils';
+import { stripUrlQueryAndFragment } from '@sentry/utils';
 import type { NodeClient } from '../sdk/client';
 import { setIsolationScope } from '../sdk/scope';
 import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module';
@@ -65,13 +66,10 @@ const _httpIntegration = ((options: HttpOptions = {}) => {
               return true;
             }
 
-            // SUPER HACK:
-            // We store the URL on the headers object, because this is passed to the propagator
-            // Where we can pick the URL off to deterime if we should attach trace headers.
+            // We keep the URL on the headers (which are the carrier for the propagator)
+            // so we can access it in the propagator to check against tracePropagationTargets
             const headers = request.headers || {};
-            // Can't use a non-enumerable property because http instrumentation clones this
-            // We remove this in the propagator
-            headers['__requestUrl'] = url;
+            storeRequestUrlOnPropagationCarrier(headers, url);
             request.headers = headers;
 
             if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url)) {

packages/opentelemetry/src/constants.ts

@@ -6,6 +6,7 @@ export const SENTRY_BAGGAGE_HEADER = 'baggage';
 export const SENTRY_TRACE_STATE_DSC = 'sentry.dsc';
 export const SENTRY_TRACE_STATE_PARENT_SPAN_ID = 'sentry.parent_span_id';
 export const SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING = 'sentry.sampled_not_recording';
+export const SENTRY_TRACE_STATE_URL = 'sentry.url';
 
 export const SENTRY_SCOPES_CONTEXT_KEY = createContextKey('sentry_scopes');
 

packages/opentelemetry/src/index.ts

@@ -17,6 +17,7 @@ export {
 } from './utils/spanTypes';
 
 export { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
+export { storeRequestUrlOnPropagationCarrier } from './utils/storeRequestUrlForPropagation';
 
 export { isSentryRequestSpan } from './utils/isSentryRequest';