From 45dcc239add6b36ac40e0eec9c3597721c02e73b Mon Sep 17 00:00:00 2001 From: Baasit Date: Fri, 8 Dec 2023 01:04:36 +0100 Subject: [PATCH] feat: add --logs flag to local command (#2187) Added `--logs` subcommand to local command. Defaults to `debug.log` when no file is provided. Closes: #1357 --------- Co-authored-by: Vaibhav Rabber --- .gitignore | 3 + crates/glaredb/src/args/local.rs | 4 ++ crates/glaredb/src/bin/main.rs | 16 +++-- crates/glaredb/tests/log_file_test.rs | 87 +++++++++++++++++++++++++++ crates/logutil/src/lib.rs | 54 ++++++++++++++--- crates/testing/src/slt/cli.rs | 2 +- 6 files changed, 153 insertions(+), 13 deletions(-) create mode 100644 crates/glaredb/tests/log_file_test.rs diff --git a/.gitignore b/.gitignore index 4f2e978b3..f651b8fcf 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,6 @@ node_modules # Benchmark data bench_data/ + +# Logs +*.log \ No newline at end of file diff --git a/crates/glaredb/src/args/local.rs b/crates/glaredb/src/args/local.rs index dfd70972c..00d882636 100644 --- a/crates/glaredb/src/args/local.rs +++ b/crates/glaredb/src/args/local.rs @@ -14,6 +14,10 @@ pub struct LocalArgs { /// Execute an SQL file. pub file: Option, + + /// File for logs to be written to + #[clap(long, value_parser)] + pub log_file: Option, } #[derive(Debug, Clone, Parser)] diff --git a/crates/glaredb/src/bin/main.rs b/crates/glaredb/src/bin/main.rs index 33450c1be..388d031e3 100644 --- a/crates/glaredb/src/bin/main.rs +++ b/crates/glaredb/src/bin/main.rs @@ -51,11 +51,17 @@ fn main() -> Result<()> { None => Commands::Local(cli.local_args), }; - // Disable logging when running locally since it'll clobber the repl - // _unless_ the user specified a logging related option. - match (&command, cli.log_mode, cli.verbose) { - (Commands::Local { .. }, None, 0) => (), - _ => logutil::init(cli.verbose, cli.log_mode.unwrap_or_default().into()), + match &command { + Commands::Local(args) => { + if let Some(file) = &args.log_file { + logutil::init( + cli.verbose, + cli.log_mode.unwrap_or_default().into(), + Some(file), + ) + } + } + _ => logutil::init(cli.verbose, cli.log_mode.unwrap_or_default().into(), None), } command.run() diff --git a/crates/glaredb/tests/log_file_test.rs b/crates/glaredb/tests/log_file_test.rs new file mode 100644 index 000000000..d54a510c5 --- /dev/null +++ b/crates/glaredb/tests/log_file_test.rs @@ -0,0 +1,87 @@ +mod setup; + +use std::{ + fs::{remove_file, OpenOptions}, + io::Read, +}; + +use crate::setup::{make_cli, DEFAULT_TIMEOUT}; + +#[test] +fn test_log_file() -> Result<(), Box> { + let mut cmd = make_cli(); + let log_file_name = "test.log"; + cmd.timeout(DEFAULT_TIMEOUT) + .arg("-q") + .arg("select 1;") + .arg("--log-file") + .arg(log_file_name) + .assert() + .success(); + + let mut log_file = std::env::current_dir().expect("failed to retrieve current dir"); + log_file.push(log_file_name); + assert!(log_file.exists()); + + let mut file = OpenOptions::new().read(true).open(&log_file)?; + assert!(file.metadata()?.len() > 0); + + let mut content = String::new(); + file.read_to_string(&mut content)?; + + assert!(content.contains("INFO")); + assert!(!content.contains("DEBUG")); + assert!(!content.contains("TRACE")); + + assert!(content.contains("Starting in-memory metastore")); + + remove_file(log_file)?; + Ok(()) +} + +#[test] +fn test_log_file_verbosity_level() -> Result<(), Box> { + let mut cmd = make_cli(); + let log_file_name = "test.log"; + cmd.timeout(DEFAULT_TIMEOUT) + .arg("-v") + .arg("-q") + .arg("select 1;") + .arg("--log-file") + .arg(log_file_name) + .assert() + .success(); + + let mut log_file = std::env::current_dir().expect("failed to retrieve current dir"); + log_file.push(log_file_name); + assert!(log_file.exists()); + + let mut file = OpenOptions::new().read(true).open(&log_file)?; + assert!(file.metadata()?.len() > 0); + + let mut content = String::new(); + file.read_to_string(&mut content)?; + + assert!(content.contains("DEBUG")); + assert!(!content.contains("TRACE")); + + remove_file(log_file)?; + Ok(()) +} + +#[test] +fn test_skipping_logs() { + let mut cmd = make_cli(); + let log_file_name = "/usr/bin/debug.log"; + cmd.timeout(DEFAULT_TIMEOUT) + .arg("-q") + .arg("select 1;") + .arg("--log-file") + .arg(log_file_name) + .assert() + .success(); + + let mut log_file = std::env::current_dir().expect("failed to retrieve current dir"); + log_file.push(log_file_name); + assert!(!log_file.exists()); +} diff --git a/crates/logutil/src/lib.rs b/crates/logutil/src/lib.rs index 8a3c9cd89..472af673d 100644 --- a/crates/logutil/src/lib.rs +++ b/crates/logutil/src/lib.rs @@ -1,4 +1,6 @@ //! Utilities for logging and tracing. +use std::{fs::File, path::PathBuf, sync::Arc}; + use tracing::{subscriber, trace, Level}; use tracing_subscriber::{ filter::EnvFilter, @@ -71,7 +73,7 @@ pub fn init_test() { /// Initialize a trace subsriber printing to the console using the given /// verbosity count. -pub fn init(verbosity: impl Into, mode: LoggingMode) { +pub fn init(verbosity: impl Into, mode: LoggingMode, log_file: Option<&PathBuf>) { let verbosity: Verbosity = verbosity.into(); let level: Level = verbosity.into(); @@ -80,16 +82,54 @@ pub fn init(verbosity: impl Into, mode: LoggingMode) { let env_filter = env_filter(level); match mode { LoggingMode::Json => { - let subscriber = json_fmt(level).with_env_filter(env_filter).finish(); - subscriber::set_global_default(subscriber) + let subscriber = json_fmt(level).with_env_filter(env_filter); + + if let Some(file) = log_file { + let debug_log = match File::create(file) { + Ok(file) => Arc::new(file), + Err(_) => { + eprintln!("Failed to create file: {:#?}", file); + return subscriber::set_global_default(subscriber.finish()).unwrap(); + } + }; + + subscriber::set_global_default(subscriber.with_writer(debug_log).finish()) + } else { + subscriber::set_global_default(subscriber.finish()) + } } LoggingMode::Full => { - let subscriber = full_fmt(level).with_env_filter(env_filter).finish(); - subscriber::set_global_default(subscriber) + let subscriber = full_fmt(level).with_env_filter(env_filter); + + if let Some(file) = log_file { + let debug_log = match File::create(file) { + Ok(file) => Arc::new(file), + Err(_) => { + eprintln!("Failed to create file: {:#?}", file); + return subscriber::set_global_default(subscriber.finish()).unwrap(); + } + }; + + subscriber::set_global_default(subscriber.with_writer(debug_log).finish()) + } else { + subscriber::set_global_default(subscriber.finish()) + } } LoggingMode::Compact => { - let subscriber = compact_fmt(level).with_env_filter(env_filter).finish(); - subscriber::set_global_default(subscriber) + let subscriber = compact_fmt(level).with_env_filter(env_filter); + if let Some(file) = log_file { + let debug_log = match File::create(file) { + Ok(file) => Arc::new(file), + Err(_) => { + eprintln!("Failed to create file: {:#?}", file); + return subscriber::set_global_default(subscriber.finish()).unwrap(); + } + }; + + subscriber::set_global_default(subscriber.with_writer(debug_log).finish()) + } else { + subscriber::set_global_default(subscriber.finish()) + } } } .unwrap(); diff --git a/crates/testing/src/slt/cli.rs b/crates/testing/src/slt/cli.rs index b827790f7..ddd20cc8f 100644 --- a/crates/testing/src/slt/cli.rs +++ b/crates/testing/src/slt/cli.rs @@ -107,7 +107,7 @@ impl Cli { Verbosity::Debug => LoggingMode::Full, Verbosity::Trace => LoggingMode::Full, }; - logutil::init(cli.verbose, log_mode); + logutil::init(cli.verbose, log_mode, None); // Abort the program on panic. This will ensure that slt tests will // never pass if there's a panic somewhere.