Refactored to remove BackpressureMonitor from the SentryOptions

Author: jamescrosswell

benchmarks/Sentry.Benchmarks/BackgroundWorkerFlushBenchmarks.cs

@@ -20,7 +20,7 @@ public Task SendEnvelopeAsync(Envelope envelope, CancellationToken cancellationT
     [IterationSetup]
     public void IterationSetup()
     {
-        _backgroundWorker = new BackgroundWorker(new FakeTransport(), new SentryOptions { MaxQueueItems = 1000 });
+        _backgroundWorker = new BackgroundWorker(new FakeTransport(), new SentryOptions { MaxQueueItems = 1000 }, null);
         _event = new SentryEvent();
         _envelope = Envelope.FromEvent(_event);
 

src/Sentry/Http/HttpTransportBase.cs

@@ -16,6 +16,7 @@ public abstract class HttpTransportBase
     internal const string DefaultErrorMessage = "No message";
 
     private readonly SentryOptions _options;
+    private readonly BackpressureMonitor? _backpressureMonitor;
     private readonly ISystemClock _clock;
     private readonly Func<string, string?> _getEnvironmentVariable;
 
@@ -42,6 +43,24 @@ protected HttpTransportBase(SentryOptions options,
         _typeName = GetType().Name;
     }
 
+    /// <summary>
+    /// Constructor for this class.
+    /// </summary>
+    /// <param name="options">The Sentry options.</param>
+    /// <param name="backpressureMonitor">The Sentry options.</param>
+    /// <param name="getEnvironmentVariable">An optional method used to read environment variables.</param>
+    /// <param name="clock">An optional system clock - used for testing.</param>
+    internal HttpTransportBase(SentryOptions options, BackpressureMonitor? backpressureMonitor,
+        Func<string, string?>? getEnvironmentVariable = default,
+        ISystemClock? clock = default)
+    {
+        _options = options;
+        _backpressureMonitor = backpressureMonitor;
+        _clock = clock ?? SystemClock.Clock;
+        _getEnvironmentVariable = getEnvironmentVariable ?? options.SettingLocator.GetEnvironmentVariable;
+        _typeName = GetType().Name;
+    }
+
     // Keep track of rate limits and their expiry dates.
     // Internal for testing.
     internal ConcurrentDictionary<RateLimitCategory, DateTimeOffset> CategoryLimitResets { get; } = new();
@@ -256,7 +275,7 @@ private void ExtractRateLimits(HttpHeaders responseHeaders)
         }
 
         var now = _clock.GetUtcNow();
-        _options.BackpressureMonitor?.RecordRateLimitHit(now);
+        _backpressureMonitor?.RecordRateLimitHit(now);
 
         // Join to a string to handle both single-header and multi-header cases
         var rateLimitsEncoded = string.Join(",", rateLimitHeaderValues);

src/Sentry/Internal/BackgroundWorker.cs

@@ -9,6 +9,7 @@ internal class BackgroundWorker : IBackgroundWorker, IDisposable
 {
     private readonly ITransport _transport;
     private readonly SentryOptions _options;
+    private readonly BackpressureMonitor? _backpressureMonitor;
     private readonly ConcurrentQueueLite<Envelope> _queue;
     private readonly int _maxItems;
     private readonly CancellationTokenSource _shutdownSource;
@@ -26,11 +27,13 @@ internal class BackgroundWorker : IBackgroundWorker, IDisposable
     public BackgroundWorker(
         ITransport transport,
         SentryOptions options,
+        BackpressureMonitor? backpressureMonitor,
         CancellationTokenSource? shutdownSource = null,
         ConcurrentQueueLite<Envelope>? queue = null)
     {
         _transport = transport;
         _options = options;
+        _backpressureMonitor = backpressureMonitor;
         _queue = queue ?? new ConcurrentQueueLite<Envelope>();
         _maxItems = options.MaxQueueItems;
         _shutdownSource = shutdownSource ?? new CancellationTokenSource();
@@ -66,7 +69,7 @@ public bool EnqueueEnvelope(Envelope envelope, bool process)
         var eventId = envelope.TryGetEventId(_options.DiagnosticLogger);
         if (Interlocked.Increment(ref _currentItems) > _maxItems)
         {
-            _options.BackpressureMonitor?.RecordQueueOverflow();
+            _backpressureMonitor?.RecordQueueOverflow();
             Interlocked.Decrement(ref _currentItems);
             _options.ClientReportRecorder.RecordDiscardedEvents(DiscardReason.QueueOverflow, envelope);
             _options.LogInfo("Discarding envelope {0} because the queue is full.", eventId);

src/Sentry/Internal/Http/HttpTransport.cs

@@ -15,10 +15,10 @@ public HttpTransport(SentryOptions options, HttpClient httpClient)
         _httpClient = httpClient;
     }
 
-    internal HttpTransport(SentryOptions options, HttpClient httpClient,
+    internal HttpTransport(SentryOptions options, HttpClient httpClient, BackpressureMonitor? backpressureMonitor,
         Func<string, string?>? getEnvironmentVariable = default,
         ISystemClock? clock = default)
-        : base(options, getEnvironmentVariable, clock)
+        : base(options, backpressureMonitor, getEnvironmentVariable, clock)
     {
         _httpClient = httpClient;
     }

src/Sentry/Internal/Http/LazyHttpTransport.cs

@@ -7,9 +7,9 @@ internal class LazyHttpTransport : ITransport
 {
     private readonly Lazy<HttpTransport> _httpTransport;
 
-    public LazyHttpTransport(SentryOptions options)
+    public LazyHttpTransport(SentryOptions options, BackpressureMonitor? backpressureMonitor)
     {
-        _httpTransport = new Lazy<HttpTransport>(() => new HttpTransport(options, options.GetHttpClient()));
+        _httpTransport = new Lazy<HttpTransport>(() => new HttpTransport(options, options.GetHttpClient(), backpressureMonitor));
     }
 
     public Task SendEnvelopeAsync(Envelope envelope, CancellationToken cancellationToken = default)

src/Sentry/Internal/Hub.cs

@@ -18,6 +18,7 @@ internal class Hub : IHub, IDisposable
     private readonly RandomValuesFactory _randomValuesFactory;
     private readonly IReplaySession _replaySession;
     private readonly List<IDisposable> _integrationsToCleanup = new();
+    private readonly BackpressureMonitor? _backpressureMonitor;
 
 #if MEMORY_DUMP_SUPPORTED
     private readonly MemoryMonitor? _memoryMonitor;
@@ -46,7 +47,8 @@ internal Hub(
         IInternalScopeManager? scopeManager = null,
         RandomValuesFactory? randomValuesFactory = null,
         IReplaySession? replaySession = null,
-        ISampleRandHelper? sampleRandHelper = null)
+        ISampleRandHelper? sampleRandHelper = null,
+        BackpressureMonitor? backpressureMonitor = null)
     {
         if (string.IsNullOrWhiteSpace(options.Dsn))
         {
@@ -61,9 +63,9 @@ internal Hub(
         _randomValuesFactory = randomValuesFactory ?? new SynchronizedRandomValuesFactory();
         _sessionManager = sessionManager ?? new GlobalSessionManager(options);
         _clock = clock ?? SystemClock.Clock;
-        if (_options.EnableBackpressureHandling && _options.BackpressureMonitor is null)
+        if (_options.EnableBackpressureHandling)
         {
-            _options.BackpressureMonitor = new BackpressureMonitor(_options.DiagnosticLogger, clock);
+            _backpressureMonitor = backpressureMonitor ?? new BackpressureMonitor(_options.DiagnosticLogger, clock);
         }
         client ??= new SentryClient(options, randomValuesFactory: _randomValuesFactory, sessionManager: _sessionManager);
         _replaySession = replaySession ?? ReplaySession.Instance;
@@ -197,7 +199,7 @@ internal ITransactionTracer StartTransaction(
             if (tracesSampler(samplingContext) is { } samplerSampleRate)
             {
                 // The TracesSampler trumps all other sampling decisions (even the trace header)
-                sampleRate = samplerSampleRate * _options.BackpressureMonitor.GetDownsampleFactor();
+                sampleRate = samplerSampleRate * _backpressureMonitor.GetDownsampleFactor();
                 isSampled = SampleRandHelper.IsSampled(sampleRand, sampleRate.Value);
                 if (isSampled is false)
                 {
@@ -216,7 +218,7 @@ internal ITransactionTracer StartTransaction(
         if (isSampled == null)
         {
             var optionsSampleRate = _options.TracesSampleRate ?? 0.0;
-            sampleRate = optionsSampleRate * _options.BackpressureMonitor.GetDownsampleFactor();
+            sampleRate = optionsSampleRate * _backpressureMonitor.GetDownsampleFactor();
             isSampled = context.IsSampled ?? SampleRandHelper.IsSampled(sampleRand, sampleRate.Value);
             if (isSampled is false)
             {
@@ -867,11 +869,7 @@ public void Dispose()
         }
         //Don't dispose of ScopeManager since we want dangling transactions to still be able to access tags.
 
-        if (_options.BackpressureMonitor is { } backpressureMonitor)
-        {
-            _options.BackpressureMonitor = null;
-            backpressureMonitor.Dispose();
-        }
+        _backpressureMonitor?.Dispose();
 
 #if __IOS__
             // TODO

src/Sentry/Internal/SdkComposer.cs

@@ -8,22 +8,24 @@ namespace Sentry.Internal;
 internal class SdkComposer
 {
     private readonly SentryOptions _options;
+    private readonly BackpressureMonitor? _backpressureMonitor;
 
-    public SdkComposer(SentryOptions options)
+    public SdkComposer(SentryOptions options, BackpressureMonitor? backpressureMonitor)
     {
         _options = options ?? throw new ArgumentNullException(nameof(options));
         if (options.Dsn is null)
         {
             throw new ArgumentException("No DSN defined in the SentryOptions");
         }
+        _backpressureMonitor = backpressureMonitor;
     }
 
     private ITransport CreateTransport()
     {
         _options.LogDebug("Creating transport.");
 
         // Start from either the transport given on options, or create a new HTTP transport.
-        var transport = _options.Transport ?? new LazyHttpTransport(_options);
+        var transport = _options.Transport ?? new LazyHttpTransport(_options, _backpressureMonitor);
 
         // When a cache directory path is given, wrap the transport in a caching transport.
         if (!string.IsNullOrWhiteSpace(_options.CacheDirectoryPath))
@@ -87,6 +89,6 @@ public IBackgroundWorker CreateBackgroundWorker()
 
         var transport = CreateTransport();
 
-        return new BackgroundWorker(transport, _options);
+        return new BackgroundWorker(transport, _options, _backpressureMonitor);
     }
 }

src/Sentry/SentryClient.cs

@@ -16,6 +16,7 @@ namespace Sentry;
 public class SentryClient : ISentryClient, IDisposable
 {
     private readonly SentryOptions _options;
+    private readonly BackpressureMonitor? _backpressureMonitor;
     private readonly ISessionManager _sessionManager;
     private readonly RandomValuesFactory _randomValuesFactory;
     private readonly Enricher _enricher;
@@ -41,9 +42,11 @@ internal SentryClient(
         SentryOptions options,
         IBackgroundWorker? worker = null,
         RandomValuesFactory? randomValuesFactory = null,
-        ISessionManager? sessionManager = null)
+        ISessionManager? sessionManager = null,
+        BackpressureMonitor? backpressureMonitor = null)
     {
         _options = options ?? throw new ArgumentNullException(nameof(options));
+        _backpressureMonitor = backpressureMonitor;
         _randomValuesFactory = randomValuesFactory ?? new SynchronizedRandomValuesFactory();
         _sessionManager = sessionManager ?? new GlobalSessionManager(options);
         _enricher = new Enricher(options);
@@ -52,7 +55,7 @@ internal SentryClient(
 
         if (worker == null)
         {
-            var composer = new SdkComposer(options);
+            var composer = new SdkComposer(options, backpressureMonitor);
             Worker = composer.CreateBackgroundWorker();
         }
         else
@@ -375,7 +378,7 @@ private SentryId DoSendEvent(SentryEvent @event, SentryHint? hint, Scope? scope)
         if (_options.SampleRate != null)
         {
             var sampleRate = _options.SampleRate.Value;
-            var downsampledRate = sampleRate * _options.BackpressureMonitor.GetDownsampleFactor();
+            var downsampledRate = sampleRate * _backpressureMonitor.GetDownsampleFactor();
             var sampleRand = _randomValuesFactory.NextDouble();
             if (sampleRand >= downsampledRate)
             {

src/Sentry/SentryOptions.cs

@@ -100,14 +100,6 @@ public bool IsGlobalModeEnabled
     /// </summary>
     public bool EnableBackpressureHandling { get; set; } = false;
 
-    /// <summary>
-    /// Stores a reference to the BackpressureMonitor for use by the rest of the SDK.
-    /// </summary>
-    /// <remarks>Note: this is necessary since the BackpressureMonitor is used by <see cref="HttpTransportBase"/>,
-    /// which is a public abstract class. Adding parameters to the constructor of that class would be a breaking change.
-    /// </remarks>
-    internal BackpressureMonitor? BackpressureMonitor { get; set; }
-
     /// <summary>
     /// This holds a reference to the current transport, when one is active.
     /// If set manually before initialization, the provided transport will be used instead of the default transport.

test/Sentry.Tests/HubTests.cs

@@ -5,11 +5,11 @@
 
 namespace Sentry.Tests;
 
-public partial class HubTests
+public partial class HubTests : IDisposable
 {
     private readonly ITestOutputHelper _output;
 
-    private class Fixture
+    private class Fixture : IDisposable
     {
         public SentryOptions Options { get; }
         public ISentryClient Client { get; set; }
@@ -18,6 +18,7 @@ private class Fixture
         public ISystemClock Clock { get; set; }
         public IReplaySession ReplaySession { get; }
         public ISampleRandHelper SampleRandHelper { get; set; }
+        public BackpressureMonitor BackpressureMonitor { get; set; }
 
         public Fixture()
         {
@@ -27,14 +28,22 @@ public Fixture()
                 TracesSampleRate = 1.0,
                 AutoSessionTracking = false
             };
-
             Client = Substitute.For<ISentryClient>();
-
             ReplaySession = Substitute.For<IReplaySession>();
         }
 
+        public void Dispose()
+        {
+            BackpressureMonitor?.Dispose();
+        }
+
         public Hub GetSut() => new(Options, Client, SessionManager, Clock, ScopeManager, replaySession: ReplaySession,
-            sampleRandHelper: SampleRandHelper);
+            sampleRandHelper: SampleRandHelper, backpressureMonitor: BackpressureMonitor);
+    }
+
+    public void Dispose()
+    {
+        _fixture.Dispose();
     }
 
     private readonly Fixture _fixture = new();
@@ -725,11 +734,11 @@ public void StartTransaction_Backpressure_Downsamples(bool usesTracesSampler)
         var transactionContext = new TransactionContext("name", "operation");
 
         var clock = new MockClock(DateTimeOffset.UtcNow);
-        var backpressureMonitor = new BackpressureMonitor(null, clock, enablePeriodicHealthCheck: false);
-        backpressureMonitor.SetDownsampleLevel(1);
-        _fixture.Options.BackpressureMonitor = backpressureMonitor;
+        _fixture.Options.EnableBackpressureHandling = true;
+        _fixture.BackpressureMonitor = new BackpressureMonitor(null, clock, enablePeriodicHealthCheck: false);
+        _fixture.BackpressureMonitor.SetDownsampleLevel(1);
         var sampleRate = 0.5f;
-        var expectedDownsampledRate = sampleRate * backpressureMonitor.DownsampleFactor;
+        var expectedDownsampledRate = sampleRate * _fixture.BackpressureMonitor.DownsampleFactor;
         if (usesTracesSampler)
         {
             _fixture.Options.TracesSampler = _ => sampleRate;
@@ -771,9 +780,9 @@ public void StartTransaction_Backpressure_SetsDiscardReason(bool usesTracesSampl
         var clock = new MockClock(DateTimeOffset.UtcNow);
         _fixture.SampleRandHelper = Substitute.For<ISampleRandHelper>();
         _fixture.SampleRandHelper.GenerateSampleRand(Arg.Any<string>()).Returns(sampleRand);
-        var backpressureMonitor = new BackpressureMonitor(null, clock, enablePeriodicHealthCheck: false);
-        backpressureMonitor.SetDownsampleLevel(1);
-        _fixture.Options.BackpressureMonitor = backpressureMonitor;
+        _fixture.Options.EnableBackpressureHandling = true;
+        _fixture.BackpressureMonitor = new BackpressureMonitor(null, clock, enablePeriodicHealthCheck: false);
+        _fixture.BackpressureMonitor.SetDownsampleLevel(1);
         var sampleRate = 0.5f;
         if (usesTracesSampler)
         {