Fixed filename with archived files (roll per run/roll on size)

[Katulus]

Our current application logging is using log4net which has a nice (for us) behavior to start with a fresh file every time you run an application.
It works like this:

• You run application and a new log file called for example application.log is created
• If there is an old application.log file it's renamed to application.log.1.
• If there is an old application.log.1 file it's renamed to application.log.2, etc.
• If log file size reaches defined threshold it creates a new application.log and renames all older files according to schema described above.

Having this behavior available in Serilog would be great. While file-per-day is useful in some scenarios it can be difficult to find when application started. In lot of cases some important work is being done during start and that's when you need to investigate log messages. If you know that they are always at the beginning of the log it saves you a lot of time.

[nblumhardt]

Thank you for all the extra detail, appreciate the follow-up. Need to have a think about this one and loop back 👍

[optical]

👍 For behavior like, or similar to this. Being able to always identify the latest log file without having to look over the existing logs is super useful, as it means you can always use the same command for tailing for example.

[derFunk]

Same here, especially we want to get rid of the mandatorily added -{Date} suffix for the main log file (via this

serilog-sinks-rollingfile/src/Serilog.Sinks.RollingFile/Sinks/RollingFile/TemplatedPathRoller.cs

Lines 53 to 71 in 1c88099

	if (!Specifier.TryGetSpecifier(filenameTemplate, out _specifier))
	{
	_specifier = Specifier.Date;
	filenameTemplate = Path.GetFileNameWithoutExtension(filenameTemplate) + DefaultSeparator +
	_specifier.Token + Path.GetExtension(filenameTemplate);
	}
	var indexOfSpecifier = filenameTemplate.IndexOf(_specifier.Token, StringComparison.Ordinal);
	var prefix = filenameTemplate.Substring(0, indexOfSpecifier);
	var suffix = filenameTemplate.Substring(indexOfSpecifier + _specifier.Token.Length);
	_filenameMatcher = new Regex(
	"^" +
	Regex.Escape(prefix) +
	"(?<" + SpecifierMatchGroup + ">\\d{" + _specifier.Format.Length + "})" +
	"(?<" + SequenceNumberMatchGroup + ">_[0-9]{3,}){0,1}" +
	Regex.Escape(suffix) +
	"$");
	DirectorySearchPattern = filenameTemplate.Replace(_specifier.Token, "*");

).
From my pov it would be okay to only have the retained files date-tagged, but the initial log file should be able to exist without any added suffix (like the proposed application.log).

Context: We're also migrating from Log4Net. We're forwarding all logs to Logstash, and I want to keep using the file forwarder I'm already using, don't want to use the serilog->logstash sink. For this to work the primary filename should be always the same, since the filename itself is functioning as a meta information in our ELK cluster, so we cannot have dates in it.

[DanHarman]

I have a related but slightly different issue. With log4net I can set it to have a constant file name, and to append the date only on rolling the current log to become the previous log. This is extremely useful for systems where we have 3rd party log monitoring, which can't really cope with a changing log file name. I did have a quick look at making a pull request to modify this behavior in the rolling appender, but its pretty involved as the date is baked into the foundation of it all.

I also looked at log4net to see how it discovered the date of the current file to know whether to roll it on startup, and it is simply using System.IO.File.GetLastWriteTimeUtc().

For those also struggling with this issue, one option would seem to be to use the log4net sink. Personally I'd prefer not to have to pull in log4net at all, but it does mean we can get the file appender behaviour we want.

[nblumhardt]

Are all the scenarios mentioned in this thread related closely enough that a single implementation could cover (essentially) all of them?

The rolling file sink, as it stands today, is a pretty simple wrapper around the basic file sink. We could, I think, create a similarly simple wrapper that implements the fixed-filename/numbered-archive policy.

No idea if this would be distinct enough to provide a meaningful alternative name (FixedFileSink? ArchivedFileSink? errm...) but since the rolling file sink here is purposefully an implementation of the file-per-day pattern, it seems like creating a new implementation to stand beside it will take the brakes off of this, and keep each a bit cleaner/more focused.

Any thoughts? Anyone interested in helping spike it up?

[optical]

I think there are two, both useful suggestions here which might warrant the creation of another sink.

