File retention policy by date/time #90

thiagosgarcia · 2019-03-14T00:03:09Z

Add configuration to discard files older than X time defined by a TimeSpan variable.

As discussed in issue #39, I think (and it did work for me) that would be nice to have a dateTime-driven file retention policy.
There are some security policies that, for example, demands that all log files should be wiped after a few days or weeks. At the same time, you can have as much as possible data in logs, so, for some cases, the strongest rule for deleting log files would be date, not file count or size.

In the example below, files older than 7 days are deleted by RollingFileSink.ApplyRetentionPolicy(_) at a maximum count of 100 files, rolling by day or at a limit of 50MB per file

  "Serilog": {
    "Using": [ "Serilog.Sinks.File" ],
    "MinimumLevel": "Debug",
    "WriteTo": [
      { "Name": "Console" },
      {
        "Name": "File",
        "Args": {
          "path": "App_Data\\Logs\\app_.log",
          "rollingInterval": "Day",
          "fileSizeLimitBytes": "52428800",
          "rollOnFileSizeLimit": "true",
          "retainedFileCountLimit": "100",
          "retainedFileTimeLimit": "7.00:00:00", //Deletes files older than 7 days
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj}{NewLine}{Exception}"
        }
      }
    ]
  },

nblumhardt

Sorry about the slow response, here! Thanks for taking the time to send this.

It seems like a nice, clean, change; there's a bit to digest. I can see how there could be some 'jitter' around the roll points if events are logged out-of-order and there's a very short/zero retention time; this issue is present with very small rolling sets, too, though.

One glitch is that if the app stops logging, the policy will not be triggered and the old files will be kept - may not be a major problem (the same thing occurs if the app is shut down :-)) but worth considering, perhaps with a note.

If you're keen to push ahead with this one, it'd be great also to include a couple of tests that exercise it.

Thanks again!

src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs

src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs

thiagosgarcia · 2019-04-23T15:35:20Z

Thanks a lot for your review @nblumhardt
I'm going to push the suggestions you made as soon as I can.

thiagosgarcia · 2019-04-30T18:11:54Z

@nblumhardt, thanks again for the review and suggestions. I pushed the new code including your suggestions, but I just noticed that I didn't updated my forked code with the new FileLifecycleHooks parameter on the constructors. I'm going to fix it later tonight, then I push again.

Update Fork

thiagosgarcia · 2019-04-30T20:37:05Z

Just rebased my changes onto the upstream, fixed conflicts and ran unit and manual tests here.
If there's any other thoughts, I'll be glad to hear.
Cheers!

…der than X time Fixed constructors for compatibility; Unified where clause to retain files Adjust error messages New test cases

nblumhardt

Looking great! Thanks for the update, sorry it's taken so long to get some eyes on this.

Had a couple of points to consider - should be getting down to the last of it now :-)

src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs

src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs

nblumhardt · 2019-05-24T04:16:56Z

src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs

-                .Where(n => StringComparer.OrdinalIgnoreCase.Compare(currentFileName, n) != 0)
-                .Skip(_retainedFileCountLimit.Value - 1)
+                .Where(n => StringComparer.OrdinalIgnoreCase.Compare(currentFileName, n.Filename) != 0)
+                .SkipWhile((x, i) => (i < (_retainedFileCountLimit - 1 ?? 0)) &&


The default of 0 for a null _retainedFileCountLimit seems wrong here, since i will never be less than it (null is intended to indicate "no limit").

Could pulling this expression out into a local function (with some locals, or structured return (true|false) make the logic a bit clearer?

I put 0 there, thinking that if retainedFileCountLimit got there as null, then I'll ignore this condition. Do you think maybe 31 (the default value) would be better instead (in case of null)?

I'll extract a function to get this piece of code clearer. I also didn't really like it the way it is.

I think it'll be hard to get the logic right here without checking retainedFileCountLimit against null, but guessing this will all come out more clearly with some reorganisation :-)

thiagosgarcia · 2019-05-24T20:44:41Z

No worries =)
Gonna push this changes as soon as I can.

thiagosgarcia · 2019-06-01T21:14:15Z

@nblumhardt I was able to push the requested changes. If there's anything else, please, let me know and I'll correct right away.

I did give some thoughts on that SkipWhile filter, and tried to create a more self-explanatory function, hope it's good to ship now. =)

iamandymcinnes · 2019-07-29T13:54:09Z

Just wondered if there is any update on getting this PR merged?

nblumhardt · 2019-07-30T02:26:29Z

Thanks for the nudge, @iamandymcinnes - I'm keen to merge this PR, I need to sanity check myself before pushing the button (there's a binary breaking change which might necessitate downstream changes, too), I'll try to loop back around ASAP, if no one else beats me to it :-)

