feat(browser): Create spans as children of root span by default (#10986)

You can opt out of this by setting `parentSpanIsAlwaysRootSpan=false` in
your client options.

This means that in browser, any span will always be added to the active
root span, not the active span. This way, we should be able to avoid
problems with execution contexts etc.

Closes #10944

packages/browser/src/client.ts

@@ -52,12 +52,17 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
    * @param options Configuration options for this SDK.
    */
   public constructor(options: BrowserClientOptions) {
+    const opts = {
+      // We default this to true, as it is the safer scenario
+      parentSpanIsAlwaysRootSpan: true,
+      ...options,
+    };
     const sdkSource = WINDOW.SENTRY_SDK_SOURCE || getSDKSource();
-    applySdkMetadata(options, 'browser', ['browser'], sdkSource);
+    applySdkMetadata(opts, 'browser', ['browser'], sdkSource);
 
-    super(options);
+    super(opts);
 
-    if (options.sendClientReports && WINDOW.document) {
+    if (opts.sendClientReports && WINDOW.document) {
       WINDOW.document.addEventListener('visibilitychange', () => {
         if (WINDOW.document.visibilityState === 'hidden') {
           this._flushOutcomes();

packages/core/src/tracing/trace.ts

@@ -1,5 +1,4 @@
 import type { ClientOptions, Scope, SentrySpanArguments, Span, SpanTimeInput, StartSpanOptions } from '@sentry/types';
-
 import { propagationContextFromHeaders } from '@sentry/utils';
 import type { AsyncContextStrategy } from '../asyncContext';
 import { getMainCarrier } from '../asyncContext';
@@ -11,13 +10,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE
 import { handleCallbackErrors } from '../utils/handleCallbackErrors';
 import { hasTracingEnabled } from '../utils/hasTracingEnabled';
 import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope';
-import {
-  addChildSpanToSpan,
-  getActiveSpan,
-  spanIsSampled,
-  spanTimeInputToSeconds,
-  spanToJSON,
-} from '../utils/spanUtils';
+import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
 import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
 import { logSpanStart } from './logSpans';
 import { sampleSpan } from './sampling';
@@ -47,7 +40,7 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) =
   const spanContext = normalizeContext(context);
 
   return withScope(context.scope, scope => {
-    const parentSpan = _getSpanForScope(scope) as SentrySpan | undefined;
+    const parentSpan = getParentSpan(scope);
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
     const activeSpan = shouldSkipSpan
@@ -94,7 +87,7 @@ export function startSpanManual<T>(context: StartSpanOptions, callback: (span: S
   const spanContext = normalizeContext(context);
 
   return withScope(context.scope, scope => {
-    const parentSpan = _getSpanForScope(scope) as SentrySpan | undefined;
+    const parentSpan = getParentSpan(scope);
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
     const activeSpan = shouldSkipSpan
@@ -141,18 +134,16 @@ export function startInactiveSpan(context: StartSpanOptions): Span {
   }
 
   const spanContext = normalizeContext(context);
-  const parentSpan = context.scope
-    ? (_getSpanForScope(context.scope) as SentrySpan | undefined)
-    : (getActiveSpan() as SentrySpan | undefined);
+
+  const scope = context.scope || getCurrentScope();
+  const parentSpan = getParentSpan(scope);
 
   const shouldSkipSpan = context.onlyIfParent && !parentSpan;
 
   if (shouldSkipSpan) {
     return new SentryNonRecordingSpan();
   }
 
-  const scope = context.scope || getCurrentScope();
-
   return createChildSpanOrTransaction({
     parentSpan,
     spanContext,
@@ -381,3 +372,19 @@ function _startChildSpan(parentSpan: Span, scope: Scope, spanArguments: SentrySp
 
   return childSpan;
 }
+
+function getParentSpan(scope: Scope): SentrySpan | undefined {
+  const span = _getSpanForScope(scope) as SentrySpan | undefined;
+
+  if (!span) {
+    return undefined;
+  }
+
+  const client = getClient();
+  const options: Partial<ClientOptions> = client ? client.getOptions() : {};
+  if (options.parentSpanIsAlwaysRootSpan) {
+    return getRootSpan(span) as SentrySpan;
+  }
+
+  return span;
+}

packages/core/test/lib/tracing/trace.test.ts

@@ -417,6 +417,48 @@ describe('startSpan', () => {
     });
   });
 
+  describe('parentSpanIsAlwaysRootSpan', () => {
+    it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
+      const options = getDefaultTestClientOptions({
+        tracesSampleRate: 1,
+        parentSpanIsAlwaysRootSpan: true,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      startSpan({ name: 'parent span' }, span => {
+        expect(spanToJSON(span).parent_span_id).toBe(undefined);
+        startSpan({ name: 'child span' }, childSpan => {
+          expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
+          startSpan({ name: 'grand child span' }, grandChildSpan => {
+            expect(spanToJSON(grandChildSpan).parent_span_id).toBe(span.spanContext().spanId);
+          });
+        });
+      });
+    });
+
+    it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
+      const options = getDefaultTestClientOptions({
+        tracesSampleRate: 1,
+        parentSpanIsAlwaysRootSpan: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      startSpan({ name: 'parent span' }, span => {
+        expect(spanToJSON(span).parent_span_id).toBe(undefined);
+        startSpan({ name: 'child span' }, childSpan => {
+          expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
+          startSpan({ name: 'grand child span' }, grandChildSpan => {
+            expect(spanToJSON(grandChildSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
+          });
+        });
+      });
+    });
+  });
+
   it('samples with a tracesSampler', () => {
     const tracesSampler = jest.fn(() => {
       return true;
@@ -751,6 +793,54 @@ describe('startSpanManual', () => {
     });
   });
 
+  describe('parentSpanIsAlwaysRootSpan', () => {
+    it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
+      const options = getDefaultTestClientOptions({
+        tracesSampleRate: 1,
+        parentSpanIsAlwaysRootSpan: true,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      startSpanManual({ name: 'parent span' }, span => {
+        expect(spanToJSON(span).parent_span_id).toBe(undefined);
+        startSpanManual({ name: 'child span' }, childSpan => {
+          expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
+          startSpanManual({ name: 'grand child span' }, grandChildSpan => {
+            expect(spanToJSON(grandChildSpan).parent_span_id).toBe(span.spanContext().spanId);
+            grandChildSpan.end();
+          });
+          childSpan.end();
+        });
+        span.end();
+      });
+    });
+
+    it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
+      const options = getDefaultTestClientOptions({
+        tracesSampleRate: 1,
+        parentSpanIsAlwaysRootSpan: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      startSpanManual({ name: 'parent span' }, span => {
+        expect(spanToJSON(span).parent_span_id).toBe(undefined);
+        startSpanManual({ name: 'child span' }, childSpan => {
+          expect(spanToJSON(childSpan).parent_span_id).toBe(span.spanContext().spanId);
+          startSpanManual({ name: 'grand child span' }, grandChildSpan => {
+            expect(spanToJSON(grandChildSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
+            grandChildSpan.end();
+          });
+          childSpan.end();
+        });
+        span.end();
+      });
+    });
+  });
+
   it('sets a child span reference on the parent span', () => {
     expect.assertions(1);
     startSpan({ name: 'outer' }, (outerSpan: any) => {
@@ -995,6 +1085,64 @@ describe('startInactiveSpan', () => {
     });
   });
 
+  describe('parentSpanIsAlwaysRootSpan', () => {
+    it('creates a span as child of root span if parentSpanIsAlwaysRootSpan=true', () => {
+      const options = getDefaultTestClientOptions({
+        tracesSampleRate: 1,
+        parentSpanIsAlwaysRootSpan: true,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+      expect(spanToJSON(inactiveSpan).parent_span_id).toBe(undefined);
+
+      startSpan({ name: 'parent span' }, span => {
+        const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+        expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
+
+        startSpan({ name: 'child span' }, () => {
+          const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+          expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
+
+          startSpan({ name: 'grand child span' }, () => {
+            const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+            expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
+          });
+        });
+      });
+    });
+
+    it('does not creates a span as child of root span if parentSpanIsAlwaysRootSpan=false', () => {
+      const options = getDefaultTestClientOptions({
+        tracesSampleRate: 1,
+        parentSpanIsAlwaysRootSpan: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+      expect(spanToJSON(inactiveSpan).parent_span_id).toBe(undefined);
+
+      startSpan({ name: 'parent span' }, span => {
+        const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+        expect(spanToJSON(inactiveSpan).parent_span_id).toBe(span.spanContext().spanId);
+
+        startSpan({ name: 'child span' }, childSpan => {
+          const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+          expect(spanToJSON(inactiveSpan).parent_span_id).toBe(childSpan.spanContext().spanId);
+
+          startSpan({ name: 'grand child span' }, grandChildSpan => {
+            const inactiveSpan = startInactiveSpan({ name: 'inactive span' });
+            expect(spanToJSON(inactiveSpan).parent_span_id).toBe(grandChildSpan.spanContext().spanId);
+          });
+        });
+      });
+    });
+  });
+
   it('includes the scope at the time the span was started when finished', async () => {
     const beforeSendTransaction = jest.fn(event => event);
 

packages/svelte/src/performance.ts

@@ -3,7 +3,7 @@ import type { Span } from '@sentry/types';
 import { afterUpdate, beforeUpdate, onMount } from 'svelte';
 import { current_component } from 'svelte/internal';
 
-import { getRootSpan, startInactiveSpan, withActiveSpan } from '@sentry/core';
+import { startInactiveSpan } from '@sentry/core';
 import { DEFAULT_COMPONENT_NAME, UI_SVELTE_INIT, UI_SVELTE_UPDATE } from './constants';
 import type { TrackComponentOptions } from './types';
 
@@ -34,17 +34,16 @@ export function trackComponent(options?: TrackComponentOptions): void {
   // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
   const componentName = `<${customComponentName || current_component.constructor.name || DEFAULT_COMPONENT_NAME}>`;
 
-  let initSpan: Span | undefined = undefined;
   if (mergedOptions.trackInit) {
-    initSpan = recordInitSpan(componentName);
+    recordInitSpan(componentName);
   }
 
   if (mergedOptions.trackUpdates) {
-    recordUpdateSpans(componentName, initSpan);
+    recordUpdateSpans(componentName);
   }
 }
 
-function recordInitSpan(componentName: string): Span | undefined {
+function recordInitSpan(componentName: string): void {
   const initSpan = startInactiveSpan({
     onlyIfParent: true,
     op: UI_SVELTE_INIT,
@@ -55,35 +54,21 @@ function recordInitSpan(componentName: string): Span | undefined {
   onMount(() => {
     initSpan.end();
   });
-
-  return initSpan;
 }
 
-function recordUpdateSpans(componentName: string, initSpan?: Span): void {
+function recordUpdateSpans(componentName: string): void {
   let updateSpan: Span | undefined;
   beforeUpdate(() => {
-    // We need to get the active transaction again because the initial one could
-    // already be finished or there is currently no transaction going on.
+    // If there is no active span, we skip
     const activeSpan = getActiveSpan();
     if (!activeSpan) {
       return;
     }
 
-    // If we are initializing the component when the update span is started, we start it as child
-    // of the init span. Else, we start it as a child of the transaction.
-    const parentSpan =
-      initSpan && initSpan.isRecording() && getRootSpan(initSpan) === getRootSpan(activeSpan)
-        ? initSpan
-        : getRootSpan(activeSpan);
-
-    if (!parentSpan) return;
-
-    updateSpan = withActiveSpan(parentSpan, () => {
-      return startInactiveSpan({
-        op: UI_SVELTE_UPDATE,
-        name: componentName,
-        attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
-      });
+    updateSpan = startInactiveSpan({
+      op: UI_SVELTE_UPDATE,
+      name: componentName,
+      attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
     });
   });
 

packages/svelte/test/performance.test.ts

@@ -33,7 +33,7 @@ describe('Sentry.trackComponent()', () => {
     });
   });
 
-  it('creates nested init and update spans on component initialization', async () => {
+  it('creates init and update spans on component initialization', async () => {
     startSpan({ name: 'outer' }, span => {
       expect(span).toBeDefined();
       render(DummyComponent, { props: { options: {} } });
@@ -73,7 +73,7 @@ describe('Sentry.trackComponent()', () => {
       description: '<Dummy$>',
       op: 'ui.svelte.update',
       origin: 'auto.ui.svelte',
-      parent_span_id: initSpanId,
+      parent_span_id: rootSpanId,
       span_id: expect.any(String),
       start_timestamp: expect.any(Number),
       timestamp: expect.any(Number),
@@ -128,7 +128,7 @@ describe('Sentry.trackComponent()', () => {
       description: '<Dummy$>',
       op: 'ui.svelte.update',
       origin: 'auto.ui.svelte',
-      parent_span_id: initSpanId,
+      parent_span_id: rootSpanId,
       span_id: expect.any(String),
       start_timestamp: expect.any(Number),
       timestamp: expect.any(Number),

packages/types/src/options.ts

@@ -94,6 +94,19 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
    */
   enableTracing?: boolean;
 
+  /**
+   * If this is enabled, any spans started will always have their parent be the active root span,
+   * if there is any active span.
+   *
+   * This is necessary because in some environments (e.g. browser),
+   * we cannot guarantee an accurate active span.
+   * Because we cannot properly isolate execution environments,
+   * you may get wrong results when using e.g. nested `startSpan()` calls.
+   *
+   * To solve this, in these environments we'll by default enable this option.
+   */
+  parentSpanIsAlwaysRootSpan?: boolean;
+
   /**
    * Initial data to populate scope.
    */