Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flush caching transport with main flush #1890

Merged
merged 4 commits into from
Sep 1, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Fix environment name casing issue ([#1861](https://github.com/getsentry/sentry-dotnet/pull/1861))
- Null check HttpContext in SystemWebVersionLocator ([#1881](https://github.com/getsentry/sentry-dotnet/pull/1881))
- Fix detection of .NET Framework 4.8.1 ([#1885](https://github.com/getsentry/sentry-dotnet/pull/1885))
- Flush caching transport with main flush ([#1890](https://github.com/getsentry/sentry-dotnet/pull/1890))

## 3.20.1

Expand Down
33 changes: 16 additions & 17 deletions src/Sentry/Internal/BackgroundWorker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading.Tasks;
using Sentry.Extensibility;
using Sentry.Internal.Extensions;
using Sentry.Internal.Http;
using Sentry.Protocol.Envelopes;

namespace Sentry.Internal
Expand Down Expand Up @@ -244,12 +245,6 @@ private async Task DoFlushAsync(CancellationToken cancellationToken)
return;
}

if (_queue.IsEmpty)
{
_options.LogDebug("No events to flush.");
return;
}

var completionSource = new TaskCompletionSource<bool>();
cancellationToken.Register(() => completionSource.TrySetCanceled());

Expand All @@ -270,24 +265,28 @@ void EventFlushedCallback(object? _, EventArgs __)

try
{
// now we're subscribed and counting, make sure it's not already empty.
var trackedDepth = _queue.Count;
if (trackedDepth == 0) // now we're subscribed and counting, make sure it's not already empty.
if (trackedDepth != 0)
{
return;
Interlocked.Exchange(ref depth, trackedDepth);
_options.LogDebug("Tracking depth: {0}.", trackedDepth);

// Check if the worker didn't finish flushing before we set the depth
if (counter < depth)
{
// Await until event is flushed (or we have cancelled)
await completionSource.Task.ConfigureAwait(false);
}
}

Interlocked.Exchange(ref depth, trackedDepth);
_options.LogDebug("Tracking depth: {0}.", trackedDepth);
_options.LogDebug("Successfully flushed all events up to call to FlushAsync.");

if (counter >= depth) // When the worker finished flushing before we set the depth
if (_transport is CachingTransport cachingTransport && !cancellationToken.IsCancellationRequested)
{
return;
_options.LogDebug("Flushing caching transport with remaining flush time.");
await cachingTransport.FlushAsync(cancellationToken).ConfigureAwait(false);
}

// Await until event is flushed (or we have cancelled)
await completionSource.Task.ConfigureAwait(false);

_options.LogDebug("Successfully flushed all events up to call to FlushAsync.");
}
catch (OperationCanceledException)
{
Expand Down
37 changes: 29 additions & 8 deletions test/Sentry.Tests/Internals/BackgroundWorkerTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Collections.Concurrent;
using System.Diagnostics;
using NSubstitute.ExceptionExtensions;
using Sentry.Internal.Http;
using Sentry.Testing;

namespace Sentry.Tests.Internals;
Expand Down Expand Up @@ -41,6 +42,7 @@ public Fixture(ITestOutputHelper outputHelper)
return token.IsCancellationRequested ? Task.FromCanceled(token) : Task.CompletedTask;
});

SentryOptions.Dsn = ValidDsn;
SentryOptions.Debug = true;
SentryOptions.DiagnosticLogger = Logger;
SentryOptions.ClientReportRecorder = ClientReportRecorder;
Expand Down Expand Up @@ -319,14 +321,6 @@ public async Task FlushAsync_DisposedWorker_LogsAndReturns()
_fixture.Logger.Received(1).Log(SentryLevel.Debug, "Worker disposed. Nothing to flush.");
}

[Fact]
public async Task FlushAsync_EmptyQueue_LogsAndReturns()
{
using var sut = _fixture.GetSut();
await sut.FlushAsync(TimeSpan.MaxValue);
_fixture.Logger.Received(1).Log(SentryLevel.Debug, "No events to flush.");
}

[Fact]
public async Task FlushAsync_SingleEvent_FlushReturnsAfterEventSent()
{
Expand Down Expand Up @@ -469,4 +463,31 @@ await _fixture.Transport.Received(1).SendEnvelopeAsync(
Arg.Is<Envelope>(e => e.Items.Count == 1 && e.Items[0].TryGetType() == "client_report"),
Arg.Any<CancellationToken>());
}

[Fact]
public async Task FlushAsync_Calls_CachingTransport_FlushAsync()
{
// Arrange
var fileSystem = new FakeFileSystem();
using var tempDir = new TempDirectory(fileSystem);

var options = _fixture.SentryOptions;
options.FileSystem = fileSystem;
options.CacheDirectoryPath = tempDir.Path;

var innerTransport = _fixture.Transport;
_fixture.Transport = CachingTransport.Create(innerTransport, options, startWorker: false);

using var sut = _fixture.GetSut();
var envelope = Envelope.FromEvent(new SentryEvent());

// Act
sut.EnqueueEnvelope(envelope, process: false);
sut.ProcessQueuedItems(1);
await sut.FlushAsync(Timeout.InfiniteTimeSpan);

// Assert
_fixture.Logger.Received(1)
.Log(SentryLevel.Debug, "External FlushAsync invocation: flushing cached envelopes.");
}
}