ericvb · 2019-09-06T05:42:45Z

Same question as @iamandymcinnes, what is the status of this PR?

pnstickne · 2019-10-31T00:09:16Z

Bump. The file-count-only stock retention policy for files is very limiting. Services / programs that restart many times in a short period of time (and create a new file each time without log sharing) run the risk of losing logs.

There is probably further consideration on how to handle 'groups of' log files in general, including PID/AppDomain-time distinction to avoid shared files.

nblumhardt · 2019-11-01T04:32:30Z

@pnstickne @ericvb thanks for the bump; sorry again about the delay, @thiagosgarcia - I'll get there! 😅

Paul, check out https://github.com/serilog/serilog-sinks-map as one way to address the file-set-per-PID type scenarios (assuming you've tried and ruled out passing shared: true to your file sink).

asalhani · 2020-02-03T12:25:51Z

@thiagosgarcia Thank you for your effort to include this as a feature.

@nblumhardt Any update on this PR? Any expected time when it will be closed?

…onals

nblumhardt · 2020-02-04T06:34:07Z

Fixed my test breakage locally on the way through the works :-)

nblumhardt · 2020-02-04T06:54:38Z

Out in the 5.0-dev build 🎉 - thanks @thiagosgarcia :-)

thiagosgarcia · 2020-02-04T13:50:13Z

Thank you @nblumhardt! If there's anything else I can do, please let me know =)

ThilinaJ · 2020-11-05T11:55:06Z

Hello,
With what release will this feature be released on NuGet?

abrasat · 2021-12-10T14:30:24Z

@thiagosgarcia @nblumhardt
Is this feature already available ? I would like to use serilog to log for lets say 30 days, but the daily files should be split in several files, each of them with a max size of for example 50 MB (yes, my application is logging a lot, especially during commissioning).
It is important that no log data gets lost. Is this behavior possible with the actual serilog file sink? If yes, can you please provide a c# configuration sample for .NET 6? Thanks!

bartelink · 2021-12-10T16:43:03Z

@abrasat per https://www.nuget.org/packages/serilog.sinks.file it is released. If you're having trouble configuring it, the best thing to do is to raise an issue on stackoverflow.com tagged serilog showing what you've tried so far. In general, usage issues don't get covered via these GitHub Issues - one glance at the download numbers should convince you that this would never scale.

Youssef-Ehab77 · 2023-10-04T13:06:09Z

Hello,
I used retainedFileTimeLimit: TimeSpan.FromMinutes(10), but I'm facing weird behavior, it deletes the old file once it reaches its fileSizeLimitBytes: 10000 and creates a new file even thought I'm still inside the 10 minutes time frame and my retainedFileCountLimit: 50 and rollingInterval: RollingInterval.Day. But when I make retainedFileTimeLimit: TimeSpan.FromDays(10) it creates a new file once the file size limit exceeds and it doesn't delete the old file.
also posted at: Stack overflow Link
@bartelink Thanks for your comments and help ❤

thiagosgarcia mentioned this pull request Mar 14, 2019

Date-driven (rather than count-driven) file retention option #39

Closed

thiagosgarcia changed the title ~~File retention policy by date~~ File retention policy by date/time Mar 14, 2019

nblumhardt reviewed Apr 20, 2019

