fix(node-core): Recycle propagationContext for each request

dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/server.js

@@ -0,0 +1,23 @@
+const { loggingTransport } = require('@sentry-internal/node-core-integration-tests');
+const Sentry = require('@sentry/node-core');
+const { setupOtel } = require('../../../utils/setupOtel.js');
+
+const client = Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+  tracesSampleRate: 1.0,
+});
+
+setupOtel(client);
+
+const express = require('express');
+const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-core-integration-tests');
+
+const app = express();
+
+app.get('/test', (_req, res) => {
+  Sentry.captureException(new Error('test error'));
+  res.json({ success: true });
+});
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts

@@ -0,0 +1,39 @@
+import { afterAll, expect, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('errors from in different requests each get a unique traceId when tracing is enabled', async () => {
+  const eventTraceIds: string[] = [];
+
+  const runner = createRunner(__dirname, 'server.js')
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .start();
+
+  await runner.makeRequest('get', '/test');
+  await runner.makeRequest('get', '/test');
+  await runner.makeRequest('get', '/test');
+
+  await runner.completed();
+
+  expect(new Set(eventTraceIds).size).toBe(3);
+  for (const traceId of eventTraceIds) {
+    expect(traceId).toMatch(/^[a-f\d]{32}$/);
+  }
+});

dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/server.js

@@ -0,0 +1,23 @@
+const { loggingTransport } = require('@sentry-internal/node-core-integration-tests');
+const Sentry = require('@sentry/node-core');
+const { setupOtel } = require('../../../utils/setupOtel.js');
+
+const client = Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+});
+
+setupOtel(client);
+
+const express = require('express');
+const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-core-integration-tests');
+
+const app = express();
+
+app.get('/test', (_req, res) => {
+  Sentry.captureException(new Error('test error'));
+  const traceId = Sentry.getCurrentScope().getPropagationContext().traceId;
+  res.json({ traceId });
+});
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/test.ts

@@ -0,0 +1,43 @@
+import { afterAll, expect, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('each request gets a unique traceId when tracing is disabled', async () => {
+  const eventTraceIds: string[] = [];
+
+  const runner = createRunner(__dirname, 'server.js')
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .start();
+
+  const propagationContextTraceIds = [
+    ((await runner.makeRequest('get', '/test')) as { traceId: string }).traceId,
+    ((await runner.makeRequest('get', '/test')) as { traceId: string }).traceId,
+    ((await runner.makeRequest('get', '/test')) as { traceId: string }).traceId,
+  ];
+
+  await runner.completed();
+
+  expect(new Set(propagationContextTraceIds).size).toBe(3);
+  for (const traceId of propagationContextTraceIds) {
+    expect(traceId).toMatch(/^[a-f\d]{32}$/);
+  }
+
+  expect(eventTraceIds).toEqual(propagationContextTraceIds);
+});

dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/server.ts

@@ -0,0 +1,23 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  transport: loggingTransport,
+  tracesSampleRate: 1.0,
+});
+
+import express from 'express';
+
+const app = express();
+
+app.get('/test', async (_req, res) => {
+  Sentry.captureException(new Error('test error'));
+  // calling Sentry.flush() here to ensure that the order in which we send transaction and errors
+  // is guaranteed to be 1. error, 2. transaction (repeated 3x in test)
+  await Sentry.flush();
+  res.json({ success: true });
+});
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts

