Farfetch/kafkaflow

[Bug Report]: MicrosoftLogHandler is serializing exceptions using System.Text.Json

cothman opened this issue · 2 comments

Prerequisites

  • I have searched issues to ensure it has not already been reported

Description

MicrosoftLogHandler is using System.Text.Json to serialize data, sometimes this data can be an Exception like in the method below

private void ProduceTelemetry(
    string topicName,
    int partition,
    IEnumerable<IMessageConsumer> consumers,
    IMessageProducer producer)
{
    try
    {
        var items = consumers
            .SelectMany(
                c =>
                {
                    var consumerLag = c.GetTopicPartitionsLag();

                    return c.Topics.Select(
                        topic => new ConsumerTelemetryMetric
                        {
                            ConsumerName = c.ConsumerName,
                            Topic = topic,
                            GroupId = c.GroupId,
                            InstanceName = $"{Environment.MachineName}-{s_processId}",
                            PausedPartitions = c.PausedPartitions
                                .Where(p => p.Topic == topic)
                                .Select(p => p.Partition.Value),
                            RunningPartitions = c.RunningPartitions
                                .Where(p => p.Topic == topic)
                                .Select(p => p.Partition.Value),
                            WorkersCount = c.WorkersCount,
                            Status = c.Status,
                            Lag = consumerLag.Where(l => l.Topic == topic).Sum(l => l.Lag),
                            SentAt = DateTime.UtcNow,
                        });
                });

        foreach (var item in items)
        {
            producer.Produce(topicName, Guid.NewGuid().ToByteArray(), item, partition: partition);
        }
    }
    catch (Exception e)
    {
        _logHandler.Warning("Error producing telemetry data", new { Exception = e });
    }
}

Which will lead to a System.NotSupportedException: Serialization and deserialization of 'System.IntPtr' instances are not supported since System.Text.Json doesn't support serialization of exceptions, links to some microsoft discussions dotnet/runtime#43026 (comment) dotnet/runtime#43482

Steps to reproduce

Every step that will lead to trigger microsoft logger with an exception as Data

Expected behavior

Serialize data even if its an exception

Actual behavior

Throws a `System.NotSupportedException: Serialization and deserialization of 'System.IntPtr' exception

KafkaFlow version

3.0.3

Hi @cothman,

Thank you for bringing this topic to discussion.

Can you please share more about the need to have the exception serialized instead of throwing the exception in your use case?

Hi @JoaoRodriguesGithub,

I'm not trying to serialize an Exception, its the ProduceTelemetry that when it catch an Exception it'll call the _logHandler.Warning("Error producing telemetry data", new { Exception = e }); , which later, if using a UseMicrosoftLog, will try to serialize the exception.

internal class MicrosoftLogHandler : ILogHandler
{
    private readonly ILogger _logger;

    public MicrosoftLogHandler(ILoggerFactory loggerFactory)
    {
        _logger = loggerFactory.CreateLogger("KafkaFlow");
    }

    public void Error(string message, Exception ex, object data)
    {
        _logger.LogError(ex, "{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
    }

    public void Warning(string message, object data)
    {
        _logger.LogWarning("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
    }

    public void Info(string message, object data)
    {
        _logger.LogInformation("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
    }

    public void Verbose(string message, object data)
    {
        _logger.LogDebug("{Message} | Data: {Data}", message, JsonSerializer.Serialize(data));
    }
}