Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented #542: Column option ResolveHierarchicalPropertyName to force non-hierarchical handling #599

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# 8.1.0
* Implemented #542: Column option `ResolveHierarchicalPropertyName` to force non-hierarchical handling

# 8.0.1
* Refactoring and performance optimizations in batched and audit sink
* Create perftest result on release
Expand Down
9 changes: 7 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ Each Standard Column in the `ColumnOptions.Store` list and any custom columns yo

* `ColumnName`
* `PropertyName`
* `ResolveHierarchicalPropertyName`
* `DataType`
* `AllowNull`
* `DataLength`
Expand All @@ -352,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 @@ -385,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,7 +2,7 @@

<PropertyGroup>
<Description>A Serilog sink that writes events to Microsoft SQL Server and Azure SQL</Description>
<VersionPrefix>8.0.1</VersionPrefix>
<VersionPrefix>8.1.0</VersionPrefix>
<EnablePackageValidation>true</EnablePackageValidation>
<PackageValidationBaselineVersion>8.0.0</PackageValidationBaselineVersion>
<Authors>Michiel van Oudheusden;Christian Kadluba;Serilog Contributors</Authors>
Expand Down
30 changes: 25 additions & 5 deletions src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class SqlColumn
private SqlDbType _dataType = SqlDbType.VarChar; // backwards-compatibility default
private string _columnName = string.Empty;
private string _propertyName;
private bool _resolveHierarchicalPropertyName = true;
private readonly List<string> _propertyNameHierarchy = new List<string>();

/// <summary>
Expand Down Expand Up @@ -109,7 +110,20 @@ public string PropertyName
set
{
_propertyName = value;
ParseHierarchicalPropertyName(value);
ParseHierarchicalPropertyName();
}
}

/// <summary>
/// Controls whether hierarchical expressions in `PropertyName` are evaluated. The default is `true`.
/// </summary>
public bool ResolveHierarchicalPropertyName
{
get => _resolveHierarchicalPropertyName;
set
{
_resolveHierarchicalPropertyName = value;
ParseHierarchicalPropertyName();
}
}

Expand All @@ -130,8 +144,6 @@ internal bool HasHierarchicalPropertyName
// and allows casting back to the Standard Column without a lot of switch gymnastics.
internal StandardColumn? StandardColumnIdentifier { get; set; }

internal Type StandardColumnType { get; set; }

/// <summary>
/// Converts a SQL sink SqlColumn object to a System.Data.DataColumn object. The original
/// SqlColumn object is stored in the DataColumn's ExtendedProperties collection.
Expand Down Expand Up @@ -170,9 +182,17 @@ internal void SetDataTypeFromConfigString(string requestedSqlType)
DataType = sqlType;
}

