Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow tracing-appender sized log and rotation #1940

Open
IniterWorker opened this issue Feb 18, 2022 · 6 comments · May be fixed by #2497 or #2904
Open

Allow tracing-appender sized log and rotation #1940

IniterWorker opened this issue Feb 18, 2022 · 6 comments · May be fixed by #2497 or #2904
Labels
crate/appender Related to the `tracing-appender` crate. kind/feature New feature or request

Comments

@IniterWorker
Copy link

IniterWorker commented Feb 18, 2022

Feature Request

Crates

The tracing-appender

Motivation

In a constraint environment, we should be able to contain the logging size and period. Let's say we want temporary logging in tmpfs, we are not able to manage the size of the log file or the rotation pool size.

Proposal

We should create a new rotate_sized mod with all the sized logic.

  • The ability to rotate log files based on a maximum size (rather than duration)
  • A configuration to limit the maximum number of previous log files to keep, and delete older ones.
  • An option to create a symlink to the latest log file, and update it when log files are rotated.
-- logs
   --- log (sym link -> current log)
   --- log.0
   --- log.1
   --- log.2 (maximum set 3)

Alternatives

We could improve the rolling. But, it seems to make simple things more complicated.

@hawkw hawkw added crate/appender Related to the `tracing-appender` crate. kind/feature New feature or request labels Feb 18, 2022
@hawkw
Copy link
Member

hawkw commented Feb 18, 2022

If I understand correctly, this issue is proposing a few features:

  • The ability to rotate log files based on a maximum size (rather than duration)
  • A configuration to limit the maximum number of previous log files to keep, and delete older ones.
  • An option to create a symlink to the latest log file, and update it when log files are rotated.

I would be very happy review to pull requests that add all of these features, and I can provide some guidance for implementing them.

Also, note that there was a previous feature request specifically for size-based file rotation, #858. I believe there was also an attempt to implement it at the time, but there were some issues with the implementation.

@IniterWorker
Copy link
Author

Hi @hawkw,

I updated the issue with your summary. Thank you by the way.

The search engine didn't come with #858 . I will look into it.

No time for me right now, maybe few weeks,

B. R

@CBenoit
Copy link
Contributor

CBenoit commented Jun 7, 2022

Hi! I have the exact same needs.

I would be very happy review to pull requests that add all of these features, and I can provide some guidance for implementing them.

My company allowed me to get an initial implementation for that done on my working hours, and it would be a pleasure for me to send pull requests.
I did read a bit the code of the rolling module, but any additional guidance would be greatly appreciated to get started faster 🙂

Thank you!

@hawkw
Copy link
Member

hawkw commented Jun 7, 2022

Hi @CBenoit, thanks for your interest in contributing this change! Are there specific aspects of the implementation that you have questions about?

From the API side, I think at some point, as more features are added to the rolling file appender, we'll probably want to add a builder-style interface for configuring the rolling file appender's behavior. For this change, though, I think we'll want to add a new Rotation type for size-based rotation.

@CBenoit
Copy link
Contributor

CBenoit commented Jun 8, 2022

Hi @CBenoit, thanks for your interest in contributing this change! Are there specific aspects of the implementation that you have questions about?

Rather than questions about aspects of the current implementation, I would like to know more about how you envisioned this feature (even a rough idea). For instance, if you have a specific API, or specific implementation in mind? Maybe it’s simpler to just go ahead and open a draft PR though. For the API side, I was thinking about adding a whole new API surface using some kind of builder pattern as you suggested.

For this change, though, I think we'll want to add a new Rotation type for size-based rotation.

So if I understand correctly, you suggest to create a completely different Rotation type, maybe inside of its own module rotate_sized as suggested by OP?
Since Rotation is currently opaque we could also change the internal representation to allow for more complex combinations (timed only, sized only, timed AND sized, file name format, symlink, delete older files…).

CBenoit added a commit to CBenoit/tracing that referenced this issue Mar 4, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
CBenoit added a commit to CBenoit/tracing that referenced this issue Mar 4, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
@CBenoit CBenoit linked a pull request Mar 4, 2023 that will close this issue
CBenoit added a commit to CBenoit/tracing that referenced this issue Mar 4, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
CBenoit added a commit to CBenoit/tracing that referenced this issue Mar 4, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
CBenoit added a commit to CBenoit/tracing that referenced this issue Mar 4, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
@CBenoit
Copy link
Contributor

CBenoit commented Mar 4, 2023

I opened a pull request to address the following point:

The ability to rotate log files based on a maximum size (rather than duration)

This pull request already addressed the following point:

A configuration to limit the maximum number of previous log files to keep, and delete older ones.

In order to track progress efficiently, I suggest creating a separate issue for the remaining point:

An option to create a symlink to the latest log file, and update it when log files are rotated.

I intend to tackle this last point as well.

CBenoit added a commit to CBenoit/tracing that referenced this issue Mar 4, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
CBenoit added a commit to CBenoit/tracing that referenced this issue Mar 4, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
CBenoit added a commit to CBenoit/tracing that referenced this issue Sep 5, 2023
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
CBenoit added a commit to CBenoit/tracing that referenced this issue Feb 13, 2024
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
@x3ccd4828 x3ccd4828 linked a pull request Mar 7, 2024 that will close this issue
dovahcrow pushed a commit to dovahcrow/tracing that referenced this issue May 11, 2024
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
dovahcrow pushed a commit to dovahcrow/tracing that referenced this issue May 11, 2024
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
dovahcrow pushed a commit to dovahcrow/tracing that referenced this issue Jun 17, 2024
This patch adds size-based rotation to tracing-appender.

In a constraint environment, we should be able to contain the logging
size and period.

Related issues:
- tokio-rs#1940
- tokio-rs#858
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/appender Related to the `tracing-appender` crate. kind/feature New feature or request
Projects
None yet
3 participants