Fixed filename with rolling archive files

[bartkusters]

When logging rolls, all new entries are written to the rolled file. Can't this be changed so that logging keeps getting written to "sample.log" and the rolled files "sample001.log", "sample002.log", etc. keep the history?

[hali4831]

This is something I am looking for too

[jawadatgithub]

Me too, to explain the benefit of that, I usually open the log.txt in notepad++ to keep monitoring current logs, and in case of issue happened while ago, I go to rolled file based on last modified date.

So most of the times I just need to have log.txt opened, and rarely check rolled files. and that's the default behavior used in nlog, but for .net core 2 apps I still prefer serilog.

[bitchkat]

I was actually just going to ask about this exact same behavior. The way that bartkusters describes is very traditional -- at least in the linux/unix world. If I'm tailing a log file, it just silently stops when the log file rolls over. Also you have to ls all the files to find the last one to start a tail when the log file has been rolled over.

[nblumhardt]

Thanks for all the input 👍

Is anyone interested in exploring this one a bit further, to see what the implementation effort would involve, and how the file logger would need to change in order to incorporate this scenario?

[nblumhardt]

See also some earlier discussion in: serilog/serilog-sinks-rollingfile#26

[bartelink]

While its true that log4net and NLog both do rolling renaming stuff as described (at least by default), I'd like to point out the following reasons to maintain [at a minimum, backcompat with] the status quo:

• I've seen cases where that's problematic (e.g. backup, viewing or AV software can block a rolling set of renames)
• doing such a rename in the context of concurrent writing to a shared resource is fraught with danger and hard to implement in a safe way
• log forwarders in general (and def Splunk one) needs to go to effort to fingerprint content to keep track of it all (and it's messy for tailing tools to cope with too - though I guess most proper ones can handle it)
• Splunk and/or other files can not track the renamed state of the file whereas this scheme keeps it all predictable - if I want to track back to a source file it's there as long as it has not been aged out.

For me, starting Visual Studio Code with code /logs where that's my log folder and then clicking from file to file works pretty effectively (recent versions are pretty snappy and seem to handle files changing properly, and of course pretty prints it pretty nicely indeed) and lets me get perspective on how much writing happened and when vs having to infer stuff from file dates etc etc.

But... the reason I'm here is to ask for a feature/tweak... @nblumhardt or others, is anyone interested in having rolling by size trigger usage of suffix universally? i.e. if the name requested is x/app.log then the first file written will be x/app_000.log. This would make it easier to explain a naming pattern to someone ingesting and/or avoid surprises vs having to explain the special casing. While not trivial, it does not look crazy difficult to impl based on the current code.

[nblumhardt]

@bartelink the _000 naming makes sense - the only drawback is that we would risk breaking existing apps by making it the default, and yet it seems too minor to warrant a new top-level configuration option.

To move this sink forward, we might need a means of plugging in naming/rolling policies - tricky work, but probably inevitable. Low on time, but happy to help out if anyone is interested in writing up an API proposal.

[vatara]

I have a similar need, I want the fixed filename, but don't care about the past logs. I made the change here: vatara@6bccc72

It adds a trimFileSizeBytes option. When the file reaches the fileSizeLimitBytes size, it trims the file down to the trimFileSizeBytes size keeping the end(most recent) part of the file. This way it's not trimming the log every time you write to it.

There's a couple issues with my changes. I used a int? for trimFileSizeBytes, instead of a long? like fileSizeLimitBytes, because when it truncates the file, FileStream.Read takes int, not long. I also had to remove readonly from a few FileSink members to allow closing and re-opening the stream.

It would be great to have a feature like this officially.

[joshuaboniface]

I'm also very interested in this. We're using Serilog file sink in https://github.com/jellyfin/jellyfin, and to package for Debian I'd like this to work like syslog does by default, that is, on-roll move jellyfin.log to jellyfin.log.0 (and up the chain), then continue writing to jellyfin.log. We would care about the history in this case. Is it possible to implement this with the current code/config options (the readme isn't clear), or would a feature be required?

[bartelink]

@joshuaboniface log4net etc also implement that renaming strategy. IME it can also be highly problematic - the race implicit in multiple writers is one reason, but also file locking from backups etc. can interfere with the process too. I'd suggest most other users feel the same (I don't recall this being asked for by anyone in the last year for instance) and hence the complexity introduced by having two modes would be unwarranted. The fact that the naming of the first file in the series is inconsistent would be nice to fix to make it more intuitive though.

