nreco / logging

Generic file logger for .NET Core (FileLoggerProvider) with minimal dependencies
MIT License
284 stars 56 forks source link

Log files not being deleted automatically #25

Closed gallargit closed 2 years ago

gallargit commented 3 years ago

Hi, I've noticed old log files do not get deleted automatically in my project, all other configurations work though: filename, filesize limit. Here's my configuration

  "Logging": {
    "LogLevel": {
      "Default": "Debug",
      "Microsoft": "Warning",
      "Microsoft.Hosting.Lifetime": "Warning"
    },
    "File": {
      "Path": "Log-{0:yyyy}{0:MM}{0:dd}.log",
      "Append": "True",
      "FileSizeLimitBytes": 10485760,
      "MaxRollingFiles": 5
    }
  },
public static IHostBuilder CreateHostBuilder(string[] args)
{
    return Host.CreateDefaultBuilder(args)
    .ConfigureWebHostDefaults(webBuilder => webBuilder.UseStartup<Startup>())
    .ConfigureLogging((config, logging) =>
        logging
            .AddFile(config.Configuration.GetSection("Logging"), fileLoggerOpts =>
            {
                fileLoggerOpts.FormatLogFileName = fName => string.Format(fName, DateTime.UtcNow);
                fileLoggerOpts.MaxRollingFiles = 5; //try set rollfiles manually, no effect
            }
            )
            .AddConsole()
            .AddEventSourceLogger()
            .AddDebug()
     ) .....

As seen in the code I tried adding the MaxRollingFiles option manually, but it had no effect. I checked the nreco source files and saw no File.Delete at all. Is the deletion supposed to be provided automatically by the .NET framework maybe? I'm using netcore 3.1 and the deletion does not work neither when local developing nor in an azure service.

VitaliyMF commented 3 years ago

When you specify "MaxRollingFiles" FileLogger re-uses 5 files in this way: (no suffix) -> 1 -> 2 -> 3 -> 4 -> (no suffix) -> 1 -> ... This 'rolling' behaviour works only when filename is the same AND "FileSizeLimitBytes" is set.

In your case there is no limit on the max file size and file name is based on the date. Even if you'll add max file size this will limit only max number of files for the concrete date.

gallargit commented 3 years ago

In my case there is max file size, unless there is a variable other than this one:

"FileSizeLimitBytes": 10485760,

But this feature is not what I thought it was, I need a "delete old files" feature. I will implement this myself, if you're interested I can share the code later.

VitaliyMF commented 3 years ago

@gallargit when "FormatLogFileName" handler is used to configure a 'dynamic' log file name I don't know how to implement "delete old files" that will work in ALL possible cases (after app restarts etc). For example, folder also may change in time and outside "FormatLogFileName" handler there is no possibility to get the list of all generated log files.

I think that when "FormatLogFileName" is used it is better to remove (or archive) old logs outside FileLogger - with a shell script executed by "Task Scheduler" (on Windows) or cron (on Linux) or background task of .NET App itself.

gallargit commented 3 years ago

True, it's actually hard to find out where they are and when to delete them (or how to detect a file name change). I've done this, maybe it'll help someone. It's not very thoroughly tested, for example the Regex for file names is not good enough, but so far it works for me.

    "File": {
      "Path": "Log-{0:yyyy}{0:MM}{0:dd}.log",
      "Append": "True",
      "FileSizeLimitBytes": 10485760,
      "MaxRollingFiles": 5,
      "RetainedFileCountLimit": 7 // max number of retained log files
    }
.AddFile(config.Configuration.GetSection("Logging"), fileLoggerOpts =>
{
    fileLoggerOpts.FormatLogFileName = fName => string.Format(fName, DateTime.UtcNow);
    try
    {
        int? retained = config.Configuration.GetSection("Logging").GetSection("File").GetValue<int?>("RetainedFileCountLimit");
        var path = config.Configuration.GetSection("Logging").GetSection("File").GetValue<string>("Path");
        var validFileNames = new List<string>();
        if (retained > 0)
        {
            var rex = new Regex(path.Replace("{0:yyyy}", @"(\d{4})").Replace("{0:MM}", @"(\d{2})").Replace("{0:dd}", @"(\d{2})"));
            var filenametoday = string.Format(path, DateTime.UtcNow);
            var listLogs = new List<string>();
            foreach (var file in Directory.GetFiles(".", "*.*"))
            {
                var plainfile = Path.GetRelativePath(".", file);
                if (rex.IsMatch(plainfile) &&
                    //ignore future files
                    string.Compare(plainfile, filenametoday) <= 0)
                {
                    listLogs.Add(file);
                }
            }
            if (listLogs.Count > retained)
            {
                listLogs.OrderByDescending(s => s).Skip(retained.Value).ToList().ForEach(file => File.Delete(file));
            }
        }
}

the RetainedFileCountLimit variable is used for example in AzureFileLoggerOptions, it would be useful to have it in NReco.Logging.File.FileLoggerOptions.

okolvik-avento commented 2 years ago

Sorry, linked the wrong issue in another post