Skip to content

Commit

Permalink
Merge pull request #604 from serilog-mssql/dev
Browse files Browse the repository at this point in the history
* Implemented #542: Column option ResolveHierarchicalPropertyName to force non-hierarchical handling
* Removed unnecessary exception handlers and let Serilog Core do the SelfLog()
* 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
  • Loading branch information
ckadluba authored Dec 6, 2024
2 parents 2d9f79d + f7fb888 commit 441f3c5
Show file tree
Hide file tree
Showing 43 changed files with 799 additions and 368 deletions.
14 changes: 13 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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

9 changes: 5 additions & 4 deletions .github/ISSUE_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
14 changes: 9 additions & 5 deletions Build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,17 @@ try
{
Push-Location "$testProjectPath"

echo "build: Testing project in $testProjectPath"
& dotnet test -c Release --collect "XPlat Code Coverage"
if ($LASTEXITCODE -ne 0)
# Run tests for different targets in sequence to avoid database tests
# to fail because of concurrency problems
foreach ($tfm in @( "net462", "net472", "net8.0" ))
{
exit 2
echo "build: Testing project in $testProjectPath for target $tfm"
& dotnet test -c Release --collect "XPlat Code Coverage" --framework "$tfm" --results-directory "./TestResults/$tfm"
if ($LASTEXITCODE -ne 0)
{
exit 2
}
}

}
finally
{
Expand Down
9 changes: 9 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
# 8.1.0
* Implemented #542: Column option `ResolveHierarchicalPropertyName` to force non-hierarchical handling
* Removed unnecessary exception handlers and let Serilog Core do the SelfLog()
* 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
Expand Down
11 changes: 9 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):

