Skip to content

File retention policy by date/time #90

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 25 additions & 11 deletions src/Serilog.Sinks.File/FileLoggerConfigurationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ public static LoggerConfiguration File(
/// including the current log file. For unlimited retention, pass null. The default is 31.</param>
/// <param name="encoding">Character encoding used to write the text file. The default is UTF-8 without BOM.</param>
/// <returns>Configuration object allowing method chaining.</returns>
/// <remarks>The file will be written using the UTF-8 character set.</remarks>
[Obsolete("New code should not be compiled against this obsolete overload"), EditorBrowsable(EditorBrowsableState.Never)]
public static LoggerConfiguration File(
this LoggerSinkConfiguration sinkConfiguration,
Expand Down Expand Up @@ -165,7 +166,7 @@ public static LoggerConfiguration File(
/// <param name="sinkConfiguration">Logger sink configuration.</param>
/// <param name="formatter">A formatter, such as <see cref="JsonFormatter"/>, to convert the log events into
/// text for the file. If control of regular text formatting is required, use the other
/// overload of <see cref="File(LoggerSinkConfiguration, string, LogEventLevel, string, IFormatProvider, long?, LoggingLevelSwitch, bool, bool, TimeSpan?, RollingInterval, bool, int?, Encoding, FileLifecycleHooks)"/>
/// overload of <see cref="File(LoggerSinkConfiguration, string, LogEventLevel, string, IFormatProvider, long?, LoggingLevelSwitch, bool, bool, TimeSpan?, RollingInterval, bool, int?, Encoding, FileLifecycleHooks, TimeSpan?)"/>
/// and specify the outputTemplate parameter instead.
/// </param>
/// <param name="path">Path to the file.</param>
Expand All @@ -181,7 +182,7 @@ public static LoggerConfiguration File(
/// <param name="shared">Allow the log file to be shared by multiple processes. The default is false.</param>
/// <param name="flushToDiskInterval">If provided, a full disk flush will be performed periodically at the specified interval.</param>
/// <param name="rollingInterval">The interval at which logging will roll over to a new file.</param>
/// <param name="rollOnFileSizeLimit">If <code>true</code>, a new file will be created when the file size limit is reached. Filenames
/// <param name="rollOnFileSizeLimit">If <code>true</code>, a new file will be created when the file size limit is reached. Filenames
/// will have a number appended in the format <code>_NNN</code>, with the first filename given no number.</param>
/// <param name="retainedFileCountLimit">The maximum number of log files that will be retained,
/// including the current log file. For unlimited retention, pass null. The default is 31.</param>
Expand Down Expand Up @@ -227,13 +228,18 @@ public static LoggerConfiguration File(
/// <param name="shared">Allow the log file to be shared by multiple processes. The default is false.</param>
/// <param name="flushToDiskInterval">If provided, a full disk flush will be performed periodically at the specified interval.</param>
/// <param name="rollingInterval">The interval at which logging will roll over to a new file.</param>
/// <param name="rollOnFileSizeLimit">If <code>true</code>, a new file will be created when the file size limit is reached. Filenames
/// <param name="rollOnFileSizeLimit">If <code>true</code>, a new file will be created when the file size limit is reached. Filenames
/// will have a number appended in the format <code>_NNN</code>, with the first filename given no number.</param>
/// <param name="retainedFileCountLimit">The maximum number of log files that will be retained,
/// including the current log file. For unlimited retention, pass null. The default is 31.</param>
/// <param name="encoding">Character encoding used to write the text file. The default is UTF-8 without BOM.</param>
/// <param name="hooks">Optionally enables hooking into log file lifecycle events.</param>
/// <param name="retainedFileTimeLimit">The maximum time after the end of an interval that a rolling log file will be retained.
/// Must be greater than or equal to <see cref="TimeSpan.Zero"/>.
/// Ignored if <paramref see="rollingInterval"/> is <see cref="RollingInterval.Infinite"/>.
/// The default is to retain files indefinitely.</param>
/// <returns>Configuration object allowing method chaining.</returns>
/// <remarks>The file will be written using the UTF-8 character set.</remarks>
public static LoggerConfiguration File(
this LoggerSinkConfiguration sinkConfiguration,
string path,
Expand All @@ -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));
Expand All @@ -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);
}

/// <summary>
Expand All @@ -267,7 +274,7 @@ public static LoggerConfiguration File(
/// <param name="sinkConfiguration">Logger sink configuration.</param>
/// <param name="formatter">A formatter, such as <see cref="JsonFormatter"/>, to convert the log events into
/// text for the file. If control of regular text formatting is required, use the other
/// overload of <see cref="File(LoggerSinkConfiguration, string, LogEventLevel, string, IFormatProvider, long?, LoggingLevelSwitch, bool, bool, TimeSpan?, RollingInterval, bool, int?, Encoding, FileLifecycleHooks)"/>
/// overload of <see cref="File(LoggerSinkConfiguration, string, LogEventLevel, string, IFormatProvider, long?, LoggingLevelSwitch, bool, bool, TimeSpan?, RollingInterval, bool, int?, Encoding, FileLifecycleHooks, TimeSpan?)"/>
/// and specify the outputTemplate parameter instead.
/// </param>
/// <param name="path">Path to the file.</param>
Expand All @@ -289,6 +296,10 @@ public static LoggerConfiguration File(
/// including the current log file. For unlimited retention, pass null. The default is 31.</param>
/// <param name="encoding">Character encoding used to write the text file. The default is UTF-8 without BOM.</param>
/// <param name="hooks">Optionally enables hooking into log file lifecycle events.</param>
/// <param name="retainedFileTimeLimit">The maximum time after the end of an interval that a rolling log file will be retained.
/// Must be greater than or equal to <see cref="TimeSpan.Zero"/>.
/// Ignored if <paramref see="rollingInterval"/> is <see cref="RollingInterval.Infinite"/>.
/// The default is to retain files indefinitely.</param>
/// <returns>Configuration object allowing method chaining.</returns>
public static LoggerConfiguration File(
this LoggerSinkConfiguration sinkConfiguration,
Expand All @@ -304,15 +315,16 @@ 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));
if (path == null) throw new ArgumentNullException(nameof(path));

return ConfigureFile(sinkConfiguration.Sink, formatter, path, restrictedToMinimumLevel, fileSizeLimitBytes, levelSwitch,
buffered, false, shared, flushToDiskInterval, encoding, rollingInterval, rollOnFileSizeLimit,
retainedFileCountLimit, hooks);
retainedFileCountLimit, hooks, retainedFileTimeLimit);
}

/// <summary>
Expand Down Expand Up @@ -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(
Expand All @@ -450,21 +462,23 @@ 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));

ILogEventSink sink;

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
{
Expand Down
25 changes: 17 additions & 8 deletions src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using Serilog.Debugging;
using Serilog.Events;
using Serilog.Formatting;
using System.Collections.Generic;

namespace Serilog.Sinks.File
{
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -173,25 +178,29 @@ 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);

// We consider the current file to exist, even if nothing's been written yet,
// 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)) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@thiagosgarcia thiagosgarcia May 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :-)

(!_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)
Expand Down
57 changes: 57 additions & 0 deletions test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down