RotatingFileAppender

[CfirTsabari]

Is your feature request related to a problem? Please describe.
I want to supply logs to the application that I write by using tracing, tracing-subscriber, and tracing-appender, but it's crucial to enforce small log files and a boundary to the total size of all the logs.
Small files - since its easier to read.
Total size - since disk space might be limited.

Describe the solution you'd like
I propose creating another Rotation
that will be based on size instead of time.
this rotation will be a configuration of two parameters:

• The max size of each file
• The maximum number of logs files to keep.

This rotation will create a new file every time the current file exceeds the allowed size and will bump all current old files indexes. In case the maximum number of files is reached it will delete the oldest file.

Describe alternatives you've considered
The current time-based rotation is not enough since it might vary a lot even in the apps.

Additional context
Python RotatingFileHandler

[hawkw]

I transferred this to tokio-rs/tracing. Also, tagging @zekisherif, since it's related to the tracing-appender crate.

[CfirTsabari]

I am intending on creating a PR for this.
I do wonder how exactly to design it.
my few first thought are:

1. Change RotationKind to include parameters and different variants something like

#[derive(Clone, Eq, PartialEq, Debug)]
enum RotationKind {
    TimeBased(TimeBasedRotationKind),
    SizeBased(u64-max size),
}
enum TimeBasedRotationKind {
    Minutely,
    Hourly,
    Daily,
    Never,
}

2. Change InnerAppender:

• Move all time-based rotation to another struct, and leave only the base file rotation code.
• create another struct for size-based rotation
• Add another option to control the number of file backups to keep.

[zekisherif]

PR #828 will likely change the public api a bit so we'll have to keep this into consideration for any work done here.

1. Changing the RotationKind will be a breaking change as RollingFileAppender is public and people may be using it directly instead of one of the helper functions. Though I agree it is the correct way to define RotationKind as you've done here and better for extendible.

2. Instead of using InnerAppender struct directly as we do today. RollingFileAppender should replace inner with a trait (InnerFileRotator?, even InnerAppender would work, we'd just have to move all implementations out of it) which you then can make structs which implement this trait and abstract the implementation of size-based and time based rotation.

Example trait definition which will look very similar to the current impl of InnerAppender. I don't think much has to really change here except the naming of the methods. But mayber there is something I'm overlooking here.

pub(crate) trait InnerAppender {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize>; // Handles writing to the log file
     fn refresh(&mut self); // Handles creating a new file to write to and modifying existing file names in the case of size based.
     fn should_rollover(&self) -> bool // Determines whether a rollover should occur or not based of the rotation criteria.
}

I don't envision many issues with what you are proposing outside of the changes to the api which are breaking changes.

[hawkw]

@zekisherif

1. Changing the RotationKind will be a breaking change as RollingFileAppender is public and people may be using it directly instead of one of the helper functions.

I don't think this a breaking change? The public Rotation type is represented internally by a RotationKind. We can change RotationKind as much as we like, as it is private. We would just add new constructors to Rotation for constructing a file-size based rotation, which would internally use the size-based RotationKind.

@CfirTsabari

#[derive(Clone, Eq, PartialEq, Debug)]
enum RotationKind {
    TimeBased(TimeBasedRotationKind),
    SizeBased(u64-max size),
}
enum TimeBasedRotationKind {
    Minutely,
    Hourly,
    Daily,
    Never,
}

IMO, it would probably be simpler to just write

#[derive(Clone, Eq, PartialEq, Debug)]
 enum RotationKind {
     Minutely,
     Hourly,
     Daily,
     Never,
     MaxSize(u64),
}

Unless I'm missing something? What does the inner enum get us here?

[zekisherif]

@zekisherif

1. Changing the RotationKind will be a breaking change as RollingFileAppender is public and people may be using it directly instead of one of the helper functions.

I don't think this a breaking change? The public Rotation type is represented internally by a RotationKind. We can change RotationKind as much as we like, as it is private. We would just add new constructors to Rotation for constructing a file-size based rotation, which would internally use the size-based RotationKind.

You're right. For some reason I thought RotationKind was public. It won't be a breaking change.

[CfirTsabari]

but RotationKind is not public, so it won't break the API.
sorry just saw that @hawkw already said that.

[CfirTsabari]

@hawkw but grouping the time base rotation will enable easier group handling.
I do have a starting draft on the main design I thought about(that doesn't build yet).
Should I open a draft PR?

[hawkw]

Should I open a draft PR?

If you like, please do!

[CfirTsabari]

So I just created a draft PR with the complete skeleton(and it does build).
I think I should do some work on the weekend.

[CfirTsabari]

The PR is ready for review :-)

[brianjcj]

actually we will need specify time and size at the same time. Whenever size limit or time limit exceed, the log will be rotated. That is the usual way log rotation being implemented in other languages and system.

[kevinhoffman]

@brianjcj - we had a similar need, with rotation conditions based on date/time and/or size, also to rotate based on the local time of the system rather than UTC, and to use a "Debian-style" log rotation scheme where the current logfile always has the same name.

Since this had some different design requirements than the RollingFileAppender here in tracing-appender, rather than add so many options/variants to RollingFileAppender here, we found it cleaner just to implement our own and then combine that with NonBlocking, which is working well. We were surprised there didn't seem to be another crate implementing such a thing (at least as far as we could find), so we've published this as the rolling-file crate. All feedback welcome.

[Christoph-AK]

Until tracing became a necessity for me I loved to use flexi_logger with

flexi_logger::Logger::with(LogSpecification::info())
    .use_windows_line_ending()
    .log_to_file(flexi_logger::FileSpec::default().directory("logs"))
    .duplicate_to_stdout(flexi_logger::Duplicate::Info)
    .append()
    .rotate(
        flexi_logger::Criterion::Size(100 * 1024_u64 * 1024_u64),
        flexi_logger::Naming::Timestamps,
        flexi_logger::Cleanup::KeepCompressedFiles(1000),
    )
    .cleanup_in_background_thread(true)

This enables async file compression, as long as you keep a token alive. Before exiting the program log_handle.shutdown(); needs to be called, which waits for the logging and compression to return from any open tasks.

Is something like this an option for the rolling appender?