Skip to content

Commit eacbca6

Browse files
committed
Core implementation without config handling.
* Added class SqlConnectionStringBuilderWrapper to add Enlist= parameter. Used in in SqlConnectionFactory. * Added new sink option PreventEnlistInTransaction in code and documented it in README. * Tests
1 parent 66e6197 commit eacbca6

File tree

14 files changed

+347
-22
lines changed

14 files changed

+347
-22
lines changed

README.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ Basic settings of the sink are configured using the properties in a `MSSqlServer
242242
* `TableName`
243243
* `SchemaName`
244244
* `AutoCreateSqlTable`
245+
* `PreventEnlistInTransaction`
245246
* `BatchPostingLimit`
246247
* `BatchPeriod`
247248
* `EagerlyEmitFirstEvent`
@@ -261,6 +262,15 @@ An optional parameter specifiying the database schema where the log events table
261262

262263
A flag specifiying if the log events table should be created if it does not exist. It defaults to `false`.
263264

265+
### PreventEnlistInTransaction
266+
267+
A flag to prevent logging SQL commands from taking part in ambient transactions. It defaults to `true`.
268+
Logging operations could be affected from surrounding `TransactionScope´s in the code, leading to log data
269+
being removed on a transation rollback. This is by default prevented by the sink adding `Enlist=false` to
270+
the `ConnectionString` that is passed. This option can be used to change this behaviour so that no `Enlist=false`
271+
is added and the passed `ConnectionString` is used as is and logging commands might be part of transations.
272+
Only change this option to `false` if you really know what you are doing!
273+
264274
### BatchPostingLimit
265275

266276
Specifies a maximum number of log events that the non-audit sink writes per batch. The default is 50.
@@ -280,7 +290,7 @@ This setting is not used by the audit sink as it writes each event immediately a
280290

281291
A flag specifiying to use Azure Managed Identities for authenticating with an Azure SQL server. It defaults to `false`. If enabled the property `AzureServiceTokenProviderResource` must be set as well.
282292

283-
**IMPORTANT:** Azure Managed Identities is only supported for the target frameworks .NET Framework 4.7.2+ and .NET Core 2.2+. Setting this to `true` when targeting a different framework results in an exception.
293+
**IMPORTANT:** Azure Managed Identities is only supported for the target frameworks .NET Framework 4.7.2+ and .NET (Core) 2.2+. Setting this to `true` when targeting a different framework results in an exception.
284294

285295
See [Azure AD-managed identities for Azure resources documentation](https://docs.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/) for details on how to configure and use Azure Managed Identitites.
286296

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Serilog.Formatting;
33
using Serilog.Sinks.MSSqlServer.Output;
44
using Serilog.Sinks.MSSqlServer.Platform;
5+
using Serilog.Sinks.MSSqlServer.Platform.SqlClient;
56

67
namespace Serilog.Sinks.MSSqlServer.Dependencies
78
{
@@ -19,7 +20,9 @@ internal static SinkDependencies Create(
1920

2021
var sqlConnectionFactory =
2122
new SqlConnectionFactory(connectionString,
23+
sinkOptions?.PreventEnlistInTransaction ?? true,
2224
sinkOptions?.UseAzureManagedIdentity ?? default,
25+
new SqlConnectionStringBuilderWrapper(),
2326
new AzureManagedServiceAuthenticator(
2427
sinkOptions?.UseAzureManagedIdentity ?? default,
2528
sinkOptions.AzureServiceTokenProviderResource,

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerSinkOptions.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public class MSSqlServerSinkOptions
1313
public MSSqlServerSinkOptions()
1414
{
1515
SchemaName = MSSqlServerSink.DefaultSchemaName;
16+
PreventEnlistInTransaction = true;
1617
BatchPostingLimit = MSSqlServerSink.DefaultBatchPostingLimit;
1718
BatchPeriod = MSSqlServerSink.DefaultPeriod;
1819
EagerlyEmitFirstEvent = true;
@@ -47,6 +48,11 @@ internal MSSqlServerSinkOptions(
4748
/// </summary>
4849
public bool AutoCreateSqlTable { get; set; }
4950

51+
/// <summary>
52+
/// Flag to prevent logging SQL commands from taking part in ambient transactions (default: true)
53+
/// </summary>
54+
public bool PreventEnlistInTransaction { get; set; }
55+
5056
/// <summary>
5157
/// Limits how many log events are written to the database per batch (default: 50)
5258
/// </summary>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace Serilog.Sinks.MSSqlServer.Platform.SqlClient
2+
{
3+
internal interface ISqlConnectionStringBuilderWrapper
4+
{
5+
string ConnectionString { get; set; }
6+
bool Enlist { set; }
7+
}
8+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#if NET452
2+
using System.Data.SqlClient;
3+
#else
4+
using Microsoft.Data.SqlClient;
5+
#endif
6+
7+
namespace Serilog.Sinks.MSSqlServer.Platform.SqlClient
8+
{
9+
internal class SqlConnectionStringBuilderWrapper : ISqlConnectionStringBuilderWrapper
10+
{
11+
private readonly SqlConnectionStringBuilder _sqlConnectionStringBuilder;
12+
13+
public SqlConnectionStringBuilderWrapper()
14+
{
15+
_sqlConnectionStringBuilder = new SqlConnectionStringBuilder();
16+
}
17+
18+
public string ConnectionString
19+
{
20+
get => _sqlConnectionStringBuilder.ConnectionString;
21+
set => _sqlConnectionStringBuilder.ConnectionString = value;
22+
}
23+
24+
public bool Enlist
25+
{
26+
set => _sqlConnectionStringBuilder.Enlist = value;
27+
}
28+
}
29+
}

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,35 @@ internal class SqlConnectionFactory : ISqlConnectionFactory
77
{
88
private readonly string _connectionString;
99
private readonly bool _useAzureManagedIdentity;
10+
private readonly ISqlConnectionStringBuilderWrapper _sqlConnectionStringBuilderWrapper;
1011
private readonly IAzureManagedServiceAuthenticator _azureManagedServiceAuthenticator;
1112

12-
public SqlConnectionFactory(string connectionString, bool useAzureManagedIdentity, IAzureManagedServiceAuthenticator azureManagedServiceAuthenticator)
13+
public SqlConnectionFactory(
14+
string connectionString,
15+
bool preventEnlistInTransaction,
16+
bool useAzureManagedIdentity,
17+
ISqlConnectionStringBuilderWrapper sqlConnectionStringBuilderWrapper,
18+
IAzureManagedServiceAuthenticator azureManagedServiceAuthenticator)
1319
{
1420
if (string.IsNullOrWhiteSpace(connectionString))
1521
{
1622
throw new ArgumentNullException(nameof(connectionString));
1723
}
18-
19-
_connectionString = connectionString;
20-
_useAzureManagedIdentity = useAzureManagedIdentity;
24+
_sqlConnectionStringBuilderWrapper = sqlConnectionStringBuilderWrapper
25+
?? throw new ArgumentNullException(nameof(sqlConnectionStringBuilderWrapper));
2126
_azureManagedServiceAuthenticator = azureManagedServiceAuthenticator
2227
?? throw new ArgumentNullException(nameof(azureManagedServiceAuthenticator));
28+
29+
// Add 'Enlist=false', so that ambient transactions (TransactionScope) will not affect/rollback logging
30+
// unless sink option PreventEnlistInTransaction is set to false.
31+
_sqlConnectionStringBuilderWrapper.ConnectionString = connectionString;
32+
if (preventEnlistInTransaction)
33+
{
34+
_sqlConnectionStringBuilderWrapper.Enlist = false;
35+
}
36+
_connectionString = _sqlConnectionStringBuilderWrapper.ConnectionString;
37+
38+
_useAzureManagedIdentity = useAzureManagedIdentity;
2339
}
2440

2541
public ISqlConnectionWrapper Create()

test/Serilog.Sinks.MSSqlServer.Tests/Configuration/Factories/MSSqlServerAuditSinkFactoryTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public void MSSqlServerAuditSinkFactoryCreateReturnsInstance()
1515
var sut = new MSSqlServerAuditSinkFactory();
1616

1717
// Act
18-
var result = sut.Create("TestConnectionString", sinkOptions, null, new MSSqlServer.ColumnOptions(), null);
18+
var result = sut.Create(DatabaseFixture.LogEventsConnectionString, sinkOptions, null, new MSSqlServer.ColumnOptions(), null);
1919

2020
// Assert
2121
Assert.IsType<MSSqlServerAuditSink>(result);

test/Serilog.Sinks.MSSqlServer.Tests/Configuration/Factories/MSSqlServerSinkFactoryTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public void MSSqlServerSinkFactoryCreateReturnsInstance()
1515
var sut = new MSSqlServerSinkFactory();
1616

1717
// Act
18-
var result = sut.Create("TestConnectionString", sinkOptions, null, new MSSqlServer.ColumnOptions(), null);
18+
var result = sut.Create(DatabaseFixture.LogEventsConnectionString, sinkOptions, null, new MSSqlServer.ColumnOptions(), null);
1919

2020
// Assert
2121
Assert.IsType<MSSqlServerSink>(result);

test/Serilog.Sinks.MSSqlServer.Tests/MiscFeaturesTests.cs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Data;
4+
using System.Transactions;
45
using FluentAssertions;
56
using Serilog.Sinks.MSSqlServer.Tests.TestUtils;
67
using Xunit;
@@ -291,5 +292,72 @@ public void LogEventStoreAsEnum()
291292
VerifyCustomQuery<LogEventColumn>($"SELECT Id from {DatabaseFixture.LogTableName}",
292293
e => e.Should().HaveCount(1));
293294
}
295+
296+
[Fact]
297+
public void LogsAreNotAffectedByTransactionsByDefault()
298+
{
299+
// Arrange
300+
Log.Logger = new LoggerConfiguration()
301+
.WriteTo.MSSqlServer
302+
(
303+
connectionString: DatabaseFixture.LogEventsConnectionString,
304+
new MSSqlServerSinkOptions
305+
{
306+
TableName = DatabaseFixture.LogTableName,
307+
AutoCreateSqlTable = true,
308+
EagerlyEmitFirstEvent = false,
309+
BatchPeriod = TimeSpan.FromSeconds(30),
310+
}
311+
)
312+
.CreateLogger();
313+
314+
using (var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
315+
{
316+
// Act
317+
Log.Logger.Information("Logging message");
318+
319+
// Flush message so it is written on foreground thread instead of timer
320+
// So we can test if it is affected by transaction
321+
Log.CloseAndFlush();
322+
}
323+
324+
// Assert after rollback, the message should still be persisted
325+
VerifyCustomQuery<LogEventColumn>($"SELECT Id from {DatabaseFixture.LogTableName}",
326+
e => e.Should().HaveCount(1));
327+
}
328+
329+
[Fact]
330+
public void LogsAreAffectedByTransactionsIfPreventEnlistInTransactionIsFalse()
331+
{
332+
// Arrange
333+
Log.Logger = new LoggerConfiguration()
334+
.WriteTo.MSSqlServer
335+
(
336+
connectionString: DatabaseFixture.LogEventsConnectionString,
337+
new MSSqlServerSinkOptions
338+
{
339+
TableName = DatabaseFixture.LogTableName,
340+
AutoCreateSqlTable = true,
341+
PreventEnlistInTransaction = false,
342+
EagerlyEmitFirstEvent = false,
343+
BatchPeriod = TimeSpan.FromSeconds(30),
344+
}
345+
)
346+
.CreateLogger();
347+
348+
using (var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
349+
{
350+
// Act
351+
Log.Logger.Information("Logging message");
352+
353+
// Flush message so it is written on foreground thread instead of timer
354+
// So we can test if it is affected by transaction
355+
Log.CloseAndFlush();
356+
}
357+
358+
// Assert after rollback, the message should still be persisted
359+
VerifyCustomQuery<LogEventColumn>($"SELECT Id from {DatabaseFixture.LogTableName}",
360+
e => e.Should().HaveCount(0));
361+
}
294362
}
295363
}

test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlClient/SqlBulkCopyWrapperTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
using Serilog.Sinks.MSSqlServer.Tests.TestUtils;
99
using Xunit;
1010

11-
namespace Serilog.Sinks.MSSqlServer.Tests.Sinks.MSSqlServer.Platform.SqlClient
11+
namespace Serilog.Sinks.MSSqlServer.Tests.Platform.SqlClient
1212
{
1313
[Trait(TestCategory.TraitName, TestCategory.Unit)]
1414
public class SqlBulkCopyWrapperTests

test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapperTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
using Serilog.Sinks.MSSqlServer.Tests.TestUtils;
99
using Serilog.Sinks.MSSqlServer.Platform.SqlClient;
1010

11-
namespace Serilog.Sinks.MSSqlServer.Tests.Sinks.MSSqlServer.Platform.SqlClient
11+
namespace Serilog.Sinks.MSSqlServer.Tests.Platform.SqlClient
1212
{
1313
[Trait(TestCategory.TraitName, TestCategory.Unit)]
1414
public class SqlCommandWrapperTests

0 commit comments

Comments
 (0)