diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index 6c9113aca2..88a81089c4 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -134,13 +134,15 @@ private void CreateChildSpan(Activity data, ISpan parentSpan, SpanId? parentSpan }; var span = parentSpan.StartChild(context); - // Used to filter out spans that are not recorded when finishing a transaction - span.SetFused(data); + // Fuse the Activity via a WeakReference so _map does not pin it, letting PruneFilteredSpans evict + // never-ended spans. #3198 intended weak refs here but SetFused stores the value strongly. + SetFusedActivity(span, data); if (span is SpanTracer spanTracer) { spanTracer.Origin = OpenTelemetryOrigin; spanTracer.StartTimestamp = data.StartTimeUtc; - spanTracer.IsFiltered = () => spanTracer.GetFused() is { IsAllDataRequested: false, Recorded: false }; + // Used to filter out spans that are not recorded when finishing a transaction. + spanTracer.IsFiltered = () => GetFusedActivity(spanTracer) is { IsAllDataRequested: false, Recorded: false }; } _map[data.SpanId] = span; } @@ -176,7 +178,8 @@ private void CreateRootSpan(Activity data) { scope.Transaction ??= transaction; }, transaction); - transaction.SetFused(data); + // Fuse weakly so _map does not pin the Activity. + SetFusedActivity(transaction, data); _map[data.SpanId] = transaction; } @@ -304,9 +307,9 @@ internal void PruneFilteredSpans(bool force = false) foreach (var mappedItem in _map) { var (spanId, span) = mappedItem; - var activity = span.GetFused(); - // Also prune when the activity has been GC'd (weak ref returns null): the activity is gone, so it - // can never call OnEnd, and the span will never be removed otherwise — causing a memory leak. + var activity = GetFusedActivity(span); + // Prune when the activity has been GC'd (weak ref returns null): it can no longer call OnEnd, so + // the span would otherwise stay in _map forever. if (activity is null or { Recorded: false, IsAllDataRequested: false }) { _map.TryRemove(spanId, out _); @@ -314,6 +317,18 @@ internal void PruneFilteredSpans(bool force = false) } } + private const string ActivityPropertyName = "Activity"; + + // Fuses the Activity onto the span via a WeakReference so _map does not pin it. + private static void SetFusedActivity(ISpan span, Activity data) => + span.SetFused(ActivityPropertyName, new WeakReference(data)); + + // Returns the fused Activity, or null once it has been garbage-collected. + private static Activity? GetFusedActivity(ISpan span) => + span.GetFused>(ActivityPropertyName) is { } weakRef && weakRef.TryGetTarget(out var activity) + ? activity + : null; + private bool NeedsPruning() { var lastPruned = Interlocked.Read(ref _lastPruned); diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs index 1c3adbbea5..e008efee3a 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs @@ -965,6 +965,25 @@ public void PruneFilteredSpans_GarbageCollectedActivity_Pruned() Assert.False(sut._map.TryGetValue(spanId, out _)); } + [Fact] + public void OnStart_FusesActivityWeakly() + { + // Arrange + _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; + var sut = _fixture.GetSut(); + + using var activity = Tracer.StartActivity("test")!; + + // Act + sut.OnStart(activity); + + // The Activity must be fused via a WeakReference, not strongly. A strong reference is pinned by + // _map, which prevents GC and defeats PruneFilteredSpans, leaking never-ended spans. + sut._map.TryGetValue(activity.SpanId, out var span).Should().BeTrue(); + span.GetFused>("Activity").Should().NotBeNull(); + span.GetFused("Activity").Should().BeNull("the Activity must not be fused with a strong reference"); + } + [Fact] public void PruneFilteredSpans_RecentlyPruned_DoesNothing() {