[fragilerus]

I also need this. This is actually the only thing preventing me from moving from log4Net (FileBeats/Logstash configuration point to a static file). @bartelink while I agree that the rolling file/backup lock can be a cause for concern, I think the need for this probably outweighs the concern (IMO), which is why the other frameworks have implemented it. I, for one, would be ok with queuing the current to-be-rolled file with a temp file name (.pending?) until the rolling move of the other files succeeds.

[bartelink]

@fragilerus While I appreciate the desire and wouldn't moan too heavily if such a feature was added

a) I'd want to switch it off to reduce moving parts and/or things to go wrong (fragility if you'll pardon the pun)
b) I'd prefer the code to stay simple (adding another mode would make the code harder to test/read)

Having non-rolling filenames (serilog style) is a pretty normal thing for a forwarder to manage (and it means the forwarder doesn't need to fingerprint file internals in order to deduplicate them) - using the 'stable' filename also risks events being missed if the forwarder has an outage (if two renames take place)

I'd be really surprised if Filebeat can't be configured to work this way (my personal experience is based on splunk forwarders, which can handle it either way but are more efficient when using Serilog's stable naming scheme)

But, back to the point regarding this being nigh-on-impossible to do cleanly the other way: the key thing is that its very hard to correctly manage having multiple writers concurrently writing to a series of files [and trimming them]. The serilog scheme works well in that there is always a deterministically correct file name to write to, and the ripple of renaming can be gated by locking on that. This reality suggests to me that someone actually doing a PR for this is not terribly likely to happen.

[etreff-tillster]

Even if such a feature is full of pitfalls, I think it would be wise to open up the ability to let us specify our own path roller (or similar policy), either via interface or base class. The default, tested implementation can remain as is, while those using Serilog for VERY basic logging have the ability to change the naming strategy. This also moves such implementation details into the developer's court, removing all liability from Serilog's side.

[bartelink]

If it's worth doing, it's worth doing right. There is no liability except keeping things simple and not avoidably enabling people painting themselves into corners. This means that such a feature needs to be mapped out into a proper feature proposal; things like enabling the normal configuration system to be used to specify arguments shouldn't be sidestepped on this sort of basis.

An alternate path may simply be to fork the repo. Reasons to suggest this?:

• the logic and resulting behaviours necessitated by your desired name rolling strategy are fundamentally incompatible with the bulk of what is in general a relatively simple implementation (there are other needs like being able to tokenize the base name and/or path which are competing for complexity budget around here)
• I suspect that it will be difficult to define the full required API to enable such a thing while still having the existing tests meaningfully cover the possible interactions with such a general hook - doing that in a different context can ease that task

... but I may well be wrong - the .File sink is after all derived from .RollingFile, and there's no reason that, given the other behavior gains traction, that the changes can't then be brought back into here

[cocowalla]

@etreff-tillster this sink recently got the concept of FileLifecycleHooks - there might be an opportunity here to introduce a new extensibility point to customise file rolling.

If you're really interested in having this, perhaps you could create a fork and see if you can propose something?

[leonelcm]

Hi All, I would be interested in this feature as well. Do you know by any chance if this will pick up soon?
Thank you

[numito]

Hi Everyone,

I have made this feature, but I believe the way I made it is not the best. It's functional but hacky.
I added a new configuration option called "preserveLogFilename" to method "LoggerConfiguration File...".
On roll, the method will copy the file to the new filename and flush the current file to restart with an empty file. Filename configuration is preserved (aka naming: date, or increment etc..) So all other configuration options work the same.
I can push my fork if anyone is interested. It is in production since 4 months and we encountered no issue.

Regards,

Numa

[JinsPeter]

@numito How can I get this?

[poteb]

I'm gonna join with the others who need this feature. There are so many integrations that hook into a static file name, which will fail without this.
@numito Please share your fork so we can have a look at it.

[nblumhardt]

Thanks for all the feedback and discussion; I think @bartelink sums up the current situation and a good path forwards in #40 (comment)

⚠️ ⚠️ ⚠️ ⚠️ ⚠️
Folks, so that this issue doesn't just become a long, confusing trail of "me too"s (I know this is a desirable feature for many people), and to try something different, can we please have no further comments here except to notify everyone of an available fork with this functionality?
⚠️ ⚠️ ⚠️ ⚠️ ⚠️

If you're keen to show your support, the 👍 button on the original issue description above will register this.

Someone doing the work to create a fully functional, maintainable version of this feature is really the only way forward: there's no likelihood the maintainers will add this in the near future, but we'd be supportive of working through the details in a separate package, and it back into this project if they prove robust and supportable in real-world use. Thanks in advance to whoever can take this on, whether it's in this package or not, it would be a great contribution to the project.

[numito]

Hello, I can push my changes, what do you prefer a new fork on GitHub or a push request ? I have working code, that is deployed in production since 6 month. I think we can do better than this first version but at least it’s a step forward. Best regards <http://www.dfacto.ch/> Numa Schmeder www.dfacto.ch <http://www.dfacto.ch/> [email protected] <mailto:[email protected]> | M +41 79 538 30 01 DIGITAL STRATEGY | DESIGN | DEVELOPMENT

…

Le 28 févr. 2020 à 00:17, Nicholas Blumhardt ***@***.***> a écrit : Thanks for all the feedback and discussion; I think @bartelink <https://github.com/bartelink> sums up the current situation and a good path forwards in #40 (comment) <#40 (comment)> ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ Folks, so that this issue doesn't just become a long, confusing trail of "me too"s (I know this is a desirable feature for many people), and to try something different, can we please have no further comments here except to notify everyone of an available fork with this functionality? ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ If you're keen to show your support, the 👍 button on the original issue description above will register this. Someone doing the work to create a fully functional, maintainable version of this feature is really the only way forward: there's no likelihood the maintainers will add this in the near future, but we'd be supportive of working through the details in a separate package, and it back into this project if they prove robust and supportable in real-world use. Thanks in advance to whoever can take this on, whether it's in this package or not, it would be a great contribution to the project. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40?email_source=notifications&email_token=AAZXCF77ZEZEBJIWPQJUPPTRFBCYZA5CNFSM4EDUP6H2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENGKTRA#issuecomment-592226756>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZXCF7HXOHMHR4YC6THYX3RFBCYZANCNFSM4EDUP6HQ>.

[nblumhardt]

Hi @numito - thanks for your reply, this is great.

We woudn't currently accept this into Serilog.Sinks.File as a PR, so I think the best way to release it would be to create a standalone repository and package, e.g. numito/serilog-sinks-fixedfilename and Serilog.Sinks.FixedFilename, and publish your code there. That way you can accept and publish bug fixes and improvements as the package gets wider use.

Thanks again! 🎉

[numito]

Hi Guys, Sorry for the late answer I was pretty busy these days. I have pushed a new repository containing my changes on: https://github.com/dfacto-lab/serilog-sinks-file <https://github.com/dfacto-lab/serilog-sinks-file> Best regards, <http://www.dfacto.ch/> Numa Schmeder www.dfacto.ch <http://www.dfacto.ch/> [email protected] <mailto:[email protected]> | M +41 79 538 30 01 DIGITAL STRATEGY | DESIGN | DEVELOPMENT

…

Le 28 févr. 2020 à 09:08, Nicholas Blumhardt ***@***.***> a écrit : Hi @numito <https://github.com/numito> - thanks for your reply, this is great. We woudn't currently accept this into Serilog.Sinks.File as a PR, so I think the best way to release it would be to create a standalone repository and package, e.g. numito/serilog-sinks-fixedfilename and Serilog.Sinks.FixedFilename, and publish your code there. That way you can accept and publish bug fixes and improvements as the package gets wider use. Thanks again! 🎉 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40?email_source=notifications&email_token=AAZXCF36Q2LXNO4IJ7KGFBDRFDA6ZA5CNFSM4EDUP6H2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENHUUXA#issuecomment-592398940>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZXCFYUD6IX6IYHGI75BZTRFDA6ZANCNFSM4EDUP6HQ>.

[nblumhardt]

Thanks @numito! It looks like there are still some TODOs before it will be consumable, like renaming the package so that there's no name conflict on NuGet/in apps; should others following this thread pop over there and write up some issues/send some PRs?

[Mart124]

Someone doing the work to create a fully functional, maintainable version of this feature is really the only way forward: there's no likelihood the maintainers will add this in the near future, but we'd be supportive of working through the details in a separate package, and it back into this project if they prove robust and supportable in real-world use.

We woudn't currently accept this into Serilog.Sinks.File as a PR, so I think the best way to release it would be to create a standalone repository and package

Why ? 😕
If the current original behavior is kept untouched, but the new rolling method discussed here can be enabled thanks to an option, then there's no risk ?
Proper CI tests could be written too.
We would then stick to your own package, rather than going with an "unknown" one, which unlikely we will, you would then get almost no report...
I think here we all want to have this as an option in this current package :)
This makes sense.
Even flagged as a beta option in the first release, we don't mind.
Thank you again 👍

[nblumhardt]

Hi @Mart124

I think here we all want to have this as an option in this current package

The reasons everyone wants the feature in this package are:

• The maintainers do the work to make sure every feature in this package works reliably, and
• The maintainers put in the nights and weekends to deal with all the testing, docs, edge cases, user questions, etc.

As things stand right now, we could merge a "beta" proposal, but then, the quality bar would be dropped for this package, and we wouldn't have a confident path forwards for users who depended on it expecting the usual quality bar.

If you'd like to help move this forward, the best way is to grab the forked package mentioned above, try it, fix issues you find in it, and keep sending feedback so that others know about it and can do the same.

[numito]

Hi, I am not sure what issues you found. But the change is backward compatible and keeps the same api, it also has an additional test case for the feature and all tests are passing. Could you clarify what is the issue ? Best regards, <http://www.dfacto.ch/> Numa Schmeder www.dfacto.ch <http://www.dfacto.ch/> [email protected] <mailto:[email protected]> | M +41 79 538 30 01 DIGITAL STRATEGY | DESIGN | DEVELOPMENT

…

Le 1 juin 2020 à 03:34, Nicholas Blumhardt ***@***.***> a écrit : Hi @Mart124 <https://github.com/Mart124> I think here we all want to have this as an option in this current package The reasons everyone wants the feature in this package are: The maintainers do the work to make sure every feature in this package works reliably, and The maintainers put in the nights and weekends to deal with all the testing, docs, edge cases, user questions, etc. As things stand right now, we could merge a "beta" proposal, but then, the quality bar would be dropped for this package, and we wouldn't have a confident path forwards for users who depended on it expecting the usual quality bar. If you'd like to help move this forward, the best way is to grab the forked package mentioned above, try it, fix issues you find in it, and keep sending feedback so that others know about it and can do the same. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZXCF4ZLLBLMGFK57BUVETRUMAQTANCNFSM4EDUP6HQ>.

[nblumhardt]

Hi @numito !

It's a perfectly good implementation for people who want this feature; hence my encouraging people to give it a try, sorry if my hasty comments above gave you the wrong impression.

To handle the kind of use this package gets, it'll need much more testing in different configurations, though.

For example, I just had another skim through, and in shared mode, in:

https://github.com/dfacto-lab/serilog-sinks-file/blob/b396b047d0ce0674127e7a917e87b4ef2b30ca94/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs#L149

it looks like there's a race condition between the Exists(), construction of FileInfo, and the later Move() call, which will result in dropped events when two processes concurrently attempt to roll the file (the resulting exception will be due to a missing file and not a locked file).

Nothing huge to worry about, and especially if you're not using it in shared mode, but these are the kinds of tricky things to flush out and debug that take a lot of time, given the absence of transactional file system APIs, and differences between platforms.

Shipping the forked sink is a great way to do that - thanks again for rolling up your sleeves and making it happen! Please don't interpret my resistance to bringing those changes in as disparagement - it's a great starting point and enables everyone wanting this functionality to contribute to a solid implementation.

[numito]

Hi @nblumhardt, Yes indeed if it’s used in shared mode we should lock while moving the file, to avoid a race condition, this could be done by locking this portion of the code using a monitor. I can add it, if people need it. If there is anything else, just drop me a line. Best <http://www.dfacto.ch/> Numa Schmeder www.dfacto.ch <http://www.dfacto.ch/> [email protected] <mailto:[email protected]> | M +41 79 538 30 01 DIGITAL STRATEGY | DESIGN | DEVELOPMENT

…

Le 3 juin 2020 à 01:04, Nicholas Blumhardt ***@***.***> a écrit : Hi @numito <https://github.com/numito> ! It's a perfectly good implementation for people who want this feature; hence my encouraging people to give it a try, sorry if my hasty comments above gave you the wrong impression. To handle the kind of use this package gets, it'll need much more testing in different configurations, though. For example, I just had another skim through, and in shared mode, in: https://github.com/dfacto-lab/serilog-sinks-file/blob/b396b047d0ce0674127e7a917e87b4ef2b30ca94/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs#L149 <https://github.com/dfacto-lab/serilog-sinks-file/blob/b396b047d0ce0674127e7a917e87b4ef2b30ca94/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs#L149> it looks like there's a race condition between the Exists(), construction of FileInfo, and the later Move() call, which will result in dropped events when two processes concurrently attempt to roll the file (the resulting exception will be due to a missing file and not a locked file). Nothing huge to worry about, and especially if you're not using it in shared mode, but these are the kinds of tricky things to flush out and debug that take a lot of time to flush out, given the absence of transactional file system APIs, and differences between platforms. Shipping the forked sink is a great way to do that - thanks again for rolling up your sleeves and making it happen! Please don't interpret my resistance to bringing those changes in as disparagement - it's a great starting point and enables everyone wanting this functionality to contribute to a solid implementation. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZXCF6KBI4LA3ZZGF4QI2TRUWAPFANCNFSM4EDUP6HQ>.

[SpeedSX]

Hi @numito,

I tried implementation you provided and got into the issue when latest file is already overflowed.
For example, with this code:

var log = new LoggerConfiguration()`
            .WriteTo.File("log.txt", fileSizeLimitBytes: 1000, rollOnFileSizeLimit: true, preserveLogFilename: true)
            .CreateLogger();
var longString = new String('0', 1000);
log.Information(longString);

After two runs, when log.txt and log_001.txt both exist and are 1039 bytes each, it refuses to log with exception:
System.IO.IOException: 'Cannot create a file when that file already exists.'

Console application, target .NET 4.6.2

[numito]

Hi, That’s strange, I will investigate. It uses the same rolling mechanism as previously. It should detect an existing file and increment.

…

Le 3 juin 2020 à 14:37, speedsx ***@***.***> a écrit : Hi @numito <https://github.com/numito>, I tried implementation you provided and got into the issue when latest file is already overflowed. For example, with this code: var log = new LoggerConfiguration()` .WriteTo.File("log.txt", fileSizeLimitBytes: 1000, rollOnFileSizeLimit: true, preserveLogFilename: true) .CreateLogger(); var longString = new String('0', 1000); log.Information(longString); After two runs, when log.txt and log_001.txt both exist and are 1039 bytes each, it refuses to log with exception: System.IO.IOException: 'Cannot create a file when that file already exists.' — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZXCF2ZZS4LJQ7ZCPE4OMLRUY7YDANCNFSM4EDUP6HQ>.

[nblumhardt]

Cool, great to see some feedback coming through!

Tracking issues in https://github.com/dfacto-lab/serilog-sinks-file/issues will avoid a lot of noise here. I think to conserve time/energy here we should close this thread as on-hold; it would be great to catch up on how things are going in a few months.

[wardboumans]

Retarted this is still not implemented. Im using serilog since somehow it seems to be the standard lib for .netcore. But as a NLog user for years in fullframework, im going back to NLog.

[nblumhardt]

@wardboumans Hello Ward; you're welcome to contribute to the work, via the project at https://github.com/dfacto-lab/serilog-sinks-file, if this feature is important to you. Please ensure your comments follow our code of conduct, if you wish to participate in discussions here. Thank you.

[numito]

Hi @numito,

I tried implementation you provided and got into the issue when latest file is already overflowed.
For example, with this code:

var log = new LoggerConfiguration()`
            .WriteTo.File("log.txt", fileSizeLimitBytes: 1000, rollOnFileSizeLimit: true, preserveLogFilename: true)
            .CreateLogger();
var longString = new String('0', 1000);
log.Information(longString);

After two runs, when log.txt and log_001.txt both exist and are 1039 bytes each, it refuses to log with exception:
System.IO.IOException: 'Cannot create a file when that file already exists.'

Console application, target .NET 4.6.2

Hi @SpeedSX

I think I found the issue, i will post an update during the day.

[nblumhardt]

Hi all; it would be great if issues can be discussed/tracked over in https://github.com/dfacto-lab/serilog-sinks-file/issues - watching that repository will be the best way to keep up :-)