Skip to content

Commit

Permalink
feat: add --logs flag to local command (#2187)
Browse files Browse the repository at this point in the history
Added `--logs` subcommand to local command. Defaults to `debug.log` when
no file is provided.
Closes: #1357

---------

Co-authored-by: Vaibhav Rabber <vrongmeal@gmail.com>
  • Loading branch information
Lilit0x and vrongmeal committed Dec 8, 2023
1 parent da785d9 commit 45dcc23
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ node_modules

# Benchmark data
bench_data/

# Logs
*.log
4 changes: 4 additions & 0 deletions crates/glaredb/src/args/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ pub struct LocalArgs {

/// Execute an SQL file.
pub file: Option<String>,

/// File for logs to be written to
#[clap(long, value_parser)]
pub log_file: Option<PathBuf>,
}

#[derive(Debug, Clone, Parser)]
Expand Down
16 changes: 11 additions & 5 deletions crates/glaredb/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
87 changes: 87 additions & 0 deletions crates/glaredb/tests/log_file_test.rs
Original file line number Diff line number Diff line change
@@ -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<dyn std::error::Error>> {
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<dyn std::error::Error>> {
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());
}
54 changes: 47 additions & 7 deletions crates/logutil/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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<Verbosity>, mode: LoggingMode) {
pub fn init(verbosity: impl Into<Verbosity>, mode: LoggingMode, log_file: Option<&PathBuf>) {
let verbosity: Verbosity = verbosity.into();
let level: Level = verbosity.into();

Expand All @@ -80,16 +82,54 @@ pub fn init(verbosity: impl Into<Verbosity>, 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();
Expand Down
2 changes: 1 addition & 1 deletion crates/testing/src/slt/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 45dcc23

Please sign in to comment.