Merge pull request #490 from nhart12/dev

ckadluba · web-flow · commit 480d36b485f3 · 2023-11-20T13:15:32.000Z
Truncate additional columns, allow varchar on standard columns
diff --git a/README.md b/README.md
@@ -512,7 +512,7 @@ The content of this column is rendered as JSON by default or with a custom IText
 
 ## Custom Property Columns
 
-By default, any log event properties you include in your log statements will be saved to the XML `Properties` column or the JSON `LogEvent` column. But they can also be stored in their own individual columns via the `AdditionalColumns` collection. This adds overhead to write operations but is very useful for frequently-queried properties. Only `ColumnName` is required; the default configuration is `varchar(max)`.
+By default, any log event properties you include in your log statements will be saved to the XML `Properties` column or the JSON `LogEvent` column. But they can also be stored in their own individual columns via the `AdditionalColumns` collection. This adds overhead to write operations but is very useful for frequently-queried properties. Only `ColumnName` is required; the default configuration is `varchar(max)`. If you specify a DataLength on a column of character data types (NVarChar, VarChar, Char, NChar) the string will be automatically truncated to the datalength to fit in the column.
 
 ```csharp
 var columnOptions = new ColumnOptions
diff --git a/src/Serilog.Sinks.MSSqlServer/Extensions/StringExtensions.cs b/src/Serilog.Sinks.MSSqlServer/Extensions/StringExtensions.cs
@@ -4,6 +4,11 @@ namespace Serilog.Sinks.MSSqlServer.Extensions
 {
     internal static class StringExtensions
     {
+        public static string TruncateOutput(this string value, int dataLength) =>
+            dataLength < 0
+                ? value     // No need to truncate if length set to maximum
+                : value.Truncate(dataLength, "...");
+
         public static string Truncate(this string value, int maxLength, string suffix)
         {
             if (value == null) return null;
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/ExceptionColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/ExceptionColumnOptions.cs
@@ -27,8 +27,8 @@ public ExceptionColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (value != SqlDbType.NVarChar)
-                        throw new ArgumentException("The Standard Column \"Exception\" must be NVarChar.");
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"Exception\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LevelColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LevelColumnOptions.cs
@@ -28,8 +28,8 @@ public LevelColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (value != SqlDbType.NVarChar && value != SqlDbType.TinyInt)
-                        throw new ArgumentException("The Standard Column \"Level\" must be of data type NVarChar or TinyInt.");
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value) && value != SqlDbType.TinyInt)
+                        throw new ArgumentException("The Standard Column \"Level\" must be of data type NVarChar, VarChar or TinyInt.");
                     base.DataType = value;
                 }
             }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LogEventColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/LogEventColumnOptions.cs
@@ -27,8 +27,8 @@ public LogEventColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (value != SqlDbType.NVarChar)
-                        throw new ArgumentException("The Standard Column \"LogEvent\" must be NVarChar.");
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"LogEvent\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageColumnOptions.cs
@@ -27,8 +27,8 @@ public MessageColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (value != SqlDbType.NVarChar)
-                        throw new ArgumentException("The Standard Column \"Message\" must be NVarChar.");
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"Message\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageTemplateColumnOptions.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/ColumnOptions/MessageTemplateColumnOptions.cs
@@ -27,8 +27,8 @@ public MessageTemplateColumnOptions() : base()
                 get => base.DataType;
                 set
                 {
-                    if (value != SqlDbType.NVarChar)
-                        throw new ArgumentException("The Standard Column \"MessageTemplate\" must be NVarChar.");
+                    if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"MessageTemplate\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/AdditionalColumnDataGenerator.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/AdditionalColumnDataGenerator.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.ComponentModel;
 using Serilog.Events;
+using Serilog.Sinks.MSSqlServer.Extensions;
 
 namespace Serilog.Sinks.MSSqlServer.Output
 {
@@ -45,6 +46,11 @@ public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn ad
             var columnType = additionalColumn.AsDataColumn().DataType;
             if (columnType.IsAssignableFrom(scalarValue.Value.GetType()))
             {
+                if (SqlDataTypes.DataLengthRequired.Contains(additionalColumn.DataType))
+                {
+                    return new KeyValuePair<string, object>(columnName, scalarValue.Value.ToString().TruncateOutput(additionalColumn.DataLength));
+
+                }
                 return new KeyValuePair<string, object>(columnName, scalarValue.Value);
             }
 
@@ -54,6 +60,9 @@ public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn ad
             }
             else
             {
+                if (additionalColumn.AllowNull) {
+                    return new KeyValuePair<string, object>(columnName, DBNull.Value);
+                }
                 return new KeyValuePair<string, object>(columnName, property.Value.ToString());
             }
         }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/StandardColumnDataGenerator.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/StandardColumnDataGenerator.cs
@@ -45,15 +45,15 @@ public KeyValuePair<string, object> GetStandardColumnNameAndValue(StandardColumn
             switch (column)
             {
                 case StandardColumn.Message:
-                    return new KeyValuePair<string, object>(_columnOptions.Message.ColumnName, TruncateOutput(logEvent.RenderMessage(_formatProvider), _columnOptions.Message.DataLength));
+                    return new KeyValuePair<string, object>(_columnOptions.Message.ColumnName, logEvent.RenderMessage(_formatProvider).TruncateOutput(_columnOptions.Message.DataLength));
                 case StandardColumn.MessageTemplate:
-                    return new KeyValuePair<string, object>(_columnOptions.MessageTemplate.ColumnName, TruncateOutput(logEvent.MessageTemplate.Text, _columnOptions.MessageTemplate.DataLength));
+                    return new KeyValuePair<string, object>(_columnOptions.MessageTemplate.ColumnName, logEvent.MessageTemplate.Text.TruncateOutput(_columnOptions.MessageTemplate.DataLength));
                 case StandardColumn.Level:
                     return new KeyValuePair<string, object>(_columnOptions.Level.ColumnName, _columnOptions.Level.StoreAsEnum ? (object)logEvent.Level : logEvent.Level.ToString());
                 case StandardColumn.TimeStamp:
                     return GetTimeStampStandardColumnNameAndValue(logEvent);
                 case StandardColumn.Exception:
-                    return new KeyValuePair<string, object>(_columnOptions.Exception.ColumnName, TruncateOutput(logEvent.Exception?.ToString(), _columnOptions.Exception.DataLength));
+                    return new KeyValuePair<string, object>(_columnOptions.Exception.ColumnName, logEvent.Exception?.ToString().TruncateOutput(_columnOptions.Exception.DataLength));
                 case StandardColumn.Properties:
                     return new KeyValuePair<string, object>(_columnOptions.Properties.ColumnName, ConvertPropertiesToXmlStructure(logEvent.Properties));
                 case StandardColumn.LogEvent:
@@ -63,11 +63,6 @@ public KeyValuePair<string, object> GetStandardColumnNameAndValue(StandardColumn
             }
         }
 
-        private static string TruncateOutput(string value, int dataLength) =>
-            dataLength < 0
-                ? value     // No need to truncate if length set to maximum
-                : value.Truncate(dataLength, "...");
-
         private KeyValuePair<string, object> GetTimeStampStandardColumnNameAndValue(LogEvent logEvent)
         {
             var dateTimeOffset = _columnOptions.TimeStamp.ConvertToUtc ? logEvent.Timestamp.ToUniversalTime() : logEvent.Timestamp;
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlDataTypes.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlDataTypes.cs
@@ -1,5 +1,6 @@
 ﻿using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Data;
 
 namespace Serilog.Sinks.MSSqlServer
@@ -51,6 +52,14 @@ public static class SqlDataTypes
             // not supported by enum: numeric, FILESTREAM, rowversion
         };
 
+        /// <summary>
+        /// SQL column types for supported strings
+        /// </summary>
+        public static readonly ReadOnlyCollection<SqlDbType> VariableCharacterColumnTypes = new ReadOnlyCollection<SqlDbType>(new List<SqlDbType> {
+            SqlDbType.NVarChar,
+            SqlDbType.VarChar
+        });
+
         /// <summary>
         /// The SQL column types which require a non-zero DataLength property.
         /// </summary>
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Misc/SqlTypesTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Misc/SqlTypesTests.cs
@@ -108,9 +108,12 @@ public void AuditLogCharacterDataSqlTypes()
             Log.Information("NVarChar {NVarChar}", twentyChars);
             Log.Information("VarChar {VarChar}", twentyChars);
 
-            // should throw truncation exception
+            // should truncate but not throw
 
-            Assert.Throws<AggregateException>(() => Log.Information("Char {Char}", thirtyChars));
+            Log.Information("Char {Char}", thirtyChars);
+            Log.Information("NChar {NChar}", thirtyChars);
+            Log.Information("NVarChar {NVarChar}", thirtyChars);
+            Log.Information("VarChar {VarChar}", thirtyChars);
 
             Log.CloseAndFlush();
         }
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Output/AdditionalColumnDataGeneratorTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Output/AdditionalColumnDataGeneratorTests.cs
@@ -114,7 +114,7 @@ public void GetAdditionalColumnNameAndValueReturnsNullForNotFoundHierachicalProp
         }
 
         [Fact]
-        public void GetAdditionalColumnNameAndValueConvertsValueTypeToStringIfConversionToColumnTypeFails()
+        public void GetAdditionalColumnNameAndValueUsesNullIfConversionToColumnTypeFails()
         {
             // Arrange
             const string columnName = "AdditionalProperty1";
@@ -131,8 +131,8 @@ public void GetAdditionalColumnNameAndValueConvertsValueTypeToStringIfConversion
             // Assert
             _columnSimplePropertyValueResolver.Verify(r => r.GetPropertyValueForColumn(additionalColumn, properties), Times.Once);
             Assert.Equal(columnName, result.Key);
-            Assert.IsType<string>(result.Value);
-            Assert.Equal("1", result.Value);  // Cannot convert int to SqlDbType.DateTimeOffset so returns string
+            Assert.IsType<DBNull>(result.Value);
+            Assert.Equal(DBNull.Value, result.Value);  // Cannot convert int to SqlDbType.DateTimeOffset so returns null
         }
 
         [Fact]
@@ -180,5 +180,27 @@ public void GetAdditionalColumnNameAndValueConvertsNullValueForNullable()
             Assert.IsType<DBNull>(result.Value);
             Assert.Equal(DBNull.Value, result.Value);
         }
+
+        [Fact]
+        public void GetAdditionalColumnNameAndValueReturnsTruncatedForCharacterTypesWithDataLength()
+        {
+            // Arrange
+            const string columnName = "AdditionalProperty1";
+            const string propertyValue = "Additional Property Value";
+            var additionalColumn = new SqlColumn(columnName, SqlDbType.NVarChar);
+            additionalColumn.DataLength = 10;
+            var properties = new Dictionary<string, LogEventPropertyValue>();
+            _columnSimplePropertyValueResolver.Setup(r => r.GetPropertyValueForColumn(
+                It.IsAny<SqlColumn>(), It.IsAny<IReadOnlyDictionary<string, LogEventPropertyValue>>()))
+                .Returns(new KeyValuePair<string, LogEventPropertyValue>(columnName, new ScalarValue(propertyValue)));
+
+            // Act
+            var result = _sut.GetAdditionalColumnNameAndValue(additionalColumn, properties);
+
+            // Assert
+            _columnSimplePropertyValueResolver.Verify(r => r.GetPropertyValueForColumn(additionalColumn, properties), Times.Once);
+            Assert.Equal(columnName, result.Key);
+            Assert.Equal("Additio...", result.Value);
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,11 @@ namespace Serilog.Sinks.MSSqlServer.Extensions`
`4`	`4`	`{`
`5`	`5`	`internal static class StringExtensions`
`6`	`6`	`{`
	`7`	`+ public static string TruncateOutput(this string value, int dataLength) =>`
	`8`	`+ dataLength < 0`
	`9`	`+ ? value // No need to truncate if length set to maximum`
	`10`	`+ : value.Truncate(dataLength, "...");`
	`11`	`+`
