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

Create a symlink to latest log file #1979

Open
wants to merge 1 commit into
base: v0.1.x
Choose a base branch
from
Open

Conversation

clia
Copy link

@clia clia commented Mar 9, 2022

Motivation

When monitoring logs of rotating files, it will be very helpful to create a symlink
to the latest log file when creating and rotating the log file.

Then you can tail -F symlink_file to keep watching the new created log file.

Solution

Create a symlink file when creating and rotating the log file.

Use a symlink crate to achieve a system compatible implementation.

It's necessary to remove the old symlink file before creating it, otherwise it may cause failure.

@clia clia requested a review from a team as a code owner March 9, 2022 12:18
@hawkw
Copy link
Member

hawkw commented Mar 9, 2022

@clia in general, we prefer to merge new features to the master branch, and then backport them to v0.1.x to be released. Can I get you to change this PR to be based on the master branch please?

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, I agree that this is definitely a feature that's worth having!

I think we probably want to make the symlinking behavior configurable, rather than always doing it by default. I left some comments discussing this. Also, if you're able to, it would be nice to add some tests to ensure this works as intended?

@@ -24,6 +24,7 @@ rust-version = "1.53.0"
crossbeam-channel = "0.5.0"
time = { version = "0.3", default-features = false, features = ["formatting"] }
parking_lot = { optional = true, version = "0.12.0" }
symlink = "0.1.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this adds a new dependency, and some users may not want the symlink feature, I think it would be preferable to feature flag this?


// Create a symlink to latest log file.
let latest_path = Path::new(&filename);
let symlink_path = Path::new(&log_directory).join(&log_filename_prefix);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if using the log filename prefix for the name of the symlink is the best solution. The prefix may contain a trailing separator character. For example, I might want my log files to have names like myapp-log-<DATE>, so the prefix might be myapp-log-. Therefore, I think we probably want to allow users to configure the name of the symlink separately from the log file's name. Also, some users may want to give the symlink a special name like <LOGFILE_NAME>.latest or similar.

PR #1779 adds a builder interface for configuring new RollingFileAppender instances, in order to support compression of log files. I think once that PR merges, we may want to add a builder method for configuring the symlink generation, instead?

Comment on lines +156 to +162
// Create a symlink to latest log file.
let latest_path = Path::new(&filename);
let symlink_path = Path::new(&log_directory).join(&log_filename_prefix);
let _ = remove_symlink_file(&symlink_path);
if let Err(err) = symlink_file(&latest_path, &symlink_path) {
eprintln!("Couldn't create symlink: {}", err);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

take it or leave it: the code for re-creating the symlink in Inner::refresh_writer is identical to this code...I wonder if we want to consider making a function for creating/updating the symlink, and call it in both places?

@isak102
Copy link

isak102 commented Jul 13, 2024

Any status on this? Would be a great feature to have. I'm willing to work on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants