Elastic.CommonSchema.NLog - MetadataDictionary with safe values

elastic · Sep 10, 2022 · e9c5ce5 · e9c5ce5
1 parent 4ca53d2
commit e9c5ce5
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 1 deletion.
diff --git 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
@@ -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) =>
 		{