fix(core): Remove sampled flag from dynamic sampling context in Tracing without Performance mode (#13753)

Fix a bug in Core that surfaced in Node apps configured for
Tracing without Performance.

Previously, we'd incorrectly add `sampled: false` flag when creating the
DSC from an active span if the application was configured for TwP. This
is possible because in TwP, Otel still emits non-recording spans for the
incoming requests. Our implementation in Core didn't account for this
edge case yet.

Author: Lms24

dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/server.js

@@ -4,25 +4,22 @@ const Sentry = require('@sentry/node');
 Sentry.init({
   dsn: 'https://[email protected]/1337',
   transport: loggingTransport,
-  beforeSend(event) {
-    event.contexts = {
-      ...event.contexts,
-      traceData: {
-        ...Sentry.getTraceData(),
-        metaTags: Sentry.getTraceMetaTags(),
-      },
-    };
-    return event;
-  },
 });
 
 // express must be required after Sentry is initialized
 const express = require('express');
 
 const app = express();
 
-app.get('/test', () => {
-  throw new Error('test error');
+app.get('/test', (_req, res) => {
+  Sentry.withScope(scope => {
+    scope.setContext('traceData', {
+      ...Sentry.getTraceData(),
+      metaTags: Sentry.getTraceMetaTags(),
+    });
+    Sentry.captureException(new Error('test error 2'));
+  });
+  res.status(200).send();
 });
 
 Sentry.setupExpressErrorHandler(app);

dev-packages/node-integration-tests/suites/tracing/meta-tags-twp-errors/test.ts

@@ -5,7 +5,7 @@ describe('errors in TwP mode have same trace in trace context and getTraceData()
     cleanupChildProcesses();
   });
 
-  test('in incoming request', async () => {
+  test('in incoming request', done => {
     createRunner(__dirname, 'server.js')
       .expect({
         event: event => {
@@ -17,14 +17,16 @@ describe('errors in TwP mode have same trace in trace context and getTraceData()
           const traceData = contexts?.traceData || {};
 
           expect(traceData['sentry-trace']).toEqual(`${trace_id}-${span_id}`);
+
           expect(traceData.baggage).toContain(`sentry-trace_id=${trace_id}`);
+          expect(traceData.baggage).not.toContain('sentry-sampled=');
 
           expect(traceData.metaTags).toContain(`<meta name="sentry-trace" content="${trace_id}-${span_id}"/>`);
-          expect(traceData.metaTags).toContain(`sentr y-trace_id=${trace_id}`);
+          expect(traceData.metaTags).toContain(`sentry-trace_id=${trace_id}`);
           expect(traceData.metaTags).not.toContain('sentry-sampled=');
         },
       })
-      .start()
+      .start(done)
       .makeRequest('get', '/test');
   });
 
@@ -41,6 +43,7 @@ describe('errors in TwP mode have same trace in trace context and getTraceData()
 
           expect(traceData['sentry-trace']).toEqual(`${trace_id}-${span_id}`);
           expect(traceData.baggage).toContain(`sentry-trace_id=${trace_id}`);
+          expect(traceData.baggage).not.toContain('sentry-sampled=');
 
           expect(traceData.metaTags).toContain(`<meta name="sentry-trace" content="${trace_id}-${span_id}"/>`);
           expect(traceData.metaTags).toContain(`sentry-trace_id=${trace_id}`);

dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts

@@ -27,5 +27,6 @@ describe('getTraceMetaTags', () => {
     expect(sentryBaggageContent).toContain('sentry-environment=production');
     expect(sentryBaggageContent).toContain('sentry-public_key=public');
     expect(sentryBaggageContent).toContain(`sentry-trace_id=${traceId}`);
+    expect(sentryBaggageContent).not.toContain('sentry-sampled=');
   });
 });

packages/core/src/tracing/dynamicSamplingContext.ts

@@ -9,6 +9,7 @@ import {
 import { DEFAULT_ENVIRONMENT } from '../constants';
 import { getClient } from '../currentScopes';
 import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes';
+import { hasTracingEnabled } from '../utils/hasTracingEnabled';
 import { getRootSpan, spanIsSampled, spanToJSON } from '../utils/spanUtils';
 
 /**
@@ -103,7 +104,12 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
     dsc.transaction = name;
   }
 
-  dsc.sampled = String(spanIsSampled(rootSpan));
+  // How can we even land here with hasTracingEnabled() returning false?
+  // Otel creates a Non-recording span in Tracing Without Performance mode when handling incoming requests
+  // So we end up with an active span that is not sampled (neither positively nor negatively)
+  if (hasTracingEnabled()) {
+    dsc.sampled = String(spanIsSampled(rootSpan));
+  }
 
   client.emit('createDsc', dsc, rootSpan);
 

packages/core/test/lib/tracing/dynamicSamplingContext.test.ts

@@ -2,6 +2,7 @@ import type { Span, SpanContextData, TransactionSource } from '@sentry/types';
 import {
   SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  getClient,
   setCurrentClient,
 } from '../../../src';
 import { SentrySpan, getDynamicSamplingContextFromSpan, startInactiveSpan } from '../../../src/tracing';
@@ -68,7 +69,7 @@ describe('getDynamicSamplingContextFromSpan', () => {
       environment: 'production',
       sampled: 'true',
       sample_rate: '0.56',
-      trace_id: expect.any(String),
+      trace_id: expect.stringMatching(/^[a-f0-9]{32}$/),
       transaction: 'tx',
     });
   });
@@ -85,7 +86,7 @@ describe('getDynamicSamplingContextFromSpan', () => {
       environment: 'production',
       sampled: 'true',
       sample_rate: '1',
-      trace_id: expect.any(String),
+      trace_id: expect.stringMatching(/^[a-f0-9]{32}$/),
       transaction: 'tx',
     });
   });
@@ -107,7 +108,7 @@ describe('getDynamicSamplingContextFromSpan', () => {
       environment: 'production',
       sampled: 'true',
       sample_rate: '0.56',
-      trace_id: expect.any(String),
+      trace_id: expect.stringMatching(/^[a-f0-9]{32}$/),
       transaction: 'tx',
     });
   });
@@ -144,4 +145,23 @@ describe('getDynamicSamplingContextFromSpan', () => {
       expect(dsc.transaction).toEqual('tx');
     });
   });
+
+  it("doesn't return the sampled flag in the DSC if in Tracing without Performance mode", () => {
+    const rootSpan = new SentrySpan({
+      name: 'tx',
+      sampled: undefined,
+    });
+
+    // Simulate TwP mode by deleting the tracesSampleRate option set in beforeEach
+    delete getClient()?.getOptions().tracesSampleRate;
+
+    const dynamicSamplingContext = getDynamicSamplingContextFromSpan(rootSpan);
+
+    expect(dynamicSamplingContext).toStrictEqual({
+      release: '1.0.1',
+      environment: 'production',
+      trace_id: expect.stringMatching(/^[a-f0-9]{32}$/),
+      transaction: 'tx',
+    });
+  });
 });