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

proposal: implement Drop for LoggerHandle #72

Closed tigeran2020 closed 3 years ago

tigeran2020 commented 3 years ago

When I want to use buffer, I must keep the LoggerHandle which returned by start and then call shutdown myself, like this:

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let logger = Logger::with_str("info")
       .log_target(LogTarget::File)
       .buffer_and_flush()
       .start()?;
    // ... do all your work ...
    logger.shutdown();
    Ok(())
}

I think it's very unrust to do this, and implement a drop method for LoggerHandle make it more rust, the main function will envolved to like this:

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let _logger_guard = Logger::with_str("info")
       .log_target(LogTarget::File)
       .buffer_and_flush()
       .start()?;
    // ... do all your work ...
    Ok(())
}
emabee commented 3 years ago

I agree, the Drop is more rusty :-)

The original purpose of the LoggerHandle was to allow programmatic changes to the LogSpecification. Users who didn't want to use this (=nearly everybody), could ignore the handle, so that it was dropped immediately. The flush and the cleanup functionality came later. The reason why I didn't implement Drop originally is that the shutdown method really shuts down the various writers; in case of the provided file logger, the cleanup thread is shut down; in case of self-implemented writers, I do not know what happens... So implementing Drop can affect users who don't keep the handle alive and use features that are affected by shutdown.

I see three options: a) leave it as is b) Implement Drop, but only do the flush (that would help your case, and do no harm) c) do the full shutdown in Drop; this would require a version update, and whatever else to make people aware that they might need to introduce your let _logger_guard = "trick", to avoid bad surprises.

emabee commented 3 years ago

Thanks for the proposal - it is now realized with version 0.18.0!