SUPER HACK: fix it

Author: mydea

dev-packages/node-integration-tests/suites/express/sentry-trace/trace-header-assign/server.ts

@@ -0,0 +1,35 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+export type TestAPIResponse = { test_data: { host: string; 'sentry-trace': string; baggage: string } };
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  environment: 'prod',
+  integrations: [
+    // TODO: This used to use the Express integration
+  ],
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+});
+
+import http from 'http';
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import cors from 'cors';
+import express from 'express';
+
+const app = express();
+
+app.use(cors());
+
+app.get('/test/express', (_req, res) => {
+  const headers = http.get('http://somewhere.not.sentry/').getHeaders();
+
+  // Responding with the headers outgoing request headers back to the assertions.
+  res.send({ test_data: headers });
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/sentry-trace/trace-header-assign/test.ts

@@ -7,7 +7,7 @@ afterAll(() => {
 });
 
 test('Should assign `sentry-trace` header which sets parent trace id of an outgoing request.', async () => {
-  const runner = createRunner(__dirname, '..', 'server.ts').start();
+  const runner = createRunner(__dirname, 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
     'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',

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

@@ -1,27 +1,31 @@
 import { createRunner } from '../../../utils/runner';
 import { createTestServer } from '../../../utils/server';
 
-test('HttpIntegration should instrument correct requests even without tracesSampleRate xxx', done => {
-  expect.assertions(11);
+test('HttpIntegration should instrument correct requests even without tracesSampleRate', done => {
+  expect.assertions(15);
 
-  createTestServer(done)
+  createTestServer(() => {})
     .get('/api/v0', headers => {
-      expect(headers['baggage']).toBeUndefined();
       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['baggage']).toBeUndefined();
       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 => {

dev-packages/node-integration-tests/suites/tracing/tracingUnsampled/scenario.ts

@@ -13,10 +13,13 @@ Sentry.init({
 import * as http from 'http';
 
 async function run(): Promise<void> {
-  await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`);
-  await makeHttpRequest(`${process.env.SERVER_URL}/api/v1`);
-  await makeHttpRequest(`${process.env.SERVER_URL}/api/v2`);
-  await makeHttpRequest(`${process.env.SERVER_URL}/api/v3`);
+  // Wrap in span that is not sampled
+  await Sentry.startSpan({ name: 'outer' }, async () => {
+    await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`);
+    await makeHttpRequest(`${process.env.SERVER_URL}/api/v1`);
+    await makeHttpRequest(`${process.env.SERVER_URL}/api/v2`);
+    await makeHttpRequest(`${process.env.SERVER_URL}/api/v3`);
+  });
 
   Sentry.captureException(new Error('foo'));
 }

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

@@ -6,12 +6,12 @@ test('HttpIntegration should instrument correct requests even when not sampled',
 
   createTestServer(done)
     .get('/api/v0', headers => {
-      expect(headers['baggage']).toBeUndefined();
+      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']).toBeUndefined();
+      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');
     })

packages/node/src/integrations/http.ts

@@ -17,7 +17,7 @@ import {
 import { getClient, getRequestSpanData, getSpanKind } from '@sentry/opentelemetry';
 import type { IntegrationFn } from '@sentry/types';
 
-import { stripUrlQueryAndFragment } from '@sentry/utils';
+import { addNonEnumerableProperty, stripUrlQueryAndFragment } from '@sentry/utils';
 import type { NodeClient } from '../sdk/client';
 import { setIsolationScope } from '../sdk/scope';
 import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module';
@@ -65,6 +65,15 @@ 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.
+            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;
+            request.headers = headers;
+
             if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url)) {
               return true;
             }
@@ -93,18 +102,18 @@ const _httpIntegration = ((options: HttpOptions = {}) => {
           requestHook: (span, req) => {
             addOriginToSpan(span, 'auto.http.otel.http');
 
+            const scopes = getCapturedScopesOnSpan(span);
+
+            const isolationScope = (scopes.isolationScope || getIsolationScope()).clone();
+            const scope = scopes.scope || getCurrentScope();
+
             // both, incoming requests and "client" requests made within the app trigger the requestHook
             // we only want to isolate and further annotate incoming requests (IncomingMessage)
             if (_isClientRequest(req)) {
               return;
             }
 
-            const scopes = getCapturedScopesOnSpan(span);
-
             // Update the isolation scope, isolate this request
-            const isolationScope = (scopes.isolationScope || getIsolationScope()).clone();
-            const scope = scopes.scope || getCurrentScope();
-
             isolationScope.setSDKProcessingMetadata({ request: req });
 
             const client = getClient<NodeClient>();

packages/opentelemetry/src/propagator.ts

@@ -1,18 +1,19 @@
-import type { Baggage, Context, Span, SpanContext, TextMapGetter, TextMapSetter } from '@opentelemetry/api';
+import { Baggage, Context, ROOT_CONTEXT, Span, SpanContext, TextMapGetter, TextMapSetter } from '@opentelemetry/api';
 import { context } from '@opentelemetry/api';
 import { TraceFlags, propagation, trace } from '@opentelemetry/api';
 import { TraceState, W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
-import { continueTrace, hasTracingEnabled } from '@sentry/core';
+import { continueTrace, getDefaultCurrentScope, getDefaultIsolationScope, hasTracingEnabled } from '@sentry/core';
 import { getRootSpan } from '@sentry/core';
 import { spanToJSON } from '@sentry/core';
 import { getClient, getCurrentScope, getDynamicSamplingContextFromClient, getIsolationScope } from '@sentry/core';
-import type { DynamicSamplingContext, Options, PropagationContext } from '@sentry/types';
+import type { DynamicSamplingContext, Options, PolymorphicRequest, PropagationContext } from '@sentry/types';
 import {
   LRUMap,
   SENTRY_BAGGAGE_KEY_PREFIX,
   baggageHeaderToDynamicSamplingContext,
   dynamicSamplingContextToSentryBaggageHeader,
+  extractRequestData,
   generateSentryTraceHeader,
   logger,
   parseBaggageHeader,
@@ -84,7 +85,8 @@ export class SentryPropagator extends W3CBaggagePropagator {
     }
 
     const activeSpan = trace.getSpan(context);
-    const url = activeSpan && spanToJSON(activeSpan).data?.[SemanticAttributes.HTTP_URL];
+    const url = getCurrentURL(carrier as { __requestUrl?: string }, activeSpan);
+
     const tracePropagationTargets = getClient()?.getOptions()?.tracePropagationTargets;
     if (
       typeof url === 'string' &&
@@ -120,7 +122,10 @@ export class SentryPropagator extends W3CBaggagePropagator {
       }, baggage);
     }
 
-    setter.set(carrier, SENTRY_TRACE_HEADER, generateSentryTraceHeader(traceId, spanId, sampled));
+    // We also want to avoid setting the default OTEL trace ID, if we get that for whatever reason
+    if (traceId && traceId !== '00000000000000000000000000000000') {
+      setter.set(carrier, SENTRY_TRACE_HEADER, generateSentryTraceHeader(traceId, spanId, sampled));
+    }
 
     super.inject(propagation.setBaggage(context, baggage), carrier, setter);
   }
@@ -230,40 +235,15 @@ function getInjectionData(context: Context): {
   }
 
   // Else we try to use the propagation context from the scope
-  const scope = getScopesFromContext(context)?.scope;
-
-  if (scope) {
-    const propagationContext = scope.getPropagationContext();
-    const dynamicSamplingContext = getDynamicSamplingContext(propagationContext, propagationContext.traceId);
-    return {
-      dynamicSamplingContext,
-      traceId: propagationContext.traceId,
-      spanId: propagationContext.spanId,
-      sampled: propagationContext.sampled,
-    };
-  }
-
-  // Else, we look at the remote span context
-  if (span) {
-    const spanContext = span.spanContext();
-    const propagationContext = getPropagationContextFromSpan(span);
-    const dynamicSamplingContext = getDynamicSamplingContext(propagationContext, spanContext.traceId);
+  const scope = getScopesFromContext(context)?.scope || getCurrentScope();
 
-    return {
-      dynamicSamplingContext,
-      traceId: spanContext.traceId,
-      spanId: spanContext.spanId,
-      sampled: getSamplingDecision(spanContext),
-    };
-  }
-
-  // If we have neither, there is nothing much we can do, but that should not happen usually
-  // Unless there is a detached OTEL context being passed around
+  const propagationContext = scope.getPropagationContext();
+  const dynamicSamplingContext = getDynamicSamplingContext(propagationContext, propagationContext.traceId);
   return {
-    dynamicSamplingContext: undefined,
-    traceId: undefined,
-    spanId: undefined,
-    sampled: undefined,
+    dynamicSamplingContext,
+    traceId: propagationContext.traceId,
+    spanId: propagationContext.spanId,
+    sampled: propagationContext.sampled,
   };
 }
 
@@ -334,3 +314,25 @@ function getExistingBaggage(carrier: unknown): string | undefined {
     return undefined;
   }
 }
+
+function getCurrentURL(carrier: { __requestUrl?: string }, span: Span | undefined): string | undefined {
+  try {
+    if (carrier.__requestUrl) {
+      const url = carrier.__requestUrl;
+      delete carrier.__requestUrl;
+      return url;
+    }
+  } catch {
+    // ignore errors here
+  }
+
+  if (span) {
+    const url = spanToJSON(span).data?.[SemanticAttributes.HTTP_URL];
+
+    if (url) {
+      return url;
+    }
+  }
+
+  return undefined;
+}