borisermakof / serilog-sinks-fluentd

A Sink that writes logs into Fluentd
Apache License 2.0
20 stars 30 forks source link

❌This lib causes memory leaks. DO NOT USE IT. #22

Open ilya-chumakov opened 2 years ago

ilya-chumakov commented 2 years ago

We found a serious memory leak on sending logs to the collector. As the project is dead (no PR accepted in recent years) and because of other serious issues like this, a quick fix could be: 1) fork it 2) fix https://github.com/borisermakof/serilog-sinks-fluentd/blob/0ca957c9156229bb1e480def9f70ec35acb8e993/src/Serilog.Sinks.Fluentd/Sinks/Fluentd/FluentdEmitter.cs to

        public static readonly SerializationContext SerializationContext
            = new SerializationContext(PackerCompatibilityOptions.PackBinaryAsRaw);

        public FluentdEmitter(Stream stream)
        {
            //actual fix is here: allow generated stuff to be GC collected
            SerializationContext.SerializerOptions.GeneratorOption = SerializationMethodGeneratorOption.CanCollect;
            SerializationContext.Serializers.Register(new OrdinaryDictionarySerializer());
            _packer = Packer.Create(stream);
        }

BTW, the serializer used there is also dead :)

I do not intend to publish a nuget package with this fix or open my fork and I recommend you to do the same. This lib has too many existing flaws. In my opinion, it has to be re-written from scratch using a different MskPack serializer to get a stable production-ready solution.

liona-k-tanaka commented 2 years ago

Thanks @ilya-chumakov. I found this problem in our production server too. But I couldn't solved by your codes... Finally, I changed serialization library from MsgPack.Cli to MessagePack for C# and confirmed the memory leak is fixed.

I sent my fix as PR #23. I don't published fixed version as nuget package yet because I want to fix original version. (I published only GitHub package for our project.)

liona-k-tanaka commented 1 year ago

Maybe My PR is not checked... But I found that klyse/serilog-sinks-fluentd merged it. I will try to use Serilog.Sinks.Fluentd.Revived package.