Merge pull request #598 from ckadluba/perf-refactor

Refactoring and Performance Optimizations

Author: ckadluba

.editorconfig

@@ -57,12 +57,24 @@ csharp_style_var_for_built_in_types = true:warning
 csharp_style_var_when_type_is_apparent = true:warning
 csharp_using_directive_placement = outside_namespace:warning
 
+# Naming rules for non-public fields
 dotnet_naming_rule.private_members_with_underscore.symbols = private_fields
 dotnet_naming_rule.private_members_with_underscore.style = prefix_underscore
 dotnet_naming_rule.private_members_with_underscore.severity = error
 dotnet_naming_symbols.private_fields.applicable_kinds = field
-dotnet_naming_symbols.private_fields.applicable_accessibilities = private,protected
+dotnet_naming_symbols.private_fields.applicable_accessibilities = private, protected
 dotnet_naming_style.prefix_underscore.capitalization = camel_case
 dotnet_naming_style.prefix_underscore.required_prefix = _
+
+# Naming rules for const or static public fields
+dotnet_naming_rule.public_fields_pascal_case.symbols = public_constant_static_fields
+dotnet_naming_rule.public_fields_pascal_case.style = pascal_case
+dotnet_naming_rule.public_fields_pascal_case.severity = error
+dotnet_naming_symbols.public_constant_static_fields.applicable_kinds = field
+dotnet_naming_symbols.public_constant_static_fields.applicable_accessibilities = public
+dotnet_naming_symbols.public_constant_static_fields.required_modifiers = const, static
+dotnet_naming_style.pascal_case.capitalization = pascal_case
+
 dotnet_sort_system_directives_first = true
 dotnet_style_require_accessibility_modifiers = always:error
+

.github/ISSUE_TEMPLATE.md

@@ -8,17 +8,18 @@ If you are opening a feature request, you can ignore this template. Bug reports
 
 >> List the names and versions of all Serilog packages used in the project:
 
-- Serilog: 
-- Serilog.Sinks.MSSqlServer: 
+- Serilog:
+- Serilog.Sinks.MSSqlServer:
 - (configuration, etc.)
 
 >> Target framework and operating system:
 
-[ ] .NET 6
+[ ] .NET 9
+[ ] .NET 8
 [ ] .NET Framework 4.8
 [ ] .NET Framework 4.7
 [ ] .NET Framework 4.6
-OS: 
+OS:
 
 >> Provide a *simple* reproduction of your Serilog configuration code:
 

.github/workflows/release.yml

@@ -39,6 +39,12 @@ jobs:
         run: ./RunPerfTests.ps1 -Filter "*QuickBenchmarks*"
         shell: pwsh
 
+      - name: Upload perf test results artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: perftestresults
+          path: artifacts\perftests
+
       - name: Get last commit message
         id: last_commit
         if: success() && github.ref == 'refs/heads/main'

CHANGES.md

@@ -1,3 +1,10 @@
+# 8.0.1
+* Refactoring and performance optimizations in batched and audit sink
+* Create perftest result on release
+* Updated issue template
+* Updated editorconfig
+* Added specific documentation about when SQL SELECT permission is not required
+
 # 8.0.0
 * Updated to .NET 8
 * Updated nearly all dependencies

README.md

