Refactoring and performance optimizations

Refactoring
* Removed DataTableCreator from class MsSqlServerSink. Pushed it down to SqlBulkBatchWriter. This also made it possible to remove reference to System.Data in MsSqlServerSink.
* Removed DataTableCreator from SinkDependencies.
* Removed IBulkBatchWriter interface from SqlInsertStatementWriter. MSSqlServerSink class now has separate instances of ISqlBulkBatchWriter and ISqlLogEventWriter and chooses which one to use based on sink option `UseSqlBulkCopy`.
* Added new class SqlCommandFactory with interface ISqlCommandFactory

Performance optimizations in batched sink
* Generate schema and table string only once and do not use string.Format().
* Do not cast on each column of each log event.

Performance optimizations in audit sink
* Render INSERT string only once and not for each log event since it will not change between log events.
* Do not create a separate SqlCommand for each log event. Reuse the same and set only new SqlConnection, CommandText and parameters for each log event.

Added new tests and adapted existing ones.

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

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

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Data;
-using System.Globalization;
 using System.Linq;
 using System.Threading.Tasks;
 using Serilog.Debugging;
@@ -12,45 +11,52 @@ namespace Serilog.Sinks.MSSqlServer.Platform
 {
     internal class SqlBulkBatchWriter : ISqlBulkBatchWriter
     {
-        private readonly string _tableName;
-        private readonly string _schemaName;
         private readonly bool _disableTriggers;
+        private readonly DataTable _dataTable;
         private readonly ISqlConnectionFactory _sqlConnectionFactory;
         private readonly ILogEventDataGenerator _logEventDataGenerator;
+        private readonly string _schemaAndTableName;
+
+        private bool _disposedValue;
 
         public SqlBulkBatchWriter(
             string tableName,
             string schemaName,
             bool disableTriggers,
+            IDataTableCreator dataTableCreator,
             ISqlConnectionFactory sqlConnectionFactory,
             ILogEventDataGenerator logEventDataGenerator)
         {
-            _tableName = tableName ?? throw new ArgumentNullException(nameof(tableName));
-            _schemaName = schemaName ?? throw new ArgumentNullException(nameof(schemaName));
+            if (tableName == null) throw new ArgumentNullException(nameof(tableName));
+            if (schemaName == null) throw new ArgumentNullException(nameof(schemaName));
             _disableTriggers = disableTriggers;
             _sqlConnectionFactory = sqlConnectionFactory ?? throw new ArgumentNullException(nameof(sqlConnectionFactory));
             _logEventDataGenerator = logEventDataGenerator ?? throw new ArgumentNullException(nameof(logEventDataGenerator));
+            _schemaAndTableName = "[" + schemaName + "].[" + tableName + "]";
+
+            _dataTable = dataTableCreator == null
+                ? throw new ArgumentNullException(nameof(dataTableCreator))
+                : dataTableCreator.CreateDataTable();
         }
 
-        public async Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable)
+        public async Task WriteBatch(IEnumerable<LogEvent> events)
         {
             try
             {
-                FillDataTable(events, dataTable);
+                FillDataTable(events);
 
                 using (var cn = _sqlConnectionFactory.Create())
                 {
                     await cn.OpenAsync().ConfigureAwait(false);
-                    using (var copy = cn.CreateSqlBulkCopy(_disableTriggers,
-                        string.Format(CultureInfo.InvariantCulture, "[{0}].[{1}]", _schemaName, _tableName)))
+                    using (var copy = cn.CreateSqlBulkCopy(_disableTriggers, _schemaAndTableName))
                     {
-                        foreach (var column in dataTable.Columns)
+                        for (var i = 0; i < _dataTable.Columns.Count; i++)
                         {
-                            var columnName = ((DataColumn)column).ColumnName;
+                            var columnName = _dataTable.Columns[i].ColumnName;
                             copy.AddSqlBulkCopyColumnMapping(columnName, columnName);
                         }
 
-                        await copy.WriteToServerAsync(dataTable).ConfigureAwait(false);
+                        await copy.WriteToServerAsync(_dataTable).ConfigureAwait(false);
                     }
                 }
             }
@@ -62,26 +68,56 @@ public async Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable)
             }
             finally
             {
-                dataTable.Clear();
+                _dataTable.Clear();
             }
         }
 
-        private void FillDataTable(IEnumerable<LogEvent> events, DataTable dataTable)
+        private void FillDataTable(IEnumerable<LogEvent> events)
         {
-            // Add the new rows to the collection. 
+            // Add the new rows to the collection.
+            _dataTable.BeginLoadData();
             foreach (var logEvent in events)
             {
-                var row = dataTable.NewRow();
+                var row = _dataTable.NewRow();
 
                 foreach (var field in _logEventDataGenerator.GetColumnsAndValues(logEvent))
                 {
                     row[field.Key] = field.Value;
                 }
 
-                dataTable.Rows.Add(row);
+                _dataTable.Rows.Add(row);
             }
 
-            dataTable.AcceptChanges();
+            _dataTable.EndLoadData();
+            _dataTable.AcceptChanges();
+        }
+
+
+        /// <summary>
+        /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
+        /// </summary>
+        public void Dispose()
+        {
+            Dispose(disposing: true);
+            GC.SuppressFinalize(this);
+        }
+
+        /// <summary>
+        /// Releases the unmanaged resources used by the Serilog.Sinks.MSSqlServer.Platform.SqlBulkBatchWriter and optionally
+        /// releases the managed resources.
+        /// </summary>
+        /// <param name="disposing">True to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
+        protected virtual void Dispose(bool disposing)
+        {
+            if (!_disposedValue)
+            {
+                if (disposing)
+                {
+                    _dataTable.Dispose();
+                }
+
+                _disposedValue = true;
+            }
         }
     }
 }

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Data;
 using System.Threading.Tasks;
+using Microsoft.Data.SqlClient;
 
 namespace Serilog.Sinks.MSSqlServer.Platform.SqlClient
 {
@@ -9,6 +10,8 @@ internal interface ISqlCommandWrapper : IDisposable
         CommandType CommandType { get; set; }
         string CommandText { get; set; }
 
+        void SetConnection(ISqlConnectionWrapper sqlConnectionWrapper);
+        void ClearParameters();
         void AddParameter(string parameterName, object value);
         int ExecuteNonQuery();
         Task<int> ExecuteNonQueryAsync();