feat: Streamline isolation scope handling & reset in isolation scopes

Author: mydea

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<BrowserTracingOption
 
         maybeEndActiveSpan();
 
-        getIsolationScope().setPropagationContext({
-          traceId: generateTraceId(),
-          sampleRand: Math.random(),
-          propagationSpanId: hasSpansEnabled() ? undefined : generateSpanId(),
-        });
-
         const scope = getCurrentScope();
         scope.setPropagationContext({
           traceId: generateTraceId(),

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

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());
     });
   }

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<ReturnType<Scope['getScopeData']>, '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');

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);

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,
@@ -58,6 +60,18 @@ export function setAsyncLocalStorageAsyncContextStrategy(): void {
     const scope = getScopes().scope;
     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);
     });

packages/server-utils/test/async-context.test.ts

@@ -156,4 +156,32 @@ describe('withIsolationScope()', () => {
         done();
       });
     }));
+
+  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);
+    });
+  });
 });

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<typeof spanToJSON>[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<typeof spanToJSON>[0]).trace_id!;
+        childSpan.end();
+
+        await new Promise(resolve => setTimeout(resolve, delay));
+
+        return { traceId, childTraceId };
+      }),
+    ),
   );
 }