From 57d96c2c263aff760997fc68fc3e2e4c3358a923 Mon Sep 17 00:00:00 2001 From: Marcin_Osada Date: Mon, 8 Jul 2019 13:14:19 +0200 Subject: [PATCH 1/4] Add OnFileRemoving life cycle hook #106 --- .../Sinks/File/FileLifecycleHooks.cs | 14 ++++++++++++++ .../Sinks/File/RollingFileSink.cs | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs b/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs index 3dbddeb..bda4c63 100644 --- a/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs +++ b/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs @@ -35,5 +35,19 @@ public abstract class FileLifecycleHooks /// The encoding to use when reading/writing to the stream. /// The Serilog should use when writing events to the log file. public virtual Stream OnFileOpened(Stream underlyingStream, Encoding encoding) => underlyingStream; + + /// + /// Method called on log files that were marked as obsolete (old) and by default would be deleted. + /// This can be used to move old logs to archive location or send to backup server + /// + /// + /// Executing long synchronous operation may affect responsiveness of application + /// + /// Log file full path + /// + /// Return if Serilog should delete file. + /// Warning: returning false and keeping file in same place will result in calling this method again in next scan for obsolete files + /// + public virtual bool OnFileRemoving(string fullPath) => true; } } diff --git a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs index 2db6f24..fd81a54 100644 --- a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs +++ b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs @@ -199,11 +199,12 @@ void ApplyRetentionPolicy(string currentFilePath) var fullPath = Path.Combine(_roller.LogFileDirectory, obsolete); try { - System.IO.File.Delete(fullPath); + if (_hooks == null || _hooks.OnFileRemoving(fullPath)) + System.IO.File.Delete(fullPath); } catch (Exception ex) { - SelfLog.WriteLine("Error {0} while removing obsolete log file {1}", ex, fullPath); + SelfLog.WriteLine("Error {0} while processing obsolete log file {1}", ex, fullPath); } } } From 4f1ae37474c229b016d1858f075797a39694bdf1 Mon Sep 17 00:00:00 2001 From: Marcin_Osada Date: Wed, 10 Jul 2019 10:54:22 +0200 Subject: [PATCH 2/4] OnFileRemoving will not interfere with Serilog delete --- .../Sinks/File/FileLifecycleHooks.cs | 10 +++------- src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs | 5 +++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs b/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs index bda4c63..5949219 100644 --- a/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs +++ b/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs @@ -37,17 +37,13 @@ public abstract class FileLifecycleHooks public virtual Stream OnFileOpened(Stream underlyingStream, Encoding encoding) => underlyingStream; /// - /// Method called on log files that were marked as obsolete (old) and by default would be deleted. - /// This can be used to move old logs to archive location or send to backup server + /// Method called on log files that were marked as obsolete (old) and would be deleted. + /// This can be used to copy old logs to archive location or send to backup server /// /// /// Executing long synchronous operation may affect responsiveness of application /// /// Log file full path - /// - /// Return if Serilog should delete file. - /// Warning: returning false and keeping file in same place will result in calling this method again in next scan for obsolete files - /// - public virtual bool OnFileRemoving(string fullPath) => true; + public virtual void OnFileRemoving(string fullPath) {} } } diff --git a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs index fd81a54..3b0ac61 100644 --- a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs +++ b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs @@ -199,8 +199,9 @@ void ApplyRetentionPolicy(string currentFilePath) var fullPath = Path.Combine(_roller.LogFileDirectory, obsolete); try { - if (_hooks == null || _hooks.OnFileRemoving(fullPath)) - System.IO.File.Delete(fullPath); + if (_hooks != null) _hooks.OnFileRemoving(fullPath); + + System.IO.File.Delete(fullPath); } catch (Exception ex) { From 735efa9699035e979c73c3f4970ae0ab41ebabd7 Mon Sep 17 00:00:00 2001 From: Marcin_Osada Date: Fri, 12 Jul 2019 12:01:11 +0200 Subject: [PATCH 3/4] Update naming and summaries --- .../Sinks/File/FileLifecycleHooks.cs | 12 +++++------- src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs | 3 +-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs b/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs index 5949219..fbaf133 100644 --- a/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs +++ b/src/Serilog.Sinks.File/Sinks/File/FileLifecycleHooks.cs @@ -19,6 +19,7 @@ namespace Serilog.Sinks.File { /// /// Enables hooking into log file lifecycle events. + /// Hooks run synchronously and therefore may affect responsiveness of the application if long operations are performed. /// public abstract class FileLifecycleHooks { @@ -37,13 +38,10 @@ public abstract class FileLifecycleHooks public virtual Stream OnFileOpened(Stream underlyingStream, Encoding encoding) => underlyingStream; /// - /// Method called on log files that were marked as obsolete (old) and would be deleted. - /// This can be used to copy old logs to archive location or send to backup server + /// Called before an obsolete (rolling) log file is deleted. + /// This can be used to copy old logs to an archive location or send to a backup server. /// - /// - /// Executing long synchronous operation may affect responsiveness of application - /// - /// Log file full path - public virtual void OnFileRemoving(string fullPath) {} + /// The full path to the file being deleted. + public virtual void OnFileDeleting(string path) {} } } diff --git a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs index 3b0ac61..43e5fad 100644 --- a/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs +++ b/src/Serilog.Sinks.File/Sinks/File/RollingFileSink.cs @@ -199,8 +199,7 @@ void ApplyRetentionPolicy(string currentFilePath) var fullPath = Path.Combine(_roller.LogFileDirectory, obsolete); try { - if (_hooks != null) _hooks.OnFileRemoving(fullPath); - + _hooks?.OnFileDeleting(fullPath); System.IO.File.Delete(fullPath); } catch (Exception ex) From f6d1cffa6bd69649d76d366a92e1647dbde8745f Mon Sep 17 00:00:00 2001 From: Marcin_Osada Date: Mon, 15 Jul 2019 17:05:31 +0200 Subject: [PATCH 4/4] Add test --- .../RollingFileSinkTests.cs | 22 ++++++++++++ .../Support/ArchiveOldLogsHook.cs | 35 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 test/Serilog.Sinks.File.Tests/Support/ArchiveOldLogsHook.cs diff --git a/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs b/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs index 2e9f613..70408b3 100644 --- a/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs +++ b/test/Serilog.Sinks.File.Tests/RollingFileSinkTests.cs @@ -71,6 +71,28 @@ public void WhenRetentionCountIsSetOldFilesAreDeleted() }); } + [Fact] + public void WhenRetentionCountAndArchivingHookIsSetOldFilesAreCopiedAndOriginalDeleted() + { + const string archiveDirectory = "OldLogs"; + LogEvent e1 = Some.InformationEvent(), + e2 = Some.InformationEvent(e1.Timestamp.AddDays(1)), + e3 = Some.InformationEvent(e2.Timestamp.AddDays(5)); + + TestRollingEventSequence( + (pf, wt) => wt.File(pf, retainedFileCountLimit: 2, rollingInterval: RollingInterval.Day, hooks: new ArchiveOldLogsHook(archiveDirectory)), + 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])); + + Assert.True(System.IO.File.Exists(ArchiveOldLogsHook.AddTopDirectory(files[0], archiveDirectory))); + }); + } + [Fact] public void WhenSizeLimitIsBreachedNewFilesCreated() { diff --git a/test/Serilog.Sinks.File.Tests/Support/ArchiveOldLogsHook.cs b/test/Serilog.Sinks.File.Tests/Support/ArchiveOldLogsHook.cs new file mode 100644 index 0000000..96dcb8c --- /dev/null +++ b/test/Serilog.Sinks.File.Tests/Support/ArchiveOldLogsHook.cs @@ -0,0 +1,35 @@ +using System; +using System.IO; +using System.Text; + +namespace Serilog.Sinks.File.Tests.Support +{ + internal class ArchiveOldLogsHook : FileLifecycleHooks + { + private readonly string _relativeArchiveDir; + + public ArchiveOldLogsHook(string relativeArchiveDir) + { + _relativeArchiveDir = relativeArchiveDir; + } + + public override void OnFileDeleting(string path) + { + base.OnFileDeleting(path); + var newFile = AddTopDirectory(path, _relativeArchiveDir, true); + System.IO.File.Copy(path, newFile, false); + } + + public static string AddTopDirectory(string path, string directoryToAdd, bool createOnNonExist = false) + { + string file = Path.GetFileName(path); + string directory = Path.Combine(Path.GetDirectoryName(path) ?? throw new InvalidOperationException(), directoryToAdd); + + if (createOnNonExist && !Directory.Exists(directory)) + { + Directory.CreateDirectory(directory); + } + return Path.Combine(directory, file); + } + } +}