View reviewed changes

nblumhardt mentioned this pull request Apr 22, 2019

4.1.0 Release #78

Merged

thiagosgarcia closed this Apr 30, 2019

thiagosgarcia reopened this Apr 30, 2019

thiagosgarcia closed this Apr 30, 2019

Merge pull request #1 from serilog/dev

ab568af

Update Fork

thiagosgarcia reopened this Apr 30, 2019

File retention policy by date - Add configuration to discard files ol…

1596f80

…der than X time Fixed constructors for compatibility; Unified where clause to retain files Adjust error messages New test cases

nblumhardt requested changes May 24, 2019

View reviewed changes

Refactoring file filter; fixing docs

b213a7a

nblumhardt added 2 commits February 4, 2020 16:10

Remove explicit private modifier; name tweak; reorganize some conditi…

1b863ff

…onals

Merge branch 'dev' into dev

00e3e94

nblumhardt approved these changes Feb 4, 2020

View reviewed changes

nblumhardt merged commit 00e3e94 into serilog:dev Feb 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File retention policy by date/time #90

File retention policy by date/time #90

thiagosgarcia commented Mar 14, 2019 •

edited

Loading

nblumhardt left a comment

thiagosgarcia commented Apr 23, 2019

thiagosgarcia commented Apr 30, 2019 •

edited

Loading

thiagosgarcia commented Apr 30, 2019

nblumhardt left a comment

nblumhardt May 24, 2019

thiagosgarcia May 24, 2019 •

edited

Loading

nblumhardt May 24, 2019

thiagosgarcia commented May 24, 2019

thiagosgarcia commented Jun 1, 2019

iamandymcinnes commented Jul 29, 2019

nblumhardt commented Jul 30, 2019

ericvb commented Sep 6, 2019

pnstickne commented Oct 31, 2019 •

edited

Loading

nblumhardt commented Nov 1, 2019

asalhani commented Feb 3, 2020

nblumhardt commented Feb 4, 2020

nblumhardt commented Feb 4, 2020

thiagosgarcia commented Feb 4, 2020

ThilinaJ commented Nov 5, 2020

abrasat commented Dec 10, 2021

bartelink commented Dec 10, 2021

Youssef-Ehab77 commented Oct 4, 2023 •

edited

Loading

File retention policy by date/time #90

File retention policy by date/time #90

Conversation

thiagosgarcia commented Mar 14, 2019 • edited Loading

nblumhardt left a comment

Choose a reason for hiding this comment

thiagosgarcia commented Apr 23, 2019

thiagosgarcia commented Apr 30, 2019 • edited Loading

thiagosgarcia commented Apr 30, 2019

nblumhardt left a comment

Choose a reason for hiding this comment

nblumhardt May 24, 2019

Choose a reason for hiding this comment

thiagosgarcia May 24, 2019 • edited Loading

Choose a reason for hiding this comment

nblumhardt May 24, 2019

Choose a reason for hiding this comment

thiagosgarcia commented May 24, 2019

thiagosgarcia commented Jun 1, 2019

iamandymcinnes commented Jul 29, 2019

nblumhardt commented Jul 30, 2019

ericvb commented Sep 6, 2019

pnstickne commented Oct 31, 2019 • edited Loading

nblumhardt commented Nov 1, 2019

asalhani commented Feb 3, 2020

nblumhardt commented Feb 4, 2020

nblumhardt commented Feb 4, 2020

thiagosgarcia commented Feb 4, 2020

ThilinaJ commented Nov 5, 2020

abrasat commented Dec 10, 2021

bartelink commented Dec 10, 2021

Youssef-Ehab77 commented Oct 4, 2023 • edited Loading

thiagosgarcia commented Mar 14, 2019 •

edited

Loading

thiagosgarcia commented Apr 30, 2019 •

edited

Loading

thiagosgarcia May 24, 2019 •

edited

Loading

pnstickne commented Oct 31, 2019 •

edited

Loading

Youssef-Ehab77 commented Oct 4, 2023 •

edited

Loading