@@ -0,0 +1,57 @@
+import { afterAll, expect, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('errors and transactions get a unique traceId per request, when tracing is enabled', async () => {
+  const eventTraceIds: string[] = [];
+  const transactionTraceIds: string[] = [];
+
+  const runner = createRunner(__dirname, 'server.ts')
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      transaction: transaction => {
+        transactionTraceIds.push(transaction.spans?.[0]?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      transaction: transaction => {
+        transactionTraceIds.push(transaction.spans?.[0]?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      transaction: transaction => {
+        transactionTraceIds.push(transaction.spans?.[0]?.trace_id || '');
+      },
+    })
+    .start();
+
+  await runner.makeRequest('get', '/test');
+  await runner.makeRequest('get', '/test');
+  await runner.makeRequest('get', '/test');
+
+  await runner.completed();
+
+  expect(new Set(transactionTraceIds).size).toBe(3);
+  for (const traceId of transactionTraceIds) {
+    expect(traceId).toMatch(/^[a-f\d]{32}$/);
+  }
+
+  expect(eventTraceIds).toEqual(transactionTraceIds);
+});

dev-packages/node-integration-tests/suites/tracing/traceid-recycling/server.ts

@@ -0,0 +1,20 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  transport: loggingTransport,
+});
+
+import express from 'express';
+
+const app = express();
+
+app.get('/test', (_req, res) => {
+  Sentry.captureException(new Error('test error'));
+  const traceId = Sentry.getCurrentScope().getPropagationContext().traceId;
+  res.json({ traceId });
+});
+
+startExpressServerAndSendPortToRunner(app);

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

@@ -0,0 +1,43 @@
+import { afterAll, expect, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('each request gets a unique traceId when tracing is disabled', async () => {
+  const eventTraceIds: string[] = [];
+
+  const runner = createRunner(__dirname, 'server.ts')
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .expect({
+      event: event => {
+        eventTraceIds.push(event.contexts?.trace?.trace_id || '');
+      },
+    })
+    .start();
+
+  const propagationContextTraceIds = [
+    ((await runner.makeRequest('get', '/test')) as { traceId: string }).traceId,
+    ((await runner.makeRequest('get', '/test')) as { traceId: string }).traceId,
+    ((await runner.makeRequest('get', '/test')) as { traceId: string }).traceId,
+  ];
+
+  await runner.completed();
+
+  expect(new Set(propagationContextTraceIds).size).toBe(3);
+  for (const traceId of propagationContextTraceIds) {
+    expect(traceId).toMatch(/^[a-f\d]{32}$/);
+  }
+
+  expect(eventTraceIds).toEqual(propagationContextTraceIds);
+});

packages/node-core/src/integrations/http/httpServerIntegration.ts

@@ -6,9 +6,11 @@ import type { Socket } from 'node:net';
 import { context, createContextKey, propagation } from '@opentelemetry/api';
 import type { AggregationCounts, Client, Integration, IntegrationFn, Scope } from '@sentry/core';
 import {
+  _INTERNAL_safeMathRandom,
   addNonEnumerableProperty,
   debug,
   generateSpanId,
+  generateTraceId,
   getClient,
   getCurrentScope,
   getIsolationScope,
@@ -218,10 +220,19 @@ function instrumentServer(
       }
 
       return withIsolationScope(isolationScope, () => {
-        // Set a new propagationSpanId for this request
-        // We rely on the fact that `withIsolationScope()` will implicitly also fork the current scope
-        // This way we can save an "unnecessary" `withScope()` invocation
-        getCurrentScope().getPropagationContext().propagationSpanId = generateSpanId();
+        const newPropagationContext = {
+          traceId: generateTraceId(),
+          sampleRand: _INTERNAL_safeMathRandom(),
+          propagationSpanId: generateSpanId(),
+        };
+        // - Set a fresh propagation context so each request gets a unique traceId.
+        //   When there are incoming trace headers, propagation.extract() below sets a remote
+        //   span on the OTel context which takes precedence in getTraceContextForScope().
+        // - We can write directly to the current scope here because it is forked implicitly via
+        //   `context.with` in `withIsolationScope` (See `SentryContextManager`).
+        // - explicitly making a deep copy to avoid mutation of original PC on the other scope
+        getCurrentScope().setPropagationContext({ ...newPropagationContext });
+        isolationScope.setPropagationContext({ ...newPropagationContext });
 
         const ctx = propagation
           .extract(context.active(), normalizedRequest.headers)