emabee / flexi_logger

A flexible logger for rust programs that can write to stderr, stdout, and/or to log files
Apache License 2.0
307 stars 50 forks source link

creating vs modification time #159

Closed jb-alvarado closed 3 months ago

jb-alvarado commented 3 months ago

I started to dive in a bit in the code.

First I found this:

https://github.com/emabee/flexi_logger/blob/ecd8bb0af7292b73e3a553f910b4d7db99d2518f/src/writers/file_log_writer/state.rs#L563-L585

You have here this function call:

try_get_creation_date(path)
    .or_else(|_| try_get_modification_date(path))
    .unwrap_or_else(|_| get_current_date())

Which calls:

fn try_get_creation_date(path: &Path) -> Result<DateTime<Local>, FlexiLoggerError> {
    Ok(std::fs::metadata(path)?.created()?.into())
}

and when it fails:

fn try_get_modification_date(path: &Path) -> Result<DateTime<Local>, FlexiLoggerError> {
    let md = std::fs::metadata(path)?;
    let d = md.created().or_else(|_| md.modified())?;
    Ok(d.into())
}

But is this not redundant? Because of let d = md.created().or_else(|_| md.modified())?; it always tries first to get the creating date and after it fails it gets modification date.

Another question is: why you try to get creating date and not always just uses modification date? Modification date would make it a bit easier to test the code, because this value you can fake.

emabee commented 3 months ago

The logically correct property to decide when an age-based rotation is necessary is the creation date of the existing log file.

I wanted to express in the code and the comments what was intended, what is done instead and why. The redundancy doesn't hurt, the code is used infrequently.