Firstly though I have to wonder about the current official rolling file sink. Practically I am unsure if it really is of much use in any serious server side application. This is primarily due to its lack of ability to roll over based on size file size. Realistically any application performing significant amounts of logging will need to limit their log file sizes, and simply halting logging after the current file reaches a certain threshold sounds like an undesirable action in all but the most dire of situations. Practically I think we could probably extend the current rolling file sink to also roll over on size when configured to do so, this would make it a much more attractive option.

The second feature is the fixed name per run, which I think would probably be better suited as a separate sink, as implementing it here while retaining the simplicity and cleanliness of the code could be a challenge. I imagine it will roll per run, optionally roll on size, have a configurable retention policy and generally support a similar set of features here. I suspect it wouldn't support multiple writers against a single file and would blow up if that situation is detected.

[DanHarman]

I actually think the features discussed are a natural part of a complete rolling sink. We are basically talking about abstracting the naming and rolling criteria. In terms if that intersecting with the event logging call, then we could probably have a single interface of GetCurrentLog(). Everything else could be encapsulated. 
I'll have another look at the code to see if I can help. Not sure though as I don't think I know enough about the architecture. 
It may be cleanest to start anew rather than trying to fix the current version, but a complete implementation should probably be able to cover the existing scenario too. 

[DanHarman]

Ok so the first thing that strikes me is that the serilog-sinks-file needs a way of exposing that a file size limit is breached on an Emit(). Not sure how best to do that - changing ILogEventSink seems like a bad idea, so it might have to be via an exception. Not sure of the impact of that though given it currently silently fails and throwing might break existing sinks. We could either keep things consistent with existing and make it a switchable behaviour, or just keep things consistent across the board and accept it can throw on breach. Thoughts?

As an aside, I do wonder about that code as well as it's only checking for breaches post write as I think the textFormatter is writing direct to the stream.

The other thing to consider is how to manage sequence numbers. Currently they are used for when a file is locked, that's pretty orthogonal to whats requested above, specifically in that I'm not sure we can rely on renaming a locked file, so we could get into a real mess trying to ripple up the sequence number on existing files when 1 or more is locked. At least with the date strategy I want, its only a case of renaming the current/previous log file to have the date suffix, not the last n.

[nblumhardt]

Thanks for all the input.!

@optical just on:

Practically I am unsure if it really is of much use in any serious server side application. This is primarily due to its lack of ability to roll over based on size file size. Realistically any application performing significant amounts of logging will need to limit their log file sizes, and simply halting logging after the current file reaches a certain threshold sounds like an undesirable action in all but the most dire of situations.

Many newer server applications using Serilog have a primary sink that's network-based (Seq, Splunk, ELK, Loggly, etc.), so a simple rolling file implementation has gone a long way. I suspect things look different moving across from another logging setup that's been built around different assumptions (ship-from-file the obvious one in this thread).

Digging into the merits of one rolling strategy or another probably isn't worth it here of course, since everyone has an idea of what they'd like to do, and Serilog will ideally facilitate that 👍. There's work already going on in the file sink repo to enable #33, which is the tracking issue for roll-on-size. @DanHarman this is the exposed file size you mention; checking file size post-write may lead to a few more bytes here and there, but exact file sizes don't seem to be a significant requirement at this point.

It seems like the consensus is then:

• Continue work on Feature to roll on file size and date #33 - roll on file size; then,
• Take advantage of those changes in the file sink to implement a fixed-filename/archiving version of the rolling file.

If that's correct, can anyone propose a WriteTo.X() signature for the fixed filename version that would enable it to sit comfortably alongside WriteTo.RollingFile() in this package? It would be great if the new implementation was still behind WriteTo.RollingFile() and was obviously distinguished from the roll-on-date implementation according to the method signature. If that's not possible - do we want to do this in a new repo/package (if so, what name), or through a slightly different method name in the current package?

Cheers!

[nblumhardt]

Okay, here's my proposal..

