Merge pull request #59 from serilog/dev

3.1.0 Release

src/Serilog.Sinks.PeriodicBatching/Serilog.Sinks.PeriodicBatching.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <Description>Buffer batches of log events to be flushed asynchronously.</Description>
-    <VersionPrefix>3.0.0</VersionPrefix>
+    <VersionPrefix>3.1.0</VersionPrefix>
     <Authors>Serilog Contributors</Authors>
     <TargetFrameworks>net45;netstandard1.1;netstandard1.2;netstandard2.0;netstandard2.1</TargetFrameworks>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>

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.
     /// </remarks>
-    public sealed class PeriodicBatchingSink : ILogEventSink, IDisposable
+    public class PeriodicBatchingSink : ILogEventSink, IDisposable, IBatchedLogEventSink
 #if FEATURE_ASYNCDISPOSABLE
         , IAsyncDisposable
 #endif
     {
+        /// <summary>
+        /// Constant used with legacy constructor to indicate that the internal queue shouldn't be limited.
+        /// </summary>
+        [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.</param>
         /// <param name="options">Options controlling behavior of the sink.</param>
         public PeriodicBatchingSink(IBatchedLogEventSink batchedSink, PeriodicBatchingSinkOptions options)
+            : this(options)
         {
-            if (options == null) throw new ArgumentNullException(nameof(options));
             _batchedLogEventSink = batchedSink ?? throw new ArgumentNullException(nameof(batchedSink));
+        }
+
+        /// <summary>
+        /// Construct a <see cref="PeriodicBatchingSink"/>. New code should avoid subclassing
+        /// <see cref="PeriodicBatchingSink"/> and use
+        /// <see cref="PeriodicBatchingSink(Serilog.Sinks.PeriodicBatching.IBatchedLogEventSink,Serilog.Sinks.PeriodicBatching.PeriodicBatchingSinkOptions)"/>
+        /// instead.
+        /// </summary>
+        /// <param name="batchSizeLimit">The maximum number of events to include in a single batch.</param>
+        /// <param name="period">The time to wait between checking for event batches.</param>
+        [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;
+        }
+
+        /// <summary>
+        /// Construct a <see cref="PeriodicBatchingSink"/>. New code should avoid subclassing
+        /// <see cref="PeriodicBatchingSink"/> and use
+        /// <see cref="PeriodicBatchingSink(Serilog.Sinks.PeriodicBatching.IBatchedLogEventSink,Serilog.Sinks.PeriodicBatching.PeriodicBatchingSinkOptions)"/>
+        /// instead.
+        /// </summary>
+        /// <param name="batchSizeLimit">The maximum number of events to include in a single batch.</param>
+        /// <param name="period">The time to wait between checking for event batches.</param>
+        /// <param name="queueLimit">Maximum number of events in the queue - use <see cref="NoQueueLimit"/> for an unbounded queue.</param>
+        [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());
-        }
 
-        /// <inheritdoc/>
+            // Initialized by externally-callable constructors.
+            _batchedLogEventSink = null!;
+        }
+
+        /// <summary>
+        /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
+        /// </summary>
+        /// <filterpriority>2</filterpriority>
         public void Dispose()
         {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        /// <summary>
+        /// Free resources held by the sink.
+        /// </summary>
+        /// <param name="disposing">If true, called because the object is being disposed; if false,
+        /// the object is being disposed from the finalizer.</param>
+        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
 
+        /// <summary>
+        /// Emit a batch of log events, running to completion synchronously.
+        /// </summary>
+        /// <param name="events">The events to emit.</param>
+        /// <remarks>Override either <see cref="EmitBatch"/> or <see cref="EmitBatchAsync"/>,
+        /// not both.</remarks>
+        protected virtual void EmitBatch(IEnumerable<LogEvent> events)
+        {
+        }
+
+        /// <summary>
+        /// Emit a batch of log events, running asynchronously.
+        /// </summary>
+        /// <param name="events">The events to emit.</param>
+        /// <remarks>Override either <see cref="EmitBatchAsync"/> or <see cref="EmitBatch"/>,
+        /// not both. </remarks>
+#pragma warning disable 1998
+        protected virtual async Task EmitBatchAsync(IEnumerable<LogEvent> 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);
         }
+
+        /// <summary>
+        /// Determine whether a queued log event should be included in the batch. If
+        /// an override returns false, the event will be dropped.
+        /// </summary>
+        /// <param name="logEvent">An event to test for inclusion.</param>
+        /// <returns>True if the event should be included in the batch; otherwise, false.</returns>
+        // ReSharper disable once UnusedParameter.Global
+        protected virtual bool CanInclude(LogEvent logEvent)
+        {
+            return true;
+        }
+
+        /// <summary>
+        /// Allows derived sinks to perform periodic work without requiring additional threads
+        /// or timers (thus avoiding additional flush/shut-down complexity).
+        /// </summary>
+        /// <remarks>Override either <see cref="OnEmptyBatch"/> or <see cref="OnEmptyBatchAsync"/>,
+        /// not both. </remarks>
+        protected virtual void OnEmptyBatch()
+        {
+        }
+
+        /// <summary>
+        /// Allows derived sinks to perform periodic work without requiring additional threads
+        /// or timers (thus avoiding additional flush/shut-down complexity).
+        /// </summary>
+        /// <remarks>Override either <see cref="OnEmptyBatchAsync"/> or <see cref="OnEmptyBatch"/>,
+        /// not both. </remarks>
+#pragma warning disable 1998
+        protected virtual async Task OnEmptyBatchAsync()
+#pragma warning restore 1998
+        {
+            // ReSharper disable once MethodHasAsyncOverload
+            OnEmptyBatch();
+        }
+
+        Task IBatchedLogEventSink.EmitBatchAsync(IEnumerable<LogEvent> batch) => EmitBatchAsync(batch);
+        Task IBatchedLogEventSink.OnEmptyBatchAsync() => OnEmptyBatchAsync();
     }
 }

test/Serilog.Sinks.PeriodicBatching.PerformanceTests/Serilog.Sinks.PeriodicBatching.PerformanceTests.csproj

@@ -5,6 +5,7 @@
     <AssemblyOriginatorKeyFile>../../assets/Serilog.snk</AssemblyOriginatorKeyFile>
     <SignAssembly>true</SignAssembly>
     <PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
+    <CheckEolTargetFramework>false</CheckEolTargetFramework>
   </PropertyGroup>
 
   <ItemGroup>

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

test/Serilog.Sinks.PeriodicBatching.Tests/Serilog.Sinks.PeriodicBatching.Tests.csproj

@@ -5,6 +5,7 @@
     <AssemblyOriginatorKeyFile>../../assets/Serilog.snk</AssemblyOriginatorKeyFile>
     <SignAssembly>true</SignAssembly>
     <PublicSign Condition=" '$(OS)' != 'Windows_NT' ">true</PublicSign>
+    <CheckEolTargetFramework>false</CheckEolTargetFramework>
   </PropertyGroup>
 
   <PropertyGroup Condition=" '$(TargetFramework)' == 'net6.0' ">
@@ -19,6 +20,7 @@
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.2.0" />
     <PackageReference Include="xunit" Version="2.2.0" />
+    <PackageReference Include="Serilog" Version="2.12.0" />
   </ItemGroup>
 
   <ItemGroup Condition=" '$(TargetFramework)' == 'net452' ">

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;
+        }
+    }
+}