@@ -200,6 +200,8 @@ CREATE TABLE [Logs] (
 
 At a minimum, writing log entries requires SELECT and INSERT permissions for the log table. (SELECT is required because the sink's batching behavior uses bulk inserts which reads the schema before the write operations begin).
 
+If the audit version of the sink is used or the sink option [UseSqlBulkCopy](#usesqlbulkcopy) is set to `true`, only INSERT statements are used and no SELECT permission is required.
+
 SQL permissions are a very complex subject. Here is an example of one possible solution (valid for SQL 2012 or later):
 
 ```

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Dependencies/SinkDependencies.cs

@@ -4,7 +4,6 @@ namespace Serilog.Sinks.MSSqlServer.Dependencies
 {
     internal class SinkDependencies
     {
-        public IDataTableCreator DataTableCreator { get; set; }
         public ISqlCommandExecutor SqlDatabaseCreator { get; set; }
         public ISqlCommandExecutor SqlTableCreator { get; set; }
         public ISqlBulkBatchWriter SqlBulkBatchWriter { get; set; }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Dependencies/SinkDependenciesFactory.cs

@@ -23,6 +23,7 @@ internal static SinkDependencies Create(
             var sqlConnectionStringBuilderWrapper = new SqlConnectionStringBuilderWrapper(
                 connectionString, sinkOptions.EnlistInTransaction);
             var sqlConnectionFactory = new SqlConnectionFactory(sqlConnectionStringBuilderWrapper);
+            var sqlCommandFactory = new SqlCommandFactory();
             var dataTableCreator = new DataTableCreator(sinkOptions.TableName, columnOptions);
             var sqlCreateTableWriter = new SqlCreateTableWriter(sinkOptions.SchemaName,
                 sinkOptions.TableName, columnOptions, dataTableCreator);
@@ -47,21 +48,16 @@ internal static SinkDependencies Create(
 
             var sinkDependencies = new SinkDependencies
             {
-                DataTableCreator = dataTableCreator,
                 SqlDatabaseCreator = new SqlDatabaseCreator(
-                    sqlCreateDatabaseWriter, sqlConnectionFactoryNoDb),
+                    sqlCreateDatabaseWriter, sqlConnectionFactoryNoDb, sqlCommandFactory),
                 SqlTableCreator = new SqlTableCreator(
-                    sqlCreateTableWriter, sqlConnectionFactory),
-                SqlBulkBatchWriter = sinkOptions.UseSqlBulkCopy
-                    ? (ISqlBulkBatchWriter)new SqlBulkBatchWriter(
-                        sinkOptions.TableName, sinkOptions.SchemaName, columnOptions.DisableTriggers,
-                        sqlConnectionFactory, logEventDataGenerator)
-                    : (ISqlBulkBatchWriter)new SqlInsertStatementWriter(
-                        sinkOptions.TableName, sinkOptions.SchemaName,
-                        sqlConnectionFactory, logEventDataGenerator),
+                    sqlCreateTableWriter, sqlConnectionFactory, sqlCommandFactory),
+                SqlBulkBatchWriter = new SqlBulkBatchWriter(
+                    sinkOptions.TableName, sinkOptions.SchemaName, columnOptions.DisableTriggers,
+                    dataTableCreator, sqlConnectionFactory, logEventDataGenerator),
                 SqlLogEventWriter = new SqlInsertStatementWriter(
                     sinkOptions.TableName, sinkOptions.SchemaName,
-                    sqlConnectionFactory, logEventDataGenerator)
+                    sqlConnectionFactory, sqlCommandFactory, logEventDataGenerator)
             };
 
             return sinkDependencies;

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerAuditSink.cs

@@ -1,11 +1,11 @@
-// Copyright 2024 Serilog Contributors 
-// 
+// Copyright 2024 Serilog Contributors
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -30,11 +30,13 @@ public class MSSqlServerAuditSink : ILogEventSink, IDisposable
     {
         private readonly ISqlLogEventWriter _sqlLogEventWriter;
 
+        private bool _disposedValue;
+
         /// <summary>
         /// Construct a sink posting to the specified database.
         ///
         /// Note: this is the legacy version of the extension method. Please use the new one using MSSqlServerSinkOptions instead.
-        /// 
+        ///
         /// </summary>
         /// <param name="connectionString">Connection string to access the database.</param>
         /// <param name="tableName">Name of the table to store the data in.</param>
@@ -113,7 +115,15 @@ public void Dispose()
         /// <param name="disposing">True to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
         protected virtual void Dispose(bool disposing)
         {
-            // This class needn't to dispose anything. This is just here for sink interface compatibility.
+            if (!_disposedValue)
+            {
+                if (disposing)
+                {
+                    _sqlLogEventWriter.Dispose();
+                }
+
+                _disposedValue = true;
+            }
         }
 
         private static void ValidateParameters(MSSqlServerSinkOptions sinkOptions, ColumnOptions columnOptions)
@@ -134,11 +144,6 @@ private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
                 throw new ArgumentNullException(nameof(sinkDependencies));
             }
 
-            if (sinkDependencies.DataTableCreator == null)
-            {
-                throw new InvalidOperationException("DataTableCreator is not initialized!");
-            }
-
             if (sinkDependencies.SqlTableCreator == null)
             {
                 throw new InvalidOperationException("SqlTableCreator is not initialized!");

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerSink.cs

@@ -1,11 +1,11 @@
-// Copyright 2024 Serilog Contributors 
-// 
+// Copyright 2024 Serilog Contributors
+//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -14,7 +14,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Data;
 using System.Threading.Tasks;
 using Serilog.Events;
 using Serilog.Formatting;
@@ -29,8 +28,9 @@ namespace Serilog.Sinks.MSSqlServer
     /// </summary>
     public class MSSqlServerSink : IBatchedLogEventSink, IDisposable
     {
+        private readonly MSSqlServerSinkOptions _sinkOptions;
         private readonly ISqlBulkBatchWriter _sqlBulkBatchWriter;
-        private readonly DataTable _eventTable;
+        private readonly ISqlLogEventWriter _sqlLogEventWriter; // Used if sink option UseSqlBulkCopy is set to false
 
         /// <summary>
         /// The default database schema name.
@@ -53,7 +53,7 @@ public class MSSqlServerSink : IBatchedLogEventSink, IDisposable
         /// Construct a sink posting to the specified database.
         ///
         /// Note: this is the legacy version of the extension method. Please use the new one using MSSqlServerSinkOptions instead.
-        /// 
+        ///
         /// </summary>
         /// <param name="connectionString">Connection string to access the database.</param>
         /// <param name="tableName">Name of the table to store the data in.</param>
@@ -108,8 +108,9 @@ internal MSSqlServerSink(
             ValidateParameters(sinkOptions);
             CheckSinkDependencies(sinkDependencies);
 
+            _sinkOptions = sinkOptions;
             _sqlBulkBatchWriter = sinkDependencies.SqlBulkBatchWriter;
-            _eventTable = sinkDependencies.DataTableCreator.CreateDataTable();
+            _sqlLogEventWriter = sinkDependencies.SqlLogEventWriter;
 
             CreateDatabaseAndTable(sinkOptions, sinkDependencies);
         }
@@ -119,7 +120,9 @@ internal MSSqlServerSink(
         /// </summary>
         /// <param name="batch">The events to emit.</param>
         public Task EmitBatchAsync(IReadOnlyCollection<LogEvent> batch) =>
-            _sqlBulkBatchWriter.WriteBatch(batch, _eventTable);
+            _sinkOptions.UseSqlBulkCopy
+                ? _sqlBulkBatchWriter.WriteBatch(batch)
+                : _sqlLogEventWriter.WriteEvents(batch);
 
         /// <summary>
         /// Called upon batchperiod if no data is in batch. Not used by this sink.
@@ -148,7 +151,7 @@ protected virtual void Dispose(bool disposing)
             {
                 if (disposing)
                 {
-                    _eventTable.Dispose();
+                    _sqlLogEventWriter.Dispose();
                 }
 
                 _disposedValue = true;
@@ -170,11 +173,6 @@ private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
                 throw new ArgumentNullException(nameof(sinkDependencies));
             }
 
-            if (sinkDependencies.DataTableCreator == null)
-            {
-                throw new InvalidOperationException("DataTableCreator is not initialized!");
-            }
-
             if (sinkDependencies.SqlTableCreator == null)
             {
                 throw new InvalidOperationException("SqlTableCreator is not initialized!");
@@ -184,6 +182,11 @@ private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
             {
                 throw new InvalidOperationException("SqlBulkBatchWriter is not initialized!");
             }
+
+            if (sinkDependencies.SqlLogEventWriter == null)
+            {
+                throw new InvalidOperationException("SqlLogEventWriter is not initialized!");
+            }
         }
 
         private void CreateDatabaseAndTable(MSSqlServerSinkOptions sinkOptions, SinkDependencies sinkDependencies)

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlBulkBatchWriter.cs

@@ -1,12 +1,12 @@
-using System.Collections.Generic;
-using System.Data;
+using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Serilog.Events;
 
 namespace Serilog.Sinks.MSSqlServer.Platform
 {
-    internal interface ISqlBulkBatchWriter
+    internal interface ISqlBulkBatchWriter : IDisposable
     {
-        Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable);
+        Task WriteBatch(IEnumerable<LogEvent> events);
     }
 }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlCommandFactory.cs

@@ -0,0 +1,10 @@
+using Serilog.Sinks.MSSqlServer.Platform.SqlClient;
+
+namespace Serilog.Sinks.MSSqlServer.Platform
+{
+    internal interface ISqlCommandFactory
+    {
+        ISqlCommandWrapper CreateCommand(ISqlConnectionWrapper sqlConnection);
+        ISqlCommandWrapper CreateCommand(string cmdText, ISqlConnectionWrapper sqlConnection);
+    }
+}

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlLogEventWriter.cs

@@ -1,9 +1,14 @@
-using Serilog.Events;
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Serilog.Events;
 
 namespace Serilog.Sinks.MSSqlServer.Platform
 {
-    internal interface ISqlLogEventWriter
+    internal interface ISqlLogEventWriter : IDisposable
     {
         void WriteEvent(LogEvent logEvent);
+
+        Task WriteEvents(IEnumerable<LogEvent> events);
     }
 }