ref: Remove spanRecorder and all related code (#10977)

This is now not used anymore, we use `getSpanDescendants()` internally
now everywhere.

Author: mydea

packages/core/src/tracing/hubextensions.ts

@@ -42,9 +42,6 @@ function _startTransaction(
     },
     ...customSamplingContext,
   });
-  if (transaction.isRecording()) {
-    transaction.initSpanRecorder();
-  }
   if (client) {
     client.emit('startTransaction', transaction);
     client.emit('spanStart', transaction);

packages/core/src/tracing/sentrySpan.ts

@@ -28,38 +28,6 @@ import {
   spanToTraceContext,
 } from '../utils/spanUtils';
 
-/**
- * Keeps track of finished spans for a given transaction
- * @internal
- * @hideconstructor
- * @hidden
- */
-export class SpanRecorder {
-  public spans: SentrySpan[];
-
-  private readonly _maxlen: number;
-
-  public constructor(maxlen: number = 1000) {
-    this._maxlen = maxlen;
-    this.spans = [];
-  }
-
-  /**
-   * This is just so that we don't run out of memory while recording a lot
-   * of spans. At some point we just stop and flush out the start of the
-   * trace tree (i.e.the first n spans with the smallest
-   * start_timestamp).
-   */
-  public add(span: SentrySpan): void {
-    if (this.spans.length > this._maxlen) {
-      // eslint-disable-next-line deprecation/deprecation
-      span.spanRecorder = undefined;
-    } else {
-      this.spans.push(span);
-    }
-  }
-}
-
 /**
  * Span contains all data about a span
  */
@@ -71,13 +39,6 @@ export class SentrySpan implements Span {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   public data: { [key: string]: any };
 
-  /**
-   * List of spans that were finalized
-   *
-   * @deprecated This property will no longer be public. Span recording will be handled internally.
-   */
-  public spanRecorder?: SpanRecorder;
-
   /**
    * @inheritDoc
    * @deprecated Use top level `Sentry.getRootSpan()` instead
@@ -278,14 +239,6 @@ export class SentrySpan implements Span {
       traceId: this._traceId,
     });
 
-    // eslint-disable-next-line deprecation/deprecation
-    childSpan.spanRecorder = this.spanRecorder;
-    // eslint-disable-next-line deprecation/deprecation
-    if (childSpan.spanRecorder) {
-      // eslint-disable-next-line deprecation/deprecation
-      childSpan.spanRecorder.add(childSpan);
-    }
-
     // To allow for interoperability we track the children of a span twice: Once with the span recorder (old) once with
     // the `addChildSpanToSpan`. Eventually we will only use `addChildSpanToSpan` and drop the span recorder.
     // To ensure interoperability with the `startSpan` API, `addChildSpanToSpan` is also called here.

packages/core/src/tracing/transaction.ts

@@ -21,7 +21,7 @@ import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary';
 import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes';
 import { getSpanDescendants, spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils';
 import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
-import { SentrySpan, SpanRecorder } from './sentrySpan';
+import { SentrySpan } from './sentrySpan';
 import { getCapturedScopesOnSpan } from './utils';
 
 /** JSDoc */
@@ -128,20 +128,6 @@ export class Transaction extends SentrySpan implements TransactionInterface {
     return this;
   }
 
-  /**
-   * Attaches SpanRecorder to the span itself
-   * @param maxlen maximum number of spans that can be recorded
-   */
-  public initSpanRecorder(maxlen: number = 1000): void {
-    // eslint-disable-next-line deprecation/deprecation
-    if (!this.spanRecorder) {
-      // eslint-disable-next-line deprecation/deprecation
-      this.spanRecorder = new SpanRecorder(maxlen);
-    }
-    // eslint-disable-next-line deprecation/deprecation
-    this.spanRecorder.add(this);
-  }
-
   /**
    * Set the context of a transaction event.
    * @deprecated Use either `.setAttribute()`, or set the context on the scope before creating the transaction.

packages/node/test/handlers.test.ts

@@ -461,7 +461,6 @@ describe('tracingHandler', () => {
   it('waits to finish transaction until all spans are finished, even though `transaction.end()` is registered on `res.finish` event first', done => {
     // eslint-disable-next-line deprecation/deprecation
     const transaction = new Transaction({ name: 'mockTransaction', sampled: true });
-    transaction.initSpanRecorder();
     // eslint-disable-next-line deprecation/deprecation
     const span = transaction.startChild({
       name: 'reallyCoolHandler',

packages/node/test/integrations/http.test.ts

@@ -1,7 +1,7 @@
 import * as http from 'http';
 import * as https from 'https';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
-import type { Hub, SentrySpan } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getSpanDescendants } from '@sentry/core';
+import type { Hub } from '@sentry/core';
 import { getCurrentHub, getIsolationScope, setCurrentClient } from '@sentry/core';
 import { Transaction } from '@sentry/core';
 import { getCurrentScope, setUser, spanToJSON, startInactiveSpan } from '@sentry/core';
@@ -81,12 +81,10 @@ describe('tracing', () => {
     nock('http://dogs.are.great').get('/').reply(200);
 
     const transaction = createTransactionOnScope();
-    // eslint-disable-next-line deprecation/deprecation
-    const spans = (transaction as unknown as SentrySpan).spanRecorder?.spans as SentrySpan[];
 
     http.get('http://dogs.are.great/');
 
-    expect(spans.length).toEqual(2);
+    const spans = getSpanDescendants(transaction);
 
     // our span is at index 1 because the transaction itself is at index 0
     expect(spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/');
@@ -97,11 +95,11 @@ describe('tracing', () => {
     nock('http://squirrelchasers.ingest.sentry.io').get('/api/12312012/store/').reply(200);
 
     const transaction = createTransactionOnScope();
-    // eslint-disable-next-line deprecation/deprecation
-    const spans = (transaction as unknown as SentrySpan).spanRecorder?.spans as SentrySpan[];
 
     http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/');
 
+    const spans = getSpanDescendants(transaction);
+
     // only the transaction itself should be there
     expect(spans.length).toEqual(1);
     expect(spanToJSON(spans[0]).description).toEqual('dogpark');
@@ -263,11 +261,11 @@ describe('tracing', () => {
     nock('http://dogs.are.great').get('/spaniel?tail=wag&cute=true#learn-more').reply(200);
 
     const transaction = createTransactionOnScope();
-    // eslint-disable-next-line deprecation/deprecation
-    const spans = (transaction as unknown as SentrySpan).spanRecorder?.spans as SentrySpan[];
 
     http.get('http://dogs.are.great/spaniel?tail=wag&cute=true#learn-more');
 
+    const spans = getSpanDescendants(transaction);
+
     expect(spans.length).toEqual(2);
 
     // our span is at index 1 because the transaction itself is at index 0
@@ -286,11 +284,11 @@ describe('tracing', () => {
     nock('http://dogs.are.great').get('/spaniel?tail=wag&cute=true#learn-more').reply(200);
 
     const transaction = createTransactionOnScope();
-    // eslint-disable-next-line deprecation/deprecation
-    const spans = (transaction as unknown as SentrySpan).spanRecorder?.spans as SentrySpan[];
 
     http.request({ method: 'GET', host: 'dogs.are.great', path: '/spaniel?tail=wag&cute=true#learn-more' });
 
+    const spans = getSpanDescendants(transaction);
+
     expect(spans.length).toEqual(2);
 
     const spanAttributes = spanToJSON(spans[1]).data || {};
@@ -314,11 +312,11 @@ describe('tracing', () => {
     nock(`http://${auth}@dogs.are.great`).get('/').reply(200);
 
     const transaction = createTransactionOnScope();
-    // eslint-disable-next-line deprecation/deprecation
-    const spans = (transaction as unknown as SentrySpan).spanRecorder?.spans as SentrySpan[];
 
     http.get(`http://${auth}@dogs.are.great/`);
 
+    const spans = getSpanDescendants(transaction);
+
     expect(spans.length).toEqual(2);
 
     // our span is at index 1 because the transaction itself is at index 0
@@ -374,11 +372,11 @@ describe('tracing', () => {
         );
 
         const transaction = createTransactionAndPutOnScope();
-        // eslint-disable-next-line deprecation/deprecation
-        const spans = (transaction as unknown as SentrySpan).spanRecorder?.spans as SentrySpan[];
 
         const request = http.get(url);
 
+        const spans = getSpanDescendants(transaction);
+
         // There should be no http spans
         const httpSpans = spans.filter(span => spanToJSON(span).op?.startsWith('http'));
         expect(httpSpans.length).toBe(0);
@@ -483,11 +481,11 @@ describe('tracing', () => {
         );
 
         const transaction = createTransactionAndPutOnScope();
-        // eslint-disable-next-line deprecation/deprecation
-        const spans = (transaction as unknown as SentrySpan).spanRecorder?.spans as SentrySpan[];
 
         const request = http.get(url);
 
+        const spans = getSpanDescendants(transaction);
+
         // There should be no http spans
         const httpSpans = spans.filter(span => spanToJSON(span).op?.startsWith('http'));
         expect(httpSpans.length).toBe(0);

packages/node/test/integrations/undici.test.ts

@@ -6,6 +6,7 @@ import {
   getCurrentScope,
   getIsolationScope,
   getMainCarrier,
+  getSpanDescendants,
   setCurrentClient,
   spanToJSON,
   startSpan,
@@ -125,8 +126,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
       await fetch(request, requestInit);
 
       expect(outerSpan).toBeInstanceOf(Transaction);
-      // eslint-disable-next-line deprecation/deprecation
-      const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
+      const spans = getSpanDescendants(outerSpan);
 
       expect(spans.length).toBe(2);
 
@@ -144,8 +144,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
       }
 
       expect(outerSpan).toBeInstanceOf(Transaction);
-      // eslint-disable-next-line deprecation/deprecation
-      const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
+      const spans = getSpanDescendants(outerSpan);
 
       expect(spans.length).toBe(2);
 
@@ -165,8 +164,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
       }
 
       expect(outerSpan).toBeInstanceOf(Transaction);
-      // eslint-disable-next-line deprecation/deprecation
-      const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
+      const spans = getSpanDescendants(outerSpan);
 
       expect(spans.length).toBe(2);
 
@@ -187,8 +185,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
       }
 
       expect(outerSpan).toBeInstanceOf(Transaction);
-      // eslint-disable-next-line deprecation/deprecation
-      const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
+      const spans = getSpanDescendants(outerSpan);
 
       expect(spans.length).toBe(1);
     });
@@ -207,18 +204,17 @@ conditionalTest({ min: 16 })('Undici integration', () => {
   it('does create a span if `shouldCreateSpanForRequest` is defined', async () => {
     await startSpan({ name: 'outer-span' }, async outerSpan => {
       expect(outerSpan).toBeInstanceOf(Transaction);
-      // eslint-disable-next-line deprecation/deprecation
-      const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
+      expect(getSpanDescendants(outerSpan).length).toBe(1);
 
       const undoPatch = patchUndici({ shouldCreateSpanForRequest: url => url.includes('yes') });
 
       await fetch('http://localhost:18100/no', { method: 'POST' });
 
-      expect(spans.length).toBe(1);
+      expect(getSpanDescendants(outerSpan).length).toBe(1);
 
       await fetch('http://localhost:18100/yes', { method: 'POST' });
 
-      expect(spans.length).toBe(2);
+      expect(getSpanDescendants(outerSpan).length).toBe(2);
 
       undoPatch();
     });
@@ -232,8 +228,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
     await withIsolationScope(async () => {
       await startSpan({ name: 'outer-span' }, async outerSpan => {
         expect(outerSpan).toBeInstanceOf(Transaction);
-        // eslint-disable-next-line deprecation/deprecation
-        const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
+        const spans = getSpanDescendants(outerSpan);
 
         await fetch('http://localhost:18100', { method: 'POST' });
 
@@ -307,8 +302,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
 
     await startSpan({ name: 'outer-span' }, async outerSpan => {
       expect(outerSpan).toBeInstanceOf(Transaction);
-      // eslint-disable-next-line deprecation/deprecation
-      const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
+      const spans = getSpanDescendants(outerSpan);
 
       expect(spans.length).toBe(1);
 

packages/node/test/performance.test.ts

@@ -81,6 +81,7 @@ describe('startSpan()', () => {
 
     const transactionEvent = await transactionEventPromise;
 
+    expect(transactionEvent.spans).toHaveLength(1);
     expect(transactionEvent.spans?.[0].description).toBe('second');
   });
 });

packages/opentelemetry/src/custom/transaction.ts

@@ -12,9 +12,6 @@ export function startTransaction(hub: HubInterface, transactionContext: Transact
 
   // eslint-disable-next-line deprecation/deprecation
   const transaction = new Transaction(transactionContext, hub as Hub);
-  // Since we do not do sampling here, we assume that this is _always_ sampled
-  // Any sampling decision happens in OpenTelemetry's sampler
-  transaction.initSpanRecorder();
 
   if (client) {
     client.emit('startTransaction', transaction);

packages/opentelemetry/test/custom/transaction.test.ts

@@ -1,4 +1,11 @@
-import { Transaction, getCurrentHub, setCurrentClient, spanToJSON } from '@sentry/core';
+import {
+  Transaction,
+  getCurrentHub,
+  getSpanDescendants,
+  setCurrentClient,
+  spanIsSampled,
+  spanToJSON,
+} from '@sentry/core';
 import { startTransaction } from '../../src/custom/transaction';
 import { TestClient, getDefaultTestClientOptions } from '../helpers/TestClient';
 
@@ -7,7 +14,7 @@ describe('startTranscation', () => {
     jest.resetAllMocks();
   });
 
-  it('creates a Transaction', () => {
+  it('creates an unsampled transaction', () => {
     const client = new TestClient(getDefaultTestClientOptions());
     // eslint-disable-next-line deprecation/deprecation
     const hub = getCurrentHub();
@@ -18,10 +25,37 @@ describe('startTranscation', () => {
 
     expect(transaction).toBeInstanceOf(Transaction);
     expect(transaction['_sampled']).toBe(undefined);
+    expect(spanIsSampled(transaction)).toBe(false);
+    // unsampled span is filtered out here
+    expect(getSpanDescendants(transaction)).toHaveLength(0);
     // eslint-disable-next-line deprecation/deprecation
-    expect(transaction.spanRecorder).toBeDefined();
+    expect(transaction.metadata).toEqual({
+      spanMetadata: {},
+    });
+
+    expect(spanToJSON(transaction)).toEqual(
+      expect.objectContaining({
+        origin: 'manual',
+        span_id: expect.any(String),
+        start_timestamp: expect.any(Number),
+        trace_id: expect.any(String),
+      }),
+    );
+  });
+
+  it('creates a sampled transaction', () => {
+    const client = new TestClient(getDefaultTestClientOptions());
     // eslint-disable-next-line deprecation/deprecation
-    expect(transaction.spanRecorder?.spans).toHaveLength(1);
+    const hub = getCurrentHub();
+    setCurrentClient(client);
+    client.init();
+
+    const transaction = startTransaction(hub, { name: 'test', sampled: true });
+
+    expect(transaction).toBeInstanceOf(Transaction);
+    expect(transaction['_sampled']).toBe(true);
+    expect(spanIsSampled(transaction)).toBe(true);
+    expect(getSpanDescendants(transaction)).toHaveLength(1);
     // eslint-disable-next-line deprecation/deprecation
     expect(transaction.metadata).toEqual({
       spanMetadata: {},