Skip to content

Commit

Permalink
Merge pull request #599 from ckadluba/issue-542-disable-hierarchical-…
Browse files Browse the repository at this point in the history
…props

Implemented #542: Column option ResolveHierarchicalPropertyName to force non-hierarchical handling
  • Loading branch information
ckadluba authored Nov 20, 2024
2 parents 71ca19a + 458b867 commit 1ad392b
Show file tree
Hide file tree
Showing 12 changed files with 243 additions and 25 deletions.
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

0 comments on commit 1ad392b

Please sign in to comment.