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..4917fa4191c6 --- /dev/null +++ b/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}$/); + } +}); 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-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); +}); 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..986be8d4c8ff 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,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)