fix(tracing): Ensure you can pass null as parentSpan in startSpan* (#12928)

Noticed this while writing docs, this makes this a bit harder to
understand. With this change you can say that `parentSpan` behaves the
same and accepts the same as `withActiveSpan`.

See getsentry/sentry-docs#10729

Author: mydea

packages/core/src/tracing/trace.ts

@@ -157,7 +157,7 @@ export function startInactiveSpan(options: StartSpanOptions): Span {
   // If `options.parentSpan` is defined, we want to wrap the callback in `withActiveSpan`
   const wrapper = options.scope
     ? (callback: () => Span) => withScope(options.scope, callback)
-    : customParentSpan
+    : customParentSpan !== undefined
       ? (callback: () => Span) => withActiveSpan(customParentSpan, callback)
       : (callback: () => Span) => callback();
 
@@ -445,8 +445,8 @@ function getParentSpan(scope: Scope): SentrySpan | undefined {
   return span;
 }
 
-function getActiveSpanWrapper<T>(parentSpan?: Span): (callback: () => T) => T {
-  return parentSpan
+function getActiveSpanWrapper<T>(parentSpan: Span | undefined | null): (callback: () => T) => T {
+  return parentSpan !== undefined
     ? (callback: () => T) => {
         return withActiveSpan(parentSpan, callback);
       }

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

@@ -282,6 +282,14 @@ describe('startSpan', () => {
     expect(getActiveSpan()).toBe(undefined);
   });
 
+  it('allows to pass parentSpan=null', () => {
+    startSpan({ name: 'GET users/[id]' }, () => {
+      startSpan({ name: 'GET users/[id]', parentSpan: null }, span => {
+        expect(spanToJSON(span).parent_span_id).toBe(undefined);
+      });
+    });
+  });
+
   it('allows to force a transaction with forceTransaction=true', async () => {
     const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
     client = new TestClient(options);
@@ -693,6 +701,15 @@ describe('startSpanManual', () => {
     expect(getActiveSpan()).toBe(undefined);
   });
 
+  it('allows to pass parentSpan=null', () => {
+    startSpan({ name: 'GET users/[id]' }, () => {
+      startSpanManual({ name: 'child', parentSpan: null }, span => {
+        expect(spanToJSON(span).parent_span_id).toBe(undefined);
+        span.end();
+      });
+    });
+  });
+
   it('allows to force a transaction with forceTransaction=true', async () => {
     const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
     client = new TestClient(options);
@@ -1014,6 +1031,14 @@ describe('startInactiveSpan', () => {
     expect(getActiveSpan()).toBeUndefined();
   });
 
+  it('allows to pass parentSpan=null', () => {
+    startSpan({ name: 'outer' }, () => {
+      const span = startInactiveSpan({ name: 'GET users/[id]', parentSpan: null });
+      expect(spanToJSON(span).parent_span_id).toBe(undefined);
+      span.end();
+    });
+  });
+
   it('allows to force a transaction with forceTransaction=true', async () => {
     const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 });
     client = new TestClient(options);

packages/opentelemetry/src/trace.ts

@@ -286,8 +286,8 @@ export function continueTrace<T>(options: Parameters<typeof baseContinueTrace>[0
   });
 }
 
-function getActiveSpanWrapper<T>(parentSpan?: Span | SentrySpan): (callback: () => T) => T {
-  return parentSpan
+function getActiveSpanWrapper<T>(parentSpan: Span | SentrySpan | undefined | null): (callback: () => T) => T {
+  return parentSpan !== undefined
     ? (callback: () => T) => {
         // We cast this, because the OTEL Span has a few more methods than our Span interface
         // TODO: Add these missing methods to the Span interface

packages/opentelemetry/test/trace.test.ts

@@ -325,6 +325,16 @@ describe('trace', () => {
       expect(getActiveSpan()).toBe(undefined);
     });
 
+    it('allows to pass parentSpan=null', () => {
+      startSpan({ name: 'GET users/[id' }, () => {
+        startSpan({ name: 'child', parentSpan: null }, span => {
+          // Due to the way we propagate the scope in OTEL,
+          // the parent_span_id is not actually undefined here, but comes from the propagation context
+          expect(spanToJSON(span).parent_span_id).toBe(getCurrentScope().getPropagationContext().spanId);
+        });
+      });
+    });
+
     it('allows to force a transaction with forceTransaction=true', async () => {
       const client = getClient()!;
       const transactionEvents: Event[] = [];
@@ -577,6 +587,17 @@ describe('trace', () => {
       expect(getActiveSpan()).toBe(undefined);
     });
 
+    it('allows to pass parentSpan=null', () => {
+      startSpan({ name: 'outer' }, () => {
+        const span = startInactiveSpan({ name: 'test span', parentSpan: null });
+
+        // Due to the way we propagate the scope in OTEL,
+        // the parent_span_id is not actually undefined here, but comes from the propagation context
+        expect(spanToJSON(span).parent_span_id).toBe(getCurrentScope().getPropagationContext().spanId);
+        span.end();
+      });
+    });
+
     it('allows to force a transaction with forceTransaction=true', async () => {
       const client = getClient()!;
       const transactionEvents: Event[] = [];
@@ -856,6 +877,17 @@ describe('trace', () => {
       expect(getActiveSpan()).toBe(undefined);
     });
 
+    it('allows to pass parentSpan=null', () => {
+      startSpan({ name: 'outer' }, () => {
+        startSpanManual({ name: 'GET users/[id]', parentSpan: null }, span => {
+          // Due to the way we propagate the scope in OTEL,
+          // the parent_span_id is not actually undefined here, but comes from the propagation context
+          expect(spanToJSON(span).parent_span_id).toBe(getCurrentScope().getPropagationContext().spanId);
+          span.end();
+        });
+      });
+    });
+
     it('allows to force a transaction with forceTransaction=true', async () => {
       const client = getClient()!;
       const transactionEvents: Event[] = [];

packages/types/src/startSpanOptions.ts

@@ -20,8 +20,9 @@ export interface StartSpanOptions {
   /**
    * If provided, make the new span a child of this span.
    * If this is not provided, the new span will be a child of the currently active span.
+   * If this is set to `null`, the new span will have no parent span.
    */
-  parentSpan?: Span;
+  parentSpan?: Span | null;
 
   /**
    * If set to true, this span will be forced to be treated as a transaction in the Sentry UI, if possible and applicable.