feat(core): Ensure trace APIs always return a span (#10942)

When tracing is disabled or otherwise skipped, they return a non
recording span instead of undefined.

This aligns the `startSpan`, `startSpanManual` and `startInactiveSpan`
APIs with opentelemetry, where you also always get a span back.

Author: mydea

packages/bun/src/integrations/bunserver.ts

@@ -96,9 +96,7 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
                     typeof serveOptions.fetch
                   >);
                   if (response && response.status) {
-                    if (span) {
-                      setHttpStatus(span, response.status);
-                    }
+                    setHttpStatus(span, response.status);
                     if (span instanceof Transaction) {
                       const scope = getCurrentScope();
                       scope.setContext('response', {

packages/core/src/fetch.ts

@@ -14,6 +14,7 @@ import {
   setHttpStatus,
   startInactiveSpan,
 } from './tracing';
+import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
 import { hasTracingEnabled } from './utils/hasTracingEnabled';
 import { spanToTraceHeader } from './utils/spanUtils';
 
@@ -92,12 +93,10 @@ export function instrumentFetchRequest(
         },
         op: 'http.client',
       })
-    : undefined;
+    : new SentryNonRecordingSpan();
 
-  if (span) {
-    handlerData.fetchData.__span = span.spanContext().spanId;
-    spans[span.spanContext().spanId] = span;
-  }
+  handlerData.fetchData.__span = span.spanContext().spanId;
+  spans[span.spanContext().spanId] = span;
 
   if (shouldAttachHeaders(handlerData.fetchData.url) && client) {
     const request: string | Request = handlerData.args[0];

packages/core/src/index.ts

@@ -90,6 +90,7 @@ export {
   spanToJSON,
   spanIsSampled,
   spanToTraceContext,
+  getSpanDescendants,
 } from './utils/spanUtils';
 export { getRootSpan } from './utils/getRootSpan';
 export { applySdkMetadata } from './utils/sdkMetadata';

packages/core/src/tracing/idleSpan.ts

@@ -3,10 +3,12 @@ import { logger, timestampInSeconds } from '@sentry/utils';
 import { getClient, getCurrentScope } from '../currentScopes';
 
 import { DEBUG_BUILD } from '../debug-build';
-import { spanToJSON } from '../utils/spanUtils';
+import { hasTracingEnabled } from '../utils/hasTracingEnabled';
+import { getSpanDescendants, removeChildSpanFromSpan, spanToJSON } from '../utils/spanUtils';
+import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
 import { SPAN_STATUS_ERROR } from './spanstatus';
 import { startInactiveSpan } from './trace';
-import { getActiveSpan, getSpanDescendants, removeChildSpanFromSpan } from './utils';
+import { getActiveSpan } from './utils';
 
 export const TRACING_DEFAULTS = {
   idleTimeout: 1_000,
@@ -71,10 +73,7 @@ interface IdleSpanOptions {
  * An idle span is a span that automatically finishes. It does this by tracking child spans as activities.
  * An idle span is always the active span.
  */
-export function startIdleSpan(
-  startSpanOptions: StartSpanOptions,
-  options: Partial<IdleSpanOptions> = {},
-): Span | undefined {
+export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Partial<IdleSpanOptions> = {}): Span {
   // Activities store a list of active spans
   const activities = new Map<string, boolean>();
 
@@ -101,21 +100,13 @@ export function startIdleSpan(
 
   const client = getClient();
 
-  if (!client) {
-    return;
+  if (!client || !hasTracingEnabled()) {
+    return new SentryNonRecordingSpan();
   }
 
   const scope = getCurrentScope();
   const previousActiveSpan = getActiveSpan();
-  const _span = _startIdleSpan(startSpanOptions);
-
-  // Span _should_ always be defined here, but TS does not know that...
-  if (!_span) {
-    return;
-  }
-
-  // For TS, so that we know everything below here has a span
-  const span = _span;
+  const span = _startIdleSpan(startSpanOptions);
 
   /**
    * Cancels the existing idle timeout, if there is one.
@@ -319,15 +310,13 @@ export function startIdleSpan(
   return span;
 }
 
-function _startIdleSpan(options: StartSpanOptions): Span | undefined {
+function _startIdleSpan(options: StartSpanOptions): Span {
   const span = startInactiveSpan(options);
 
   // eslint-disable-next-line deprecation/deprecation
   getCurrentScope().setSpan(span);
 
-  if (span) {
-    DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`);
-  }
+  DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`);
 
   return span;
 }

packages/core/src/tracing/index.ts

@@ -2,9 +2,10 @@ export { startIdleTransaction, addTracingExtensions } from './hubextensions';
 export { IdleTransaction, TRACING_DEFAULTS } from './idletransaction';
 export type { BeforeFinishCallback } from './idletransaction';
 export { SentrySpan } from './sentrySpan';
+export { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
 export { Transaction } from './transaction';
 // eslint-disable-next-line deprecation/deprecation
-export { getActiveTransaction, getActiveSpan, getSpanDescendants } from './utils';
+export { getActiveTransaction, getActiveSpan } from './utils';
 export {
   setHttpStatus,
   getSpanStatusFromHttpCode,

packages/core/src/tracing/sentryNonRecordingSpan.ts

@@ -0,0 +1,62 @@
+import type {
+  Span,
+  SpanAttributeValue,
+  SpanAttributes,
+  SpanContext,
+  SpanContextData,
+  SpanStatus,
+  SpanTimeInput,
+} from '@sentry/types';
+import { uuid4 } from '@sentry/utils';
+import { TRACE_FLAG_NONE } from '../utils/spanUtils';
+
+/**
+ * A Sentry Span that is non-recording, meaning it will not be sent to Sentry.
+ */
+export class SentryNonRecordingSpan implements Span {
+  private _traceId: string;
+  private _spanId: string;
+
+  public constructor(spanContext: SpanContext = {}) {
+    this._traceId = spanContext.traceId || uuid4();
+    this._spanId = spanContext.spanId || uuid4().substring(16);
+  }
+
+  /** @inheritdoc */
+  public spanContext(): SpanContextData {
+    return {
+      spanId: this._spanId,
+      traceId: this._traceId,
+      traceFlags: TRACE_FLAG_NONE,
+    };
+  }
+
+  /** @inheritdoc */
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  public end(_timestamp?: SpanTimeInput): void {}
+
+  /** @inheritdoc */
+  public setAttribute(_key: string, _value: SpanAttributeValue | undefined): this {
+    return this;
+  }
+
+  /** @inheritdoc */
+  public setAttributes(_values: SpanAttributes): this {
+    return this;
+  }
+
+  /** @inheritdoc */
+  public setStatus(_status: SpanStatus): this {
+    return this;
+  }
+
+  /** @inheritdoc */
+  public updateName(_name: string): this {
+    return this;
+  }
+
+  /** @inheritdoc */
+  public isRecording(): boolean {
+    return false;
+  }
+}

packages/core/src/tracing/sentrySpan.ts

@@ -22,12 +22,12 @@ import { getRootSpan } from '../utils/getRootSpan';
 import {
   TRACE_FLAG_NONE,
   TRACE_FLAG_SAMPLED,
+  addChildSpanToSpan,
   getStatusMessage,
   spanTimeInputToSeconds,
   spanToJSON,
   spanToTraceContext,
 } from '../utils/spanUtils';
-import { addChildSpanToSpan } from './utils';
 
 /**
  * Keeps track of finished spans for a given transaction

packages/core/src/tracing/trace.ts

@@ -7,11 +7,12 @@ import { DEBUG_BUILD } from '../debug-build';
 import { getCurrentHub } from '../hub';
 import { handleCallbackErrors } from '../utils/handleCallbackErrors';
 import { hasTracingEnabled } from '../utils/hasTracingEnabled';
-import { spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
+import { addChildSpanToSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
 import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
+import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
 import type { SentrySpan } from './sentrySpan';
 import { SPAN_STATUS_ERROR } from './spanstatus';
-import { addChildSpanToSpan, getActiveSpan, setCapturedScopesOnSpan } from './utils';
+import { getActiveSpan, setCapturedScopesOnSpan } from './utils';
 
 /**
  * Wraps a function with a transaction/span and finishes the span after the function is done.
@@ -24,7 +25,7 @@ import { addChildSpanToSpan, getActiveSpan, setCapturedScopesOnSpan } from './ut
  * or you didn't set `tracesSampleRate`, this function will not generate spans
  * and the `span` returned from the callback will be undefined.
  */
-export function startSpan<T>(context: StartSpanOptions, callback: (span: Span | undefined) => T): T {
+export function startSpan<T>(context: StartSpanOptions, callback: (span: Span) => T): T {
   const spanContext = normalizeContext(context);
 
   return withScope(context.scope, scope => {
@@ -35,18 +36,16 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
     const activeSpan = shouldSkipSpan
-      ? undefined
+      ? new SentryNonRecordingSpan()
       : createChildSpanOrTransaction(hub, {
           parentSpan,
           spanContext,
           forceTransaction: context.forceTransaction,
           scope,
         });
 
-    if (activeSpan) {
-      // eslint-disable-next-line deprecation/deprecation
-      scope.setSpan(activeSpan);
-    }
+    // eslint-disable-next-line deprecation/deprecation
+    scope.setSpan(activeSpan);
 
     return handleCallbackErrors(
       () => callback(activeSpan),
@@ -75,10 +74,7 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |
  * or you didn't set `tracesSampleRate`, this function will not generate spans
  * and the `span` returned from the callback will be undefined.
  */
-export function startSpanManual<T>(
-  context: StartSpanOptions,
-  callback: (span: Span | undefined, finish: () => void) => T,
-): T {
+export function startSpanManual<T>(context: StartSpanOptions, callback: (span: Span, finish: () => void) => T): T {
   const spanContext = normalizeContext(context);
 
   return withScope(context.scope, scope => {
@@ -89,18 +85,16 @@ export function startSpanManual<T>(
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
     const activeSpan = shouldSkipSpan
-      ? undefined
+      ? new SentryNonRecordingSpan()
       : createChildSpanOrTransaction(hub, {
           parentSpan,
           spanContext,
           forceTransaction: context.forceTransaction,
           scope,
         });
 
-    if (activeSpan) {
-      // eslint-disable-next-line deprecation/deprecation
-      scope.setSpan(activeSpan);
-    }
+    // eslint-disable-next-line deprecation/deprecation
+    scope.setSpan(activeSpan);
 
     function finishAndSetSpan(): void {
       activeSpan && activeSpan.end();
@@ -131,11 +125,7 @@ export function startSpanManual<T>(
  * or you didn't set `tracesSampleRate` or `tracesSampler`, this function will not generate spans
  * and the `span` returned from the callback will be undefined.
  */
-export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
-  if (!hasTracingEnabled()) {
-    return undefined;
-  }
-
+export function startInactiveSpan(context: StartSpanOptions): Span {
   const spanContext = normalizeContext(context);
   // eslint-disable-next-line deprecation/deprecation
   const hub = getCurrentHub();
@@ -147,7 +137,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
   const shouldSkipSpan = context.onlyIfParent && !parentSpan;
 
   if (shouldSkipSpan) {
-    return undefined;
+    return new SentryNonRecordingSpan();
   }
 
   const scope = context.scope || getCurrentScope();
@@ -275,14 +265,14 @@ function createChildSpanOrTransaction(
     forceTransaction?: boolean;
     scope: Scope;
   },
-): Span | undefined {
+): Span {
   if (!hasTracingEnabled()) {
-    return undefined;
+    return new SentryNonRecordingSpan();
   }
 
   const isolationScope = getIsolationScope();
 
-  let span: Span | undefined;
+  let span: Span;
   if (parentSpan && !forceTransaction) {
     // eslint-disable-next-line deprecation/deprecation
     span = parentSpan.startChild(spanContext);

packages/core/src/tracing/transaction.ts

@@ -19,10 +19,10 @@ import { DEBUG_BUILD } from '../debug-build';
 import { getCurrentHub } from '../hub';
 import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary';
 import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes';
-import { spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils';
+import { getSpanDescendants, spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils';
 import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
 import { SentrySpan, SpanRecorder } from './sentrySpan';
-import { getCapturedScopesOnSpan, getSpanDescendants } from './utils';
+import { getCapturedScopesOnSpan } from './utils';
 
 /** JSDoc */
 export class Transaction extends SentrySpan implements TransactionInterface {

packages/core/src/tracing/utils.ts

@@ -31,54 +31,6 @@ export function getActiveSpan(): Span | undefined {
   return getCurrentScope().getSpan();
 }
 
-const CHILD_SPANS_FIELD = '_sentryChildSpans';
-
-type SpanWithPotentialChildren = Span & {
-  [CHILD_SPANS_FIELD]?: Set<Span>;
-};
-
-/**
- * Adds an opaque child span reference to a span.
- */
-export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void {
-  if (span[CHILD_SPANS_FIELD] && span[CHILD_SPANS_FIELD].size < 1000) {
-    span[CHILD_SPANS_FIELD].add(childSpan);
-  } else {
-    span[CHILD_SPANS_FIELD] = new Set([childSpan]);
-  }
-}
-
-/** This is only used internally by Idle Spans. */
-export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void {
-  if (span[CHILD_SPANS_FIELD]) {
-    span[CHILD_SPANS_FIELD].delete(childSpan);
-  }
-}
-
-/**
- * Returns an array of the given span and all of its descendants.
- */
-export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] {
-  const resultSet = new Set<Span>();
-
-  function addSpanChildren(span: SpanWithPotentialChildren): void {
-    // This exit condition is required to not infinitely loop in case of a circular dependency.
-    if (resultSet.has(span)) {
-      return;
-    } else {
-      resultSet.add(span);
-      const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : [];
-      for (const childSpan of childSpans) {
-        addSpanChildren(childSpan);
-      }
-    }
-  }
-
-  addSpanChildren(span);
-
-  return Array.from(resultSet);
-}
-
 const SCOPE_ON_START_SPAN_FIELD = '_sentryScope';
 const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope';