Skip to content

Commit 2b0c9c8

Browse files
ckadlubackadluba
committed
Changes after review from @jonorossi.
* Renamed PreventEnlistInTransactions to EnlistInTransaction and changed default to false. * Fixed typos in README.
1 parent d7cbe54 commit 2b0c9c8

File tree

11 files changed

+31
-99
lines changed

11 files changed

+31
-99
lines changed

‎README.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ Basic settings of the sink are configured using the properties in a `MSSqlServer
242242
* `TableName`
243243
* `SchemaName`
244244
* `AutoCreateSqlTable`
245-
* `PreventEnlistInTransaction`
245+
* `EnlistInTransaction`
246246
* `BatchPostingLimit`
247247
* `BatchPeriod`
248248
* `EagerlyEmitFirstEvent`
@@ -262,14 +262,14 @@ An optional parameter specifiying the database schema where the log events table
262262

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

265-
### PreventEnlistInTransaction
265+
### EnlistInTransaction
266266

267-
A flag to prevent logging SQL commands from taking part in ambient transactions. It defaults to `true`.
267+
A flag to make logging SQL commands take part in ambient transactions. It defaults to `false`.
268268
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!
269+
being removed on a transaction 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 behavior so that `Enlist=true`
271+
is added instead (which is the default for SQL connections) and logging commands might be part of transactions.
272+
Only change this option to `true` if you have a good reason and really know what you are doing!
273273

274274
### BatchPostingLimit
275275

‎src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/Microsoft.Extensions.Configuration/MicrosoftExtensionsSinkOptionsProvider.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ private static void ReadTableOptions(IConfigurationSection config, MSSqlServerSi
2525
SetProperty.IfNotNull<string>(config["tableName"], val => sinkOptions.TableName = val);
2626
SetProperty.IfNotNull<string>(config["schemaName"], val => sinkOptions.SchemaName = val);
2727
SetProperty.IfNotNull<bool>(config["autoCreateSqlTable"], val => sinkOptions.AutoCreateSqlTable = val);
28-
SetProperty.IfNotNull<bool>(config["preventEnlistInTransaction"], val => sinkOptions.PreventEnlistInTransaction = val);
28+
SetProperty.IfNotNull<bool>(config["enlistInTransaction"], val => sinkOptions.EnlistInTransaction = val);
2929
}
3030

3131
private static void ReadBatchSettings(IConfigurationSection config, MSSqlServerSinkOptions sinkOptions)

‎src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/MSSqlServerConfigurationSection.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,10 @@ public ValueConfigElement AutoCreateSqlTable
157157
get => (ValueConfigElement)base[nameof(AutoCreateSqlTable)];
158158
}
159159

160-
[ConfigurationProperty(nameof(PreventEnlistInTransaction))]
161-
public ValueConfigElement PreventEnlistInTransaction
160+
[ConfigurationProperty(nameof(EnlistInTransaction))]
161+
public ValueConfigElement EnlistInTransaction
162162
{
163-
get => (ValueConfigElement)base[nameof(PreventEnlistInTransaction)];
163+
get => (ValueConfigElement)base[nameof(EnlistInTransaction)];
164164
}
165165

166166
[ConfigurationProperty(nameof(BatchPostingLimit))]

‎src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/SystemConfigurationSinkOptionsProvider.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ private static void ReadTableOptions(MSSqlServerConfigurationSection config, MSS
2121
SetProperty.IfProvided<string>(config.SchemaName, nameof(config.SchemaName.Value), value => sinkOptions.SchemaName = value);
2222
SetProperty.IfProvided<bool>(config.AutoCreateSqlTable, nameof(config.AutoCreateSqlTable.Value),
2323
value => sinkOptions.AutoCreateSqlTable = value);
24-
SetProperty.IfProvided<bool>(config.PreventEnlistInTransaction, nameof(config.PreventEnlistInTransaction.Value),
25-
value => sinkOptions.PreventEnlistInTransaction = value);
24+
SetProperty.IfProvided<bool>(config.EnlistInTransaction, nameof(config.EnlistInTransaction.Value),
25+
value => sinkOptions.EnlistInTransaction = value);
2626
}
2727

2828
private static void ReadBatchSettings(MSSqlServerConfigurationSection config, MSSqlServerSinkOptions sinkOptions)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ internal static SinkDependencies Create(
2020

2121
var sqlConnectionFactory =
2222
new SqlConnectionFactory(connectionString,
23-
sinkOptions?.PreventEnlistInTransaction ?? true,
23+
sinkOptions?.EnlistInTransaction ?? default,
2424
sinkOptions?.UseAzureManagedIdentity ?? default,
2525
new SqlConnectionStringBuilderWrapper(),
2626
new AzureManagedServiceAuthenticator(

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ public class MSSqlServerSinkOptions
1313
public MSSqlServerSinkOptions()
1414
{
1515
SchemaName = MSSqlServerSink.DefaultSchemaName;
16-
PreventEnlistInTransaction = true;
1716
BatchPostingLimit = MSSqlServerSink.DefaultBatchPostingLimit;
1817
BatchPeriod = MSSqlServerSink.DefaultPeriod;
1918
EagerlyEmitFirstEvent = true;
@@ -49,9 +48,9 @@ internal MSSqlServerSinkOptions(
4948
public bool AutoCreateSqlTable { get; set; }
5049

5150
/// <summary>
52-
/// Flag to prevent logging SQL commands from taking part in ambient transactions (default: true)
51+
/// Flag to make logging SQL commands take part in ambient transactions (default: false)
5352
/// </summary>
54-
public bool PreventEnlistInTransaction { get; set; }
53+
public bool EnlistInTransaction { get; set; }
5554

5655
/// <summary>
5756
/// Limits how many log events are written to the database per batch (default: 50)

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ internal class SqlConnectionFactory : ISqlConnectionFactory
1212

1313
public SqlConnectionFactory(
1414
string connectionString,
15-
bool preventEnlistInTransaction,
15+
bool enlistInTransaction,
1616
bool useAzureManagedIdentity,
1717
ISqlConnectionStringBuilderWrapper sqlConnectionStringBuilderWrapper,
1818
IAzureManagedServiceAuthenticator azureManagedServiceAuthenticator)
@@ -27,12 +27,9 @@ public SqlConnectionFactory(
2727
?? throw new ArgumentNullException(nameof(azureManagedServiceAuthenticator));
2828

2929
// Add 'Enlist=false', so that ambient transactions (TransactionScope) will not affect/rollback logging
30-
// unless sink option PreventEnlistInTransaction is set to false.
30+
// unless sink option EnlistInTransaction is set to true.
3131
_sqlConnectionStringBuilderWrapper.ConnectionString = connectionString;
32-
if (preventEnlistInTransaction)
33-
{
34-
_sqlConnectionStringBuilderWrapper.Enlist = false;
35-
}
32+
_sqlConnectionStringBuilderWrapper.Enlist = enlistInTransaction;
3633
_connectionString = _sqlConnectionStringBuilderWrapper.ConnectionString;
3734

3835
_useAzureManagedIdentity = useAzureManagedIdentity;

‎test/Serilog.Sinks.MSSqlServer.Tests/Configuration/Implementations/Microsoft.Extensions.Configuration/MicrosoftExtensionsSinkOptionsProviderTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,17 @@ public void ConfigureSinkOptionsSetsAutoCreateSqlTable()
7777
}
7878

7979
[Fact]
80-
public void ConfigureSinkOptionsSetsPreventEnlistInTransaction()
80+
public void ConfigureSinkOptionsSetsEnlistInTransaction()
8181
{
8282
// Arrange
83-
_configurationSectionMock.Setup(s => s["preventEnlistInTransaction"]).Returns("false");
83+
_configurationSectionMock.Setup(s => s["enlistInTransaction"]).Returns("true");
8484
var sut = new MicrosoftExtensionsSinkOptionsProvider();
8585

8686
// Act
8787
var result = sut.ConfigureSinkOptions(new MSSqlServerSinkOptions(), _configurationSectionMock.Object);
8888

8989
// Assert
90-
Assert.False(result.PreventEnlistInTransaction);
90+
Assert.True(result.EnlistInTransaction);
9191
}
9292

9393
[Fact]

‎test/Serilog.Sinks.MSSqlServer.Tests/Configuration/Implementations/System.Configuration/SystemConfigurationSinkOptionsProviderTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,19 @@ public void ConfigureSinkOptionsReadsBatchSettings()
2525
}
2626

2727
[Fact]
28-
public void ConfigureSinkOptionsReadsPreventEnlistInTransaction()
28+
public void ConfigureSinkOptionsReadsEnlistInTransaction()
2929
{
3030
// Arrange
3131
var configSection = new MSSqlServerConfigurationSection();
32-
configSection.PreventEnlistInTransaction.Value = "false";
33-
var sinkOptions = new MSSqlServerSinkOptions { PreventEnlistInTransaction = true };
32+
configSection.EnlistInTransaction.Value = "true";
33+
var sinkOptions = new MSSqlServerSinkOptions { EnlistInTransaction = false };
3434
var sut = new SystemConfigurationSinkOptionsProvider();
3535

3636
// Act
3737
sut.ConfigureSinkOptions(configSection, sinkOptions);
3838

3939
// Assert
40-
Assert.False(sinkOptions.PreventEnlistInTransaction);
40+
Assert.True(sinkOptions.EnlistInTransaction);
4141
}
4242
}
4343
}

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

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Data;
4-
using System.Transactions;
54
using FluentAssertions;
65
using Serilog.Sinks.MSSqlServer.Tests.TestUtils;
76
using Xunit;
@@ -292,68 +291,5 @@ public void LogEventStoreAsEnum()
292291
VerifyCustomQuery<LogEventColumn>($"SELECT Id from {DatabaseFixture.LogTableName}",
293292
e => e.Should().HaveCount(1));
294293
}
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-
}
309-
)
310-
.CreateLogger();
311-
312-
using (var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
313-
{
314-
// Act
315-
Log.Logger.Information("Logging message");
316-
317-
// Flush message so it is written on foreground thread instead of timer
318-
// So we can test if it is affected by transaction
319-
Log.CloseAndFlush();
320-
}
321-
322-
// Assert after rollback, the message should still be persisted
323-
VerifyCustomQuery<LogEventColumn>($"SELECT Id from {DatabaseFixture.LogTableName}",
324-
e => e.Should().HaveCount(1));
325-
}
326-
327-
[Fact]
328-
public void LogsAreAffectedByTransactionsIfPreventEnlistInTransactionIsFalse()
329-
{
330-
// Arrange
331-
Log.Logger = new LoggerConfiguration()
332-
.WriteTo.MSSqlServer
333-
(
334-
connectionString: DatabaseFixture.LogEventsConnectionString,
335-
new MSSqlServerSinkOptions
336-
{
337-
TableName = DatabaseFixture.LogTableName,
338-
AutoCreateSqlTable = true,
339-
PreventEnlistInTransaction = false
340-
}
341-
)
342-
.CreateLogger();
343-
344-
using (var scope = new TransactionScope(TransactionScopeAsyncFlowOption.Enabled))
345-
{
346-
// Act
347-
Log.Logger.Information("Logging message");
348-
349-
// Flush message so it is written on foreground thread instead of timer
350-
// So we can test if it is affected by transaction
351-
Log.CloseAndFlush();
352-
}
353-
354-
// Assert after rollback, the message should still be persisted
355-
VerifyCustomQuery<LogEventColumn>($"SELECT Id from {DatabaseFixture.LogTableName}",
356-
e => e.Should().HaveCount(0));
357-
}
358294
}
359295
}

‎test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlConnectionFactoryTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ public void IntializeThrowsIfAzureManagedServiceAuthenticatorIsNull()
5656
}
5757

5858
[Fact]
59-
public void SetsEnlistOnConnectionStringIfPreventEnlistTransactionTrue()
59+
public void SetsEnlistFalseOnConnectionStringIfEnlistTransactionFalse()
6060
{
6161
// Arrange
62-
var sut = new SqlConnectionFactory(DatabaseFixture.LogEventsConnectionString, true, false,
62+
var sut = new SqlConnectionFactory(DatabaseFixture.LogEventsConnectionString, false, false,
6363
_sqlConnectionStringBuilderWrapperMock.Object, _azureManagedServiceAuthenticatorMock.Object);
6464

6565
// Act
@@ -72,10 +72,10 @@ public void SetsEnlistOnConnectionStringIfPreventEnlistTransactionTrue()
7272
}
7373

7474
[Fact]
75-
public void DoesNotSetEnlistOnConnectionStringIfPreventEnlistTransactionFalse()
75+
public void SetsEnlistTrueOnConnectionStringIfEnlistTransactionTrue()
7676
{
7777
// Arrange
78-
var sut = new SqlConnectionFactory(DatabaseFixture.LogEventsConnectionString, false, false,
78+
var sut = new SqlConnectionFactory(DatabaseFixture.LogEventsConnectionString, true, false,
7979
_sqlConnectionStringBuilderWrapperMock.Object, _azureManagedServiceAuthenticatorMock.Object);
8080

8181
// Act
@@ -84,7 +84,7 @@ public void DoesNotSetEnlistOnConnectionStringIfPreventEnlistTransactionFalse()
8484

8585
// Assert
8686
_sqlConnectionStringBuilderWrapperMock.VerifySet(c => c.ConnectionString = DatabaseFixture.LogEventsConnectionString);
87-
_sqlConnectionStringBuilderWrapperMock.VerifySet(c => c.Enlist = false, Times.Never);
87+
_sqlConnectionStringBuilderWrapperMock.VerifySet(c => c.Enlist = true);
8888
}
8989

9090
[Fact]

0 commit comments

Comments
 (0)