```
Expand Down Expand Up @@ -337,6 +339,7 @@ Each Standard Column in the `ColumnOptions.Store` list and any custom columns yo

* `ColumnName`
* `PropertyName`
* `ResolveHierarchicalPropertyName`
* `DataType`
* `AllowNull`
* `DataLength`
Expand All @@ -350,7 +353,11 @@ Any valid SQL column name can be used. Standard Columns have default names assig

The optional name of a Serilog property to use as the value for a custom column. If not provided, the property used is the one that has the same name as the specified ColumnName. It applies only to custom columns defined in `AdditionalColumns` and is ignored for standard columns.

PropertyName can contain a simple property name like `SomeProperty` but it can also be used to hierachically reference sub-properties with expressions like `SomeProperty.SomeSubProperty.SomeThirdLevelProperty`. This can be used to easily bind additional columns to specific sub-properties following the paradigm of structured logging. Please be aware that collections are not supported. This means expressions like `SomeProperty.SomeArray[2]` will not work.
PropertyName can contain a simple property name like `SomeProperty` but it can also be used to hierarchically reference sub-properties with expressions like `SomeProperty.SomeSubProperty.SomeThirdLevelProperty`. This can be used to easily bind additional columns to specific sub-properties following the paradigm of structured logging. Please be aware that collections are not supported. This means expressions like `SomeProperty.SomeArray[2]` will not work. Hierarchical property resolution can be disabled using `ResolveHierarchicalPropertyName` in case you need property names containing dots which should not be treated as hierarchical.

### ResolveHierarchicalPropertyName

Controls whether hierarchical sub-property expressions in `PropertyName` are evaluated (see above). The default is `true`. If set to `false` any value is treated as a simple property name and no hierarchical sub-property binding is done.

### DataType

Expand Down Expand Up @@ -383,7 +390,7 @@ Numeric types use the default precision and scale. For numeric types, you are re

### AllowNull

Determines whether or not the column can store SQL `NULL` values. The default is true. Some of the other features like `PrimaryKey` have related restrictions, and some of the Standard Columns impose restrictions (for example, the `Id` column never allows nulls).
Determines whether the column can store SQL `NULL` values. The default is `true`. Some of the other features like `PrimaryKey` have related restrictions, and some of the Standard Columns impose restrictions (for example, the `Id` column never allows nulls).

### DataLength

Expand Down
6 changes: 6 additions & 0 deletions sample/WorkerServiceDemo/Worker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
};
_logger.LogInformation("{@Structured} {@Scalar}", structured, "Scalar Value");


// Logging a property with dots in its name to AdditionalColumn3
// but treat it as unstructured according to configuration in AdditionalColumns in appsettings.json
_logger.LogInformation("Non-structured property with dot-name to AdditionalColumn3 {@NonstructuredProperty.WithNameContainingDots.Name}",
new Random().Next().ToString());

while (!stoppingToken.IsCancellationRequested)
{
_logger.LogInformation("Worker running at: {time}. CustomProperty1: {CustomProperty1}",
Expand Down
21 changes: 19 additions & 2 deletions sample/WorkerServiceDemo/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"columnName": "Timestamp",
"convertToUtc": false
},
"additionalColumns": [
"customColumns": [
{
"columnName": "AdditionalColumn1",
"propertyName": "CustomProperty1",
Expand All @@ -37,6 +37,12 @@
"columnName": "AdditionalColumn2",
"propertyName": "Structured.Name",
"dataType": "12"
},
{
"columnName": "AdditionalColumn3",
"propertyName": "NonstructuredProperty.WithNameContainingDots.Name",
"resolveHierarchicalPropertyName": false,
"dataType": "12"
}
]
},
Expand All @@ -63,11 +69,22 @@
"columnName": "Timestamp",
"convertToUtc": false
},
"additionalColumns": [
"customColumns": [
{
"columnName": "AdditionalColumn1",
"propertyName": "CustomProperty1",
"dataType": "12"
},
{
"columnName": "AdditionalColumn2",
"propertyName": "Structured.Name",
"dataType": "12"
},
{
"columnName": "AdditionalColumn3",
"propertyName": "NonstructuredProperty.WithNameContainingDots.Name",
"resolveHierarchicalPropertyName": false,
"dataType": "12"
}
]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright 2015 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.
Expand All @@ -30,6 +30,12 @@ protected override object GetElementKey(ConfigurationElement element)
{
return ((ColumnConfig)element)?.ColumnName;
}

// For testing
internal void Add(ColumnConfig config)
{
BaseAdd(config);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Copyright 2015 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.
Expand Down Expand Up @@ -48,6 +48,13 @@ public virtual string PropertyName
set { this[nameof(PropertyName)] = value; }
}

[ConfigurationProperty("ResolveHierarchicalPropertyName")]
public virtual string ResolveHierarchicalPropertyName
{
get { return (string)this[nameof(ResolveHierarchicalPropertyName)]; }
set { this[nameof(ResolveHierarchicalPropertyName)] = value; }
}

[ConfigurationProperty("DataType")]
public string DataType
{
Expand Down Expand Up @@ -85,6 +92,8 @@ internal SqlColumn AsSqlColumn()

SetProperty.IfProvidedNotEmpty<string>(this, nameof(PropertyName), (val) => sqlColumn.PropertyName = val);

SetProperty.IfProvided<bool>(this, nameof(ResolveHierarchicalPropertyName), (val) => sqlColumn.ResolveHierarchicalPropertyName = val);

SetProperty.IfProvidedNotEmpty<string>(this, nameof(DataType), (val) => sqlColumn.SetDataTypeFromConfigString(val));

SetProperty.IfProvided<int>(this, nameof(DataLength), (val) => sqlColumn.DataLength = val);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

<PropertyGroup>
<Description>A Serilog sink that writes events to Microsoft SQL Server and Azure SQL</Description>
<VersionPrefix>8.0.0</VersionPrefix>
<EnablePackageValidation>false</EnablePackageValidation>
<PackageValidationBaselineVersion>7.0.0</PackageValidationBaselineVersion>
<VersionPrefix>8.1.0</VersionPrefix>
<EnablePackageValidation>true</EnablePackageValidation>
<PackageValidationBaselineVersion>8.0.0</PackageValidationBaselineVersion>
<Authors>Michiel van Oudheusden;Christian Kadluba;Serilog Contributors</Authors>
<TargetFrameworks>netstandard2.0;net462;net472;net8.0</TargetFrameworks>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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>
Expand Down Expand Up @@ -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)
Expand All @@ -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!");
Expand Down
Loading

0 comments on commit 441f3c5

Please sign in to comment.