Logger doesn't rotate files anymore

[Sella-GH]

Versions used

.NET: 8 and 9 (both don't work)
Microsoft.Extensions.Logging: 8.0.1 and 9.0.0 (both don't work)
NReco.Logging.File: 1.2.1

Description

Currently I got the issue that the rotation feature of logging isn't working anymore. I have no clue why but at the end of the day it always uses the same file (approx. until the max file size is reached).

I attach the code below, maybe you can find an issue?

Code sample

logging.AddFile(Path.Combine("Logs", $"AzzyBot_{DateTimeOffset.Now:yyyy-MM-dd}.log"), c =>
{
    c.Append = true;
    c.FileSizeLimitBytes = 10380902; // ~9.9 MB
    c.MaxRollingFiles = logDays;
    c.RollingFilesConvention = FileLoggerOptions.FileRollingConvention.AscendingStableBase;
    c.UseUtcTimestamp = false;
    c.FormatLogFileName = static (logTime) => string.Format(CultureInfo.InvariantCulture, logTime, $"{DateTimeOffset.Now:yyyy-MM-dd}");
    c.FormatLogEntry = static (message) =>
    {
        string logMessage = $"[{DateTimeOffset.Now:yyyy-MM-dd HH:mm:ss}] {message.LogLevel}: {message.LogName}[{message.EventId}] {message.Message}";
        if (message.Exception is not null)
            logMessage += Environment.NewLine + message.Exception;

        return logMessage;
    };
});

Additional comments

All in all it's a great lib! I am happy that there is a lightweight extension which handles file logging. 🙂

[VitaliyMF]

I have no clue why but at the end of the day it always uses the same file (approx. until the max file size is reached).

Could you clarify how exactly to reproduce the issue? It sounds like the same file is all the day until the max file size is reached, which is a correct behaviour. FormatLogFileName handler forces a new 'base' filename each day, and rolling files feature works only when base filename remains stable.

[Sella-GH]

Could you clarify how exactly to reproduce the issue? It sounds like the same file is all the day until the max file size is reached, which is a correct behaviour. FormatLogFileName handler forces a new 'base' filename each day, and rolling files feature works only when base filename remains stable.

That could be the issue in the end. Originally I tried to rotate the files each day with a datestamp as it's identifier (instead of the numbering) but it doesn't seem to be supported so maybe that's why I was thinking it's a bug, whoops.

[VitaliyMF]

I guess you wanted to have a constant number of log files based on date (like "last 20 days"), however current implementation doesn't store anywhere all 'previous' log file names (returned by FormatLogFileName) and therefore cannot remove 'old' dates to keep constant number of log files. I guess it is easier to do cleanup (remove old logs) by simple cron task (or Windows Scheduler).

[Sella-GH]

Actually not. I wanted to use the same system as it's implemented now but instead of creating log files with names like "Log", "Log.1", "Log.2" and so on it would've been "Log-2024-11-18", "Log-2024-11-17", "Log-2024-11-16".

[VitaliyMF]

Rotation means that log files are re-used, for instance log.txt -> log1.txt -> log2.txt -> log.txt -> log1.txt (old content is lost) -> log2.txt -> etc

It is unclear for me how rotation can work on a date basis instead of indexes. It is definitely weird if a file with old date like 'Log-2024-11-17' is reused for logs dated 2024-11-14 (for instance).

I guess what you are looking for is automatic cleanup when logs filename is based on date, and old dates are automatically removed (say only last N files are kept). I don't think that this cleanup should be a part of NReco.Logging.File because logs cleanup strategy can be very different (say, it can involve archiving old logs before removal) and this is actually an application-specific thing. This can be a background task in your .NET app, or you can call this handler in a separate thread from your custom "FormatLogFileName" handler - say, keep previously returned filename and when it changes, run a cleanup routine. Let me know if you need an example, I'll try to prepare it when I'll have a bit of free time.

[Sella-GH]

Oh the cleanup part I already got that's no issue. The part where I create logs based on a date-basis is the thing which I miss (or currently not have / can't achieve with the lib).

If that's possible to do with the lib then it'd be great to know. Otherwise I may have to search for something else or fall back to an own implementation (which I actually don't want to do..) 🙂

[VitaliyMF]

The part where I create logs based on a date-basis is the thing which I miss (or currently not have / can't achieve with the lib).
If that's possible to do with the lib then it'd be great to know

Now I lost the understanding of desired behavior. You can already create logs on a date basis via FormatLogFileName handler, isn't it? You can either use or do not use the rolling files feature (which will work only for a concrete date). And if you need to clean up old logs (= keep only last N days) you can easily do that in your app's code (this can even be a call from FormatLogFileName when 'base' file name is changed).

[chenglidev]

@VitaliyMF I think it would be good if it supports an option naming log files by date without specifying FormatLogFileName. I think it will be very useful.

[VitaliyMF]

FormatLogFileName is trivial (https://github.com/nreco/logging?tab=readme-ov-file#change-log-file-name-on-the-fly) and gives full control over file name conventions (date format, an ability to use different folders based on the date etc). Just few lines of code, I don't see any motivation to complicate this part.

[chenglidev]

I see. But I think you can consider this scenario. We might want to change logging configuration through appsettings.json only without changing any code.

[VitaliyMF]

@chenglidev technically configuration of date-based log file name through appsettings.json can be supported in this way: in FileLoggerProvider FileWriter.GetBaseLogFileName can be a check for a String.Format placeholder token {0} and if it present, String.Format may be used to handle these placeholders:

string GetBaseLogFileName() {
	var fName = FileLogPrv.LogFileName;
	if (FileLogPrv.FormatLogFileName != null)
		fName = FileLogPrv.FormatLogFileName(fName);
	else if (fName.IndexOf("{0:")>=0)
		fName = String.Format(fName,  FileLogPrv.UseUtcTimestamp ? DateTime.UtcNow : DateTime.Now);
	}
	return fName;
}

It is not obvious that {0} is actually refers to the DateTime.Now. Maybe it makes sense to use "{now}" instead and then replace "{now}" with "{0}"?..

[chenglidev]

@VitaliyMF OK. Thanks. Let me try that.