feat(core): Add span.isRecording() instead of span.sampled (#10034)

To align this with OTEL API.

Author: mydea

MIGRATION.md

@@ -57,6 +57,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
 * `span.setName(newName)`: Use `span.updateName(newName)` instead.
 * `span.toTraceparent()`: use `spanToTraceHeader(span)` util instead.
 * `span.getTraceContext()`: Use `spanToTraceContext(span)` utility function instead.
+* `span.sampled`: Use `span.isRecording()` instead.
 
 ## Deprecate `pushScope` & `popScope` in favor of `withScope`
 

packages/astro/test/server/meta.test.ts

@@ -4,7 +4,7 @@ import { vi } from 'vitest';
 import { getTracingMetaTags, isValidBaggageString } from '../../src/server/meta';
 
 const mockedSpan = {
-  sampled: true,
+  isRecording: () => true,
   traceId: '12345678901234567890123456789012',
   spanId: '1234567890123456',
   transaction: {
@@ -70,7 +70,7 @@ describe('getTracingMetaTags', () => {
     const tags = getTracingMetaTags(
       // @ts-expect-error - only passing a partial span object
       {
-        sampled: true,
+        isRecording: () => true,
         traceId: '12345678901234567890123456789012',
         spanId: '1234567890123456',
         transaction: undefined,
@@ -93,7 +93,7 @@ describe('getTracingMetaTags', () => {
     const tags = getTracingMetaTags(
       // @ts-expect-error - only passing a partial span object
       {
-        sampled: true,
+        isRecording: () => true,
         traceId: '12345678901234567890123456789012',
         spanId: '1234567890123456',
         transaction: undefined,

packages/browser/src/profiling/utils.ts

@@ -515,7 +515,7 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {
     return false;
   }
 
-  if (!transaction.sampled) {
+  if (!transaction.isRecording()) {
     if (DEBUG_BUILD) {
       logger.log('[Profiling] Discarding profile because transaction was not sampled.');
     }

packages/browser/test/unit/profiling/hubextensions.test.ts

@@ -67,7 +67,7 @@ describe('BrowserProfilingIntegration', () => {
     // @ts-expect-error force api to be undefined
     global.window.Profiler = undefined;
     // set sampled to true so that profiling does not early return
-    const mockTransaction = { sampled: true } as Transaction;
+    const mockTransaction = { isRecording: () => true } as Transaction;
     expect(() => onProfilingStartRouteTransaction(mockTransaction)).not.toThrow();
   });
   it('does not throw if constructor throws', () => {
@@ -80,8 +80,8 @@ describe('BrowserProfilingIntegration', () => {
       }
     }
 
-    // set sampled to true so that profiling does not early return
-    const mockTransaction = { sampled: true } as Transaction;
+    // set isRecording to true so that profiling does not early return
+    const mockTransaction = { isRecording: () => true } as Transaction;
 
     // @ts-expect-error override with our own constructor
     global.window.Profiler = Profiler;

packages/bun/test/integrations/bunserver.test.ts

@@ -80,7 +80,7 @@ describe('Bun Serve Integration', () => {
     client.on('finishTransaction', transaction => {
       expect(transaction.traceId).toBe(TRACE_ID);
       expect(transaction.parentSpanId).toBe(PARENT_SPAN_ID);
-      expect(transaction.sampled).toBe(true);
+      expect(transaction.isRecording()).toBe(true);
 
       expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
     });

packages/core/src/tracing/hubextensions.ts

@@ -55,6 +55,7 @@ function _startTransaction(
 The transaction will not be sampled. Please use the ${configInstrumenter} instrumentation to start transactions.`,
       );
 
+    // eslint-disable-next-line deprecation/deprecation
     transactionContext.sampled = false;
   }
 
@@ -64,7 +65,7 @@ The transaction will not be sampled. Please use the ${configInstrumenter} instru
     transactionContext,
     ...customSamplingContext,
   });
-  if (transaction.sampled) {
+  if (transaction.isRecording()) {
     transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
   }
   if (client && client.emit) {
@@ -94,7 +95,7 @@ export function startIdleTransaction(
     transactionContext,
     ...customSamplingContext,
   });
-  if (transaction.sampled) {
+  if (transaction.isRecording()) {
     transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
   }
   if (client && client.emit) {

packages/core/src/tracing/sampling.ts

@@ -21,13 +21,16 @@ export function sampleTransaction<T extends Transaction>(
 ): T {
   // nothing to do if tracing is not enabled
   if (!hasTracingEnabled(options)) {
+    // eslint-disable-next-line deprecation/deprecation
     transaction.sampled = false;
     return transaction;
   }
 
   // if the user has forced a sampling decision by passing a `sampled` value in their transaction context, go with that
+  // eslint-disable-next-line deprecation/deprecation
   if (transaction.sampled !== undefined) {
     transaction.setMetadata({
+      // eslint-disable-next-line deprecation/deprecation
       sampleRate: Number(transaction.sampled),
     });
     return transaction;
@@ -60,6 +63,7 @@ export function sampleTransaction<T extends Transaction>(
   // only valid values are booleans or numbers between 0 and 1.)
   if (!isValidSampleRate(sampleRate)) {
     DEBUG_BUILD && logger.warn('[Tracing] Discarding transaction because of invalid sample rate.');
+    // eslint-disable-next-line deprecation/deprecation
     transaction.sampled = false;
     return transaction;
   }
@@ -74,15 +78,18 @@ export function sampleTransaction<T extends Transaction>(
             : 'a negative sampling decision was inherited or tracesSampleRate is set to 0'
         }`,
       );
+    // eslint-disable-next-line deprecation/deprecation
     transaction.sampled = false;
     return transaction;
   }
 
   // Now we roll the dice. Math.random is inclusive of 0, but not of 1, so strict < is safe here. In case sampleRate is
   // a boolean, the < comparison will cause it to be automatically cast to 1 if it's true and 0 if it's false.
+  // eslint-disable-next-line deprecation/deprecation
   transaction.sampled = Math.random() < (sampleRate as number | boolean);
 
   // if we're not going to keep it, we're done
+  // eslint-disable-next-line deprecation/deprecation
   if (!transaction.sampled) {
     DEBUG_BUILD &&
       logger.log(

packages/core/src/tracing/span.ts

@@ -154,6 +154,7 @@ export class Span implements SpanInterface {
     }
     // We want to include booleans as well here
     if ('sampled' in spanContext) {
+      // eslint-disable-next-line deprecation/deprecation
       this.sampled = spanContext.sampled;
     }
     if (spanContext.op) {
@@ -193,6 +194,7 @@ export class Span implements SpanInterface {
     const childSpan = new Span({
       ...spanContext,
       parentSpanId: this.spanId,
+      // eslint-disable-next-line deprecation/deprecation
       sampled: this.sampled,
       traceId: this.traceId,
     });
@@ -351,6 +353,7 @@ export class Span implements SpanInterface {
     this.endTimestamp = spanContext.endTimestamp;
     this.op = spanContext.op;
     this.parentSpanId = spanContext.parentSpanId;
+    // eslint-disable-next-line deprecation/deprecation
     this.sampled = spanContext.sampled;
     this.spanId = spanContext.spanId || this.spanId;
     this.startTimestamp = spanContext.startTimestamp || this.startTimestamp;
@@ -400,6 +403,11 @@ export class Span implements SpanInterface {
     });
   }
 
+  /** @inheritdoc */
+  public isRecording(): boolean {
+    return !this.endTimestamp && !!this.sampled;
+  }
+
   /**
    * Get the merged data for this span.
    * For now, this combines `data` and `attributes` together,

packages/core/src/utils/spanUtils.ts

@@ -24,7 +24,7 @@ export function spanToTraceContext(span: Span): TraceContext {
  * Convert a Span to a Sentry trace header.
  */
 export function spanToTraceHeader(span: Span): string {
-  return generateSentryTraceHeader(span.traceId, span.spanId, span.sampled);
+  return generateSentryTraceHeader(span.traceId, span.spanId, span.isRecording());
 }
 
 /**

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

@@ -209,6 +209,23 @@ describe('span', () => {
     });
   });
 
+  describe('isRecording', () => {
+    it('returns true for sampled span', () => {
+      const span = new Span({ sampled: true });
+      expect(span.isRecording()).toEqual(true);
+    });
+
+    it('returns false for sampled, finished span', () => {
+      const span = new Span({ sampled: true, endTimestamp: Date.now() });
+      expect(span.isRecording()).toEqual(false);
+    });
+
+    it('returns false for unsampled span', () => {
+      const span = new Span({ sampled: false });
+      expect(span.isRecording()).toEqual(false);
+    });
+  });
+
   // Ensure that attributes & data are merged together
   describe('_getData', () => {
     it('works without data & attributes', () => {

packages/tracing-internal/src/browser/browsertracing.ts

@@ -333,6 +333,7 @@ export class BrowserTracing implements Integration {
     this._latestRouteName = finalContext.name;
     this._latestRouteSource = finalContext.metadata && finalContext.metadata.source;
 
+    // eslint-disable-next-line deprecation/deprecation
     if (finalContext.sampled === false) {
       DEBUG_BUILD && logger.log(`[Tracing] Will not send ${finalContext.op} transaction because of beforeNavigate.`);
     }

packages/types/src/span.ts

@@ -58,6 +58,8 @@ export interface SpanContext {
 
   /**
    * Was this span chosen to be sent as part of the sample?
+   *
+   * @deprecated Use `isRecording()` instead.
    */
   sampled?: boolean;
 
@@ -268,4 +270,10 @@ export interface Span extends SpanContext {
     trace_id: string;
     origin?: SpanOrigin;
   };
+
+  /**
+   * If this is span is actually recording data.
+   * This will return false if tracing is disabled, this span was not sampled or if the span is already finished.
+   */
+  isRecording(): boolean;
 }