Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Fixes

- Memory leak when finishing an unsampled Transaction that has started unsampled Spans ([#4717](https://github.com/getsentry/sentry-dotnet/pull/4717))
- backported via ([#4853](https://github.com/getsentry/sentry-dotnet/pull/4853))

### Dependencies

- Bump Java SDK from v8.24.0 to v8.25.0 ([#4679](https://github.com/getsentry/sentry-dotnet/pull/4679))
Expand Down
19 changes: 18 additions & 1 deletion src/Sentry/Internal/UnsampledTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@ namespace Sentry.Internal;

/// <summary>
/// We know already, when starting a transaction, whether it's going to be sampled or not. When it's not sampled, we can
/// avoid lots of unecessary processing. The only thing we need to track is the number of spans that would have been
/// avoid lots of unnecessary processing. The only thing we need to track is the number of spans that would have been
/// created (the client reports detailing discarded events includes this detail).
/// </summary>
internal sealed class UnsampledTransaction : NoOpTransaction
{
// Although it's a little bit wasteful to create separate individual class instances here when all we're going to
// report to sentry is the span count (in the client report), SDK users may refer to things like
// `ITransaction.Spans.Count`, so we create an actual collection
#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
private readonly ConcurrentBag<ISpan> _spans = [];
#else
private ConcurrentBag<ISpan> _spans = [];
#endif

private readonly IHub _hub;
private readonly ITransactionContext _context;
private readonly SentryOptions? _options;
Expand Down Expand Up @@ -79,6 +84,9 @@ public override void Finish()
_options?.ClientReportRecorder.RecordDiscardedEvent(discardReason, DataCategory.Span, spanCount);

_options?.LogDebug("Finished unsampled transaction");

// Release tracked spans
ReleaseSpans();
}

public override void Finish(SpanStatus status) => Finish();
Expand All @@ -103,4 +111,13 @@ public ISpan StartChild(string operation, SpanId spanId)
_spans.Add(span);
return span;
}

private void ReleaseSpans()
{
#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
_spans.Clear();
#else
_spans = [];
#endif
}
}
4 changes: 2 additions & 2 deletions src/Sentry/TransactionTracer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public IReadOnlyList<string> Fingerprint
/// <inheritdoc />
public IReadOnlyDictionary<string, string> Tags => _tags;

#if NETSTANDARD2_1_OR_GREATER
#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
private readonly ConcurrentBag<ISpan> _spans = new();
#else
private ConcurrentBag<ISpan> _spans = new();
Expand Down Expand Up @@ -429,7 +429,7 @@ public string? Origin

private void ReleaseSpans()
{
#if NETSTANDARD2_1_OR_GREATER
#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
_spans.Clear();
#else
_spans = new ConcurrentBag<ISpan>();
Expand Down
18 changes: 18 additions & 0 deletions test/Sentry.Tests/Internals/UnsampledSpanTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,24 @@ namespace Sentry.Tests.Internals;

public class UnsampledSpanTests
{
[Fact]
public void StartChild_IsUnsampledSpan_HasReferenceToUnsampledTransaction()
{
// Arrange
var hub = Substitute.For<IHub>();
ITransactionContext context = new TransactionContext("TestTransaction", "TestOperation",
new SentryTraceHeader(SentryId.Create(), SpanId.Create(), false)
);
var transaction = new UnsampledTransaction(hub, context);

// Act
var unsampledSpan = transaction.StartChild("Foo");

// Assert
Assert.IsType<UnsampledSpan>(unsampledSpan);
Assert.Same(transaction, unsampledSpan.GetTransaction());
}

[Fact]
public void GetTraceHeader_CreatesHeaderFromUnsampledTransaction()
{
Expand Down
40 changes: 40 additions & 0 deletions test/Sentry.Tests/Internals/UnsampledTransactionTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
namespace Sentry.Tests.Internals;

public class UnsampledTransactionTests
{
[Fact]
public void StartChild_CreatesSpan_IsTrackedByParent()
{
// Arrange
var hub = Substitute.For<IHub>();
ITransactionContext context = new TransactionContext("TestTransaction", "TestOperation",
new SentryTraceHeader(SentryId.Create(), SpanId.Create(), false)
);
var transaction = new UnsampledTransaction(hub, context);

// Act
var unsampledSpan = transaction.StartChild("Foo");

// Assert
var span = Assert.Single(transaction.Spans);
Assert.Same(unsampledSpan, span);
}

[Fact]
public void Finish_WithTrackedSpans_ClearsTrackedSpans()
{
// Arrange
var hub = Substitute.For<IHub>();
ITransactionContext context = new TransactionContext("TestTransaction", "TestOperation",
new SentryTraceHeader(SentryId.Create(), SpanId.Create(), false)
);
var transaction = new UnsampledTransaction(hub, context);
_ = transaction.StartChild("Foo");

// Act
transaction.Finish();

// Assert
Assert.Empty(transaction.Spans);
}
}
6 changes: 3 additions & 3 deletions test/Sentry.Tests/SpanTracerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public async Task SetExtra_DataInserted_NoDataLoss()
{
// Arrange
var hub = Substitute.For<IHub>();
var transaction = new SpanTracer(hub, null, null, SentryId.Empty, "");
var spanTracer = new SpanTracer(hub, null, null, SentryId.Empty, "");
var evt = new ManualResetEvent(false);
var ready = new ManualResetEvent(false);
var counter = 0;
Expand All @@ -27,7 +27,7 @@ public async Task SetExtra_DataInserted_NoDataLoss()

for (var i = 0; i < amount; i++)
{
transaction.SetExtra(Guid.NewGuid().ToString(), Guid.NewGuid());
spanTracer.SetExtra(Guid.NewGuid().ToString(), Guid.NewGuid());
}
})).ToList();
ready.WaitOne();
Expand All @@ -36,7 +36,7 @@ public async Task SetExtra_DataInserted_NoDataLoss()

// Arrange
// 4 tasks testing X amount should be the same amount as Extras.
Assert.Equal(4 * amount, transaction.Extra.Count);
Assert.Equal(4 * amount, spanTracer.Extra.Count);
}
}
}
35 changes: 35 additions & 0 deletions test/Sentry.Tests/TransactionTracerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
namespace Sentry.Tests;

public class TransactionTracerTests
{
[Fact]
public void StartChild_CreatesSpanTracer_TracksSpans()
{
// Arrange
var hub = Substitute.For<IHub>();
var transaction = new TransactionTracer(hub, "op", "name");

// Act
var span = transaction.StartChild("operation");

// Assert
Assert.IsType<SpanTracer>(span);
Assert.Collection(transaction.Spans,
element => Assert.Same(span, element));
}

[Fact]
public void Finish_WithTrackedSpans_ClearsTrackedSpans()
{
// Arrange
var hub = Substitute.For<IHub>();
var transaction = new TransactionTracer(hub, "op", "name");
_ = transaction.StartChild("operation");

// Act
transaction.Finish();

// Assert
Assert.Empty(transaction.Spans);
}
}
Loading