Add ZipDaysToKeep to roll-by-size-time appender #564

mmatt1967 · 2020-06-26T12:02:58Z

Implemented a new option on the RollingSizeTimeLogAppender that allows configuration of a number of days for which zipped log files should be kept.

Fixed issues with log archiving in RollingSizeTimeAppender.

src/Core/WinSWCore/LogAppenders.cs

src/Core/ServiceWrapper/winsw.csproj

…tch the configured naming pattern.

Bumps [coverlet.collector](https://github.com/coverlet-coverage/coverlet) from 1.2.0 to 1.3.0. - [Release notes](https://github.com/coverlet-coverage/coverlet/releases) - [Commits](https://github.com/coverlet-coverage/coverlet/commits) Signed-off-by: dependabot[bot] <support@github.com>

Bumps [SharpZipLib](https://github.com/icsharpcode/SharpZipLib) from 0.86.0 to 1.2.0. - [Release notes](https://github.com/icsharpcode/SharpZipLib/releases) - [Changelog](https://github.com/icsharpcode/SharpZipLib/blob/master/docs/Changes.txt) - [Commits](https://github.com/icsharpcode/SharpZipLib/commits/v1.2.0) Signed-off-by: dependabot[bot] <support@github.com>

Bumps [ilmerge](https://github.com/dotnet/ILMerge) from 3.0.29 to 3.0.40. - [Release notes](https://github.com/dotnet/ILMerge/releases) - [Commits](https://github.com/dotnet/ILMerge/commits) Signed-off-by: dependabot[bot] <support@github.com>

…tor-1.3.0 Bump coverlet.collector from 1.2.0 to 1.3.0

Bump SharpZipLib from 0.86.0 to 1.2.0

Bump ilmerge from 3.0.29 to 3.0.40

# Conflicts: # src/Core/ServiceWrapper/winsw.csproj

This reverts commit df2061b.

…-3.0.40" This reverts commit f70f02c, reversing changes made to 6aed8e6.

nxtn

@oleg-nenashev Any thoughts on the setting name?

nxtn · 2020-08-16T15:04:07Z

src/Core/ServiceWrapper/winsw.csproj

-
-  <PropertyGroup Condition="'$(TargetFramework)' != 'netcoreapp3.1'">
-    <ILMergeVersion>3.0.40</ILMergeVersion>
+    <SignAssembly>false</SignAssembly>


All changes to .csproj files should actually be reverted.

nxtn · 2020-08-16T15:04:31Z

src/Core/WinSWCore/LogAppenders.cs

@@ -395,7 +399,7 @@ private void CopyStreamWithRotation(StreamReader reader, string extension)
                            var nextFileName = Path.Combine(baseDirectory, string.Format("{0}.{1}.#{2:D4}{3}", baseFileName, now.ToString(FilePattern), nextFileNumber, extension));
                            File.Move(logFile, nextFileName);

-                            writer = CreateWriter(new FileStream(logFile, FileMode.Create));
+                            writer = CreateWriter(new FileStream(logFile, FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite));


That was an attempt to deal with a sharing violation that was more likely caused by the missing Dispose.

nxtn · 2020-08-16T15:04:59Z

src/Core/WinSWCore/LogAppenders.cs

@@ -427,6 +431,7 @@ private void CopyStreamWithRotation(StreamReader reader, string extension)
                        try
                        {
                            // roll file
+                            writer.Dispose();


Good catch, thanks!

nxtn · 2020-08-16T15:07:45Z

src/Core/WinSWCore/LogAppenders.cs

+                                    //EventLogger.LogEvent($"{zipFilePath}: datePart matches format {datePart} : {ZipDateFormat}, is older than {ZipDaysToKeep + ZipOlderThanNumDays} days, deleting ");
+                                    File.Delete(zipFilePath);
+                                }
+                                //else


Don't commit comments.

I assume you mean code that has been commented out?

nxtn · 2020-08-16T15:10:06Z

src/Core/WinSWCore/LogAppenders.cs

+                try
+                {
+                    //Get all the zipfiles that might match the naming pattern
+                    List<string> ZipFiles = new List<string>(Directory.GetFiles(directory, zipFileBaseName + ".*.zip"));


No need to instantiate a list. Just use the result array.

The compiler can now optimize foreach on arrays.

nxtn · 2020-08-16T15:13:19Z

src/Core/WinSWCore/LogAppenders.cs

+                            if (DateTime.TryParseExact(datePart, ZipDateFormat, System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.None, out DateTime date))
+                            {
+                                //the date portion of the name matches the configured ZipDateFormat
+                                if (date.CompareTo(DateTime.Now.AddDays((double)(-ZipDaysToKeep - ZipOlderThanNumDays))) < 0)


You can use the < operator in C#.

nxtn · 2020-08-16T15:16:15Z

src/Core/WinSWCore/LogAppenders.cs

+                    {
+                        if (File.Exists(zipFilePath))
+                        {
+                            //ZipDateFormat could contain literal characters, including the '.', so this gets a bit tricky


Doesn't TryParseExact handle .? It could be a real problem if zipFileBaseName contains ..

TryParseExact should handle '.', but doesn't seem to like the full file name. Implemented a fix for the possibility of ZipFileBaseName containing '.'

nxtn · 2020-08-16T15:17:23Z

src/Core/WinSWCore/LogAppenders.cs

+                                datePart = parts[parts.Length - 1];
+                            }
+                            //EventLogger.LogEvent($"{zipFilePath}: datePart = {datePart}");
+                            if (DateTime.TryParseExact(datePart, ZipDateFormat, System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.None, out DateTime date))


Do we use InvariantCulture when formatting ZipDateFormat to string?

No, CurrentCulture is used when formatting the file name. TryParseExact call changed to match.

oleg-nenashev · 2020-08-16T15:27:04Z

Added to my queue

Add ZipDaysToKeep to roll-by-size-time appender

dd64b70

mmatt1967 requested review from nxtn and oleg-nenashev as code owners June 26, 2020 12:02

nxtn requested changes Jun 27, 2020

View reviewed changes

src/Core/WinSWCore/LogAppenders.cs Outdated Show resolved Hide resolved

src/Core/ServiceWrapper/winsw.csproj Show resolved Hide resolved

mmatt1967 and others added 8 commits June 27, 2020 13:47

Merge remote-tracking branch 'upstream/master'

c9f433e

Change how zip files to delete are found to only delete files that ma…

c15d60e

…tch the configured naming pattern.

Bump ilmerge from 3.0.29 to 3.0.40

783f254

Bumps [ilmerge](https://github.com/dotnet/ILMerge) from 3.0.29 to 3.0.40. - [Release notes](https://github.com/dotnet/ILMerge/releases) - [Commits](https://github.com/dotnet/ILMerge/commits) Signed-off-by: dependabot[bot] <support@github.com>

Merge pull request #1 from mmatt1967/dependabot/nuget/coverlet.collec…

6fcaae7

…tor-1.3.0 Bump coverlet.collector from 1.2.0 to 1.3.0

Merge pull request #2 from mmatt1967/dependabot/nuget/SharpZipLib-1.2.0

6aed8e6

Bump SharpZipLib from 0.86.0 to 1.2.0

Merge pull request #3 from mmatt1967/dependabot/nuget/ilmerge-3.0.40

f70f02c

Bump ilmerge from 3.0.29 to 3.0.40

nxtn self-requested a review June 27, 2020 15:33

nxtn added this to the 2.Next milestone Jun 27, 2020

mmatt1967 added 9 commits June 28, 2020 08:31

Merge remote-tracking branch 'upstream/master'

3e8f713

update dependencies

bcf235a

Merge branch 'master' of https://github.com/mmatt1967/winsw

b64a6a1

# Conflicts: # src/Core/ServiceWrapper/winsw.csproj

Revert "Bump SharpZipLib from 0.86.0 to 1.2.0"

3d852e3

This reverts commit df2061b.

Revert "Merge pull request #3 from mmatt1967/dependabot/nuget/ilmerge…

9356db7

…-3.0.40" This reverts commit f70f02c, reversing changes made to 6aed8e6.

revert dependency changes

99213c3

Merge branch 'master' of https://github.com/mmatt1967/winsw

b58c3b3

Reverting dependency changes

d94f42c

Fix issues with log archiving in RollingSizeTimeLogAppender

c049691

nxtn requested changes Aug 16, 2020

View reviewed changes

nxtn added the new-feature label Aug 17, 2020

Updated to address comments on pull request by NextTurn

71234f3

nxtn force-pushed the master branch from 68eee96 to 48015a5 Compare December 26, 2020 12:20

TheNobleGoblin mentioned this pull request Dec 6, 2021

roll-by-size-time does not roll by size #894

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ZipDaysToKeep to roll-by-size-time appender #564

Add ZipDaysToKeep to roll-by-size-time appender #564

mmatt1967 commented Jun 26, 2020 •

edited

nxtn left a comment

nxtn Aug 16, 2020

mmatt1967 Oct 15, 2020

nxtn Aug 16, 2020

mmatt1967 Oct 15, 2020

nxtn Aug 16, 2020

nxtn Aug 16, 2020

mmatt1967 Oct 15, 2020

nxtn Aug 16, 2020 •

edited

mmatt1967 Oct 15, 2020

nxtn Aug 16, 2020

mmatt1967 Oct 15, 2020

nxtn Aug 16, 2020

mmatt1967 Oct 23, 2020

nxtn Aug 16, 2020

mmatt1967 Oct 15, 2020

oleg-nenashev commented Aug 16, 2020

Add ZipDaysToKeep to roll-by-size-time appender #564

Are you sure you want to change the base?

Add ZipDaysToKeep to roll-by-size-time appender #564

Conversation

mmatt1967 commented Jun 26, 2020 • edited

nxtn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nxtn Aug 16, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleg-nenashev commented Aug 16, 2020

mmatt1967 commented Jun 26, 2020 •

edited

nxtn Aug 16, 2020 •

edited