Size rolling

[x3ccd4828]

This patch adds size-based rotation to tracing-appender.

Closes #1940
Closes #858

cc @hawkw @IniterWorker @CfirTsabari @davidbarsky @CBenoit

There is another pull request (#2497) for this feature but I thought I would try a different approach.

Motivation

In a constrained environment, we should be able to constrain the logging size and period.

Solution

Maximum bytes per log file may be specified in the max_file_size function when building the appender. The option will only be used when selecting Rotation::SIZE.

File size is checked every write and if the size is exceeded the log file is rolled over.

None of the other rotation types use sub-seconds in the logfile name but for size-based rotation, I used it since there could be a plethora of writes in a short period and this was the simple way to ensure filename uniqueness otherwise it would write to the same file even if the size was exceeded. This is a limitation since I tried to use the same methodology as the time-based rolling without rewriting the architecture.

[x3ccd4828]

Hi @hawkw @davidbarsky @hds,

What would it take to get this reviewed? Should I just close it since nobody will ever look at it?

[hds]

@x3ccd4828 I can look at this, but I'm currently very full up with family stuff, would you mind reminding me after the 25th of January? I'll be back at work then (and therefore at my computer) and will make some time to go through old PRs and issues.

[x3ccd4828]

Tha is understandable, I will post here at the end of January

[x3ccd4828]

@hds I have rebased the branch would you be able to review it?

[JoelSatkas]

Comment doesnt seem right, shouldn't this say something like // Will roll over once file reaches 1024 bytes ?

[x3ccd4828]

thank you for catching that I fixed the comment

[JoelSatkas]

So in this doc i do not see a default file size thats used, i think would be handy to mention that?

[x3ccd4828]

There is no default since it's an option. So if the user selects size but doesn't provide a size it will do nothing. Should I capture this behaviour in the doc files?

[JoelSatkas]

I am not that experienced in Rust but i meant that from my understanding, these docs say

let appender = tracing_appender::rolling::size("/some/path", "rolling.log");

Which seems to me to be making a size rolling appender and passing it the path and log file name but there is no parameter for the size. Therefore i would expect a default size. If there is no default size and the size rollover wont work then i think its good to document that, otherwise a bit unclear how your supposed to use it or what you can expect.

[JoelSatkas]

@x3ccd4828 Could you update your branch with the latest official traceing-appenders? If i am not mistaken this branch is still on version 0.2.0?

Thats at least what my toml lock file is indicating, i was on version 0.2.3 before and had tracing working in both console and file appending using the time rotation but when i use your branch, i get errors indicating that the file appender doesnt implement MakeWriter?

expected a `Fn<()>` closure, found `RollingFileAppender`
the trait `Fn<()>` is not implemented for `RollingFileAppender`
wrap the `RollingFileAppender` in a closure with no arguments: `|| { /* code */ }`
the following other types implement trait `MakeWriter<'a>`:
  std::fs::File
  BoxMakeWriter
  TestWriter
  WithMaxLevel<M>
  WithMinLevel<M>
  WithFilter<M, F>
  tracing_subscriber::fmt::writer::OrElse<A, B>
  Tee<A, B>
and 2 others
required for `RollingFileAppender` to implement `for<'writer> MakeWriter<'writer>`

My code for tracing to the console and log file looks like this:

// Create a rolling file appender
let file_appender = RollingFileAppender::builder()
    .filename_prefix(SERVER_LOG_FILE_NAME)
    .filename_suffix("log")
    .max_file_size(5 * 1024 * 1024)
    .max_log_files(5)
    .rotation(Rotation::SIZE)
    .build(log_dir);

if file_appender.is_err() {
    tracing::error!("Failed to build file appender for server logs.");
    return EResultCode::Failure;
}

// Set up the console and file layers
let filter_layer = EnvFilter::try_from_default_env().unwrap_or_else(|_| {
    EnvFilter::new(
    "web-servicer=debug,web_servicer_library=debug,tower_http=debug",
)
});
let file_layer = fmt::layer()
    .with_writer(file_appender.unwrap())
    .with_ansi(false)
    .with_target(true)
    .with_level(true);
let console_layer = fmt::layer().with_target(true).with_level(true);

// Register the layers
tracing_subscriber::registry()
    .with(filter_layer)
    .with(console_layer)
    .with(file_layer)
    .init();

[x3ccd4828]

@JoelSatkas I rebased based on tracing master branch but for some reason master branch has tracing-appender at 0.2.0.
From the looks of it, 0.2.3 was in a branch for release purposes only. I don't think I can rebase it based on that branch for this merge.

[JoelSatkas]

@JoelSatkas I rebased based on tracing master branch but for some reason master branch has tracing-appender at 0.2.0. From the looks of it, 0.2.3 was in a branch for release purposes only. I don't think I can rebase it based on that branch for this merge.

I see. Thats okay and thank you for checking. I would love to have this feature in the project im working on but for now the time rollover will have to suffice. I hope this PR will be properly reviewed by the developers soon and thank you for contributing