Queue limit in non-durable sink should be defined in byte size

[FantasticFiasco]

As it currently is, the queue size in the non-durable sink is defined as number of events. This is problematic because how large is a log event? Well it differs. It would be better if this was defined as a size in bytes, because it would be easier to reason about the memory requirements for the sink when we run in a non-durable mode.

This issue was created because of #201.

[seruminar]

Please consider adding a non-infinite default as well. It takes some time to track down why there's a memory leak despite nothing apparent in user code (e.g. Serilog contexts are properly disposed). In my case a long-running Docker container was blowing up because its memory was filling with log events. At the very least, logging a warning may help point out that an internal queue is unlimited and can cause a crash.

[FantasticFiasco]

Hi @seruminar.

It's unfortunate that you found this behavior in production. What I can do is to log something in the Serilog SelfLog. Have you registered a callback for those events, i.e. would you have found the issue sooner with log events in the SelfLog?

The reason we opt to have infinite as default value is because there is no good default value. Any value we choose might result in the exact same behaviour as you've experienced.

[seruminar]

Yeah, an event in the SelfLog would help, it is written to the console in development so I would have noticed a message there on startup while testing locally.

I understand that a limit is application-specific; perhaps I am more reacting to this one parameter being disproportionately more important than the others. In the durable sinks, a similar one is retainedBufferFileCountLimit but it has a default value. Would it be possible to remove the default null and make it a required parameter, where passing null would equal current behavior but would force the developer to explicitly opt-in to an infinite queue?

[FantasticFiasco]

That would be even better than writing to SelfLog. And since we're currently in development towards a new major version, I see no problem with implementing a breaking change if it improves the overall experience.

Would you be willing to write this as a new issue? You can reference back to this conversation if you wish to. Thanks for the feedback!

[seruminar]

Would you be willing to write this as a new issue? You can reference back to this conversation if you wish to. Thanks for the feedback!

All done! #245