private void ParseHierarchicalPropertyName(string propertyName)
private void ParseHierarchicalPropertyName()
{
_propertyNameHierarchy.AddRange(propertyName.Split('.'));
_propertyNameHierarchy.Clear();
if (ResolveHierarchicalPropertyName)
{
_propertyNameHierarchy.AddRange(PropertyName.Split('.'));
}
else
{
_propertyNameHierarchy.Add(PropertyName);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Data;
using Moq;
using System.Linq;
using FluentAssertions;
using Serilog.Configuration;
using Serilog.Sinks.MSSqlServer.Configuration;
using Serilog.Sinks.MSSqlServer.Tests.TestUtils;
Expand Down Expand Up @@ -28,7 +29,7 @@ public void ConfigureColumnOptionsReadsTraceIdColumnOptions()
_configurationSection.TraceId.AllowNull = "false";
_configurationSection.TraceId.DataType = "22"; // VarChar
var columnOptions = new MSSqlServer.ColumnOptions();

// Act
_sut.ConfigureColumnOptions(_configurationSection, columnOptions);

Expand Down Expand Up @@ -56,5 +57,48 @@ public void ConfigureColumnOptionsReadsSpanIdColumnOptions()
Assert.False(columnOptions.SpanId.AllowNull);
Assert.Equal(SqlDbType.VarChar, columnOptions.SpanId.DataType);
}

[Fact]
public void ConfigureColumnOptionsReadsAdditionalColumnsResolveHierarchicalPropertyName()
{
// Arrange
const string columnName = "AdditionalColumn1";
var columnConfig = new ColumnConfig
{
ColumnName = columnName,
ResolveHierarchicalPropertyName = "false"
};
_configurationSection.Columns.Add(columnConfig);
var columnOptions = new MSSqlServer.ColumnOptions();

// Act
_sut.ConfigureColumnOptions(_configurationSection, columnOptions);

// Assert
var additionalColumn1 = columnOptions.AdditionalColumns.SingleOrDefault(c => c.ColumnName == columnName);
additionalColumn1.Should().NotBeNull();
additionalColumn1.ResolveHierarchicalPropertyName.Should().Be(false);
}

[Fact]
public void ConfigureColumnOptionsDefaultsAdditionalColumnsResolveHierarchicalPropertyName()
{
// Arrange
const string columnName = "AdditionalColumn1";
var columnConfig = new ColumnConfig
{
ColumnName = columnName
};
_configurationSection.Columns.Add(columnConfig);
var columnOptions = new MSSqlServer.ColumnOptions();

// Act
_sut.ConfigureColumnOptions(_configurationSection, columnOptions);

// Assert
var additionalColumn1 = columnOptions.AdditionalColumns.SingleOrDefault(c => c.ColumnName == columnName);
additionalColumn1.Should().NotBeNull();
additionalColumn1.ResolveHierarchicalPropertyName.Should().Be(true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -239,5 +239,63 @@ public void WritesLogEventWithColumnsFromHierarchicalNamedProperties()
VerifyStringColumnWritten(additionalColumn1Name, property1Value);
VerifyIntegerColumnWritten(additionalColumn2Name, property2Value);
}

[Fact]
public void WritesLogEventIgnoringHierarchicalNamedPropertiesWhenResolveHierarchicalPropertyNameIsFalse()
{
// Arrange
const string additionalColumn1Name = "AdditionalColumn1";
const string additionalProperty1Name = "AdditionalProperty1.SubProperty1";
const string additionalColumn2Name = "AdditionalColumn2";
const string additionalProperty2Name = "AdditionalProperty2.SubProperty2.SubSubProperty1";
var columnOptions = new MSSqlServer.ColumnOptions
{
AdditionalColumns = new List<SqlColumn>
{
new SqlColumn
{
ColumnName = additionalColumn1Name,
PropertyName = additionalProperty1Name,
ResolveHierarchicalPropertyName = false,
DataType = SqlDbType.NVarChar,
AllowNull = true,
DataLength = 100
},
new SqlColumn
{
ColumnName = additionalColumn2Name,
PropertyName = additionalProperty2Name,
ResolveHierarchicalPropertyName = false,
DataType = SqlDbType.Int,
AllowNull = true
}
}
};
var property1Value = "PropertyValue1";
var property2Value = 2;

// Act
Log.Logger = new LoggerConfiguration()
.WriteTo.MSSqlServer(
DatabaseFixture.LogEventsConnectionString,
sinkOptions: new MSSqlServerSinkOptions
{
TableName = DatabaseFixture.LogTableName,
AutoCreateSqlTable = true
},
columnOptions: columnOptions,
formatProvider: CultureInfo.InvariantCulture)
.CreateLogger();
// Log event properties with names containing dots can be created using ForContext()
Log.ForContext(additionalProperty1Name, property1Value)
.ForContext(additionalProperty2Name, property2Value)
.Information("Hello {@AdditionalProperty1.SubProperty1} from thread {@AdditionalProperty2.SubProperty2.SubSubProperty1}");
Log.CloseAndFlush();

// Assert
VerifyDatabaseColumnsWereCreated(columnOptions.AdditionalColumns);
VerifyStringColumnWritten(additionalColumn1Name, property1Value);
VerifyIntegerColumnWritten(additionalColumn2Name, property2Value);
}
}
}
Loading
Loading