fix(core): Use WeakRef for Span-Scope circular references

packages/core/src/utils/spanOnScope.ts

@@ -1,11 +1,12 @@
 import type { Scope } from '../scope';
 import type { Span } from '../types/span';
 import { addNonEnumerableProperty } from '../utils/object';
+import { derefWeakRef, makeWeakRef, type MaybeWeakRef } from './weakRef';
 
 const SCOPE_SPAN_FIELD = '_sentrySpan';
 
 type ScopeWithMaybeSpan = Scope & {
-  [SCOPE_SPAN_FIELD]?: Span;
+  [SCOPE_SPAN_FIELD]?: MaybeWeakRef<Span>;
 };
 
 /**
@@ -14,7 +15,8 @@ type ScopeWithMaybeSpan = Scope & {
  */
 export function _setSpanForScope(scope: Scope, span: Span | undefined): void {
   if (span) {
-    addNonEnumerableProperty(scope as ScopeWithMaybeSpan, SCOPE_SPAN_FIELD, span);
+    // Use WeakRef to avoid circular reference with span holding scope
+    addNonEnumerableProperty(scope as ScopeWithMaybeSpan, SCOPE_SPAN_FIELD, makeWeakRef(span));
   } else {
     // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
     delete (scope as ScopeWithMaybeSpan)[SCOPE_SPAN_FIELD];
@@ -26,5 +28,5 @@ export function _setSpanForScope(scope: Scope, span: Span | undefined): void {
  * NOTE: This should NOT be used directly, but is only used internally by the trace methods.
  */
 export function _getSpanForScope(scope: ScopeWithMaybeSpan): Span | undefined {
-  return scope[SCOPE_SPAN_FIELD];
+  return derefWeakRef(scope[SCOPE_SPAN_FIELD]);
 }

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

@@ -44,17 +44,16 @@ describe('tracing utils', () => {
       expect(retrieved.isolationScope).toBeUndefined();
     });
 
-    it('uses WeakRef only for isolation scopes', () => {
+    it('uses WeakRef only for isolation scope, stores scope directly', () => {
       const span = createMockSpan();
       const scope = new Scope();
       const isolationScope = new Scope();
 
       setCapturedScopesOnSpan(span, scope, isolationScope);
 
-      // Check that only isolation scope is wrapped with WeakRef
       const spanWithScopes = span as any;
-      expect(spanWithScopes._sentryScope).toBe(scope); // Regular scope stored directly
-      expect(spanWithScopes._sentryIsolationScope).toBeInstanceOf(WeakRef); // Isolation scope wrapped
+      expect(spanWithScopes._sentryScope).toBe(scope);
+      expect(spanWithScopes._sentryIsolationScope).toBeInstanceOf(WeakRef);
 
       // Verify we can still retrieve the scopes
       const retrieved = getCapturedScopesOnSpan(span);
@@ -77,14 +76,8 @@ describe('tracing utils', () => {
 
         // Check that both scopes are stored directly when WeakRef is not available
         const spanWithScopes = span as any;
-        expect(spanWithScopes._sentryScope).toBe(scope); // Regular scope always stored directly
-        expect(spanWithScopes._sentryIsolationScope).toBe(isolationScope); // Isolation scope falls back to direct storage
-
-        // When WeakRef is available, ensure regular scope is not wrapped but isolation scope would be
-        if (originalWeakRef) {
-          expect(spanWithScopes._sentryScope).not.toBeInstanceOf(originalWeakRef);
-          expect(spanWithScopes._sentryIsolationScope).not.toBeInstanceOf(originalWeakRef);
-        }
+        expect(spanWithScopes._sentryScope).toBe(scope);
+        expect(spanWithScopes._sentryIsolationScope).toBe(isolationScope);
 
         // Verify we can still retrieve the scopes
         const retrieved = getCapturedScopesOnSpan(span);
@@ -96,46 +89,42 @@ describe('tracing utils', () => {
       }
     });
 
-    it('handles WeakRef deref returning undefined gracefully', () => {
+    it('handles WeakRef deref returning undefined gracefully for isolation scope', () => {
       const span = createMockSpan();
       const scope = new Scope();
       const isolationScope = new Scope();
 
       setCapturedScopesOnSpan(span, scope, isolationScope);
 
-      // Mock WeakRef.deref to return undefined for isolation scope (simulating garbage collection)
-      // Regular scope is stored directly, so it should always be available
+      // Mock isolation scope WeakRef to return undefined (simulating garbage collection)
       const spanWithScopes = span as any;
       const mockIsolationScopeWeakRef = {
         deref: vi.fn().mockReturnValue(undefined),
       };
 
-      // Keep the regular scope as is (stored directly)
-      // Only replace the isolation scope with a mock WeakRef
       spanWithScopes._sentryIsolationScope = mockIsolationScopeWeakRef;
 
       const retrieved = getCapturedScopesOnSpan(span);
-      expect(retrieved.scope).toBe(scope); // Regular scope should still be available
-      expect(retrieved.isolationScope).toBeUndefined(); // Isolation scope should be undefined due to GC
+      expect(retrieved.scope).toBe(scope);
+      expect(retrieved.isolationScope).toBeUndefined();
       expect(mockIsolationScopeWeakRef.deref).toHaveBeenCalled();
     });
 
-    it('handles corrupted WeakRef objects gracefully', () => {
+    it('handles corrupted WeakRef objects gracefully for isolation scope', () => {
       const span = createMockSpan();
       const scope = new Scope();
 
-      // Set up a regular scope (stored directly) and a corrupted isolation scope WeakRef
       const spanWithScopes = span as any;
-      spanWithScopes._sentryScope = scope; // Regular scope stored directly
+      spanWithScopes._sentryScope = scope;
       spanWithScopes._sentryIsolationScope = {
         deref: vi.fn().mockImplementation(() => {
           throw new Error('WeakRef deref failed');
         }),
       };
 
       const retrieved = getCapturedScopesOnSpan(span);
-      expect(retrieved.scope).toBe(scope); // Regular scope should still be available
-      expect(retrieved.isolationScope).toBeUndefined(); // Isolation scope should be undefined due to error
+      expect(retrieved.scope).toBe(scope);
+      expect(retrieved.isolationScope).toBeUndefined();
     });
 
     it('preserves scope data when using WeakRef', () => {

packages/opentelemetry/src/spanExporter.ts

@@ -20,6 +20,7 @@ import {
   getCapturedScopesOnSpan,
   getDynamicSamplingContextFromSpan,
   getStatusMessage,
+  LRUMap,
   SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME,
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -41,6 +42,7 @@ type SpanNodeCompleted = SpanNode & { span: ReadableSpan };
 
 const MAX_SPAN_COUNT = 1000;
 const DEFAULT_TIMEOUT = 300; // 5 min
+const SENT_SPANS_MAX_SIZE = 10000;
 
 interface FinishedSpanBucket {
   timestampInS: number;
@@ -71,9 +73,7 @@ export class SentrySpanExporter {
   private _finishedSpanBucketSize: number;
   private _spansToBucketEntry: WeakMap<ReadableSpan, FinishedSpanBucket>;
   private _lastCleanupTimestampInS: number;
-  // Essentially a a set of span ids that are already sent. The values are expiration
-  // times in this cache so we don't hold onto them indefinitely.
-  private _sentSpans: Map<string, number>;
+  private _sentSpans: LRUMap<string, number>;
   /* Internally, we use a debounced flush to give some wiggle room to the span processor to accumulate more spans. */
   private _debouncedFlush: ReturnType<typeof debounce>;
 
@@ -85,7 +85,7 @@ export class SentrySpanExporter {
     this._finishedSpanBuckets = new Array(this._finishedSpanBucketSize).fill(undefined);
     this._lastCleanupTimestampInS = Math.floor(_INTERNAL_safeDateNow() / 1000);
     this._spansToBucketEntry = new WeakMap();
-    this._sentSpans = new Map<string, number>();
+    this._sentSpans = new LRUMap<string, number>(SENT_SPANS_MAX_SIZE);
     this._debouncedFlush = debounce(this.flush.bind(this), 1, { maxWait: 100 });
   }
 
@@ -124,7 +124,7 @@ export class SentrySpanExporter {
 
     // If the span doesn't have a local parent ID (it's a root span), we're gonna flush all the ended spans
     const localParentId = getLocalParentId(span);
-    if (!localParentId || this._sentSpans.has(localParentId)) {
+    if (!localParentId || this._sentSpans.get(localParentId)) {
       this._debouncedFlush();
     }
   }
@@ -137,7 +137,6 @@ export class SentrySpanExporter {
   public flush(): void {
     const finishedSpans = this._finishedSpanBuckets.flatMap(bucket => (bucket ? Array.from(bucket.spans) : []));
 
-    this._flushSentSpanCache();
     const sentSpans = this._maybeSend(finishedSpans);
 
     const sentSpanCount = sentSpans.size;
@@ -147,15 +146,14 @@ export class SentrySpanExporter {
         `SpanExporter exported ${sentSpanCount} spans, ${remainingOpenSpanCount} spans are waiting for their parent spans to finish`,
       );
 
-    const expirationDate = _INTERNAL_safeDateNow() + DEFAULT_TIMEOUT * 1000;
-
     for (const span of sentSpans) {
-      this._sentSpans.set(span.spanContext().spanId, expirationDate);
+      this._sentSpans.set(span.spanContext().spanId, 1);
       const bucketEntry = this._spansToBucketEntry.get(span);
       if (bucketEntry) {
         bucketEntry.spans.delete(span);
       }
     }
+
     // Cancel a pending debounced flush, if there is one
     // This can be relevant if we directly flush, circumventing the debounce
     // in that case, we want to cancel any pending debounced flush
@@ -193,7 +191,7 @@ export class SentrySpanExporter {
       const transactionEvent = createTransactionForOtelSpan(span);
 
       // Add an attribute to the transaction event to indicate that this transaction is an orphaned transaction
-      if (root.parentNode && this._sentSpans.has(root.parentNode.id)) {
+      if (root.parentNode && this._sentSpans.get(root.parentNode.id)) {
         const traceData = transactionEvent.contexts?.trace?.data;
         if (traceData) {
           traceData['sentry.parent_span_already_sent'] = true;
@@ -235,20 +233,9 @@ export class SentrySpanExporter {
     return sentSpans;
   }
 
-  /** Remove "expired" span id entries from the _sentSpans cache. */
-  private _flushSentSpanCache(): void {
-    const currentTimestamp = _INTERNAL_safeDateNow();
-    // Note, it is safe to delete items from the map as we go: https://stackoverflow.com/a/35943995/90297
-    for (const [spanId, expirationTime] of this._sentSpans.entries()) {
-      if (expirationTime <= currentTimestamp) {
-        this._sentSpans.delete(spanId);
-      }
-    }
-  }
-
   /** Check if a node is a completed root node or a node whose parent has already been sent */
   private _nodeIsCompletedRootNodeOrHasSentParent(node: SpanNode): node is SpanNodeCompleted {
-    return !!node.span && (!node.parentNode || this._sentSpans.has(node.parentNode.id));
+    return !!node.span && (!node.parentNode || !!this._sentSpans.get(node.parentNode.id));
   }
 
   /** Get all completed root nodes from a list of nodes */

packages/opentelemetry/test/integration/transactions.test.ts

@@ -612,7 +612,7 @@ describe('Integration | Transactions', () => {
     expect(finishedSpans.length).toBe(0);
   });
 
-  it('discards child spans that are finished after 5 minutes their parent span has been sent', async () => {
+  it('sends child spans that are finished after 5 minutes as orphaned transactions', async () => {
     const timeout = 5 * 60 * 1000;
     const now = Date.now();
     vi.useFakeTimers();
@@ -631,14 +631,6 @@ describe('Integration | Transactions', () => {
       },
     });
 
-    const spanProcessor = getSpanProcessor();
-
-    const exporter = spanProcessor ? spanProcessor['_exporter'] : undefined;
-
-    if (!exporter) {
-      throw new Error('No exporter found, aborting test...');
-    }
-
     startSpanManual({ name: 'test name' }, async span => {
       const subSpan = startInactiveSpan({ name: 'inner span 1' });
       subSpan.end();
@@ -654,18 +646,15 @@ describe('Integration | Transactions', () => {
 
     vi.advanceTimersByTime(timeout + 2);
 
-    expect(transactions).toHaveLength(1);
+    expect(transactions).toHaveLength(2);
     expect(transactions[0]?.spans).toHaveLength(1);
 
-    // subSpan2 is pending (and will eventually be cleaned up)
-    const finishedSpans: any = [];
-    exporter['_finishedSpanBuckets'].forEach(bucket => {
-      if (bucket) {
-        finishedSpans.push(...bucket.spans);
-      }
+    expect(transactions[1]?.transaction).toBe('inner span 2');
+    expect(transactions[1]?.contexts?.trace?.data).toEqual({
+      'sentry.parent_span_already_sent': true,
+      'sentry.origin': 'manual',
+      'sentry.source': 'custom',
     });
-    expect(finishedSpans.length).toBe(1);
-    expect(finishedSpans[0]?.name).toBe('inner span 2');
   });
 
   it('only considers sent spans, not finished spans, for flushing orphaned spans of sent spans', async () => {