diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index cb1be728f26f..b0c67a006185 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -19,7 +19,6 @@ import { getClient, getCurrentScope, getDynamicSamplingContextFromSpan, - getIsolationScope, getLocationHref, GLOBAL_OBJ, hasSpansEnabled, @@ -554,12 +553,6 @@ export const browserTracingIntegration = ((options: Partial { setCurrentClient(client); client.init(); - const oldIsolationScopePropCtx = getIsolationScope().getPropagationContext(); const oldCurrentScopePropCtx = getCurrentScope().getPropagationContext(); startBrowserTracingNavigationSpan(client, { name: 'test navigation span' }); - const newIsolationScopePropCtx = getIsolationScope().getPropagationContext(); const newCurrentScopePropCtx = getCurrentScope().getPropagationContext(); expect(oldCurrentScopePropCtx).toEqual({ @@ -950,24 +948,13 @@ describe('browserTracingIntegration', () => { propagationSpanId: expect.stringMatching(/[a-f0-9]{16}/), sampleRand: expect.any(Number), }); - expect(oldIsolationScopePropCtx).toEqual({ - traceId: expect.stringMatching(/[a-f0-9]{32}/), - sampleRand: expect.any(Number), - }); + expect(newCurrentScopePropCtx).toEqual({ traceId: expect.stringMatching(/[a-f0-9]{32}/), propagationSpanId: expect.stringMatching(/[a-f0-9]{16}/), sampleRand: expect.any(Number), }); - expect(newIsolationScopePropCtx).toEqual({ - traceId: expect.stringMatching(/[a-f0-9]{32}/), - propagationSpanId: expect.stringMatching(/[a-f0-9]{16}/), - sampleRand: expect.any(Number), - }); - - expect(newIsolationScopePropCtx.traceId).not.toEqual(oldIsolationScopePropCtx.traceId); expect(newCurrentScopePropCtx.traceId).not.toEqual(oldCurrentScopePropCtx.traceId); - expect(newIsolationScopePropCtx.propagationSpanId).not.toEqual(oldIsolationScopePropCtx.propagationSpanId); }); it("saves the span's positive sampling decision and its DSC on the propagationContext when the span finishes", () => { diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index e2322766287b..12623aa224d9 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -1,10 +1,8 @@ import type { AttributeObject, RawAttribute, RawAttributes } from './attributes'; -import { getClient, getCurrentScope, getIsolationScope, withIsolationScope } from './currentScopes'; +import { getClient, getCurrentScope, getIsolationScope } from './currentScopes'; import { DEBUG_BUILD } from './debug-build'; import type { CaptureContext } from './scope'; import { closeSession, makeSession, updateSession } from './session'; -import { startNewTrace } from './tracing/trace'; -import type { CheckIn, FinishedCheckIn, MonitorConfig } from './types/checkin'; import type { Event, EventHint } from './types/event'; import type { EventProcessor } from './types/eventprocessor'; import type { Extra, Extras } from './types/extra'; @@ -13,12 +11,9 @@ import type { Session, SessionContext } from './types/session'; import type { SeverityLevel } from './types/severity'; import type { User } from './types/user'; import { debug } from './utils/debug-logger'; -import { isThenable } from './utils/is'; -import { uuid4 } from './utils/misc'; import type { ExclusiveEventHintOrCaptureContext } from './utils/prepareEvent'; import { parseEventHintOrCaptureContext } from './utils/prepareEvent'; import { getCombinedScopeData } from './utils/scopeData'; -import { timestampInSeconds } from './utils/time'; import { GLOBAL_OBJ } from './utils/worldwide'; /** @@ -182,76 +177,6 @@ export function lastEventId(): string | undefined { return getIsolationScope().lastEventId(); } -/** - * Create a cron monitor check in and send it to Sentry. - * - * @param checkIn An object that describes a check in. - * @param upsertMonitorConfig An optional object that describes a monitor config. Use this if you want - * to create a monitor automatically when sending a check in. - */ -export function captureCheckIn(checkIn: CheckIn, upsertMonitorConfig?: MonitorConfig): string { - const scope = getCurrentScope(); - const client = getClient(); - if (!client) { - DEBUG_BUILD && debug.warn('Cannot capture check-in. No client defined.'); - } else if (!client.captureCheckIn) { - DEBUG_BUILD && debug.warn('Cannot capture check-in. Client does not support sending check-ins.'); - } else { - return client.captureCheckIn(checkIn, upsertMonitorConfig, scope); - } - - return uuid4(); -} - -/** - * Wraps a callback with a cron monitor check in. The check in will be sent to Sentry when the callback finishes. - * - * @param monitorSlug The distinct slug of the monitor. - * @param callback Callback to be monitored - * @param upsertMonitorConfig An optional object that describes a monitor config. Use this if you want - * to create a monitor automatically when sending a check in. - */ -export function withMonitor( - monitorSlug: CheckIn['monitorSlug'], - callback: () => T, - upsertMonitorConfig?: MonitorConfig, -): T { - function runCallback(): T { - const checkInId = captureCheckIn({ monitorSlug, status: 'in_progress' }, upsertMonitorConfig); - const now = timestampInSeconds(); - - function finishCheckIn(status: FinishedCheckIn['status']): void { - captureCheckIn({ monitorSlug, status, checkInId, duration: timestampInSeconds() - now }); - } - // Default behavior without isolateTrace - let maybePromiseResult: T; - try { - maybePromiseResult = callback(); - } catch (e) { - finishCheckIn('error'); - throw e; - } - - if (isThenable(maybePromiseResult)) { - return maybePromiseResult.then( - r => { - finishCheckIn('ok'); - return r; - }, - e => { - finishCheckIn('error'); - throw e; - }, - ) as T; - } - finishCheckIn('ok'); - - return maybePromiseResult; - } - - return withIsolationScope(() => (upsertMonitorConfig?.isolateTrace ? startNewTrace(runCallback) : runCallback())); -} - /** * Call `flush()` on the current client, if there is one. See {@link Client.flush}. * diff --git a/packages/core/src/monitor.ts b/packages/core/src/monitor.ts new file mode 100644 index 000000000000..59490ce90be6 --- /dev/null +++ b/packages/core/src/monitor.ts @@ -0,0 +1,93 @@ +import { getClient, getCurrentScope, withIsolationScope } from './currentScopes'; +import { DEBUG_BUILD } from './debug-build'; +import { startNewTrace } from './tracing/trace'; +import type { CheckIn, FinishedCheckIn, MonitorConfig } from './types/checkin'; +import { debug } from './utils/debug-logger'; +import { isThenable } from './utils/is'; +import { uuid4 } from './utils/misc'; +import { timestampInSeconds } from './utils/time'; + +/** + * Wraps a callback with a cron monitor check in. The check in will be sent to Sentry when the callback finishes. + * + * @param monitorSlug The distinct slug of the monitor. + * @param callback Callback to be monitored + * @param upsertMonitorConfig An optional object that describes a monitor config. Use this if you want + * to create a monitor automatically when sending a check in. + */ +export function withMonitor( + monitorSlug: CheckIn['monitorSlug'], + callback: () => T, + upsertMonitorConfig?: MonitorConfig, +): T { + function runCallback(): T { + const checkInId = captureCheckIn({ monitorSlug, status: 'in_progress' }, upsertMonitorConfig); + const now = timestampInSeconds(); + + function finishCheckIn(status: FinishedCheckIn['status']): void { + captureCheckIn({ monitorSlug, status, checkInId, duration: timestampInSeconds() - now }); + } + // Default behavior without isolateTrace + let maybePromiseResult: T; + try { + maybePromiseResult = callback(); + } catch (e) { + finishCheckIn('error'); + throw e; + } + + if (isThenable(maybePromiseResult)) { + return maybePromiseResult.then( + r => { + finishCheckIn('ok'); + return r; + }, + e => { + finishCheckIn('error'); + throw e; + }, + ) as T; + } + finishCheckIn('ok'); + + return maybePromiseResult; + } + + // With isolation scope resets the propagation context, so if we do not pass isolateTace, we want to manually continue the trace + const oldPropagationContext = getCurrentScope().getPropagationContext(); + + return withIsolationScope(() => { + if (upsertMonitorConfig?.isolateTrace) { + return startNewTrace(runCallback); + } + + // If we are not isolating the trace, in this case we want to keep the same trace as the parent + const newPropagationContext = getCurrentScope().getPropagationContext(); + if (!newPropagationContext.parentSpanId) { + getCurrentScope().setPropagationContext(oldPropagationContext); + } + + return runCallback(); + }); +} + +/** + * Create a cron monitor check in and send it to Sentry. + * + * @param checkIn An object that describes a check in. + * @param upsertMonitorConfig An optional object that describes a monitor config. Use this if you want + * to create a monitor automatically when sending a check in. + */ +export function captureCheckIn(checkIn: CheckIn, upsertMonitorConfig?: MonitorConfig): string { + const scope = getCurrentScope(); + const client = getClient(); + if (!client) { + DEBUG_BUILD && debug.warn('Cannot capture check-in. No client defined.'); + } else if (!client.captureCheckIn) { + DEBUG_BUILD && debug.warn('Cannot capture check-in. Client does not support sending check-ins.'); + } else { + return client.captureCheckIn(checkIn, upsertMonitorConfig, scope); + } + + return uuid4(); +} diff --git a/packages/core/src/shared-exports.ts b/packages/core/src/shared-exports.ts index 2d75b93b311e..e440a80e0434 100644 --- a/packages/core/src/shared-exports.ts +++ b/packages/core/src/shared-exports.ts @@ -12,8 +12,6 @@ export * from './tracing'; export * from './semanticAttributes'; export { createEventEnvelope, createSessionEnvelope } from './envelope'; export { - captureCheckIn, - withMonitor, captureException, captureEvent, captureMessage, @@ -36,6 +34,7 @@ export { captureSession, addEventProcessor, } from './exports'; +export { withMonitor, captureCheckIn } from './monitor'; export { getCurrentScope, getIsolationScope, diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 115f8097b190..22342cd1f513 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -367,7 +367,7 @@ function createChildOrRootSpan({ const isolationScope = getIsolationScope(); if (!hasSpansEnabled()) { - const scopePropagationContext = { ...isolationScope.getPropagationContext(), ...scope.getPropagationContext() }; + const scopePropagationContext = scope.getPropagationContext(); const traceId = parentSpan ? parentSpan.spanContext().traceId : scopePropagationContext.traceId; // The placeholder is a thin marker; it carries no sampling decision or DSC. Both are read from diff --git a/packages/core/test/lib/sdk.test.ts b/packages/core/test/lib/sdk.test.ts index 588585630621..8ca2f6cda39d 100644 --- a/packages/core/test/lib/sdk.test.ts +++ b/packages/core/test/lib/sdk.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, type Mock, test, vi } from 'vitest'; import type { Client } from '../../src/client'; import { getCurrentScope } from '../../src/currentScopes'; -import { captureCheckIn } from '../../src/exports'; +import { captureCheckIn } from '../../src/monitor'; import { installedIntegrations } from '../../src/integration'; import { initAndBind, setCurrentClient } from '../../src/sdk'; import type { Integration } from '../../src/types/integration'; diff --git a/packages/opentelemetry/src/asyncContextStrategy.ts b/packages/opentelemetry/src/asyncContextStrategy.ts index e13b62087aee..c7afdf06f805 100644 --- a/packages/opentelemetry/src/asyncContextStrategy.ts +++ b/packages/opentelemetry/src/asyncContextStrategy.ts @@ -2,6 +2,8 @@ import * as api from '@opentelemetry/api'; import type { Scope, TracingChannelBinding } from '@sentry/core'; import { getAsyncContextStrategy, + _INTERNAL_safeMathRandom, + generateTraceId, getDefaultCurrentScope, getDefaultIsolationScope, getMainCarrier, @@ -86,6 +88,18 @@ export function setOpenTelemetryContextAsyncContextStrategy(): AsyncLocalStorage // the OTEL context manager, which uses the presence of this key to determine if it should // fork the isolation scope, or not return api.context.with(ctx.setValue(SENTRY_FORK_ISOLATION_SCOPE_CONTEXT_KEY, true), () => { + // When forking an isolation scope, unless we are continuing an incoming trace (identified by a `parentSpanId`), + // we give the freshly forked scope its own trace. + // This way, new root spans in an isolation scope will get separate traces + const scope = getCurrentScope(); + const propagationContext = scope.getPropagationContext(); + if (!propagationContext.parentSpanId) { + scope.setPropagationContext({ + ...propagationContext, + traceId: generateTraceId(), + sampleRand: _INTERNAL_safeMathRandom(), + }); + } return callback(getIsolationScope()); }); } diff --git a/packages/opentelemetry/test/asyncContextStrategy.test.ts b/packages/opentelemetry/test/asyncContextStrategy.test.ts index a36ae15180a7..669b79dea560 100644 --- a/packages/opentelemetry/test/asyncContextStrategy.test.ts +++ b/packages/opentelemetry/test/asyncContextStrategy.test.ts @@ -19,6 +19,15 @@ import { TraceState } from '../src/utils/TraceState'; import { mockSdkInit } from './helpers/mockSdkInit'; describe('asyncContextStrategy', () => { + // `withIsolationScope` gives the forked current scope a fresh propagation context (unless it is + // continuing an incoming trace), so scope data is expected to match apart from that context. + function scopeDataWithoutPropagationContext( + scope: Scope, + ): Omit, 'propagationContext'> { + const { propagationContext: _propagationContext, ...rest } = scope.getScopeData(); + return rest; + } + beforeEach(() => { getCurrentScope().clear(); getIsolationScope().clear(); @@ -45,7 +54,8 @@ describe('asyncContextStrategy', () => { expect(scope1).not.toBe(initialScope); expect(isolationScope1).not.toBe(initialIsolationScope); - expect(scope1.getScopeData()).toEqual(initialScope.getScopeData()); + expect(scopeDataWithoutPropagationContext(scope1)).toEqual(scopeDataWithoutPropagationContext(initialScope)); + expect(scope1.getPropagationContext().traceId).not.toBe(initialScope.getPropagationContext().traceId); expect(isolationScope1.getScopeData()).toEqual(initialIsolationScope.getScopeData()); scope1.setExtra('b', 'b'); @@ -162,7 +172,8 @@ describe('asyncContextStrategy', () => { expect(scope1).not.toBe(initialScope); expect(isolationScope1).not.toBe(initialIsolationScope); - expect(scope1.getScopeData()).toEqual(initialScope.getScopeData()); + expect(scopeDataWithoutPropagationContext(scope1)).toEqual(scopeDataWithoutPropagationContext(initialScope)); + expect(scope1.getPropagationContext().traceId).not.toBe(initialScope.getPropagationContext().traceId); expect(isolationScope1.getScopeData()).toEqual(initialIsolationScope.getScopeData()); await asyncSetExtra(scope1, 'b', 'b'); @@ -207,7 +218,8 @@ describe('asyncContextStrategy', () => { expect(scope1).not.toBe(initialScope); expect(isolationScope1).not.toBe(initialIsolationScope); - expect(scope1.getScopeData()).toEqual(initialScope.getScopeData()); + expect(scopeDataWithoutPropagationContext(scope1)).toEqual(scopeDataWithoutPropagationContext(initialScope)); + expect(scope1.getPropagationContext().traceId).not.toBe(initialScope.getPropagationContext().traceId); expect(isolationScope1.getScopeData()).toEqual(initialIsolationScope.getScopeData()); scope1.setExtra('b', 'b'); @@ -244,7 +256,8 @@ describe('asyncContextStrategy', () => { expect(scope1).not.toBe(initialScope); expect(isolationScope1).not.toBe(initialIsolationScope); - expect(scope1.getScopeData()).toEqual(initialScope.getScopeData()); + expect(scopeDataWithoutPropagationContext(scope1)).toEqual(scopeDataWithoutPropagationContext(initialScope)); + expect(scope1.getPropagationContext().traceId).not.toBe(initialScope.getPropagationContext().traceId); expect(isolationScope1.getScopeData()).toEqual(initialIsolationScope.getScopeData()); scope1.setExtra('b2', 'b'); @@ -294,7 +307,8 @@ describe('asyncContextStrategy', () => { expect(scope1).not.toBe(initialScope); expect(isolationScope1).not.toBe(initialIsolationScope); - expect(scope1.getScopeData()).toEqual(initialScope.getScopeData()); + expect(scopeDataWithoutPropagationContext(scope1)).toEqual(scopeDataWithoutPropagationContext(initialScope)); + expect(scope1.getPropagationContext().traceId).not.toBe(initialScope.getPropagationContext().traceId); expect(isolationScope1.getScopeData()).toEqual(initialIsolationScope.getScopeData()); await asyncSetExtra(scope1, 'b', 'b'); @@ -331,7 +345,8 @@ describe('asyncContextStrategy', () => { expect(scope1).not.toBe(initialScope); expect(isolationScope1).not.toBe(initialIsolationScope); - expect(scope1.getScopeData()).toEqual(initialScope.getScopeData()); + expect(scopeDataWithoutPropagationContext(scope1)).toEqual(scopeDataWithoutPropagationContext(initialScope)); + expect(scope1.getPropagationContext().traceId).not.toBe(initialScope.getPropagationContext().traceId); expect(isolationScope1.getScopeData()).toEqual(initialIsolationScope.getScopeData()); scope1.setExtra('b2', 'b'); diff --git a/packages/profiling-node/src/integration.ts b/packages/profiling-node/src/integration.ts index 943ae5e35c7b..3aff24aa4d74 100644 --- a/packages/profiling-node/src/integration.ts +++ b/packages/profiling-node/src/integration.ts @@ -541,8 +541,7 @@ class ContinuousProfiler { return; } - const traceId = - getCurrentScope().getPropagationContext().traceId || getIsolationScope().getPropagationContext().traceId; + const traceId = getCurrentScope().getPropagationContext().traceId; const chunk = this._initializeChunk(traceId); CpuProfilerBindings.startProfiling(chunk.id); diff --git a/packages/server-utils/src/async-context.ts b/packages/server-utils/src/async-context.ts index 1a12ba1a898a..becaabc80d8f 100644 --- a/packages/server-utils/src/async-context.ts +++ b/packages/server-utils/src/async-context.ts @@ -3,6 +3,8 @@ import type { Scope } from '@sentry/core'; import { _INTERNAL_createTracingChannelBinding, getAsyncContextStrategy, + _INTERNAL_safeMathRandom, + generateTraceId, getDefaultCurrentScope, getDefaultIsolationScope, getMainCarrier, @@ -64,6 +66,18 @@ export function setAsyncLocalStorageAsyncContextStrategy(): void { const scope = getScopes().scope.clone(); const isolationScope = getScopes().isolationScope.clone(); + // When forking an isolation scope, unless we are continuing an incoming trace (identified by a `parentSpanId`), + // we give the freshly forked scope its own trace. + // This way, new root spans in an isolation scope will get separate traces. + const propagationContext = scope.getPropagationContext(); + if (!propagationContext.parentSpanId) { + scope.setPropagationContext({ + ...propagationContext, + traceId: generateTraceId(), + sampleRand: _INTERNAL_safeMathRandom(), + }); + } + return asyncStorage.run({ scope, isolationScope }, () => { return callback(isolationScope); }); diff --git a/packages/server-utils/test/async-context.test.ts b/packages/server-utils/test/async-context.test.ts index 3e0566d58f06..d0ee8b357140 100644 --- a/packages/server-utils/test/async-context.test.ts +++ b/packages/server-utils/test/async-context.test.ts @@ -213,6 +213,34 @@ describe('withIsolationScope()', () => { expect(initialScope.getScopeData().tags).toEqual({ aa: 'aa' }); })); + + it('gives each forked isolation scope its own trace id when not continuing an incoming trace', () => { + const traceIds: string[] = []; + + withIsolationScope(() => { + traceIds.push(getCurrentScope().getPropagationContext().traceId); + }); + withIsolationScope(() => { + traceIds.push(getCurrentScope().getPropagationContext().traceId); + }); + + expect(traceIds[0]).toMatch(/^[a-f0-9]{32}$/); + expect(traceIds[1]).toMatch(/^[a-f0-9]{32}$/); + expect(traceIds[0]).not.toBe(traceIds[1]); + }); + + it('keeps the trace id when continuing an incoming trace (parentSpanId set)', () => { + const incomingTraceId = 'cafecafecafecafecafecafecafecafe'; + getCurrentScope().setPropagationContext({ + traceId: incomingTraceId, + parentSpanId: '1234567890abcdef', + sampleRand: 0.42, + }); + + withIsolationScope(() => { + expect(getCurrentScope().getPropagationContext().traceId).toBe(incomingTraceId); + }); + }); }); describe('AsyncLocalStorage re-use', () => { diff --git a/packages/vercel-edge/test/middlewareTraceIsolation.test.ts b/packages/vercel-edge/test/middlewareTraceIsolation.test.ts index 503c692a7a8d..0338de190a0a 100644 --- a/packages/vercel-edge/test/middlewareTraceIsolation.test.ts +++ b/packages/vercel-edge/test/middlewareTraceIsolation.test.ts @@ -1,5 +1,12 @@ import { context, propagation, ROOT_CONTEXT, trace } from '@opentelemetry/api'; -import { getCurrentScope, getGlobalScope, getIsolationScope, GLOBAL_OBJ, spanToJSON } from '@sentry/core'; +import { + getCurrentScope, + getGlobalScope, + getIsolationScope, + GLOBAL_OBJ, + spanToJSON, + withIsolationScope, +} from '@sentry/core'; import { setOpenTelemetryContextAsyncContextStrategy } from '@sentry/opentelemetry'; import { AsyncLocalStorage } from 'async_hooks'; import { beforeAll, beforeEach, describe, expect, it } from 'vitest'; @@ -59,24 +66,28 @@ async function runMiddlewareRequest( delay = 0, ): Promise<{ traceId: string; childTraceId: string }> { const request = new Request(url, { method: 'GET', headers }); - return withPropagatedContext(request.headers, () => - nextMiddlewareTrace(new URL(url).pathname, async () => { - const rootSpan = trace.getActiveSpan()!; - const traceId = spanToJSON(rootSpan).trace_id!; - - await new Promise(resolve => setTimeout(resolve, delay)); - - // A child span started within the request must parent onto this request's root span (same trace id), not onto a - // concurrent request's span. - const childTracer = trace.getTracer('user'); - const childSpan = childTracer.startSpan('user-work'); - const childTraceId = spanToJSON(childSpan as unknown as Parameters[0]).trace_id!; - childSpan.end(); - - await new Promise(resolve => setTimeout(resolve, delay)); - - return { traceId, childTraceId }; - }), + // Mirror `wrapMiddlewareWithSentry`: fork an isolation scope per request, then run Next's + // `withPropagatedContext` + `Middleware.execute` trace inside it. + return withIsolationScope(() => + withPropagatedContext(request.headers, () => + nextMiddlewareTrace(new URL(url).pathname, async () => { + const rootSpan = trace.getActiveSpan()!; + const traceId = spanToJSON(rootSpan).trace_id!; + + await new Promise(resolve => setTimeout(resolve, delay)); + + // A child span started within the request must parent onto this request's root span (same trace id), not onto a + // concurrent request's span. + const childTracer = trace.getTracer('user'); + const childSpan = childTracer.startSpan('user-work'); + const childTraceId = spanToJSON(childSpan as unknown as Parameters[0]).trace_id!; + childSpan.end(); + + await new Promise(resolve => setTimeout(resolve, delay)); + + return { traceId, childTraceId }; + }), + ), ); }