Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task done
cothman opened this issue Mar 7, 2024 · 2 comments · Fixed by #539
Closed
1 task done
Labels
bug Something isn't working

Comments

@cothman
Copy link

cothman commented Mar 7, 2024

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

@cothman cothman added the bug Something isn't working label Mar 7, 2024
@cothman cothman changed the title [Bug Report]: MicrosoftLogHandler serializing exception using System.Text.Json [Bug Report]: MicrosoftLogHandler serializing exceptions using System.Text.Json Mar 7, 2024
@cothman cothman changed the title [Bug Report]: MicrosoftLogHandler serializing exceptions using System.Text.Json [Bug Report]: MicrosoftLogHandler is serializing exceptions using System.Text.Json Mar 7, 2024
@JoaoRodriguesGithub
Copy link
Contributor

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?

@cothman
Copy link
Author

cothman commented Mar 8, 2024

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));
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants