feat(core): Allow to pass forceTransaction to startSpan() APIs (#…

…10749) This will ensure a span is sent as a transaction to Sentry. This only implements this option for the core implementation, not yet for OTEL - that is a follow up here: #10807
getsentry · Feb 26, 2024 · 372e405 · 372e405
1 parent 1eeea62
commit 372e405
Show file tree

Hide file tree

Showing 5 changed files with 402 additions and 58 deletions.
diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts
@@ -1,13 +1,14 @@
 import type { Hub, Scope, Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types';
 
 import { addNonEnumerableProperty, dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils';
+import { getDynamicSamplingContextFromSpan } from '.';
 import { getCurrentScope, getIsolationScope, withScope } from '../currentScopes';
 
 import { DEBUG_BUILD } from '../debug-build';
 import { getCurrentHub } from '../hub';
 import { handleCallbackErrors } from '../utils/handleCallbackErrors';
 import { hasTracingEnabled } from '../utils/hasTracingEnabled';
-import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
+import { spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
 
 /**
  * Wraps a function with a transaction/span and finishes the span after the function is done.
@@ -21,7 +22,7 @@ import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
  * and the `span` returned from the callback will be undefined.
  */
 export function startSpan<T>(context: StartSpanOptions, callback: (span: Span | undefined) => T): T {
-  const ctx = normalizeContext(context);
+  const spanContext = normalizeContext(context);
 
   return withScope(context.scope, scope => {
     // eslint-disable-next-line deprecation/deprecation
@@ -30,10 +31,14 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |
     const parentSpan = scope.getSpan();
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
-    const activeSpan = shouldSkipSpan ? undefined : createChildSpanOrTransaction(hub, parentSpan, ctx);
-
-    // eslint-disable-next-line deprecation/deprecation
-    scope.setSpan(activeSpan);
+    const activeSpan = shouldSkipSpan
+      ? undefined
+      : createChildSpanOrTransaction(hub, {
+          parentSpan,
+          spanContext,
+          forceTransaction: context.forceTransaction,
+          scope,
+        });
 
     return handleCallbackErrors(
       () => callback(activeSpan),
@@ -66,7 +71,7 @@ export function startSpanManual<T>(
   context: StartSpanOptions,
   callback: (span: Span | undefined, finish: () => void) => T,
 ): T {
-  const ctx = normalizeContext(context);
+  const spanContext = normalizeContext(context);
 
   return withScope(context.scope, scope => {
     // eslint-disable-next-line deprecation/deprecation
@@ -75,10 +80,14 @@ export function startSpanManual<T>(
     const parentSpan = scope.getSpan();
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
-    const activeSpan = shouldSkipSpan ? undefined : createChildSpanOrTransaction(hub, parentSpan, ctx);
-
-    // eslint-disable-next-line deprecation/deprecation
-    scope.setSpan(activeSpan);
+    const activeSpan = shouldSkipSpan
+      ? undefined
+      : createChildSpanOrTransaction(hub, {
+          parentSpan,
+          spanContext,
+          forceTransaction: context.forceTransaction,
+          scope,
+        });
 
     function finishAndSetSpan(): void {
       activeSpan && activeSpan.end();
@@ -114,7 +123,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
     return undefined;
   }
 
-  const ctx = normalizeContext(context);
+  const spanContext = normalizeContext(context);
   // eslint-disable-next-line deprecation/deprecation
   const hub = getCurrentHub();
   const parentSpan = context.scope
@@ -128,41 +137,19 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
     return undefined;
   }
 
-  const isolationScope = getIsolationScope();
-  const scope = getCurrentScope();
-
-  let span: Span | undefined;
-
-  if (parentSpan) {
-    // eslint-disable-next-line deprecation/deprecation
-    span = parentSpan.startChild(ctx);
-  } else {
-    const { traceId, dsc, parentSpanId, sampled } = {
-      ...isolationScope.getPropagationContext(),
-      ...scope.getPropagationContext(),
-    };
-
-    // eslint-disable-next-line deprecation/deprecation
-    span = hub.startTransaction({
-      traceId,
-      parentSpanId,
-      parentSampled: sampled,
-      ...ctx,
-      metadata: {
-        dynamicSamplingContext: dsc,
-        // eslint-disable-next-line deprecation/deprecation
-        ...ctx.metadata,
-      },
-    });
-  }
-
-  if (parentSpan) {
-    addChildSpanToSpan(parentSpan, span);
-  }
+  const scope = context.scope || getCurrentScope();
 
-  setCapturedScopesOnSpan(span, scope, isolationScope);
+  // Even though we don't actually want to make this span active on the current scope,
+  // we need to make it active on a temporary scope that we use for event processing
+  // as otherwise, it won't pick the correct span for the event when processing it
+  const temporaryScope = scope.clone();
 
-  return span;
+  return createChildSpanOrTransaction(hub, {
+    parentSpan,
+    spanContext,
+    forceTransaction: context.forceTransaction,
+    scope: temporaryScope,
+  });
 }
 
 /**
@@ -277,20 +264,47 @@ export const continueTrace: ContinueTrace = <V>(
 
 function createChildSpanOrTransaction(
   hub: Hub,
-  parentSpan: Span | undefined,
-  ctx: TransactionContext,
+  {
+    parentSpan,
+    spanContext,
+    forceTransaction,
+    scope,
+  }: {
+    parentSpan: Span | undefined;
+    spanContext: TransactionContext;
+    forceTransaction?: boolean;
+    scope: Scope;
+  },
 ): Span | undefined {
   if (!hasTracingEnabled()) {
     return undefined;
   }
 
   const isolationScope = getIsolationScope();
-  const scope = getCurrentScope();
 
   let span: Span | undefined;
-  if (parentSpan) {
+  if (parentSpan && !forceTransaction) {
+    // eslint-disable-next-line deprecation/deprecation
+    span = parentSpan.startChild(spanContext);
+    addChildSpanToSpan(parentSpan, span);
+  } else if (parentSpan) {
+    // If we forced a transaction but have a parent span, make sure to continue from the parent span, not the scope
+    const dsc = getDynamicSamplingContextFromSpan(parentSpan);
+    const { traceId, spanId: parentSpanId } = parentSpan.spanContext();
+    const sampled = spanIsSampled(parentSpan);
+
     // eslint-disable-next-line deprecation/deprecation
-    span = parentSpan.startChild(ctx);
+    span = hub.startTransaction({
+      traceId,
+      parentSpanId,
+      parentSampled: sampled,
+      ...spanContext,
+      metadata: {
+        dynamicSamplingContext: dsc,
+        // eslint-disable-next-line deprecation/deprecation
+        ...spanContext.metadata,
+      },
+    });
   } else {
     const { traceId, dsc, parentSpanId, sampled } = {
       ...isolationScope.getPropagationContext(),
@@ -302,18 +316,20 @@ function createChildSpanOrTransaction(
       traceId,
       parentSpanId,
       parentSampled: sampled,
-      ...ctx,
+      ...spanContext,
       metadata: {
         dynamicSamplingContext: dsc,
         // eslint-disable-next-line deprecation/deprecation
-        ...ctx.metadata,
+        ...spanContext.metadata,
       },
     });
   }
 
-  if (parentSpan) {
-    addChildSpanToSpan(parentSpan, span);
-  }
+  // We always set this as active span on the scope
+  // In the case of this being an inactive span, we ensure to pass a detached scope in here in the first place
+  // But by having this here, we can ensure that the lookup through `getCapturedScopesOnSpan` results in the correct scope & span combo
+  // eslint-disable-next-line deprecation/deprecation
+  scope.setSpan(span);
 
   setCapturedScopesOnSpan(span, scope, isolationScope);
 

diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts
@@ -301,7 +301,9 @@ export class Transaction extends SentrySpan implements TransactionInterface {
         ...metadata,
         capturedSpanScope,
         capturedSpanIsolationScope,
-        dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
+        ...dropUndefinedKeys({
+          dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
+        }),
       },
       _metrics_summary: getMetricSummaryJsonForSpan(this),
       ...(source && {