search

serilog / serilog-sinks-periodicbatching

Infrastructure for Serilog sinks that process events in batches.
Apache License 2.0
71 stars 29 forks source link

Change options like batch size limit after construction? #37

Closed thnk2wn closed 4 years ago

thnk2wn commented 4 years ago

It'd be great if there was a way to change options like BatchSizeLimit, FlushPeriod, and QueueLimit after construction. We have some external configuration coming from cloud events that don't fire until later on after the sink is already created at which point it's too late to take effect.

Other good ways to work around?

Thanks

nblumhardt commented 4 years ago

Hi Geoff,

Check out this Gist: https://gist.github.com/nblumhardt/34c0c273c383da9745f4e974f12b9cac - should do what you need.

Cheers! Nick

thnk2wn commented 4 years ago

Thanks @nblumhardt.

I think that delayed initialization has some potential issues such as:

  1. Earlier log events are lost or must be kept in a separate in memory queue then 'replayed' later
  2. Have to keep a reference to the sink around somewhere (normally just .WriteTo(...) in startup)
  3. Concurrency considerations
  4. Additional wrapped configuration

Might be more ideal in ways if the sink could reconfigure itself safely (between batches etc.) to not have those drawbacks. It appears it already dynamically adjusts flush period on error so it seems feasible although I'm sure there are other considerations and potential issues of doing so.

I'll give the delayed option a try for now

nblumhardt commented 4 years ago

Thanks for the follow-up. Makes sense - but some substantial challenges to overcome before it could be done at the level of this sink. Some modification of the delayed-init approach should be a faster way to a solution, keep us posted with progress :-) 👍

thnk2wn commented 4 years ago

For our needs the wrapped, delayed sink wasn't a good fit, mostly due to loss of log events prior to full initialization.

I ended up copying the sink and making modifications to support changes after construction. Currently this is directly within our application; if ever it's useful beyond that I could potentially migrate those to a forked copy and/or create a PR upon further validation. However I'm guessing this is mostly something specific to our needs and the changes not solid enough to support all use cases and risk of changes.

nblumhardt commented 4 years ago

Cool; thanks for closing the loop, Geoff