Feature to roll on file size and date

[akshetty9]

Question : Is there any plan to include Rolling file based on size/date in this sink. AlternateRollingFileSink does not have option to specify the retainedFileCount and there is no development/pr happening on that sink. I am curious to know if this will be included here.

Update : AlternateRollingFileSink now supports Retenation policy and also ITextFormatter if anyone is looking for the same

[hmvs]

Will author accept PR if i will implement that?
Will be nice to have rolling by size. Now it just stops writing to file.

[nblumhardt]

This particular request has come up enough times now that I think it's time we made some progress on it 👍

From a PR, I think we would need:

• A fairly surgical/low-impact approach, unless a big gain can be found in further refactoring (e.g. to "roll policies")
• Consistent with the current file naming approach: appending _001, _002 etc as the files roll on a particular day
• Some basic benchmarks showing performance consistent with the current implementation for the current configuration defaults

@hmvs are you interested in taking a shot at this? Also CC'ing @jose3m, @Wedvich who have worked in this area recently and may have some additional points to consider.

Cheers!

[hmvs]

Let's imagine:

• rolling by size is enabled
• maximum numbers of files is set to 5
• limit is 10k per file

What should happen when we have more than 50k of logs?
Variants:

• 001 is overwritten, then 002 is overwritten... (content should be truncated)
• Just stops writing (i don't like that)
• 001 is deleted and start writing to 006
  • Problems in case when we will have more than 999
    • Should we increment it to _1001?

Yep, i am interested. Will do in my spare time.
Cheers.

[nblumhardt]

Great, thanks for the help.

I think the answer to the first question above should be "001 is deleted and start writing to 006".

The second question - hmm. _1001 is ugly because it breaks the lexicographic sort order, but it's probably the least surprising. Thoughts?

(> 1000 log files per day is probably a rare situation, I'd suppose.)

[hmvs]

Hmmm. Dilemma :)

[hmvs]

@nblumhardt i had a quick look into code and didn't find a way how we can determine that file is full and it's time to roll.
I think we should make changes to FileSink package first. For example we can make an interface IFileSink or ISinkWithSize or something similar.
public interface ISinkWithSize{ long CurrentSize {get;} }
After it we can make changes to this package. Without it we could hit performance badly by checking filesize everytime.

Another way is to throw an exception from FileSync when file is full. I am new to Serilog but i think it's not good.

[nblumhardt]

Agreed on the exception, too much interference with other exception-related debugging that might be going on.

The file sinks (two classes, IIRC) both support size limits already, so exposing this via an interface seems reasonable. Could alternatively shape it as ISizeLimitedFileSink.SizeLimitReached so that the logic that determines whether the sink will accept an event, and the logic that determines whether an event will be sent to the sink, are the same?

The IFlushableFileSink interface was added for a similar purpose just a little while ago - the downstream changes here were a bit inconvenient due to the lack of union types (ISink + IFlushableFileSink) but otherwise it came out fairly cleanly.

[hmvs]

Hmm. We can go even further and make it more generic. So we can call it
something like ILimitedSink or similar. So that any sink that can decline
messages can inherit it

IRejectingSink... ?

[nblumhardt]

We could, though I don't think there's another (non-file) place we'd use this at the moment. Making a general interface, not specific to the file sink, would mean adding it to Serilog.dll which would only really make the cut if multiple packages need it.

This was the reason why IFlushableFileSink is not IFlushableSink - using File in the name means that if, in the future, we decide to add the more general interface, the names won't collide :-)

[hmvs]

Ok. We can go with ISizeLimitedFileSink

[optical]

How do we feel about also supporting automatic archival (compressing) of rotated/rolled logs? I figure this is likely to be a useful feature for this sink and generally goes along with the idea of rolling based on file size. Theres probably a lot more to discuss, so might be more appropriate to discuss in a separate issue alongside this and #26 since it can be done after the work has been done here.

[narendrachava]

Is the development still active to implement this feature ??

[hmvs]

@narendrachava nope :(

[dasjestyr]

Did this fall on its face? I was going to recommend just copying how Enterprise Library does it. I think this feature is super important for folks in similar situations to myself where my logs are being pumped to Loggly which has a limit on indexing, so anything lower than Info needs to go to text files while I'm debugging :/ -- super hard when the log just stops writing after so long. I've tried increasing the size of the log, but it just becomes unmanageable.

The easiest solution (at least for the a first round) would be to have 2 limits "file size limit" and "max file count limit". That way could say something like "keep creating new files of no more than 10MB until you reach 100 files". Then have a bool option to begin overwriting older files once the file count limit is hit.

[nblumhardt]

It'd be great to get the ball rolling on this again. It's intricate work, thus pretty time-consuming, but in combination with #26 we've got a good chunk of useful functionality to add.

Maybe a slight digression, but I'm wondering now whether we might be better off doing this as a rather more drastic change. The split between Serilog.Sinks.File and Serilog.Sinks.RollingFile adds quite a bit of friction and hoops to jump through. Looking again at the work in progress, does it really make any sense to have a non-rolling file sink that is size-limited?

Digging deeper, based on the NuGet numbers, it doesn't look like the File package is even used outside the context of RollingFile. Even the alternate rolling file doesn't use it.

Perhaps this'd be easier to move forward if we combined Serilog.Sinks.File and _RollingFile_into a single, complete, rolling file package. The existing File functionality would simply be the fixed filename case with no archiving.

This might help get around some of the friction you hit in the file sink PR, too, @hmvs?

It'd be a good chunk of work and take some time; the best way to do it, I think, would be to create a third repository forked from https://github.com/serilog/serilog-sinks-file, where the work can proceed and the package can be published under a temporary name for testing. When done, we'd merge into the file sink repository, and make Serilog.Sinks.RollingFile into a wrapper package.

(We're doing something very similar for Serilog.Sinks.Console, LiterateConsole and ColoredConsole.)

The biggest challenge, still, will be time. It seems like core functionality, but most of us seem to have moved to log servers and so don't rely heavily enough on file-based rolling to need a lot of flexibility. For example, @dasjestyr, could you not just set up Seq (disclaimer, I built it), Splunk or Elasticsearch locally and use that during development instead of a giant rolling file? The experience would have to be much smoother :-).

I'm still keen to move this forward, however. Hoping I'll get some time in the next few weeks to contribute. Any thoughts on the approach outlined above?

[dasjestyr]

I considered doing something different for local, but it's not necessarily for local testing. Our workflow is test locally > deploy to remote dev (integration) and test > promote to QA > promote to prod. Working locally isn't a big deal because everyone outputs to the output window in visual studio. I think the best I'd be able to do is something like seq for the remote development environment, meaning everyone would have to learn to use seq in addition to loggly which isn't going to sit well, plus I'm not sure how well that would sit with licensing. It was easier to just dump to a text file and hook up some file tailing.

[hmvs]

@nblumhardt Hmm. I haven't thought in that way. So would be wonderful to have this two packages merged into one.
As i remember i had concern about UT coverage of ATOMIC_APPEND and OS_MUTEX functionality. As far as I am not sure how to test it manually i am afraid to break it during refactoring. Is there any chance that someone can cover it with UT?

[nblumhardt]

@hmvs the challenge with those types is that they need to be tested using operating system-level/process concurrency, IIRC. Probably one for manual testing once the rest of the port is done (as scary as that is).

[DanHarman]

@nblumhardt I agree that two file appenders doesn't really seem necessary. I've just resorted to using the log4net appender and then using its file appender which supports static file name for the current log, and appending a date when it rolls. Even with something like Seq, a lot of monitoring setups like Nagios seem best configured to watch a static log file. Also I feel more confident that a servers HDs will stay available come a problem than a remote log concentrator.

The thing that makes this seem so much work to me to implement, is the support for rolling based on file size, not just date when the current 'hot' file needs to maintain the same name. This leads to multiple files to rename both for size and date rolls. This is fiddly with all the custom naming template options.

[nblumhardt]

Thanks for the note @DanHarman. Yes, it's the intersection of many different features that seems to destroy many "rolling file" codebases. Even things like file sharing modes, flush modes and so-on start to come into play.

The major challenge with the existing design seems to be that the file sink would have to expose almost all fine-grained information about the underlying file's state in order for the separate rolling file sink to implement rolling policies and coordinate file names, which then becomes a versioning/coordination pain. Merging will overcome that, but leave a lot of design work still to be done.

I think I'm done with the Console overhaul, so ready to move onto the File overhaul soon - just waiting on some inspiration to come :-)

[akshetty9]

@nblumhardt could you please let us know if there is any work being done on combining these features and creating a single sink which can roll based on file size and date.

However, there are several improvements in alternate rolling file sink which now supports ITextFormatter, Retention policy, Hourly rolling file .

There is also a "super alternate rolling file" here which adds even more options like compact json format :)

[nblumhardt]

I've implemented this in serilog/serilog-sinks-file#35

[stephenpatten]

This PR just made my day , literally we were contemplating moving away toward NLog because of this feature. I'll incorporate ASAP and keep you posted. Thank you Serilog Team!

When will this be available on NuGet?

[nblumhardt]

Thanks @stephenpatten - I think the only thing blocking an initial merge and -dev package publish is the broken Travis build. I'll ask around and see if anyone has an idea what might be causing it.

[yoav2]

So is it possible now to creating a rolling file based on date and size?

[mikaelmello]

@yoav2, yes.