From 160b503c85f81ddc15c3bd1672a30931e4541b42 Mon Sep 17 00:00:00 2001 From: Sam Cook Date: Tue, 17 Sep 2019 18:16:00 +0100 Subject: [PATCH 1/4] Add .editorconfig file --- .editorconfig | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..494f6e3 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,10 @@ +root=true + +[*.config] +indent_style=space +indent_size=2 + +[*.cs] +indent_style=space +indent_size=4 +trim_trailing_whitespace=true From a7fa4b09062cac3c7fb384f86f44323c5c5c73ad Mon Sep 17 00:00:00 2001 From: Sam Cook Date: Tue, 17 Sep 2019 18:20:30 +0100 Subject: [PATCH 2/4] Add test to demonstrate logging failure for log events that have exceptions that can't be serialised due to a property throwing an exception when fetched. --- .../IntegrationTests.cs | 54 ++++++++++++++++++- .../TestOutputTextWriter.cs | 48 +++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 src/NLog.Targets.ElasticSearch.Tests/TestOutputTextWriter.cs diff --git a/src/NLog.Targets.ElasticSearch.Tests/IntegrationTests.cs b/src/NLog.Targets.ElasticSearch.Tests/IntegrationTests.cs index 847e97a..17b7a2c 100644 --- a/src/NLog.Targets.ElasticSearch.Tests/IntegrationTests.cs +++ b/src/NLog.Targets.ElasticSearch.Tests/IntegrationTests.cs @@ -1,12 +1,64 @@ using System; +using NLog.Common; using NLog.Config; +using NLog.Layouts; using Xunit; +using Xunit.Abstractions; namespace NLog.Targets.ElasticSearch.Tests { public class IntegrationTests { - [Fact(Skip ="Integration")] + private readonly ITestOutputHelper testOutputHelper; + + public IntegrationTests(ITestOutputHelper testOutputHelper) + { + this.testOutputHelper = testOutputHelper; + } + + private class ExceptionWithPropertiesThatThrow : Exception + { + public object ThisPropertyThrowsOnGet => throw new ObjectDisposedException("DisposedObject"); + } + + [Theory(Skip = "Integration")] + [InlineData(true)] + [InlineData(false)] + public void ExceptionSerializationTest(bool hasExceptionFieldLayout) + { + using (var testOutputTextWriter = new TestOutputTextWriter(testOutputHelper)) + { + InternalLogger.LogWriter = testOutputTextWriter; + InternalLogger.LogLevel = LogLevel.Warn; + + var elasticTarget = new ElasticSearchTarget(); + + if (hasExceptionFieldLayout) + { + elasticTarget.Fields.Add(new Field + { + Name = "exception", + Layout = Layout.FromString("${exception:format=toString,Data:maxInnerExceptionLevel=10}"), + LayoutType = typeof(string) + }); + } + + var rule = new LoggingRule("*", LogLevel.Info, elasticTarget); + + var config = new LoggingConfiguration(); + config.LoggingRules.Add(rule); + + LogManager.Configuration = config; + + var logger = LogManager.GetLogger("Example"); + + logger.Error(new ExceptionWithPropertiesThatThrow(), "Boom"); + + LogManager.Flush(); + } + } + + [Fact(Skip = "Integration")] public void SimpleLogTest() { var elasticTarget = new ElasticSearchTarget(); diff --git a/src/NLog.Targets.ElasticSearch.Tests/TestOutputTextWriter.cs b/src/NLog.Targets.ElasticSearch.Tests/TestOutputTextWriter.cs new file mode 100644 index 0000000..fe7743f --- /dev/null +++ b/src/NLog.Targets.ElasticSearch.Tests/TestOutputTextWriter.cs @@ -0,0 +1,48 @@ +using System.IO; +using System.Text; +using Xunit.Abstractions; + +namespace NLog.Targets.ElasticSearch.Tests +{ + public class TestOutputTextWriter : TextWriter + { + private StringBuilder stringBuilder; + private readonly ITestOutputHelper testOutputHelper; + + public TestOutputTextWriter(ITestOutputHelper testOutputHelper) + { + this.stringBuilder = new StringBuilder(); + this.testOutputHelper = testOutputHelper; + } + + public override Encoding Encoding => Encoding.Unicode; + + public override void Write(char value) + { + this.stringBuilder.Append(value); + } + + public override void WriteLine(string value) + { + this.stringBuilder.Append(value); + + this.Flush(); + } + + public override void Flush() + { + var sb = this.stringBuilder; + if (sb.Length > 0) + { + this.stringBuilder = new StringBuilder(); + this.testOutputHelper.WriteLine(sb.ToString()); + } + } + + protected override void Dispose(bool disposing) + { + this.Flush(); + base.Dispose(disposing); + } + } +} \ No newline at end of file From 2df3cbd12490c836be9b9f720e5b697edd5ce4d9 Mon Sep 17 00:00:00 2001 From: Sam Cook Date: Tue, 17 Sep 2019 18:22:12 +0100 Subject: [PATCH 3/4] Configure json serialisation of exceptions to tolerate errors without blowing up and log a warning in the internal log instead. --- src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs b/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs index d61bd3b..cc0aa1d 100644 --- a/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs +++ b/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs @@ -282,6 +282,11 @@ private static JsonSerializerSettings CreateJsonSerializerSettings() { var jsonSerializerSettings = new JsonSerializerSettings { ReferenceLoopHandling = ReferenceLoopHandling.Ignore, CheckAdditionalContent = true }; jsonSerializerSettings.Converters.Add(new StringEnumConverter()); + jsonSerializerSettings.Error = (sender, args) => + { + InternalLogger.Warn(args.ErrorContext.Error, $"Error serializing exception property '{args.ErrorContext.Member}', property ignored"); + args.ErrorContext.Handled = true; + }; return jsonSerializerSettings; } } From d5c79394e38bbb927a7d1b31b6061d29e082ea5f Mon Sep 17 00:00:00 2001 From: Sam Cook Date: Tue, 17 Sep 2019 18:23:43 +0100 Subject: [PATCH 4/4] Only perform exception serialisation if there isn't a custom exception layout specified. --- .../ElasticSearchTarget.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs b/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs index cc0aa1d..9c41549 100644 --- a/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs +++ b/src/NLog.Targets.ElasticSearch/ElasticSearchTarget.cs @@ -220,13 +220,6 @@ private PostData FormPayload(ICollection logEvents) {"message", RenderLogEvent(Layout, logEvent)} }; - if (logEvent.Exception != null) - { - var jsonString = JsonConvert.SerializeObject(logEvent.Exception, _jsonSerializerSettings.Value); - var ex = JsonConvert.DeserializeObject(jsonString); - document.Add("exception", ex.ReplaceDotInKeys()); - } - foreach (var field in Fields) { var renderedField = RenderLogEvent(field.Layout, logEvent); @@ -245,6 +238,13 @@ private PostData FormPayload(ICollection logEvents) } } + if (logEvent.Exception != null && !document.ContainsKey("exception")) + { + var jsonString = JsonConvert.SerializeObject(logEvent.Exception, _jsonSerializerSettings.Value); + var ex = JsonConvert.DeserializeObject(jsonString); + document.Add("exception", ex.ReplaceDotInKeys()); + } + if (IncludeAllProperties && logEvent.HasProperties) { foreach (var p in logEvent.Properties)