feat(core): Deprecate span startTimestamp & endTimestamp

Instead, you can use `spanToJSON()` to _read_ the timestamps.
Updating timestamps after span creation will not be possible in v8.

Also ensure that `span.end()` can only be called once.

Author: mydea

packages/core/src/tracing/idletransaction.ts

@@ -4,7 +4,7 @@ import { logger, timestampInSeconds } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 import type { Hub } from '../hub';
-import { spanTimeInputToSeconds } from '../utils/spanUtils';
+import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
 import type { Span } from './span';
 import { SpanRecorder } from './span';
 import { Transaction } from './transaction';
@@ -55,7 +55,7 @@ export class IdleTransactionSpanRecorder extends SpanRecorder {
       };
 
       // We should only push new activities if the span does not have an end timestamp.
-      if (span.endTimestamp === undefined) {
+      if (spanToJSON(span).timestamp === undefined) {
         this._pushActivity(span.spanContext().spanId);
       }
     }
@@ -167,18 +167,19 @@ export class IdleTransaction extends Transaction {
         }
 
         // We cancel all pending spans with status "cancelled" to indicate the idle transaction was finished early
-        if (!span.endTimestamp) {
-          span.endTimestamp = endTimestampInS;
+        if (!spanToJSON(span).timestamp) {
           span.setStatus('cancelled');
+          span.end(endTimestampInS);
           DEBUG_BUILD &&
             logger.log('[Tracing] cancelling span since transaction ended early', JSON.stringify(span, undefined, 2));
         }
 
-        const spanStartedBeforeTransactionFinish = span.startTimestamp < endTimestampInS;
+        const { start_timestamp: startTime, timestamp: endTime } = spanToJSON(span);
+        const spanStartedBeforeTransactionFinish = startTime && startTime < endTimestampInS;
 
         // Add a delta with idle timeout so that we prevent false positives
         const timeoutWithMarginOfError = (this._finalTimeout + this._idleTimeout) / 1000;
-        const spanEndedBeforeFinalTimeout = span.endTimestamp - this.startTimestamp < timeoutWithMarginOfError;
+        const spanEndedBeforeFinalTimeout = endTime && startTime && endTime - startTime < timeoutWithMarginOfError;
 
         if (DEBUG_BUILD) {
           const stringifiedSpan = JSON.stringify(span, undefined, 2);

packages/core/src/tracing/span.ts

@@ -71,16 +71,6 @@ export class Span implements SpanInterface {
    */
   public status?: SpanStatusType | string;
 
-  /**
-   * Timestamp in seconds when the span was created.
-   */
-  public startTimestamp: number;
-
-  /**
-   * Timestamp in seconds when the span ended.
-   */
-  public endTimestamp?: number;
-
   /**
    * @inheritDoc
    */
@@ -131,6 +121,10 @@ export class Span implements SpanInterface {
   protected _sampled: boolean | undefined;
   protected _name?: string;
   protected _attributes: SpanAttributes;
+  /** Epoch timestamp in seconds when the span started. */
+  protected _startTime: number;
+  /** Epoch timestamp in seconds when the span ended. */
+  protected _endTime?: number;
 
   private _logMessage?: string;
 
@@ -144,7 +138,7 @@ export class Span implements SpanInterface {
   public constructor(spanContext: SpanContext = {}) {
     this._traceId = spanContext.traceId || uuid4();
     this._spanId = spanContext.spanId || uuid4().substring(16);
-    this.startTimestamp = spanContext.startTimestamp || timestampInSeconds();
+    this._startTime = spanContext.startTimestamp || timestampInSeconds();
     // eslint-disable-next-line deprecation/deprecation
     this.tags = spanContext.tags ? { ...spanContext.tags } : {};
     // eslint-disable-next-line deprecation/deprecation
@@ -170,7 +164,7 @@ export class Span implements SpanInterface {
       this.status = spanContext.status;
     }
     if (spanContext.endTimestamp) {
-      this.endTimestamp = spanContext.endTimestamp;
+      this._endTime = spanContext.endTimestamp;
     }
   }
 
@@ -273,6 +267,38 @@ export class Span implements SpanInterface {
     this._attributes = attributes;
   }
 
+  /**
+   * Timestamp in seconds (epoch time) indicating when the span started.
+   * @deprecated Use `spanToJSON()` instead.
+   */
+  public get startTimestamp(): number {
+    return this._startTime;
+  }
+
+  /**
+   * Timestamp in seconds (epoch time) indicating when the span started.
+   * @deprecated In v8, you will not be able to update the span start time after creation.
+   */
+  public set startTimestamp(startTime: number) {
+    this._startTime = startTime;
+  }
+
+  /**
+   * Timestamp in seconds when the span ended.
+   * @deprecated Use `spanToJSON()` instead.
+   */
+  public get endTimestamp(): number | undefined {
+    return this._endTime;
+  }
+
+  /**
+   * Timestamp in seconds when the span ended.
+   * @deprecated Set the end time via `span.end()` instead.
+   */
+  public set endTimestamp(endTime: number | undefined) {
+    this._endTime = endTime;
+  }
+
   /* eslint-enable @typescript-eslint/member-ordering */
 
   /** @inheritdoc */
@@ -426,6 +452,10 @@ export class Span implements SpanInterface {
 
   /** @inheritdoc */
   public end(endTimestamp?: SpanTimeInput): void {
+    // If already ended, skip
+    if (this._endTime) {
+      return;
+    }
     const rootSpan = getRootSpan(this);
     if (
       DEBUG_BUILD &&
@@ -439,7 +469,7 @@ export class Span implements SpanInterface {
       }
     }
 
-    this.endTimestamp = spanTimeInputToSeconds(endTimestamp);
+    this._endTime = spanTimeInputToSeconds(endTimestamp);
   }
 
   /**
@@ -460,12 +490,12 @@ export class Span implements SpanInterface {
     return dropUndefinedKeys({
       data: this._getData(),
       description: this._name,
-      endTimestamp: this.endTimestamp,
+      endTimestamp: this._endTime,
       op: this.op,
       parentSpanId: this.parentSpanId,
       sampled: this._sampled,
       spanId: this._spanId,
-      startTimestamp: this.startTimestamp,
+      startTimestamp: this._startTime,
       status: this.status,
       // eslint-disable-next-line deprecation/deprecation
       tags: this.tags,
@@ -483,12 +513,12 @@ export class Span implements SpanInterface {
     this.data = spanContext.data || {};
     // eslint-disable-next-line deprecation/deprecation
     this._name = spanContext.name || spanContext.description;
-    this.endTimestamp = spanContext.endTimestamp;
+    this._endTime = spanContext.endTimestamp;
     this.op = spanContext.op;
     this.parentSpanId = spanContext.parentSpanId;
     this._sampled = spanContext.sampled;
     this._spanId = spanContext.spanId || this._spanId;
-    this.startTimestamp = spanContext.startTimestamp || this.startTimestamp;
+    this._startTime = spanContext.startTimestamp || this._startTime;
     this.status = spanContext.status;
     // eslint-disable-next-line deprecation/deprecation
     this.tags = spanContext.tags || {};
@@ -516,19 +546,19 @@ export class Span implements SpanInterface {
       op: this.op,
       parent_span_id: this.parentSpanId,
       span_id: this._spanId,
-      start_timestamp: this.startTimestamp,
+      start_timestamp: this._startTime,
       status: this.status,
       // eslint-disable-next-line deprecation/deprecation
       tags: Object.keys(this.tags).length > 0 ? this.tags : undefined,
-      timestamp: this.endTimestamp,
+      timestamp: this._endTime,
       trace_id: this._traceId,
       origin: this.origin,
     });
   }
 
   /** @inheritdoc */
   public isRecording(): boolean {
-    return !this.endTimestamp && !!this._sampled;
+    return !this._endTime && !!this._sampled;
   }
 
   /**

packages/core/src/tracing/trace.ts

@@ -242,7 +242,7 @@ export function startSpanManual<T>(
       () => callback(activeSpan, finishAndSetSpan),
       () => {
         // Only update the span status if it hasn't been changed yet, and the span is not yet finished
-        if (activeSpan && !activeSpan.endTimestamp && (!activeSpan.status || activeSpan.status === 'ok')) {
+        if (activeSpan && activeSpan.isRecording() && (!activeSpan.status || activeSpan.status === 'ok')) {
           activeSpan.setStatus('internal_error');
         }
       },

packages/core/src/tracing/transaction.ts

@@ -16,7 +16,7 @@ import { DEBUG_BUILD } from '../debug-build';
 import type { Hub } from '../hub';
 import { getCurrentHub } from '../hub';
 import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes';
-import { spanTimeInputToSeconds, spanToTraceContext } from '../utils/spanUtils';
+import { spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils';
 import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
 import { Span as SpanClass, SpanRecorder } from './span';
 
@@ -257,7 +257,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
    */
   protected _finishTransaction(endTimestamp?: number): TransactionEvent | undefined {
     // This transaction is already finished, so we should not flush it again.
-    if (this.endTimestamp !== undefined) {
+    if (this._endTime !== undefined) {
       return undefined;
     }
 
@@ -286,15 +286,15 @@ export class Transaction extends SpanClass implements TransactionInterface {
       return undefined;
     }
 
-    const finishedSpans = this.spanRecorder ? this.spanRecorder.spans.filter(s => s !== this && s.endTimestamp) : [];
+    const finishedSpans = this.spanRecorder
+      ? this.spanRecorder.spans.filter(span => span !== this && spanToJSON(span).timestamp)
+      : [];
 
     if (this._trimEnd && finishedSpans.length > 0) {
-      this.endTimestamp = finishedSpans.reduce((prev: SpanClass, current: SpanClass) => {
-        if (prev.endTimestamp && current.endTimestamp) {
-          return prev.endTimestamp > current.endTimestamp ? prev : current;
-        }
-        return prev;
-      }).endTimestamp;
+      const endTimes = finishedSpans.map(span => spanToJSON(span).timestamp).filter(Boolean) as number[];
+      this._endTime = endTimes.reduce((prev, current) => {
+        return prev > current ? prev : current;
+      });
     }
 
     // eslint-disable-next-line deprecation/deprecation
@@ -310,10 +310,10 @@ export class Transaction extends SpanClass implements TransactionInterface {
       },
       // TODO: Pass spans serialized via `spanToJSON()` here instead in v8.
       spans: finishedSpans,
-      start_timestamp: this.startTimestamp,
+      start_timestamp: this._startTime,
       // eslint-disable-next-line deprecation/deprecation
       tags: this.tags,
-      timestamp: this.endTimestamp,
+      timestamp: this._endTime,
       transaction: this._name,
       type: 'transaction',
       sdkProcessingMetadata: {

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

@@ -1,6 +1,6 @@
 import { timestampInSeconds } from '@sentry/utils';
 import { Span } from '../../../src';
-import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED } from '../../../src/utils/spanUtils';
+import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON } from '../../../src/utils/spanUtils';
 
 describe('span', () => {
   describe('name', () => {
@@ -186,31 +186,41 @@ describe('span', () => {
       const now = timestampInSeconds();
       span.end();
 
-      expect(span.endTimestamp).toBeGreaterThanOrEqual(now);
+      expect(spanToJSON(span).timestamp).toBeGreaterThanOrEqual(now);
     });
 
     it('works with endTimestamp in seconds', () => {
       const span = new Span();
       const timestamp = timestampInSeconds() - 1;
       span.end(timestamp);
 
-      expect(span.endTimestamp).toEqual(timestamp);
+      expect(spanToJSON(span).timestamp).toEqual(timestamp);
     });
 
     it('works with endTimestamp in milliseconds', () => {
       const span = new Span();
       const timestamp = Date.now() - 1000;
       span.end(timestamp);
 
-      expect(span.endTimestamp).toEqual(timestamp / 1000);
+      expect(spanToJSON(span).timestamp).toEqual(timestamp / 1000);
     });
 
     it('works with endTimestamp in array form', () => {
       const span = new Span();
       const seconds = Math.floor(timestampInSeconds() - 1);
       span.end([seconds, 0]);
 
-      expect(span.endTimestamp).toEqual(seconds);
+      expect(spanToJSON(span).timestamp).toEqual(seconds);
+    });
+
+    it('skips if span is already ended', () => {
+      const startTimestamp = timestampInSeconds() - 5;
+      const endTimestamp = timestampInSeconds() - 1;
+      const span = new Span({ startTimestamp, endTimestamp });
+
+      span.end();
+
+      expect(spanToJSON(span).timestamp).toBe(endTimestamp);
     });
   });
 

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

@@ -1,4 +1,4 @@
-import { Hub, addTracingExtensions, getCurrentScope, makeMain } from '../../../src';
+import { Hub, addTracingExtensions, getCurrentScope, makeMain, spanToJSON } from '../../../src';
 import { Scope } from '../../../src/scope';
 import {
   Span,
@@ -182,17 +182,17 @@ describe('startSpan', () => {
     let _span: Span | undefined;
     startSpan({ name: 'GET users/[id]' }, span => {
       expect(span).toBeDefined();
-      expect(span?.endTimestamp).toBeUndefined();
+      expect(spanToJSON(span!).timestamp).toBeUndefined();
       _span = span as Span;
     });
 
     expect(_span).toBeDefined();
-    expect(_span?.endTimestamp).toBeDefined();
+    expect(spanToJSON(_span!).timestamp).toBeDefined();
   });
 
   it('allows to pass a `startTime`', () => {
     const start = startSpan({ name: 'outer', startTime: [1234, 0] }, span => {
-      return span?.startTimestamp;
+      return spanToJSON(span!).start_timestamp;
     });
 
     expect(start).toEqual(1234);
@@ -235,9 +235,9 @@ describe('startSpanManual', () => {
   it('creates & finishes span', async () => {
     startSpanManual({ name: 'GET users/[id]' }, (span, finish) => {
       expect(span).toBeDefined();
-      expect(span?.endTimestamp).toBeUndefined();
+      expect(spanToJSON(span!).timestamp).toBeUndefined();
       finish();
-      expect(span?.endTimestamp).toBeDefined();
+      expect(spanToJSON(span!).timestamp).toBeDefined();
     });
   });
 
@@ -285,7 +285,7 @@ describe('startSpanManual', () => {
   it('allows to pass a `startTime`', () => {
     const start = startSpanManual({ name: 'outer', startTime: [1234, 0] }, span => {
       span?.end();
-      return span?.startTimestamp;
+      return spanToJSON(span!).start_timestamp;
     });
 
     expect(start).toEqual(1234);
@@ -297,11 +297,11 @@ describe('startInactiveSpan', () => {
     const span = startInactiveSpan({ name: 'GET users/[id]' });
 
     expect(span).toBeDefined();
-    expect(span?.endTimestamp).toBeUndefined();
+    expect(spanToJSON(span!).timestamp).toBeUndefined();
 
     span?.end();
 
-    expect(span?.endTimestamp).toBeDefined();
+    expect(spanToJSON(span!).timestamp).toBeDefined();
   });
 
   it('does not set span on scope', () => {
@@ -334,7 +334,7 @@ describe('startInactiveSpan', () => {
 
   it('allows to pass a `startTime`', () => {
     const span = startInactiveSpan({ name: 'outer', startTime: [1234, 0] });
-    expect(span?.startTimestamp).toEqual(1234);
+    expect(spanToJSON(span!).start_timestamp).toEqual(1234);
   });
 });