From 62da3eec3b9d2db3940132aeac45025d55604c26 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 Mar 2026 14:24:42 +0100 Subject: [PATCH 1/6] fix(node-core): Recycle traceId per request when tracing is disabled --- .../tracing/traceid-recycling/server.js | 23 ++++++++++ .../suites/tracing/traceid-recycling/test.ts | 43 +++++++++++++++++++ .../tracing/traceid-recycling/server.ts | 20 +++++++++ .../suites/tracing/traceid-recycling/test.ts | 43 +++++++++++++++++++ .../http/httpServerIntegration.ts | 14 ++++-- 5 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/server.js create mode 100644 dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/traceid-recycling/server.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/traceid-recycling/test.ts diff --git a/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/server.js b/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/server.js new file mode 100644 index 000000000000..f6ff7b354b19 --- /dev/null +++ b/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); diff --git a/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/test.ts b/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling/test.ts new file mode 100644 index 000000000000..c83d9de4cac4 --- /dev/null +++ b/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); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/traceid-recycling/server.ts b/dev-packages/node-integration-tests/suites/tracing/traceid-recycling/server.ts new file mode 100644 index 000000000000..11f436a1885f --- /dev/null +++ b/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); diff --git a/dev-packages/node-integration-tests/suites/tracing/traceid-recycling/test.ts b/dev-packages/node-integration-tests/suites/tracing/traceid-recycling/test.ts new file mode 100644 index 000000000000..d2be070cb091 --- /dev/null +++ b/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); +}); diff --git a/packages/node-core/src/integrations/http/httpServerIntegration.ts b/packages/node-core/src/integrations/http/httpServerIntegration.ts index f5833f1b007b..eaacef638251 100644 --- a/packages/node-core/src/integrations/http/httpServerIntegration.ts +++ b/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,14 @@ 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(); + // 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(). + getCurrentScope().setPropagationContext({ + traceId: generateTraceId(), + sampleRand: _INTERNAL_safeMathRandom(), + propagationSpanId: generateSpanId(), + }); const ctx = propagation .extract(context.active(), normalizedRequest.headers) From fc273f281948ace82b0fbeed6c5053c1c127e79f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 Mar 2026 14:36:22 +0100 Subject: [PATCH 2/6] comment --- .../node-core/src/integrations/http/httpServerIntegration.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/node-core/src/integrations/http/httpServerIntegration.ts b/packages/node-core/src/integrations/http/httpServerIntegration.ts index eaacef638251..5512066dd190 100644 --- a/packages/node-core/src/integrations/http/httpServerIntegration.ts +++ b/packages/node-core/src/integrations/http/httpServerIntegration.ts @@ -223,6 +223,8 @@ function instrumentServer( // 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`). getCurrentScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: _INTERNAL_safeMathRandom(), From 5d421b8c7a0816cb9878c2bfddb84f4534f4cb22 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 Mar 2026 14:38:00 +0100 Subject: [PATCH 3/6] also set on isolationScope --- .../src/integrations/http/httpServerIntegration.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/node-core/src/integrations/http/httpServerIntegration.ts b/packages/node-core/src/integrations/http/httpServerIntegration.ts index 5512066dd190..ca294dc52931 100644 --- a/packages/node-core/src/integrations/http/httpServerIntegration.ts +++ b/packages/node-core/src/integrations/http/httpServerIntegration.ts @@ -220,16 +220,18 @@ function instrumentServer( } return withIsolationScope(isolationScope, () => { + 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`). - getCurrentScope().setPropagationContext({ - traceId: generateTraceId(), - sampleRand: _INTERNAL_safeMathRandom(), - propagationSpanId: generateSpanId(), - }); + getCurrentScope().setPropagationContext(newPropagationContext); + isolationScope.setPropagationContext(newPropagationContext); const ctx = propagation .extract(context.active(), normalizedRequest.headers) From 54dee291a76a91cde74e7231bdd9c28fd96fd880 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 Mar 2026 15:23:18 +0100 Subject: [PATCH 4/6] add tests for when tracing is enabled --- .../traceid-recycling-with-spans/server.js | 23 ++++++++ .../traceid-recycling-with-spans/test.ts | 40 +++++++++++++ .../traceid-recycling-with-spans/server.ts | 23 ++++++++ .../traceid-recycling-with-spans/test.ts | 57 +++++++++++++++++++ 4 files changed, 143 insertions(+) create mode 100644 dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/server.js create mode 100644 dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/server.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts diff --git a/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/server.js b/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/server.js new file mode 100644 index 000000000000..bdf75643111b --- /dev/null +++ b/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); diff --git a/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts b/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts new file mode 100644 index 000000000000..46f12fdb6fff --- /dev/null +++ b/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts @@ -0,0 +1,40 @@ +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 => { + // console.log('xx event 3'); + 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}$/); + } +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/server.ts b/dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/server.ts new file mode 100644 index 000000000000..77e83d839e3b --- /dev/null +++ b/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); diff --git a/dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts new file mode 100644 index 000000000000..93d7e9d41ce6 --- /dev/null +++ b/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); +}); From de0448910cd2ba19feb607e4ea3689da70aea49c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 17 Mar 2026 15:32:34 +0100 Subject: [PATCH 5/6] deep copy PCs onto both scopes --- .../integrations/http/httpServerIntegration.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/node-core/src/integrations/http/httpServerIntegration.ts b/packages/node-core/src/integrations/http/httpServerIntegration.ts index ca294dc52931..363530d8b939 100644 --- a/packages/node-core/src/integrations/http/httpServerIntegration.ts +++ b/packages/node-core/src/integrations/http/httpServerIntegration.ts @@ -225,13 +225,14 @@ function instrumentServer( 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`). - getCurrentScope().setPropagationContext(newPropagationContext); - isolationScope.setPropagationContext(newPropagationContext); + // - 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`). + // - explicilty 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) From 6f94462934cc7f0fa263c72b9199289bcd5ce462 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 18 Mar 2026 15:03:43 +0100 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Charly Gomez --- .../suites/tracing/traceid-recycling-with-spans/test.ts | 1 - .../node-core/src/integrations/http/httpServerIntegration.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts b/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts index 46f12fdb6fff..4917fa4191c6 100644 --- a/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts +++ b/dev-packages/node-core-integration-tests/suites/tracing/traceid-recycling-with-spans/test.ts @@ -21,7 +21,6 @@ test('errors from in different requests each get a unique traceId when tracing i }) .expect({ event: event => { - // console.log('xx event 3'); eventTraceIds.push(event.contexts?.trace?.trace_id || ''); }, }) diff --git a/packages/node-core/src/integrations/http/httpServerIntegration.ts b/packages/node-core/src/integrations/http/httpServerIntegration.ts index 363530d8b939..986be8d4c8ff 100644 --- a/packages/node-core/src/integrations/http/httpServerIntegration.ts +++ b/packages/node-core/src/integrations/http/httpServerIntegration.ts @@ -230,7 +230,7 @@ function instrumentServer( // 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`). - // - explicilty making a deep copy to avoid mutation of original PC on the other scope + // - explicitly making a deep copy to avoid mutation of original PC on the other scope getCurrentScope().setPropagationContext({ ...newPropagationContext }); isolationScope.setPropagationContext({ ...newPropagationContext });