From 1916445982d082d56b24fe5ba95fd957b400df6c Mon Sep 17 00:00:00 2001 From: Marcus Bogren Date: Wed, 15 Jul 2026 16:03:13 +0200 Subject: [PATCH 1/4] fix(OpenTelemetry): fuse Activity via WeakReference so never-ended spans are pruned SentrySpanProcessor fused the Activity onto the span with a strong reference. SetFused stores its ConditionalWeakTable values strongly, so _map kept the Activity alive and PruneFilteredSpans never collected spans whose request ended without OnEnd (aborted or never-completed requests). Those built up until the process ran out of memory. Fusing a WeakReference instead lets the Activity be collected once the request ends, so the existing prune removes the orphaned span. Follows up #3198 (from #3166 and #3197), which intended weak references but stored them strongly. Co-authored-by: Claude --- .../SentrySpanProcessor.cs | 23 +++++++++++++------ .../SentrySpanProcessorTests.cs | 19 +++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index 6c9113aca2..d1cedcedfe 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. + span.SetFused("Activity", new WeakReference(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. + transaction.SetFused("Activity", new WeakReference(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,12 @@ internal void PruneFilteredSpans(bool force = false) } } + // Returns the fused Activity, or null once it has been garbage-collected. + private static Activity? GetFusedActivity(ISpan span) => + span.GetFused>("Activity") 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() { From 7eea9262235a819c3eced6e48f54161ccd4fb236 Mon Sep 17 00:00:00 2001 From: Marcus Bogren Date: Fri, 17 Jul 2026 09:47:49 +0200 Subject: [PATCH 2/4] refactor(OpenTelemetry): add symmetric SetFusedActivity helper Route both fuse call sites and GetFusedActivity through a shared ActivityPropertyName const so they can't drift from the Fused default name. Addresses review feedback on #5393. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Sentry.OpenTelemetry/SentrySpanProcessor.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index d1cedcedfe..88a81089c4 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -136,7 +136,7 @@ private void CreateChildSpan(Activity data, ISpan parentSpan, SpanId? parentSpan var span = parentSpan.StartChild(context); // 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. - span.SetFused("Activity", new WeakReference(data)); + SetFusedActivity(span, data); if (span is SpanTracer spanTracer) { spanTracer.Origin = OpenTelemetryOrigin; @@ -179,7 +179,7 @@ private void CreateRootSpan(Activity data) scope.Transaction ??= transaction; }, transaction); // Fuse weakly so _map does not pin the Activity. - transaction.SetFused("Activity", new WeakReference(data)); + SetFusedActivity(transaction, data); _map[data.SpanId] = transaction; } @@ -317,9 +317,15 @@ 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>("Activity") is { } weakRef && weakRef.TryGetTarget(out var activity) + span.GetFused>(ActivityPropertyName) is { } weakRef && weakRef.TryGetTarget(out var activity) ? activity : null; From 0ec4953fc918f4da139dbcf8142d670f0c88e673 Mon Sep 17 00:00:00 2001 From: Marcus Bogren Date: Fri, 17 Jul 2026 10:20:18 +0200 Subject: [PATCH 3/4] fix(OpenTelemetry): capture span filter decision eagerly IsFiltered read IsAllDataRequested/Recorded lazily from the weakly fused Activity, so once the Activity was garbage-collected the check returned false and a filtered (sampled-out) span was included in the transaction instead of dropped. The sampling decision is fixed at span creation, so capture it as a bool up front. Addresses Cursor Bugbot finding on #5393. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Sentry.OpenTelemetry/SentrySpanProcessor.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index 88a81089c4..985400ef59 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -141,8 +141,11 @@ private void CreateChildSpan(Activity data, ISpan parentSpan, SpanId? parentSpan { spanTracer.Origin = OpenTelemetryOrigin; spanTracer.StartTimestamp = data.StartTimeUtc; - // Used to filter out spans that are not recorded when finishing a transaction. - spanTracer.IsFiltered = () => GetFusedActivity(spanTracer) is { IsAllDataRequested: false, Recorded: false }; + // Capture the sampling decision now; it is fixed at span creation. Reading it lazily from the + // fused Activity would fail once the Activity has been garbage-collected (weak ref returns null), + // causing a filtered span to be included in the transaction instead of dropped. + var isFiltered = data is { IsAllDataRequested: false, Recorded: false }; + spanTracer.IsFiltered = () => isFiltered; } _map[data.SpanId] = span; } From 22a607ba761c595334935742260fc83dec007737 Mon Sep 17 00:00:00 2001 From: Marcus Bogren Date: Fri, 17 Jul 2026 10:55:07 +0200 Subject: [PATCH 4/4] Revert "fix(OpenTelemetry): capture span filter decision eagerly" This reverts commit 0ec4953. The eager snapshot read IsAllDataRequested/Recorded in OnStart, which runs before OTel instrumentation filters (e.g. HttpClient Filter) clear those flags, so filtered spans were captured as unfiltered. That traded a minor GC-timing edge for a worse late-filtering regression. Back to reading the flags lazily from the weakly-fused Activity. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Sentry.OpenTelemetry/SentrySpanProcessor.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index 985400ef59..88a81089c4 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -141,11 +141,8 @@ private void CreateChildSpan(Activity data, ISpan parentSpan, SpanId? parentSpan { spanTracer.Origin = OpenTelemetryOrigin; spanTracer.StartTimestamp = data.StartTimeUtc; - // Capture the sampling decision now; it is fixed at span creation. Reading it lazily from the - // fused Activity would fail once the Activity has been garbage-collected (weak ref returns null), - // causing a filtered span to be included in the transaction instead of dropped. - var isFiltered = data is { IsAllDataRequested: false, Recorded: false }; - spanTracer.IsFiltered = () => isFiltered; + // 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; }