`7`	`12`	`public static string Truncate(this string value, int maxLength, string suffix)`
`8`	`13`	`{`
`9`	`14`	`if (value == null) return null;`
Original file line number	Diff line number	Diff line change
`@@ -27,8 +27,8 @@ public ExceptionColumnOptions() : base()`
`27`	`27`	`get => base.DataType;`
`28`	`28`	`set`
`29`	`29`	`{`
`30`		`- if (value != SqlDbType.NVarChar)`
`31`		`- throw new ArgumentException("The Standard Column \"Exception\" must be NVarChar.");`
	`30`	`+ if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))`
	`31`	`+ throw new ArgumentException("The Standard Column \"Exception\" must be NVarChar or VarChar.");`
`32`	`32`	`base.DataType = value;`
`33`	`33`	`}`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,8 +28,8 @@ public LevelColumnOptions() : base()`
`28`	`28`	`get => base.DataType;`
`29`	`29`	`set`
`30`	`30`	`{`
`31`		`- if (value != SqlDbType.NVarChar && value != SqlDbType.TinyInt)`
`32`		`- throw new ArgumentException("The Standard Column \"Level\" must be of data type NVarChar or TinyInt.");`
	`31`	`+ if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value) && value != SqlDbType.TinyInt)`
	`32`	`+ throw new ArgumentException("The Standard Column \"Level\" must be of data type NVarChar, VarChar or TinyInt.");`
`33`	`33`	`base.DataType = value;`
`34`	`34`	`}`
`35`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,8 +27,8 @@ public LogEventColumnOptions() : base()`
`27`	`27`	`get => base.DataType;`
`28`	`28`	`set`
`29`	`29`	`{`
`30`		`- if (value != SqlDbType.NVarChar)`
`31`		`- throw new ArgumentException("The Standard Column \"LogEvent\" must be NVarChar.");`
	`30`	`+ if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))`
	`31`	`+ throw new ArgumentException("The Standard Column \"LogEvent\" must be NVarChar or VarChar.");`
