getsentry · mydea · Apr 10, 2024 · Mar 27, 2024 · mydea · Mar 8, 2024
diff --git a/packages/browser/src/client.ts b/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();

diff --git a/packages/core/src/tracing/trace.ts b/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;
+}
diff --git a/packages/core/test/lib/tracing/trace.test.ts b/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);
 

diff --git a/packages/svelte/src/performance.ts b/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' },
     });
   });
 

diff --git a/packages/svelte/test/performance.test.ts b/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),

diff --git a/packages/types/src/options.ts b/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.
    */