From d72362002c0deb902457942c620034c0aa794ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sun, 28 Apr 2024 16:48:51 +1200 Subject: [PATCH] feat(cli): use non-blocking logging --- Cargo.lock | 13 ++++++++++ crates/cli/Cargo.toml | 1 + crates/cli/src/args.rs | 13 ++++++---- crates/cli/src/args/logging.rs | 46 ++++++++++++++++++---------------- crates/cli/src/lib.rs | 2 +- 5 files changed, 48 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c42cb4bf..d0523be5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3685,6 +3685,18 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "tracing-appender" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3566e8ce28cc0a3fe42519fc80e6b4c943cc4c8cef275620eb8dac2d3d4e06cf" +dependencies = [ + "crossbeam-channel", + "thiserror", + "time", + "tracing-subscriber", +] + [[package]] name = "tracing-attributes" version = "0.1.27" @@ -4060,6 +4072,7 @@ dependencies = [ "termcolor", "tokio", "tracing", + "tracing-appender", "tracing-subscriber", "tracing-test", "uuid", diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 8ffc6acd..1c92ca8d 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -44,6 +44,7 @@ serde_json = "1.0.107" tempfile = "3.8.1" termcolor = "1.4.0" tracing = "0.1.40" +tracing-appender = "0.2.3" which = "6.0.1" [dependencies.blake3] diff --git a/crates/cli/src/args.rs b/crates/cli/src/args.rs index 69b52eef..27cbcfde 100644 --- a/crates/cli/src/args.rs +++ b/crates/cli/src/args.rs @@ -15,6 +15,7 @@ use clap::{ use miette::{IntoDiagnostic, Result}; use tokio::{fs::File, io::AsyncReadExt}; use tracing::{debug, info, trace, warn}; +use tracing_appender::non_blocking::WorkerGuard; use watchexec::{paths::PATH_SEPARATOR, sources::fs::WatchedPath}; use watchexec_signals::Signal; @@ -1145,7 +1146,7 @@ fn expand_args_up_to_doubledash() -> Result, std::io::Error> { } #[inline] -pub async fn get_args() -> Result { +pub async fn get_args() -> Result<(Args, Option)> { let prearg_logs = logging::preargs(); if prearg_logs { warn!("⚠ RUST_LOG environment variable set or hardcoded, logging options have no effect"); @@ -1157,9 +1158,11 @@ pub async fn get_args() -> Result { debug!("parsing arguments"); let mut args = Args::parse_from(args); - if !prearg_logs { - logging::postargs(&args.logging).await?; - } + let log_guard = if !prearg_logs { + logging::postargs(&args.logging).await? + } else { + None + }; // https://no-color.org/ if args.color == ColourMode::Auto && std::env::var("NO_COLOR").is_ok() { @@ -1290,5 +1293,5 @@ pub async fn get_args() -> Result { debug_assert!(args.workdir.is_some()); debug_assert!(args.project_origin.is_some()); info!(?args, "got arguments"); - Ok(args) + Ok((args, log_guard)) } diff --git a/crates/cli/src/args/logging.rs b/crates/cli/src/args/logging.rs index cd87a4be..2298b030 100644 --- a/crates/cli/src/args/logging.rs +++ b/crates/cli/src/args/logging.rs @@ -1,16 +1,17 @@ -use std::{path::PathBuf, env::var, fs::File, sync::Mutex}; +use std::{env::var, io::stderr, path::PathBuf}; use clap::{ArgAction, Parser, ValueHint}; -use miette::{IntoDiagnostic, Result}; +use miette::{bail, Result}; use tokio::fs::metadata; use tracing::{info, warn}; +use tracing_appender::{non_blocking, non_blocking::WorkerGuard, rolling}; #[derive(Debug, Clone, Parser)] pub struct LoggingArgs { /// Set diagnostic log level /// /// This enables diagnostic logging, which is useful for investigating bugs or gaining more - /// insight into faulty filters or "missing" events. Use multiple times to increase args.verbose. + /// insight into faulty filters or "missing" events. Use multiple times to increase verbosity. /// /// Goes up to '-vvvv'. When submitting bug reports, default to a '-vvv' log level. /// @@ -77,27 +78,30 @@ pub fn preargs() -> bool { log_on } -pub async fn postargs(args: &LoggingArgs) -> Result<()> { +pub async fn postargs(args: &LoggingArgs) -> Result> { if args.verbose == 0 { - return Ok(()); + return Ok(None); } - let log_file = if let Some(file) = &args.log_file { + let (log_writer, guard) = if let Some(file) = &args.log_file { let is_dir = metadata(&file).await.map_or(false, |info| info.is_dir()); - let path = if is_dir { - let filename = format!( - "watchexec.{}.log", - chrono::Utc::now().format("%Y-%m-%dT%H-%M-%SZ") - ); - file.join(filename) + let (dir, filename) = if is_dir { + ( + file.to_owned(), + PathBuf::from(format!( + "watchexec.{}.log", + chrono::Utc::now().format("%Y-%m-%dT%H-%M-%SZ") + )), + ) + } else if let (Some(parent), Some(file_name)) = (file.parent(), file.file_name()) { + (parent.into(), PathBuf::from(file_name)) } else { - file.to_owned() + bail!("Failed to determine log file name"); }; - // TODO: use tracing-appender instead - Some(File::create(path).into_diagnostic()?) + non_blocking(rolling::never(dir, filename)) } else { - None + non_blocking(stderr()) }; let mut builder = tracing_subscriber::fmt().with_env_filter(match args.verbose { @@ -113,16 +117,16 @@ pub async fn postargs(args: &LoggingArgs) -> Result<()> { builder = builder.with_span_events(FmtSpan::NEW | FmtSpan::CLOSE); } - match if let Some(writer) = log_file { - builder.json().with_writer(Mutex::new(writer)).try_init() + match if args.log_file.is_some() { + builder.json().with_writer(log_writer).try_init() } else if args.verbose > 3 { - builder.pretty().try_init() + builder.pretty().with_writer(log_writer).try_init() } else { - builder.try_init() + builder.with_writer(log_writer).try_init() } { Ok(()) => info!("logging initialised"), Err(e) => eprintln!("Failed to initialise logging, continuing with none\n{e}"), } - Ok(()) + Ok(Some(guard)) } diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 71431e0e..4b773601 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -116,7 +116,7 @@ async fn run_completions(shell: ShellCompletion) -> Result<()> { } pub async fn run() -> Result<()> { - let args = args::get_args().await?; + let (args, _log_guard) = args::get_args().await?; if args.manual { run_manpage(args).await