From 1596f80d66c65ca0cb22358dbaeaa871321a7b99 Mon Sep 17 00:00:00 2001 From: Thiago Garcia Date: Tue, 30 Apr 2019 09:37:12 -0300 Subject: [PATCH 1/3] File retention policy by date - Add configuration to discard files older than X time Fixed constructors for compatibility; Unified where clause to retain files Adjust error messages New test cases --- .../FileLoggerConfigurationExtensions.cs | 36 ++++++++---- .../Sinks/File/RollingFileSink.cs | 25 +++++--- .../RollingFileSinkTests.cs | 57 +++++++++++++++++++ 3 files changed, 99 insertions(+), 19 deletions(-) diff --git a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs index 490803d..c735437 100644 --- a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs +++ b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs @@ -138,6 +138,7 @@ public static LoggerConfiguration File( /// including the current log file. For unlimited retention, pass null. The default is 31. /// Character encoding used to write the text file. The default is UTF-8 without BOM. /// Configuration object allowing method chaining. + /// The file will be written using the UTF-8 character set. [Obsolete("New code should not be compiled against this obsolete overload"), EditorBrowsable(EditorBrowsableState.Never)] public static LoggerConfiguration File( this LoggerSinkConfiguration sinkConfiguration, @@ -165,7 +166,7 @@ public static LoggerConfiguration File( /// Logger sink configuration. /// A formatter, such as , to convert the log events into /// text for the file. If control of regular text formatting is required, use the other - /// overload of + /// overload of /// and specify the outputTemplate parameter instead. /// /// Path to the file. @@ -181,7 +182,7 @@ public static LoggerConfiguration File( /// Allow the log file to be shared by multiple processes. The default is false. /// If provided, a full disk flush will be performed periodically at the specified interval. /// The interval at which logging will roll over to a new file. - /// If true, a new file will be created when the file size limit is reached. Filenames + /// If true, a new file will be created when the file size limit is reached. Filenames /// will have a number appended in the format _NNN, with the first filename given no number. /// The maximum number of log files that will be retained, /// including the current log file. For unlimited retention, pass null. The default is 31. @@ -227,13 +228,18 @@ public static LoggerConfiguration File( /// Allow the log file to be shared by multiple processes. The default is false. /// If provided, a full disk flush will be performed periodically at the specified interval. /// The interval at which logging will roll over to a new file. - /// If true, a new file will be created when the file size limit is reached. Filenames + /// If true, a new file will be created when the file size limit is reached. Filenames /// will have a number appended in the format _NNN, with the first filename given no number. /// The maximum number of log files that will be retained, /// including the current log file. For unlimited retention, pass null. The default is 31. /// Character encoding used to write the text file. The default is UTF-8 without BOM. /// Optionally enables hooking into log file lifecycle events. + /// The maximum time after the end of an interval that a rolling log file will be retained. + /// Must be greater than or equal to . + /// Ignored if is . + /// The default is to retain files indefinitely. /// Configuration object allowing method chaining. + /// The file will be written using the UTF-8 character set. public static LoggerConfiguration File( this LoggerSinkConfiguration sinkConfiguration, string path, @@ -249,7 +255,8 @@ public static LoggerConfiguration File( bool rollOnFileSizeLimit = false, int? retainedFileCountLimit = DefaultRetainedFileCountLimit, Encoding encoding = null, - FileLifecycleHooks hooks = null) + FileLifecycleHooks hooks = null, + TimeSpan? retainedFileTimeLimit = null) { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); if (path == null) throw new ArgumentNullException(nameof(path)); @@ -258,7 +265,7 @@ public static LoggerConfiguration File( var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider); return File(sinkConfiguration, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, levelSwitch, buffered, shared, flushToDiskInterval, - rollingInterval, rollOnFileSizeLimit, retainedFileCountLimit, encoding, hooks); + rollingInterval, rollOnFileSizeLimit, retainedFileCountLimit, encoding, hooks, retainedFileTimeLimit); } /// @@ -267,7 +274,7 @@ public static LoggerConfiguration File( /// Logger sink configuration. /// A formatter, such as , to convert the log events into /// text for the file. If control of regular text formatting is required, use the other - /// overload of + /// overload of /// and specify the outputTemplate parameter instead. /// /// Path to the file. @@ -289,6 +296,10 @@ public static LoggerConfiguration File( /// including the current log file. For unlimited retention, pass null. The default is 31. /// Character encoding used to write the text file. The default is UTF-8 without BOM. /// Optionally enables hooking into log file lifecycle events. + /// The maximum time after the end of an interval that a rolling log file will be retained. + /// Must be greater than or equal to . + /// Ignored if is . + /// The default is to retain files indefinitely. /// Configuration object allowing method chaining. public static LoggerConfiguration File( this LoggerSinkConfiguration sinkConfiguration, @@ -304,7 +315,8 @@ public static LoggerConfiguration File( bool rollOnFileSizeLimit = false, int? retainedFileCountLimit = DefaultRetainedFileCountLimit, Encoding encoding = null, - FileLifecycleHooks hooks = null) + FileLifecycleHooks hooks = null, + TimeSpan? retainedFileTimeLimit = null) { if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); @@ -312,7 +324,7 @@ public static LoggerConfiguration File( return ConfigureFile(sinkConfiguration.Sink, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, levelSwitch, buffered, false, shared, flushToDiskInterval, encoding, rollingInterval, rollOnFileSizeLimit, - retainedFileCountLimit, hooks); + retainedFileCountLimit, hooks, retainedFileTimeLimit); } /// @@ -432,7 +444,7 @@ public static LoggerConfiguration File( if (path == null) throw new ArgumentNullException(nameof(path)); return ConfigureFile(sinkConfiguration.Sink, formatter, path, restrictedToMinimumLevel, null, levelSwitch, false, true, - false, null, encoding, RollingInterval.Infinite, false, null, hooks); + false, null, encoding, RollingInterval.Infinite, false, null, hooks, null); } static LoggerConfiguration ConfigureFile( @@ -450,13 +462,15 @@ static LoggerConfiguration ConfigureFile( RollingInterval rollingInterval, bool rollOnFileSizeLimit, int? retainedFileCountLimit, - FileLifecycleHooks hooks) + FileLifecycleHooks hooks, + TimeSpan? retainedFileTimeLimit) { if (addSink == null) throw new ArgumentNullException(nameof(addSink)); if (formatter == null) throw new ArgumentNullException(nameof(formatter)); if (path == null) throw new ArgumentNullException(nameof(path)); if (fileSizeLimitBytes.HasValue && fileSizeLimitBytes < 0) throw new ArgumentException("Negative value provided; file size limit must be non-negative.", nameof(fileSizeLimitBytes)); if (retainedFileCountLimit.HasValue && retainedFileCountLimit < 1) throw new ArgumentException("At least one file must be retained.", nameof(retainedFileCountLimit)); + if (retainedFileTimeLimit.HasValue && retainedFileTimeLimit < TimeSpan.Zero) throw new ArgumentException("Negative value provided; retained file time limit must be non-negative.", nameof(retainedFileTimeLimit)); if (shared && buffered) throw new ArgumentException("Buffered writes are not available when file sharing is enabled.", nameof(buffered)); if (shared && hooks != null) throw new ArgumentException("File lifecycle hooks are not currently supported for shared log files.", nameof(hooks)); @@ -464,7 +478,7 @@ static LoggerConfiguration ConfigureFile( if (rollOnFileSizeLimit || rollingInterval != RollingInterval.Infinite) { - sink = new RollingFileSink(path, formatter, fileSizeLimitBytes, retainedFileCountLimit, encoding, buffered, shared, rollingInterval, rollOnFileSizeLimit, hooks); + sink = new RollingFileSink(path, formatter, fileSizeLimitBytes, retainedFileCountLimit, encoding, buffered, shared, rollingInterval, rollOnFileSizeLimit, hooks, retainedFileTimeLimit); } else { diff --git a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs index 2db6f24..7d34453 100644 --- a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs +++ b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs @@ -20,6 +20,7 @@ using Serilog.Debugging; using Serilog.Events; using Serilog.Formatting; +using System.Collections.Generic; namespace Serilog.Sinks.File { @@ -29,6 +30,7 @@ sealed class RollingFileSink : ILogEventSink, IFlushableFileSink, IDisposable readonly ITextFormatter _textFormatter; readonly long? _fileSizeLimitBytes; readonly int? _retainedFileCountLimit; + readonly TimeSpan? _retainedFileTimeLimit; readonly Encoding _encoding; readonly bool _buffered; readonly bool _shared; @@ -50,16 +52,19 @@ public RollingFileSink(string path, bool shared, RollingInterval rollingInterval, bool rollOnFileSizeLimit, - FileLifecycleHooks hooks) + FileLifecycleHooks hooks, + TimeSpan? retainedFileTimeLimit) { if (path == null) throw new ArgumentNullException(nameof(path)); - if (fileSizeLimitBytes.HasValue && fileSizeLimitBytes < 0) throw new ArgumentException("Negative value provided; file size limit must be non-negative."); - if (retainedFileCountLimit.HasValue && retainedFileCountLimit < 1) throw new ArgumentException("Zero or negative value provided; retained file count limit must be at least 1."); + if (fileSizeLimitBytes.HasValue && fileSizeLimitBytes < 0) throw new ArgumentException("Negative value provided; file size limit must be non-negative"); + if (retainedFileCountLimit.HasValue && retainedFileCountLimit < 1) throw new ArgumentException("Zero or negative value provided; retained file count limit must be at least 1"); + if (retainedFileTimeLimit.HasValue && retainedFileTimeLimit < TimeSpan.Zero) throw new ArgumentException("Negative value provided; retained file time limit must be non-negative.", nameof(retainedFileTimeLimit)); _roller = new PathRoller(path, rollingInterval); _textFormatter = textFormatter; _fileSizeLimitBytes = fileSizeLimitBytes; _retainedFileCountLimit = retainedFileCountLimit; + _retainedFileTimeLimit = retainedFileTimeLimit; _encoding = encoding; _buffered = buffered; _shared = shared; @@ -173,7 +178,7 @@ void OpenFile(DateTime now, int? minSequence = null) void ApplyRetentionPolicy(string currentFilePath) { - if (_retainedFileCountLimit == null) return; + if (_retainedFileCountLimit == null && _retainedFileTimeLimit == null) return; var currentFileName = Path.GetFileName(currentFilePath); @@ -181,17 +186,21 @@ void ApplyRetentionPolicy(string currentFilePath) // because files are only opened on response to an event being processed. var potentialMatches = Directory.GetFiles(_roller.LogFileDirectory, _roller.DirectorySearchPattern) .Select(Path.GetFileName) - .Union(new [] { currentFileName }); + .Union(new[] { currentFileName }); var newestFirst = _roller .SelectMatches(potentialMatches) .OrderByDescending(m => m.DateTime) .ThenByDescending(m => m.SequenceNumber) - .Select(m => m.Filename); + .Select(m => new { m.Filename, m.DateTime }); var toRemove = newestFirst - .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)) && + (!_retainedFileTimeLimit.HasValue || + x.DateTime.HasValue && + DateTime.Now.Subtract(_retainedFileTimeLimit.Value).CompareTo(x.DateTime.Value) <= 0)) + .Select(x => x.Filename) .ToList(); foreach (var obsolete in toRemove) diff --git a/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs b/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs index 2e9f613..746d587 100644 --- a/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs +++ b/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs @@ -71,6 +71,63 @@ public void WhenRetentionCountIsSetOldFilesAreDeleted() }); } + [Fact] + public void WhenRetentionTimeIsSetOldFilesAreDeleted() + { + LogEvent e1 = Some.InformationEvent(DateTime.Today.AddDays(-5)), + e2 = Some.InformationEvent(e1.Timestamp.AddDays(2)), + e3 = Some.InformationEvent(e2.Timestamp.AddDays(5)); + + TestRollingEventSequence( + (pf, wt) => wt.File(pf, retainedFileTimeLimit: TimeSpan.FromDays(1), rollingInterval: RollingInterval.Day), + new[] {e1, e2, e3}, + files => + { + Assert.Equal(3, files.Count); + Assert.True(!System.IO.File.Exists(files[0])); + Assert.True(!System.IO.File.Exists(files[1])); + Assert.True(System.IO.File.Exists(files[2])); + }); + } + + [Fact] + public void WhenRetentionCountAndTimeIsSetOldFilesAreDeletedByTime() + { + LogEvent e1 = Some.InformationEvent(DateTime.Today.AddDays(-5)), + e2 = Some.InformationEvent(e1.Timestamp.AddDays(2)), + e3 = Some.InformationEvent(e2.Timestamp.AddDays(5)); + + TestRollingEventSequence( + (pf, wt) => wt.File(pf, retainedFileCountLimit: 2, retainedFileTimeLimit: TimeSpan.FromDays(1), rollingInterval: RollingInterval.Day), + new[] {e1, e2, e3}, + files => + { + Assert.Equal(3, files.Count); + Assert.True(!System.IO.File.Exists(files[0])); + Assert.True(!System.IO.File.Exists(files[1])); + Assert.True(System.IO.File.Exists(files[2])); + }); + } + + [Fact] + public void WhenRetentionCountAndTimeIsSetOldFilesAreDeletedByCount() + { + LogEvent e1 = Some.InformationEvent(DateTime.Today.AddDays(-5)), + e2 = Some.InformationEvent(e1.Timestamp.AddDays(2)), + e3 = Some.InformationEvent(e2.Timestamp.AddDays(5)); + + TestRollingEventSequence( + (pf, wt) => wt.File(pf, retainedFileCountLimit: 2, retainedFileTimeLimit: TimeSpan.FromDays(10), rollingInterval: RollingInterval.Day), + new[] {e1, e2, e3}, + files => + { + Assert.Equal(3, files.Count); + Assert.True(!System.IO.File.Exists(files[0])); + Assert.True(System.IO.File.Exists(files[1])); + Assert.True(System.IO.File.Exists(files[2])); + }); + } + [Fact] public void WhenSizeLimitIsBreachedNewFilesCreated() { From b213a7a8481c0f9033d0c129078c829c9cbb4187 Mon Sep 17 00:00:00 2001 From: Thiago Garcia Date: Sat, 1 Jun 2019 18:07:59 -0300 Subject: [PATCH 2/3] Refactoring file filter; fixing docs --- .../FileLoggerConfigurationExtensions.cs | 2 -- .../Sinks/File/RollingFileSink.cs | 19 +++++++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs index c735437..ad6b80b 100644 --- a/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs +++ b/src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs @@ -138,7 +138,6 @@ public static LoggerConfiguration File( /// including the current log file. For unlimited retention, pass null. The default is 31. /// Character encoding used to write the text file. The default is UTF-8 without BOM. /// Configuration object allowing method chaining. - /// The file will be written using the UTF-8 character set. [Obsolete("New code should not be compiled against this obsolete overload"), EditorBrowsable(EditorBrowsableState.Never)] public static LoggerConfiguration File( this LoggerSinkConfiguration sinkConfiguration, @@ -239,7 +238,6 @@ public static LoggerConfiguration File( /// Ignored if is . /// The default is to retain files indefinitely. /// Configuration object allowing method chaining. - /// The file will be written using the UTF-8 character set. public static LoggerConfiguration File( this LoggerSinkConfiguration sinkConfiguration, string path, diff --git a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs index 7d34453..0e61954 100644 --- a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs +++ b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs @@ -191,15 +191,11 @@ void ApplyRetentionPolicy(string currentFilePath) var newestFirst = _roller .SelectMatches(potentialMatches) .OrderByDescending(m => m.DateTime) - .ThenByDescending(m => m.SequenceNumber) - .Select(m => new { m.Filename, m.DateTime }); + .ThenByDescending(m => m.SequenceNumber); var toRemove = newestFirst .Where(n => StringComparer.OrdinalIgnoreCase.Compare(currentFileName, n.Filename) != 0) - .SkipWhile((x, i) => (i < (_retainedFileCountLimit - 1 ?? 0)) && - (!_retainedFileTimeLimit.HasValue || - x.DateTime.HasValue && - DateTime.Now.Subtract(_retainedFileTimeLimit.Value).CompareTo(x.DateTime.Value) <= 0)) + .SkipWhile(FilterFiles) .Select(x => x.Filename) .ToList(); @@ -217,6 +213,17 @@ void ApplyRetentionPolicy(string currentFilePath) } } + private bool FilterFiles(RollingLogFile file, int index) + { + var isInCountLimit = index < (_retainedFileCountLimit - 1 ?? 0); + + var isInTimeLimit = !_retainedFileTimeLimit.HasValue; + if (_retainedFileTimeLimit.HasValue && file.DateTime.HasValue) + isInTimeLimit = DateTime.Now.Subtract(_retainedFileTimeLimit.Value).CompareTo(file.DateTime.Value) <= 0; + + return isInCountLimit && isInTimeLimit; + } + public void Dispose() { lock (_syncRoot) From 1b863ff185b8760df6334f7138003d885fbdb21d Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Tue, 4 Feb 2020 16:10:19 +1000 Subject: [PATCH 3/3] Remove explicit private modifier; name tweak; reorganize some conditionals --- .../Sinks/File/RollingFileSink.cs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs index 0e61954..330dfbc 100644 --- a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs +++ b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs @@ -171,12 +171,12 @@ void OpenFile(DateTime now, int? minSequence = null) throw; } - ApplyRetentionPolicy(path); + ApplyRetentionPolicy(path, now); return; } } - void ApplyRetentionPolicy(string currentFilePath) + void ApplyRetentionPolicy(string currentFilePath, DateTime now) { if (_retainedFileCountLimit == null && _retainedFileTimeLimit == null) return; @@ -195,7 +195,7 @@ void ApplyRetentionPolicy(string currentFilePath) var toRemove = newestFirst .Where(n => StringComparer.OrdinalIgnoreCase.Compare(currentFileName, n.Filename) != 0) - .SkipWhile(FilterFiles) + .SkipWhile((f, i) => ShouldRetainFile(f, i, now)) .Select(x => x.Filename) .ToList(); @@ -213,15 +213,18 @@ void ApplyRetentionPolicy(string currentFilePath) } } - private bool FilterFiles(RollingLogFile file, int index) + bool ShouldRetainFile(RollingLogFile file, int index, DateTime now) { - var isInCountLimit = index < (_retainedFileCountLimit - 1 ?? 0); + if (_retainedFileCountLimit.HasValue && index >= _retainedFileCountLimit.Value) + return false; - var isInTimeLimit = !_retainedFileTimeLimit.HasValue; - if (_retainedFileTimeLimit.HasValue && file.DateTime.HasValue) - isInTimeLimit = DateTime.Now.Subtract(_retainedFileTimeLimit.Value).CompareTo(file.DateTime.Value) <= 0; - - return isInCountLimit && isInTimeLimit; + if (_retainedFileTimeLimit.HasValue && file.DateTime.HasValue && + file.DateTime.Value < now.Subtract(_retainedFileTimeLimit.Value)) + { + return false; + } + + return true; } public void Dispose()