Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --logs flag to local command #2187

Merged
merged 9 commits into from
Dec 8, 2023
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());
Lilit0x marked this conversation as resolved.
Show resolved Hide resolved

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