fix: log not serializable data on microsoft log handler

Farfetch · Mar 13, 2024 · 2909522 · 2909522
1 parent 0beaf75
commit 2909522
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 5 deletions.
diff --git a/src/KafkaFlow.Abstractions/ILogHandler.cs b/src/KafkaFlow.Abstractions/ILogHandler.cs
@@ -22,6 +22,14 @@ public interface ILogHandler
     /// <param name="data">Additional data related to the warning log</param>
     void Warning(string message, object data);
 
+    /// <summary>
+    /// Writes a warning log entry
+    /// </summary>
+    /// <param name="message">Warning message</param>
+    /// <param name="ex">Exception thrown</param>
+    /// <param name="data">Additional data related to the warning log</param>
+    void Warning(string message, Exception ex, object data);
+
     /// <summary>
     /// Writes a info log entry
     /// </summary>

diff --git a/src/KafkaFlow.Abstractions/NullLogHandler.cs b/src/KafkaFlow.Abstractions/NullLogHandler.cs
@@ -19,6 +19,12 @@ public void Warning(string message, object data)
         // Do nothing
     }
 
+    /// <inheritdoc />
+    public void Warning(string message, Exception ex, object data)
+    {
+        // Do nothing
+    }
+
     /// <inheritdoc />
     public void Info(string message, object data)
     {

diff --git a/src/KafkaFlow.Admin/TelemetryScheduler.cs b/src/KafkaFlow.Admin/TelemetryScheduler.cs
@@ -96,7 +96,7 @@ private void ProduceTelemetry(
         }
         catch (Exception e)
         {
-            _logHandler.Warning("Error producing telemetry data", new { Exception = e });
+            _logHandler.Warning("Error producing telemetry data", e, null);
         }
     }
 }
diff --git a/src/KafkaFlow.LogHandler.Console/ConsoleLogHandler.cs b/src/KafkaFlow.LogHandler.Console/ConsoleLogHandler.cs
@@ -24,6 +24,21 @@ public void Warning(string message, object data) => Print(
         $"\nKafkaFlow: {message} | Data: {JsonSerializer.Serialize(data)}",
         ConsoleColor.Yellow);
 
+    public void Warning(string message, Exception ex, object data)
+    {
+        var serializedException = JsonSerializer.Serialize(
+         new
+         {
+             Type = ex.GetType().FullName,
+             ex.Message,
+             ex.StackTrace,
+         });
+
+        Print(
+            $"\nKafkaFlow: {message} | Data: {JsonSerializer.Serialize(data)} | Exception: {serializedException}",
+            ConsoleColor.Yellow);
+    }
+
     public void Info(string message, object data) => Print(
         $"\nKafkaFlow: {message} | Data: {JsonSerializer.Serialize(data)}",
         ConsoleColor.Green);

diff --git a/src/KafkaFlow.LogHandler.Microsoft/MicrosoftLogHandler.cs b/src/KafkaFlow.LogHandler.Microsoft/MicrosoftLogHandler.cs
@@ -6,6 +6,8 @@ namespace KafkaFlow;
 
 internal class MicrosoftLogHandler : ILogHandler
 {
+    private const string SerializationErrorMessage = "Log data serialization error.";
+
     private readonly ILogger _logger;
 
     public MicrosoftLogHandler(ILoggerFactory loggerFactory)
@@ -15,21 +17,47 @@ public MicrosoftLogHandler(ILoggerFactory loggerFactory)
 
     public void Error(string message, Exception ex, object data)
     {
-        _logger.LogError(ex, "{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogError(ex, "{Message} | Data: {Data}", message, SerializeData(data));
     }
 
     public void Warning(string message, object data)
     {
-        _logger.LogWarning("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogWarning("{Message} | Data: {Data}", message, SerializeData(data));
+    }
+
+    public void Warning(string message, Exception ex, object data)
+    {
+        _logger.LogWarning(ex, "{Message} | Data: {Data}", message, SerializeData(data));
     }
 
     public void Info(string message, object data)
     {
-        _logger.LogInformation("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogInformation("{Message} | Data: {Data}", message, SerializeData(data));
     }
 
     public void Verbose(string message, object data)
     {
-        _logger.LogDebug("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
+        _logger.LogDebug("{Message} | Data: {Data}", message, SerializeData(data));
+    }
+
+    private string SerializeData(object data)
+    {
+        if (data is null)
+        {
+            return null;
+        }
+
+        try
+        {
+            return JsonSerializer.Serialize(data);
+        }
+        catch(Exception ex)
+        {
+            return JsonSerializer.Serialize(new
+            {
+                Error = SerializationErrorMessage,
+                Exception = $"Message: {ex.Message}, Trace: {ex.StackTrace}",
+            });
+        }
     }
 }
diff --git a/tests/KafkaFlow.IntegrationTests/Core/TraceLogHandler.cs b/tests/KafkaFlow.IntegrationTests/Core/TraceLogHandler.cs
@@ -29,6 +29,18 @@ public void Warning(string message, object data)
                 }));
     }
 
+    public void Warning(string message, Exception ex, object data)
+    {
+        Trace.TraceWarning(
+            JsonSerializer.Serialize(
+                new
+                {
+                    Message = message,
+                    Exception = ex,
+                    Data = data,
+                }));
+    }
+
     public void Info(string message, object data)
     {
         Trace.TraceInformation(

diff --git a/tests/KafkaFlow.UnitTests/LogHandlers/MicrosoftLogHandlerTests.cs b/tests/KafkaFlow.UnitTests/LogHandlers/MicrosoftLogHandlerTests.cs
@@ -1,6 +1,8 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
+using System;
+using System.Linq;
 
 namespace KafkaFlow.UnitTests.LogHandlers;
 
@@ -21,4 +23,31 @@ public void Constructor_CreatesNamedLogger()
         // Assert
         loggerFactoryMock.Verify(x => x.CreateLogger("KafkaFlow"), Times.Once);
     }
+
+    [TestMethod]
+    public void LogWarning_WithNotSerializableData_DoesNotSerializeObject()
+    {
+        // Arrange
+        var expectedMessage = "Any message";
+        var expectedSerializationErrorMessage = "Log data serialization error.";
+        var loggerMock = new Mock<ILogger>();
+        var loggerFactoryMock = new Mock<ILoggerFactory>();
+
+        loggerFactoryMock
+            .Setup(x => x.CreateLogger("KafkaFlow"))
+            .Returns(loggerMock.Object);
+
+        var handler = new MicrosoftLogHandler(loggerFactoryMock.Object);
+
+        // Act
+        handler.Warning(expectedMessage, new
+        {
+            In = new IntPtr(1)
+        });
+
+        // Assert
+        Assert.AreEqual(1, loggerMock.Invocations.Count);
+        Assert.IsTrue(loggerMock.Invocations[0].Arguments.Any(x => x.ToString().Contains(expectedMessage)));
+        Assert.IsTrue(loggerMock.Invocations[0].Arguments.Any(x => x.ToString().Contains(expectedSerializationErrorMessage)));
+    }
 }