From 79ee1115a28774bb99e29c88a11da87c6648bfb2 Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Tue, 13 Sep 2022 14:31:40 +1000 Subject: [PATCH 1/2] Dev version bump [skip ci] --- .../Serilog.Sinks.PeriodicBatching.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj b/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj index 8cbb235..dd098b6 100644 --- a/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj +++ b/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj @@ -2,7 +2,7 @@ Buffer batches of log events to be flushed asynchronously. - 3.0.0 + 3.0.1 Serilog Contributors net45;netstandard1.1;netstandard1.2;netstandard2.0;netstandard2.1 true From 833322de3dbc0d03909ada104f7314cbf5b1283d Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Thu, 15 Sep 2022 10:19:23 +1000 Subject: [PATCH 2/2] Restore obsolete inheritance-based API, without supporting AsyncDispose() for legacy sinks (using the inheritance API); also now ensures the wrapped sink is disposed whether the wrapper started or not --- .../Serilog.Sinks.PeriodicBatching.csproj | 2 +- .../PeriodicBatching/PeriodicBatchingSink.cs | 159 +++++++++++++++++- ...s.PeriodicBatching.PerformanceTests.csproj | 1 + .../BackwardsCompatibilityTests.cs | 21 +++ ...erilog.Sinks.PeriodicBatching.Tests.csproj | 2 + .../Support/LegacyDisposeTrackingSink.cs | 21 +++ 6 files changed, 199 insertions(+), 7 deletions(-) create mode 100644 test/Serilog.Sinks.PeriodicBatching.Tests/BackwardsCompatibilityTests.cs create mode 100644 test/Serilog.Sinks.PeriodicBatching.Tests/Support/LegacyDisposeTrackingSink.cs diff --git a/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj b/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj index dd098b6..4862b19 100644 --- a/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj +++ b/src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj @@ -2,7 +2,7 @@ Buffer batches of log events to be flushed asynchronously. - 3.0.1 + 3.1.0 Serilog Contributors net45;netstandard1.1;netstandard1.2;netstandard2.0;netstandard2.1 true diff --git a/src/Serilog.Sinks.PeriodicBatching/Sinks/PeriodicBatching/PeriodicBatchingSink.cs b/src/Serilog.Sinks.PeriodicBatching/Sinks/PeriodicBatching/PeriodicBatchingSink.cs index 30a338f..d56fdbd 100644 --- a/src/Serilog.Sinks.PeriodicBatching/Sinks/PeriodicBatching/PeriodicBatchingSink.cs +++ b/src/Serilog.Sinks.PeriodicBatching/Sinks/PeriodicBatching/PeriodicBatchingSink.cs @@ -32,11 +32,17 @@ namespace Serilog.Sinks.PeriodicBatching /// that want to change this behavior need to either implement from scratch, or /// embed retry logic in the batch emitting functions. /// - public sealed class PeriodicBatchingSink : ILogEventSink, IDisposable + public class PeriodicBatchingSink : ILogEventSink, IDisposable, IBatchedLogEventSink #if FEATURE_ASYNCDISPOSABLE , IAsyncDisposable #endif { + /// + /// Constant used with legacy constructor to indicate that the internal queue shouldn't be limited. + /// + [Obsolete("Implement `IBatchedLogEventSink` and use the `PeriodicBatchingSinkOptions` constructor.")] + public const int NoQueueLimit = -1; + readonly IBatchedLogEventSink _batchedLogEventSink; readonly int _batchSizeLimit; readonly bool _eagerlyEmitFirstEvent; @@ -59,9 +65,57 @@ public sealed class PeriodicBatchingSink : ILogEventSink, IDisposable /// it will dispose this object if possible. /// Options controlling behavior of the sink. public PeriodicBatchingSink(IBatchedLogEventSink batchedSink, PeriodicBatchingSinkOptions options) + : this(options) { - if (options == null) throw new ArgumentNullException(nameof(options)); _batchedLogEventSink = batchedSink ?? throw new ArgumentNullException(nameof(batchedSink)); + } + + /// + /// Construct a . New code should avoid subclassing + /// and use + /// + /// instead. + /// + /// The maximum number of events to include in a single batch. + /// The time to wait between checking for event batches. + [Obsolete("Implement `IBatchedLogEventSink` and use the `PeriodicBatchingSinkOptions` constructor.")] + protected PeriodicBatchingSink(int batchSizeLimit, TimeSpan period) + : this(new PeriodicBatchingSinkOptions + { + BatchSizeLimit = batchSizeLimit, + Period = period, + EagerlyEmitFirstEvent = true, + QueueLimit = null + }) + { + _batchedLogEventSink = this; + } + + /// + /// Construct a . New code should avoid subclassing + /// and use + /// + /// instead. + /// + /// The maximum number of events to include in a single batch. + /// The time to wait between checking for event batches. + /// Maximum number of events in the queue - use for an unbounded queue. + [Obsolete("Implement `IBatchedLogEventSink` and use the `PeriodicBatchingSinkOptions` constructor.")] + protected PeriodicBatchingSink(int batchSizeLimit, TimeSpan period, int queueLimit) + : this(new PeriodicBatchingSinkOptions + { + BatchSizeLimit = batchSizeLimit, + Period = period, + EagerlyEmitFirstEvent = true, + QueueLimit = queueLimit == NoQueueLimit ? null : queueLimit + }) + { + _batchedLogEventSink = this; + } + + PeriodicBatchingSink(PeriodicBatchingSinkOptions options) + { + if (options == null) throw new ArgumentNullException(nameof(options)); if (options.BatchSizeLimit <= 0) throw new ArgumentOutOfRangeException(nameof(options), "The batch size limit must be greater than zero."); @@ -73,14 +127,33 @@ public PeriodicBatchingSink(IBatchedLogEventSink batchedSink, PeriodicBatchingSi _status = new BatchedConnectionStatus(options.Period); _eagerlyEmitFirstEvent = options.EagerlyEmitFirstEvent; _timer = new PortableTimer(_ => OnTick()); - } - /// + // Initialized by externally-callable constructors. + _batchedLogEventSink = null!; + } + + /// + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. + /// + /// 2 public void Dispose() { + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Free resources held by the sink. + /// + /// If true, called because the object is being disposed; if false, + /// the object is being disposed from the finalizer. + protected virtual void Dispose(bool disposing) + { + if (!disposing) return; + lock (_stateLock) { - if (!_started || _unloading) + if (_unloading) return; _unloading = true; @@ -101,7 +174,7 @@ public async ValueTask DisposeAsync() { lock (_stateLock) { - if (!_started || _unloading) + if (_unloading) return; _unloading = true; @@ -111,13 +184,48 @@ public async ValueTask DisposeAsync() await OnTick().ConfigureAwait(false); + if (ReferenceEquals(_batchedLogEventSink, this)) + { + // The sink is being used in the obsolete inheritance-based mode. Old sinks won't + // override something like `DisposeAsyncCore()`; we just forward to the synchronous + // `Dispose()` method to ensure whatever cleanup they do still occurs. + Dispose(true); + return; + } + if (_batchedLogEventSink is IAsyncDisposable asyncDisposable) await asyncDisposable.DisposeAsync().ConfigureAwait(false); else (_batchedLogEventSink as IDisposable)?.Dispose(); + + GC.SuppressFinalize(this); } #endif + /// + /// Emit a batch of log events, running to completion synchronously. + /// + /// The events to emit. + /// Override either or , + /// not both. + protected virtual void EmitBatch(IEnumerable events) + { + } + + /// + /// Emit a batch of log events, running asynchronously. + /// + /// The events to emit. + /// Override either or , + /// not both. +#pragma warning disable 1998 + protected virtual async Task EmitBatchAsync(IEnumerable events) +#pragma warning restore 1998 + { + // ReSharper disable once MethodHasAsyncOverload + EmitBatch(events); + } + async Task OnTick() { try @@ -219,5 +327,44 @@ public void Emit(LogEvent logEvent) _queue.TryEnqueue(logEvent); } + + /// + /// Determine whether a queued log event should be included in the batch. If + /// an override returns false, the event will be dropped. + /// + /// An event to test for inclusion. + /// True if the event should be included in the batch; otherwise, false. + // ReSharper disable once UnusedParameter.Global + protected virtual bool CanInclude(LogEvent logEvent) + { + return true; + } + + /// + /// Allows derived sinks to perform periodic work without requiring additional threads + /// or timers (thus avoiding additional flush/shut-down complexity). + /// + /// Override either or , + /// not both. + protected virtual void OnEmptyBatch() + { + } + + /// + /// Allows derived sinks to perform periodic work without requiring additional threads + /// or timers (thus avoiding additional flush/shut-down complexity). + /// + /// Override either or , + /// not both. +#pragma warning disable 1998 + protected virtual async Task OnEmptyBatchAsync() +#pragma warning restore 1998 + { + // ReSharper disable once MethodHasAsyncOverload + OnEmptyBatch(); + } + + Task IBatchedLogEventSink.EmitBatchAsync(IEnumerable batch) => EmitBatchAsync(batch); + Task IBatchedLogEventSink.OnEmptyBatchAsync() => OnEmptyBatchAsync(); } } diff --git a/test/Serilog.Sinks.PeriodicBatching.PerformanceTests/Serilog.Sinks.PeriodicBatching.PerformanceTests.csproj b/test/Serilog.Sinks.PeriodicBatching.PerformanceTests/Serilog.Sinks.PeriodicBatching.PerformanceTests.csproj index 5f1bdf1..57704d7 100644 --- a/test/Serilog.Sinks.PeriodicBatching.PerformanceTests/Serilog.Sinks.PeriodicBatching.PerformanceTests.csproj +++ b/test/Serilog.Sinks.PeriodicBatching.PerformanceTests/Serilog.Sinks.PeriodicBatching.PerformanceTests.csproj @@ -5,6 +5,7 @@ ../../assets/Serilog.snk true true + false diff --git a/test/Serilog.Sinks.PeriodicBatching.Tests/BackwardsCompatibilityTests.cs b/test/Serilog.Sinks.PeriodicBatching.Tests/BackwardsCompatibilityTests.cs new file mode 100644 index 0000000..a1d53bc --- /dev/null +++ b/test/Serilog.Sinks.PeriodicBatching.Tests/BackwardsCompatibilityTests.cs @@ -0,0 +1,21 @@ +#if FEATURE_ASYNCDISPOSABLE + +using System.Threading.Tasks; +using Serilog.Sinks.PeriodicBatching.Tests.Support; +using Xunit; + +namespace Serilog.Sinks.PeriodicBatching.Tests; + +public class BackwardsCompatibilityTests +{ + [Fact] + public async Task LegacySinksAreDisposedWhenLoggerIsDisposedAsync() + { + var sink = new LegacyDisposeTrackingSink(); + var logger = new LoggerConfiguration().WriteTo.Sink(sink).CreateLogger(); + await logger.DisposeAsync(); + Assert.True(sink.IsDisposed); + } +} + +#endif diff --git a/test/Serilog.Sinks.PeriodicBatching.Tests/Serilog.Sinks.PeriodicBatching.Tests.csproj b/test/Serilog.Sinks.PeriodicBatching.Tests/Serilog.Sinks.PeriodicBatching.Tests.csproj index a258351..e3ff328 100644 --- a/test/Serilog.Sinks.PeriodicBatching.Tests/Serilog.Sinks.PeriodicBatching.Tests.csproj +++ b/test/Serilog.Sinks.PeriodicBatching.Tests/Serilog.Sinks.PeriodicBatching.Tests.csproj @@ -5,6 +5,7 @@ ../../assets/Serilog.snk true true + false @@ -19,6 +20,7 @@ + diff --git a/test/Serilog.Sinks.PeriodicBatching.Tests/Support/LegacyDisposeTrackingSink.cs b/test/Serilog.Sinks.PeriodicBatching.Tests/Support/LegacyDisposeTrackingSink.cs new file mode 100644 index 0000000..f97b130 --- /dev/null +++ b/test/Serilog.Sinks.PeriodicBatching.Tests/Support/LegacyDisposeTrackingSink.cs @@ -0,0 +1,21 @@ +using System; + +#pragma warning disable CS0618 + +namespace Serilog.Sinks.PeriodicBatching.Tests.Support +{ + public class LegacyDisposeTrackingSink : PeriodicBatchingSink + { + public bool IsDisposed { get; private set; } + + public LegacyDisposeTrackingSink() + : base(10, TimeSpan.FromMinutes(1)) + { + } + + protected override void Dispose(bool disposing) + { + IsDisposed = true; + } + } +}