Feat: Rolling Appender Custom Rotation

[stegaBOB]

Motivation

Currently there are only 4 distinct rotations to choose from for the rolling appender: one minute, one hour, one day, and never. This PR makes it possible to specify custom durations such as 45 minutes, 6 hours, 5 years (???), or however often you desire your logs to be rotated. Closes #778.

Solution

I added an additional Custom variant to the RotationKind enum that stores a time::Duration, along with a new custom constructor on Rotation, which validates the duration length to be positive and less than 10 years. The 10 years is completely arbitrary and is mainly used to avoid potential time overflows with the OffsetDateTime in the next_date and round_date functions. This also means a 1ns duration is allowed. I think in practice this translates to something like 1 log per file, which may be useful somehow, but also feels a bit ridiculous. Would love some feedback on whether more realistic bounds would be better here.

I made the date_format for Custom be dynamic to avoid redundant precision.

The round_date implementation is probably the most important here and is equivalent to how the chrono crate does its DurationRound trait: https://github.com/chronotope/chrono/blob/38b19bbe4e21c402f81edfa2932a43831e679a35/src/round.rs#L214. The main differences are the lack of error handling (since we can't return a Result in this context) and using the euclidean remainder operation to handle negative current DateTimes (which shouldn't really be possible in the first place in our case) instead of their manual implementation it. Not having the error handling here is fine because all inputs should already be valid: the duration must be positive and the resulting DateTime should always be valid. I left comments explaining this.

Still to do:

• Update the doc comments
• Add a custom constructor method for RollingFileAppender

I held off on the last two things to make sure my implementation here is sufficient enough. If it looks okay, I'll be happy to finish that off!

[stegaBOB]

@hawkw Not sure who best to tag here, but I see that you commented back in 2020 on the original issue so I thought I'd try you! Would sorta prefer to get this merged in if possible while its still 2023 😅. If there's anything else I can do here to help get this in, just let me know!

[stegaBOB]

Hi @davidbarsky! I saw you were active on some recent commits so thought I'd try tagging you for this. Would love to get this merged in at some point

[kaffarell]

LGTM, the unit-tests are also extensive :)

As you said, a few things are missing here, namely:

• docs
• public constructor in tracing-appender::rolling module.

Some small nits below:

[kaffarell]

I think the change looks fine! Please add the missing docs and public method now, so that it only has to be reviewed once :)

[gibbz00]

Hi!

I would love to get this merged. Would it be ok for me to open a PR that takes this past the finish line?

[stegaBOB]

Hi!

I would love to get this merged. Would it be ok for me to open a PR that takes this past the finish line?

Honestly I'd just be happy if a tracing maintainer responds at all to either of our PRs. It was a bit pathetic merging master into my branch for over a year.