The current full WriteTo.RollingFile() method signature is:

        public static LoggerConfiguration RollingFile(
            this LoggerSinkConfiguration sinkConfiguration,
            ITextFormatter formatter,
            string pathFormat,
            LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
            long? fileSizeLimitBytes = DefaultFileSizeLimitBytes,
            int? retainedFileCountLimit = DefaultRetainedFileCountLimit,
            LoggingLevelSwitch levelSwitch = null,
            bool buffered = false,
            bool shared = false,
            TimeSpan? flushToDiskInterval = null)

https://github.com/serilog/serilog-sinks-rollingfile/blob/dev/src/Serilog.Sinks.RollingFile/RollingFileLoggerConfigurationExtensions.cs

Plus, there's an overload that accepts outputTemplate instead of ITextFormatter.

For fixed-filename rolling, we could use:

        public static LoggerConfiguration RollingFile(
            this LoggerSinkConfiguration sinkConfiguration,
            ITextFormatter formatter,
            string path,
            long fileSizeLimitBytes,
            LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
            int? retainedFileCountLimit = DefaultRetainedFileCountLimit,
            LoggingLevelSwitch levelSwitch = null,
            bool buffered = false,
            bool shared = false,
            TimeSpan? flushToDiskInterval = null)

This is made unambiguous by forcing a value for fileSizeLimitBytes, and the pathFormat name has become path (no longer a format).

Behind the configuration facade, I think this would instantiate an ArchivedFileSink or similar, so that RollingFileSink stays clean and simple.

There's definitely some room for confusion between two identically-named methods with similar arguments, but the alternatives all have issues of their own.

👍 ... 👎 ?

[Suchiman]

What about

        public static LoggerConfiguration RollingFile(
            this LoggerSinkConfiguration sinkConfiguration,
            ITextFormatter formatter,
            string path,
            string archivePathFormat,
            LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
            long? fileSizeLimitBytes = DefaultFileSizeLimitBytes,
            int? retainedFileCountLimit = DefaultRetainedFileCountLimit,
            LoggingLevelSwitch levelSwitch = null,
            bool buffered = false,
            bool shared = false,
            TimeSpan? flushToDiskInterval = null)

So you would say
WriteTo.RollingFile(textFormatter, "C:\logs\current.txt", "C:\logs\archive\log-{Date}.txt");
Allows you to archive to a path
of your choice, having a different name for current and ignoring the max file size

[DanHarman]

We definitely need a template for rolling pattern although what about for when we have a sequence due to file size rolling. e.g. on T we might end up with:

app.log <--- currently written to
app_1.log <--- previous log
app_2.log <---- previous to that

then in the same folder from day before

app_16-11-27.log <---- most recent from yesterday
app_16-11-27_1,log etc

i.e. the rolling should support both date and file size if we want a proper job of this. The file size rolling is for same day only, and means renaming all the preceding log files for that day to bump their sequence. The date rolling is a date substitution on all the previous days files.

There is also a comment above wanting to replicate the default log4net behaviour of putting the sequence/date on the end of the file name, which might imply having the sequence position in the template by default. I am not that convinced by the practise of changing the file name extension on a role (seems very wrong and irritating to work with), but the flexibility probably wouldn't hurt.

One final point, is that it is probably unnecessary to go to overboard trying to emulate all the log4net options since there is always the option to use the log4net sink. I'm currently doing this for the static filename feature myself. Obviously preferable not to have to configure two different loggers etc, but it does work.

[anicoll]

Has anything come from this thread?

Is there a way to archive with a fixed logfileName but a date based archive file name?
Log4net offer this as seen in their documentation via staticLogFileName
https://logging.apache.org/log4net/release/config-examples.html

[nblumhardt]

Hi @anicoll, thanks for jumping in.

We just released version 4.0.0 of Serilog.Sinks.File, which implements roll-on-size, and crucially for this ticket, puts all of the required information about file state and the file set into a single package.

No one has pushed forward with a "static filename" rolling policy yet, though. I've burned up my personal budget of time for file sink improvements for a few more months yet (the roll-on-size change took a lot of time), but if no one else gets there with a proposal/PR first, I'll loop back to it sometime in 2018.

HTH,
Nick

[nblumhardt]

Discussion on this is moving to serilog/serilog-sinks-file#40 as I don't expect we'll make any further significant changes in the RollingFile sink. Hope you'll all keep an eye on the ticket over there - thanks for all the input 👍