`32`	`32`	`base.DataType = value;`
`33`	`33`	`}`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,8 +27,8 @@ public MessageColumnOptions() : base()`
`27`	`27`	`get => base.DataType;`
`28`	`28`	`set`
`29`	`29`	`{`
`30`		`- if (value != SqlDbType.NVarChar)`
`31`		`- throw new ArgumentException("The Standard Column \"Message\" must be NVarChar.");`
	`30`	`+ if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))`
	`31`	`+ throw new ArgumentException("The Standard Column \"Message\" must be NVarChar or VarChar.");`
`32`	`32`	`base.DataType = value;`
`33`	`33`	`}`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,8 +27,8 @@ public MessageTemplateColumnOptions() : base()`
`27`	`27`	`get => base.DataType;`
`28`	`28`	`set`
`29`	`29`	`{`
`30`		`- if (value != SqlDbType.NVarChar)`
`31`		`- throw new ArgumentException("The Standard Column \"MessageTemplate\" must be NVarChar.");`
	`30`	`+ if (!SqlDataTypes.VariableCharacterColumnTypes.Contains(value))`
	`31`	`+ throw new ArgumentException("The Standard Column \"MessageTemplate\" must be NVarChar or VarChar.");`
`32`	`32`	`base.DataType = value;`
`33`	`33`	`}`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@`
`2`	`2`	`using System.Collections.Generic;`
`3`	`3`	`using System.ComponentModel;`
`4`	`4`	`using Serilog.Events;`
	`5`	`+using Serilog.Sinks.MSSqlServer.Extensions;`
`5`	`6`
`6`	`7`	`namespace Serilog.Sinks.MSSqlServer.Output`
`7`	`8`	`{`
`@@ -45,6 +46,11 @@ public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn ad`
`45`	`46`	`var columnType = additionalColumn.AsDataColumn().DataType;`
`46`	`47`	`if (columnType.IsAssignableFrom(scalarValue.Value.GetType()))`
`47`	`48`	`{`
	`49`	`+ if (SqlDataTypes.DataLengthRequired.Contains(additionalColumn.DataType))`
	`50`	`+ {`
	`51`	`+ return new KeyValuePair<string, object>(columnName, scalarValue.Value.ToString().TruncateOutput(additionalColumn.DataLength));`
	`52`	`+`
	`53`	`+ }`
`48`	`54`	`return new KeyValuePair<string, object>(columnName, scalarValue.Value);`
`49`	`55`	`}`
`50`	`56`
`@@ -54,6 +60,9 @@ public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn ad`
`54`	`60`	`}`
`55`	`61`	`else`
`56`	`62`	`{`
	`63`	`+ if (additionalColumn.AllowNull) {`
	`64`	`+ return new KeyValuePair<string, object>(columnName, DBNull.Value);`
	`65`	`+ }`
`57`	`66`	`return new KeyValuePair<string, object>(columnName, property.Value.ToString());`
`58`	`67`	`}`
`59`	`68`	`}`