From e9c5ce5da763354669a3cff507e58e66d9b4afc1 Mon Sep 17 00:00:00 2001
From: Rolf Kristensen <sweaty1@hotmail.com>
Date: Sat, 10 Sep 2022 16:55:37 +0200
Subject: [PATCH] Elastic.CommonSchema.NLog - MetadataDictionary with safe
 values

---
 src/Elastic.CommonSchema.NLog/EcsLayout.cs    | 36 ++++++++++++++-
 .../MessageTests.cs                           | 44 +++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/src/Elastic.CommonSchema.NLog/EcsLayout.cs b/src/Elastic.CommonSchema.NLog/EcsLayout.cs
index a50d3063..da12b754 100644
--- a/src/Elastic.CommonSchema.NLog/EcsLayout.cs
+++ b/src/Elastic.CommonSchema.NLog/EcsLayout.cs
@@ -303,13 +303,31 @@ private MetadataDictionary GetMetadata(LogEventInfo e)
 
 			if (IncludeEventProperties && e.HasProperties)
 			{
+				global::NLog.MessageTemplates.MessageTemplateParameters templateParameters = null; 
+
 				foreach (var prop in e.Properties)
 				{
 					var propertyName = prop.Key?.ToString();
 					if (string.IsNullOrEmpty(propertyName) || ExcludeProperties.Contains(propertyName))
 						continue;
 
-					Populate(metadata, propertyName, prop.Value);
+					var propertyValue = prop.Value;
+					if (propertyValue is null || propertyValue is IConvertible || propertyValue.GetType().IsValueType)
+					{
+						Populate(metadata, propertyName, propertyValue);
+					}
+					else
+					{
+						templateParameters = templateParameters ?? e.MessageTemplateParameters;
+						if (AllowSerializePropertyValue(propertyName, templateParameters))
+						{
+							Populate(metadata, propertyName, propertyValue);
+						}
+						else
+						{
+							Populate(metadata, propertyName, propertyValue?.ToString());
+						}
+					}
 				}
 			}
 
@@ -340,6 +358,22 @@ private MetadataDictionary GetMetadata(LogEventInfo e)
 				: null;
 		}
 
+		private bool AllowSerializePropertyValue(string propertyName, global::NLog.MessageTemplates.MessageTemplateParameters templateParameters)
+		{
+			if (templateParameters.Count > 0)
+			{
+				// System.Text.Json is very fragile, and can only handle safe objects
+				// Microsoft ASP.NET often uses message-templates for logging unsafe objects
+				foreach (var messageProperty in templateParameters)
+				{
+					if (propertyName == messageProperty.Name)
+						return messageProperty.CaptureType == global::NLog.MessageTemplates.CaptureType.Serialize;
+				}
+			}
+
+			return true;	// Not from Message-Template, then probably safe
+		}
+
 		private Log GetLog(LogEventInfo logEventInfo)
 		{
 			var logOriginMethod = LogOriginCallSiteMethod?.Render(logEventInfo);
diff --git a/tests/Elastic.CommonSchema.NLog.Tests/MessageTests.cs b/tests/Elastic.CommonSchema.NLog.Tests/MessageTests.cs
index e9755b3e..9ed1db02 100644
--- a/tests/Elastic.CommonSchema.NLog.Tests/MessageTests.cs
+++ b/tests/Elastic.CommonSchema.NLog.Tests/MessageTests.cs
@@ -49,6 +49,50 @@ public void SeesMessageWithProp() => TestLogger((logger, getLogEvents) =>
 			y.Should().HaveValue().And.Be(2.2);
 		});
 
+		[Fact]
+		public void SeesMessageWithSafeProp() => TestLogger((logger, getLogEvents) =>
+		{
+			logger.Info("Info {@SafeValue}", new NiceObject() { ValueX = "X", SomeY = 2.2 });
+
+			var logEvents = getLogEvents();
+			logEvents.Should().HaveCount(1);
+
+			var ecsEvents = ToEcsEvents(logEvents);
+
+			var (_, info) = ecsEvents.First();
+			info.Message.Should().Be("Info {\"ValueX\":\"X\", \"SomeY\":2.2}");
+			info.Metadata.Should().ContainKey("SafeValue");
+
+			var x = info.Metadata["SafeValue"] as System.Collections.Generic.Dictionary<string, object>;
+			x.Should().NotBeNull().And.NotBeEmpty();
+		});
+
+		[Fact]
+		public void SeesMessageWithUnsafeProp() => TestLogger((logger, getLogEvents) =>
+		{
+			logger.Info("Info {UnsafeValue}", new NiceObject() { ValueX = "X", SomeY = 2.2 });
+
+			var logEvents = getLogEvents();
+			logEvents.Should().HaveCount(1);
+
+			var ecsEvents = ToEcsEvents(logEvents);
+
+			var (_, info) = ecsEvents.First();
+			info.Message.Should().Be("Info X=X");
+			info.Metadata.Should().ContainKey("UnsafeValue");
+
+			var x = info.Metadata["UnsafeValue"] as string;
+			x.Should().NotBeNull().And.Be("X=X");
+		});
+
+		public class NiceObject
+		{
+			public string ValueX { get; set; }
+			public double SomeY { get; set; }
+
+			public override string ToString() => $"X={ValueX}";
+		}		 
+
 		[Fact]
 		public void SeesMessageWithException() => TestLogger((logger, getLogEvents) =>
 		{