cocowalla / serilog-sinks-file-archive

Plugin for the Serilog File sink that works with rolling log files, archiving completed log files before they are deleted by Serilog's retention mechanism
Apache License 2.0
31 stars 8 forks source link

Hook not saving archive after 1000 files #19

Closed smirnoff410 closed 2 months ago

smirnoff410 commented 4 months ago

I'm saving logs in file with names log_001.txt, log_002.txt, log_003.txt etc. Serilog auto generate files with suffix _001 and when files count more then 1000 hooks not working correctly This behavior because comparing digital filename inside string

ArchiveHooks.cs Line: 123

var filesToDelete = Directory.GetFiles(folder, searchPattern)
    .Select(f => new FileInfo(f))
    .OrderByDescending(f => f, LogFileComparer.Default)
    .Skip(this.retainedFileCountLimit)
    .ToList();

Test case:

This behavior because OrderByDescending can't correct compare string _log999.txt.gz and _log1000.txt.gz ArchiveHook create archive _log1000.txt.gz but after delete it

I think compare filename length is solution in class LogFileComparer

if (x.Name.Length > y.Name.Length)
    return 1;
if (y.Name.Length > x.Name.Length)
    return -1;

LogFileComparer full code

private class LogFileComparer : IComparer<FileInfo>
{
    public static IComparer<FileInfo> Default = new LogFileComparer();

    // This will not work correctly when the file uses a date format where lexicographical order does not 
    // correspond to chronological order - but frankly, if you are using non ISO 8601 date formats in your 
    // files you should be shot :)
    public int Compare(FileInfo x, FileInfo y)
    {
        if (x is null && y is null)
            return 0;
        if (x is null)
            return -1;
        if (y is null)
            return 1;
        if (x.Name.Length > y.Name.Length)
            return 1;
        if (y.Name.Length > x.Name.Length)
            return -1;
        return String.Compare(x.Name, y.Name, StringComparison.OrdinalIgnoreCase);
    }
}

Test code

// Test for removing old archive files in the same directory
[Fact]
public void Should_remove_old_archives()
{
    var retainedFiles = 1;
    var archiveWrapper = new ArchiveHooks(retainedFiles);

    using (var temp = TempFolder.ForCaller())
    {
        var path = temp.AllocateFilename("log");

        var logEvents = GenerateLogEvents(1002);
        // Write events, such that we end up with 1000 deleted files and 1 retained file
        WriteLogEvents(path, archiveWrapper, logEvents);

        // Get all the files in the test directory
        var files = Directory.GetFiles(temp.Path)
            .OrderBy(p => p, StringComparer.OrdinalIgnoreCase)
            .ToArray();

        // We should have a single log file, and 'retainedFiles' gz files
        files.Count(x => x.EndsWith("log")).ShouldBe(1);
        files.Count(x => x.EndsWith("gz")).ShouldBe(retainedFiles);

        // Ensure the data was GZip compressed, by decompressing and comparing against what we wrote
        int i = logEvents.Length - retainedFiles - 1;
        foreach (var gzipFile in files.Where(x => x.EndsWith("gz")))
        {
            var lines = Utils.DecompressLines(gzipFile);

            lines.Count.ShouldBe(1);
            lines[0].ShouldEndWith(logEvents[i].MessageTemplate.Text);
            i++;
        }
    }
}

/// <summary>
/// Generate array log events with custom size 
/// </summary>
/// <param name="count"></param>
/// <returns></returns>
private LogEvent[] GenerateLogEvents(int count)
{
    var logEvents = new LogEvent[count];
    for (var x = 0; x < count; x++)
        logEvents[x] = Some.LogEvent();
    return logEvents;
}
cocowalla commented 4 months ago

If you can submit a PR, I can look at this properly and see about getting it merged 👍

smirnoff410 commented 4 months ago

I don't have any permission for create and push new branches. Maybe I doing something wrong? Can you help me?

Answer from git remote: Permission to cocowalla/serilog-sinks-file-archive.git denied to smirnoff410. fatal: unable to access 'https://github.com/cocowalla/serilog-sinks-file-archive.git/': The requested URL returned error: 403 Pushing to https://github.com/cocowalla/serilog-sinks-file-archive.git

cocowalla commented 4 months ago

Ah, you have to make a fork first, then push your changes to the fork. Once you're ready, you can then create a pull request (PR) from your fork. A few links to help you get started:

Good luck with your first PR! 🍀

smirnoff410 commented 4 months ago

It's was awesome! Thx for guide :) Check please my PR