From f4a7e3d4762a179c64022fc0b22d49e3d03d0a02 Mon Sep 17 00:00:00 2001 From: rizi Date: Tue, 22 Sep 2020 09:06:00 +0200 Subject: [PATCH 1/7] TraceSourceLogger now takes exception into account(adds it to the log message) even if formatter is not null --- .../src/TraceSourceLogger.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs index 1c2f2a970a28e4..eb302aa6538fb0 100644 --- a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs +++ b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; +using System.Globalization; using DiagnosticsTraceSource = System.Diagnostics.TraceSource; namespace Microsoft.Extensions.Logging.TraceSource @@ -35,13 +36,20 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except } if (exception != null) { - message += Environment.NewLine + exception; + message += string.Format(CultureInfo.InvariantCulture, "{0}{1}", Environment.NewLine, exception); } } - if (!string.IsNullOrEmpty(message)) + + if (string.IsNullOrEmpty(message) && exception == null) + return; + + if (exception != null) { - _traceSource.TraceEvent(GetEventType(logLevel), eventId.Id, message); + string exceptionDelimiter = !string.IsNullOrEmpty(message) ? Environment.NewLine : string.Empty; + message += string.Format(CultureInfo.InvariantCulture, "{0}Error: {1}", exceptionDelimiter, exception); } + + _traceSource.TraceEvent(GetEventType(logLevel), eventId.Id, message); } public bool IsEnabled(LogLevel logLevel) From 34683e1b2e84247185487f3eede940de2cfb94c9 Mon Sep 17 00:00:00 2001 From: rizi Date: Tue, 22 Sep 2020 11:06:42 +0200 Subject: [PATCH 2/7] Fixed a logical error, that would have added the exception twice to the log message when the formatter would have been null. --- .../src/TraceSourceLogger.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs index eb302aa6538fb0..3d96223f39e231 100644 --- a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs +++ b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs @@ -3,7 +3,6 @@ using System; using System.Diagnostics; -using System.Globalization; using DiagnosticsTraceSource = System.Diagnostics.TraceSource; namespace Microsoft.Extensions.Logging.TraceSource @@ -23,7 +22,9 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except { return; } + string message = string.Empty; + if (formatter != null) { message = formatter(state, exception); @@ -34,10 +35,6 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except { message += state; } - if (exception != null) - { - message += string.Format(CultureInfo.InvariantCulture, "{0}{1}", Environment.NewLine, exception); - } } if (string.IsNullOrEmpty(message) && exception == null) @@ -46,7 +43,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except if (exception != null) { string exceptionDelimiter = !string.IsNullOrEmpty(message) ? Environment.NewLine : string.Empty; - message += string.Format(CultureInfo.InvariantCulture, "{0}Error: {1}", exceptionDelimiter, exception); + message += exceptionDelimiter + "Error: " + exception; } _traceSource.TraceEvent(GetEventType(logLevel), eventId.Id, message); From 3cf365ac76b1ec5e5b29a3a7e7c391cf4151865d Mon Sep 17 00:00:00 2001 From: rizi Date: Wed, 23 Sep 2020 06:48:55 +0200 Subject: [PATCH 3/7] Added tests for TraceSourceLogger. --- .../src/TraceSourceLogger.cs | 2 +- .../tests/Common/TraceSourceLoggerTest.cs | 33 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs index 3d96223f39e231..527fbb679c4b17 100644 --- a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs +++ b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs @@ -42,7 +42,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except if (exception != null) { - string exceptionDelimiter = !string.IsNullOrEmpty(message) ? Environment.NewLine : string.Empty; + string exceptionDelimiter = string.IsNullOrEmpty(message) ? string.Empty : Environment.NewLine; message += exceptionDelimiter + "Error: " + exception; } diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs index fbabd024136a46..a4c7e3315da32f 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs @@ -1,7 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Diagnostics; + +using Moq; + using Xunit; namespace Microsoft.Extensions.Logging.Test @@ -51,10 +55,37 @@ public static void MultipleLoggers_IsEnabledReturnsCorrectValue(SourceLevels fir .AddTraceSource(secondSwitch)); var logger = factory.CreateLogger("Test"); - + // Assert Assert.Equal(expected, logger.IsEnabled(LogLevel.Information)); } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void Log_Shoud_Add_Exception_To_Message_Whether_Formatter_Is_Null_Or_Not(bool shouldFormatterBeNull) + { + // Arrange + Mock traceListener = new Mock(); + SourceSwitch sourceSwitch = new SourceSwitch("TestSwitch") {Level = SourceLevels.All}; + + ILoggerFactory factory = TestLoggerBuilder.Create(builder => builder.AddTraceSource(sourceSwitch, traceListener.Object)); + ILogger logger = factory.CreateLogger("Test"); + + const LogLevel logLevel = LogLevel.Information; + EventId eventId = new EventId(1); + const string message = "some log message"; + Exception exception = new Exception("Some error occurred"); + Func formatter = shouldFormatterBeNull ? (Func)null : (value, passedException) => value; + + string expectedMessage = $"{message}{Environment.NewLine}Error: {exception}"; + + // Act + logger.Log(logLevel, eventId, message, exception, formatter); + + // Assert + traceListener.Verify(listener => listener.TraceEvent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), expectedMessage), Times.Once); + } } } From 8981cadf682b05d91897a3468d9d67a0721b47eb Mon Sep 17 00:00:00 2001 From: rizi Date: Thu, 24 Sep 2020 05:43:19 +0200 Subject: [PATCH 4/7] Removed unnecessary white-spaces. --- .../tests/Common/TraceSourceLoggerTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs index a4c7e3315da32f..c49dbf7edf5f52 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs @@ -49,7 +49,6 @@ public static void MultipleLoggers_IsEnabledReturnsCorrectValue(SourceLevels fir secondSwitch.Level = second; // Act - var factory = TestLoggerBuilder.Create(builder => builder .AddTraceSource(firstSwitch) .AddTraceSource(secondSwitch)); From b9a5c999ca291a271e5c7703b2d75918245a815f Mon Sep 17 00:00:00 2001 From: rizi Date: Fri, 25 Sep 2020 09:33:40 +0200 Subject: [PATCH 5/7] Make the TraceLogger output more "natural" (one line message) and similar to SystemdConsoleFormatter. --- .../src/TraceSourceLogger.cs | 19 ++++++++----------- .../tests/Common/TraceSourceLoggerTest.cs | 2 +- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs index 527fbb679c4b17..e3e81d42cf976d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs +++ b/src/libraries/Microsoft.Extensions.Logging.TraceSource/src/TraceSourceLogger.cs @@ -29,24 +29,21 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except { message = formatter(state, exception); } - else + else if (state != null) { - if (state != null) - { - message += state; - } + message += state; } - if (string.IsNullOrEmpty(message) && exception == null) - return; - if (exception != null) { - string exceptionDelimiter = string.IsNullOrEmpty(message) ? string.Empty : Environment.NewLine; - message += exceptionDelimiter + "Error: " + exception; + string exceptionDelimiter = string.IsNullOrEmpty(message) ? string.Empty : " " ; + message += exceptionDelimiter + exception; } - _traceSource.TraceEvent(GetEventType(logLevel), eventId.Id, message); + if (!string.IsNullOrEmpty(message)) + { + _traceSource.TraceEvent(GetEventType(logLevel), eventId.Id, message); + } } public bool IsEnabled(LogLevel logLevel) diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs index c49dbf7edf5f52..181710fae1800e 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs @@ -77,7 +77,7 @@ public static void Log_Shoud_Add_Exception_To_Message_Whether_Formatter_Is_Null_ Exception exception = new Exception("Some error occurred"); Func formatter = shouldFormatterBeNull ? (Func)null : (value, passedException) => value; - string expectedMessage = $"{message}{Environment.NewLine}Error: {exception}"; + string expectedMessage = $"{message} {exception}"; // Act logger.Log(logLevel, eventId, message, exception, formatter); From 5a2e1f1670dfef80c6cac376b22d2cdd98a98690 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 28 Sep 2020 10:51:17 -0700 Subject: [PATCH 6/7] Update TraceSourceLoggerTest.cs --- .../tests/Common/TraceSourceLoggerTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs index 181710fae1800e..9e10a0a692cd76 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs @@ -3,7 +3,6 @@ using System; using System.Diagnostics; - using Moq; using Xunit; From 7d7ea2f498a2090d15b4fd385213781202190b28 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 28 Sep 2020 10:52:33 -0700 Subject: [PATCH 7/7] Update TraceSourceLoggerTest.cs --- .../tests/Common/TraceSourceLoggerTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs index 9e10a0a692cd76..52632e471466cc 100644 --- a/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs +++ b/src/libraries/Microsoft.Extensions.Logging/tests/Common/TraceSourceLoggerTest.cs @@ -53,7 +53,7 @@ public static void MultipleLoggers_IsEnabledReturnsCorrectValue(SourceLevels fir .AddTraceSource(secondSwitch)); var logger = factory.CreateLogger("Test"); - + // Assert Assert.Equal(expected, logger.IsEnabled(LogLevel.Information)); }