Truncate additional columns, allow varchar on standard columns

Author: nhart12

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.VariableCharactorColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"Exception\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }

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.VariableCharactorColumnTypes.Contains(value) && value != SqlDbType.TinyInt)
+                        throw new ArgumentException("The Standard Column \"Level\" must be of data type NVarChar, VarChar or TinyInt.");
                     base.DataType = value;
                 }
             }

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.VariableCharactorColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"LogEvent\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }

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.VariableCharactorColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"Message\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }

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.VariableCharactorColumnTypes.Contains(value))
+                        throw new ArgumentException("The Standard Column \"MessageTemplate\" must be NVarChar or VarChar.");
                     base.DataType = value;
                 }
             }

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, TruncateOutput((string)scalarValue.Value, additionalColumn.DataLength));
+
+                }
                 return new KeyValuePair<string, object>(columnName, scalarValue.Value);
             }
 
@@ -54,10 +60,15 @@ public KeyValuePair<string, object> GetAdditionalColumnNameAndValue(SqlColumn ad
             }
             else
             {
-                return new KeyValuePair<string, object>(columnName, property.Value.ToString());
+                return new KeyValuePair<string, object>(columnName, DBNull.Value);
             }
         }
 
+        private static string TruncateOutput(string value, int dataLength) =>
+            dataLength < 0
+                ? value     // No need to truncate if length set to maximum
+                : value.Truncate(dataLength, string.Empty);
+
         private static bool TryChangeType(object obj, Type type, out object conversion)
         {
             conversion = null;

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/SqlDataTypes.cs

@@ -51,6 +51,15 @@ public static class SqlDataTypes
             // not supported by enum: numeric, FILESTREAM, rowversion
         };
 
+        /// <summary>
+        /// SQL column types for supported strings
+        /// </summary>
+        public static readonly HashSet<SqlDbType> VariableCharactorColumnTypes = new HashSet<SqlDbType>
+        {
+            SqlDbType.NVarChar,
+            SqlDbType.VarChar
+        };
+
         /// <summary>
         /// The SQL column types which require a non-zero DataLength property.
         /// </summary>

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();
         }

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("Additional", result.Value);
+        }
     }
 }