datalust / serilog-sinks-seq

A Serilog sink that writes events to the Seq structured log server
https://datalust.co/seq
Apache License 2.0
239 stars 50 forks source link

Possible memory leak #216

Closed jessicah closed 7 months ago

jessicah commented 7 months ago

Initially filed at: https://github.com/serilog/serilog-sinks-periodicbatching/issues/76

Description Serilog appears to end up with unbounded memory usage, creating a large number of CancellationTokenSources and ValueTasks.

Reproduction Program.cs:

Log.Logger = new LoggerConfiguration()
    .Filter.RateLimited()
    .WriteTo.Seq("http://...", payloadFormatter: new CompactJsonFormatter(), eventBodyLimitBytes: 10485760)
    .CreateLogger();

var builder = WebApplication.CreateBuilder(args);

builder.Host.UseSerilog(Log.Logger);

LoggingRateLimiter.cs:

using Serilog;
using Serilog.Configuration;
using Serilog.Events;

namespace BlazorSSR
{
    public static class LoggingExtensions
    {
        public static LoggerConfiguration RateLimited(this LoggerFilterConfiguration filterConfiguration)
        {
            var limiter = new LoggingRateLimiter();

            return filterConfiguration.ByExcluding(limiter.LimitExceeded);
        }
    }

    class LoggingRateLimiter
    {
        readonly object _sync = new();

        Dictionary<string, DateTime> _lastSuccessTimes = [];

        public bool LimitExceeded(LogEvent logEvent)
        {
            lock (_sync)
            {
                if (logEvent.Properties.TryGetValue("Endpoint", out var path))
                {
                    if (_lastSuccessTimes.TryGetValue(path.ToString(), out DateTime lastSuccess))
                    {
                        if (lastSuccess.AddSeconds(10) < DateTime.Now)
                        {
                            _lastSuccessTimes[path.ToString()] = DateTime.Now;

                            return false;
                        }

                        return true;
                    }
                    else
                    {
                        _lastSuccessTimes[path.ToString()] = DateTime.Now;

                        return false;
                    }
                }

                return false;
            }
        }
    }
}

Expected behavior Memory to remain fairly constant

Relevant package, tooling and runtime versions

Additional context CancellationTokenSource 320329705-fba30248-938b-4226-a5e6-73750e2b0a14 320329886-af812f03-7354-4ab2-942a-54845743e43d

ValueTask 320329748-80d73fdd-af59-4ab8-932a-b6573682c97c 320329777-50358f19-c0e3-4322-a3fb-be1087462a1c

nblumhardt commented 7 months ago

Thanks for raising this; closing as duplicate of https://github.com/serilog/serilog-sinks-periodicbatching/issues/76, until we figure out what project the issue is in.

jessicah commented 7 months ago

I removed seq as a sink, and used async console sink instead, and the memory leak seems to have gone away.

nblumhardt commented 7 months ago

Thanks for following up. It'd be great to get a few more details over on the other ticket regarding your testing conditions, just so we know whether there's something for us to chase down or not. Thanks again!