From f5d190c3378444d0a9c16c460ec6bbbcb5eb1d5f Mon Sep 17 00:00:00 2001 From: Jarrod Overson Date: Tue, 15 Aug 2023 11:45:05 -0400 Subject: [PATCH] refactor: consolidated include/exclude to one filter string --- Cargo.lock | 1 + crates/bins/wick/Cargo.toml | 1 + crates/bins/wick/src/main.rs | 3 - crates/bins/wick/src/options.rs | 180 ++++++++++++++++------ crates/wick/wick-logger/src/options.rs | 2 +- crates/wick/wick-settings/src/settings.rs | 6 +- 6 files changed, 135 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4bb43d4f..3990a673 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6348,6 +6348,7 @@ dependencies = [ "jaq-core", "markup-converter", "nkeys", + "once_cell", "openssl", "option-utils", "regex", diff --git a/crates/bins/wick/Cargo.toml b/crates/bins/wick/Cargo.toml index 409bf278..1b8ad856 100644 --- a/crates/bins/wick/Cargo.toml +++ b/crates/bins/wick/Cargo.toml @@ -54,6 +54,7 @@ option-utils = { workspace = true } structured-output = { workspace = true } regex = { workspace = true } dhat = { workspace = true, optional = true } +once_cell = { workspace = true } [dev-dependencies] test_bin = { workspace = true } diff --git a/crates/bins/wick/src/main.rs b/crates/bins/wick/src/main.rs index 167bb9f0..7aa148ef 100644 --- a/crates/bins/wick/src/main.rs +++ b/crates/bins/wick/src/main.rs @@ -201,9 +201,6 @@ async fn async_start() -> Result<(GlobalOptions, StructuredOutput), (GlobalOptio let mut cli = Cli::parse_from(args); let options = cli.output.clone(); - // Do a preliminary logger init to catch any logs from the local settings. - // let log_options: wick_logger::LoggingOptions = cli.logging.name(BIN_NAME).into(); - // let settings = wick_logger::with_default(&log_options, Box::new(wick_settings::Settings::new)); let settings = wick_settings::Settings::new(); apply_log_settings(&settings, &mut cli.logging); diff --git a/crates/bins/wick/src/options.rs b/crates/bins/wick/src/options.rs index 3d97c76c..7771e634 100644 --- a/crates/bins/wick/src/options.rs +++ b/crates/bins/wick/src/options.rs @@ -1,7 +1,8 @@ use std::path::PathBuf; use clap::Args; -use wick_logger::{FilterOptions, TargetLevel}; +use once_cell::sync::Lazy; +use wick_logger::{FilterOptions, LogLevel, TargetLevel}; use wick_oci_utils::{OciOptions, OnExisting}; use wick_settings::Credential; @@ -36,21 +37,13 @@ pub(crate) struct LoggingOptions { #[clap(long = "otlp", env = "OTLP_ENDPOINT", global = true, action)] pub(crate) otlp_endpoint: Option, - /// The inclusion filter to apply to events posted to STDERR. - #[clap(long = "log-keep", env = "LOG_INCLUDE", global = true, action)] - pub(crate) log_include: Option, + /// The filter to apply to events posted to STDERR. + #[clap(long = "log-filter", env = "LOG_FILTER", global = true, action)] + pub(crate) stderr_filter: Option, - /// The exclusion filter to apply to events posted to STDERR. - #[clap(long = "log-filter", env = "LOG_EXCLUDE", global = true, action)] - pub(crate) log_exclude: Option, - - /// The inclusion filter to apply to events posted to the OTLP endpoint. - #[clap(long = "otel-keep", env = "OTEL_INCLUDE", global = true, action)] - pub(crate) otel_include: Option, - - /// The exclusion filter to apply to events posted to the OTLP endpoint. - #[clap(long = "otel-filter", env = "OTEL_EXCLUDE", global = true, action)] - pub(crate) otel_exclude: Option, + /// The filter to apply to events posted to the OTLP endpoint. + #[clap(long = "otel-filter", env = "OTEL_FILTER", global = true, action)] + pub(crate) tel_filter: Option, /// The application doing the logging. #[clap(skip)] @@ -65,31 +58,80 @@ impl LoggingOptions { } } -fn parse_logstr(value: &str) -> Vec { - value +static DEFAULT_EXCLUSION_FILTER: Lazy> = Lazy::new(|| { + vec![ + TargetLevel::new("wasmrs", wick_logger::LogLevel::Error), + TargetLevel::new("wasmrs_runtime", wick_logger::LogLevel::Error), + TargetLevel::new("wasmrs_wasmtime", wick_logger::LogLevel::Error), + ] +}); + +static DEFAULT_INCLUSION_FILTER: Lazy> = + Lazy::new(|| vec![TargetLevel::new("flow", wick_logger::LogLevel::Warn)]); + +fn parse_logstr( + default_level: LogLevel, + default_inc: &[TargetLevel], + default_exc: &[TargetLevel], + value: &str, +) -> FilterOptions { + let parts: Vec<(bool, Option<&str>, LogLevel)> = value .split(',') .filter_map(|s| { - if s.is_empty() || !s.contains('=') { + let s = s.trim(); + if s.is_empty() { return None; } + if !s.contains('=') { + return Some((false, None, s.parse().ok()?)); + } let mut parts = s.split('='); - let target = parts.next()?; - let level = parts.next().unwrap_or("info"); - Some(TargetLevel { - target: target.to_owned(), - level: level.parse().ok()?, - }) + let target = parts.next()?.trim(); + let (negated, target) = if target.starts_with('!') { + (true, target.trim_start_matches('!').trim()) + } else { + (false, target) + }; + let level = parts.next().unwrap_or("info").trim(); + Some((negated, Some(target), level.parse().ok()?)) + }) + .collect(); + + let global_level = parts + .iter() + .find(|(_, target, _)| target.is_none()) + .map_or_else(|| default_level, |(_, _, level)| *level); + + let exclude = parts + .iter() + .filter_map(|(negated, target, level)| match negated { + true => target.map(|target| TargetLevel::new(target, *level)), + false => None, + }) + .collect::>(); + let include = parts + .iter() + .filter_map(|(negated, target, level)| match negated { + true => None, + false => target.map(|target| TargetLevel::new(target, *level)), }) - .collect() + .collect::>(); + + FilterOptions { + level: global_level, + // If the filter had inclusion rules, use those. Otherwise, use the default. + include: if include.is_empty() { + default_inc.to_vec() + } else { + include + }, + // If the filter had exclusion rules, add them to our defaults. + exclude: [default_exc.to_vec(), exclude].concat(), + } } impl From<&LoggingOptions> for wick_logger::LoggingOptions { fn from(value: &LoggingOptions) -> Self { - let stderr_inc = value.log_include.as_deref().map_or_else(Vec::new, parse_logstr); - let stderr_exc = value.log_exclude.as_deref().map_or_else(Vec::new, parse_logstr); - let otel_inc = value.otel_include.as_deref().map_or_else(Vec::new, parse_logstr); - let otel_exc = value.otel_exclude.as_deref().map_or_else(Vec::new, parse_logstr); - let global_level = if value.quiet { wick_logger::LogLevel::Quiet } else if value.trace { @@ -100,12 +142,20 @@ impl From<&LoggingOptions> for wick_logger::LoggingOptions { wick_logger::LogLevel::Info }; - let default_inc = vec![TargetLevel::new("flow", wick_logger::LogLevel::Warn)]; - let default_exc = vec![ - TargetLevel::new("wasmrs", wick_logger::LogLevel::Error), - TargetLevel::new("wasmrs_runtime", wick_logger::LogLevel::Error), - TargetLevel::new("wasmrs_wasmtime", wick_logger::LogLevel::Error), - ]; + let stderr_opts = parse_logstr( + global_level, + &DEFAULT_INCLUSION_FILTER, + &DEFAULT_EXCLUSION_FILTER, + value.stderr_filter.as_deref().unwrap_or_default(), + ); + + let otel_opts = parse_logstr( + global_level, + &DEFAULT_INCLUSION_FILTER, + &DEFAULT_EXCLUSION_FILTER, + value.tel_filter.as_deref().unwrap_or_default(), + ); + Self { verbose: value.verbose == 1, log_json: false, @@ -114,16 +164,8 @@ impl From<&LoggingOptions> for wick_logger::LoggingOptions { app_name: value.app_name.clone(), global: false, levels: wick_logger::LogFilters { - telemetry: FilterOptions { - level: global_level, - include: [otel_inc, default_inc.clone()].concat(), - exclude: [otel_exc, default_exc.clone()].concat(), - }, - stderr: FilterOptions { - level: global_level, - include: [stderr_inc, default_inc].concat(), - exclude: [stderr_exc, default_exc].concat(), - }, + telemetry: otel_opts, + stderr: stderr_opts, }, } } @@ -149,12 +191,10 @@ pub(crate) fn apply_log_settings(settings: &wick_settings::Settings, options: &m options.verbose = 1; } if let Some(otel_settings) = &settings.trace.telemetry { - options.otel_include = otel_settings.include.clone(); - options.otel_exclude = otel_settings.exclude.clone(); + options.tel_filter = otel_settings.filter.clone(); } if let Some(log_settings) = &settings.trace.stderr { - options.log_include = log_settings.include.clone(); - options.log_exclude = log_settings.exclude.clone(); + options.stderr_filter = log_settings.filter.clone(); } if options.otlp_endpoint.is_none() { options.otlp_endpoint = settings.trace.otlp.clone(); @@ -219,3 +259,43 @@ pub(crate) fn reconcile_fetch_options( } oci_opts } + +#[cfg(test)] +mod test { + use anyhow::Result; + + use super::*; + + type ExpectedLogRule = (LogLevel, Vec, Vec); + + #[rstest::rstest] + #[case(LogLevel::Info, "trace", (LogLevel::Trace, DEFAULT_INCLUSION_FILTER.to_vec(),vec![]))] + #[case(LogLevel::Info, "wick=trace", (LogLevel::Info, vec![TargetLevel::new("wick", LogLevel::Trace)],vec![]))] + #[case(LogLevel::Info, "debug,wick=trace", (LogLevel::Debug, vec![TargetLevel::new("wick", LogLevel::Trace)],vec![]))] + #[case(LogLevel::Info, "wick=trace,debug", (LogLevel::Debug, vec![TargetLevel::new("wick", LogLevel::Trace)],vec![]))] + #[case(LogLevel::Info, " wick=trace , debug ,,, ,", (LogLevel::Debug, vec![TargetLevel::new("wick", LogLevel::Trace)],vec![]))] + #[case(LogLevel::Info, "wick=trace,!flow=info", (LogLevel::Info, vec![TargetLevel::new("wick", LogLevel::Trace)],vec![TargetLevel::new("flow", LogLevel::Info)]))] + #[case(LogLevel::Info, "wick=trace,!flow=info,wasmrs=info", (LogLevel::Info, vec![TargetLevel::new("wick", LogLevel::Trace),TargetLevel::new("wasmrs", LogLevel::Info)],vec![TargetLevel::new("flow", LogLevel::Info)]))] + #[case(LogLevel::Info, "wick = trace, ! flow = info, wasmrs = info", (LogLevel::Info, vec![TargetLevel::new("wick", LogLevel::Trace),TargetLevel::new("wasmrs", LogLevel::Info)],vec![TargetLevel::new("flow", LogLevel::Info)]))] + fn test_log_rules( + #[case] default_loglevel: LogLevel, + #[case] filter: &str, + #[case] expected: ExpectedLogRule, + ) -> Result<()> { + let filter = parse_logstr( + default_loglevel, + &DEFAULT_INCLUSION_FILTER, + &DEFAULT_EXCLUSION_FILTER, + filter, + ); + assert_eq!(filter.level, expected.0); + assert_eq!(filter.include, expected.1); + + // prepend the default exclusion filter so we don't need to include it in test cases above. + let expected_exclude = [DEFAULT_EXCLUSION_FILTER.to_vec(), expected.2].concat(); + + assert_eq!(filter.exclude, expected_exclude); + + Ok(()) + } +} diff --git a/crates/wick/wick-logger/src/options.rs b/crates/wick/wick-logger/src/options.rs index f7c3ce86..dbe8685f 100644 --- a/crates/wick/wick-logger/src/options.rs +++ b/crates/wick/wick-logger/src/options.rs @@ -113,7 +113,7 @@ where } /// The log level for specific targets. -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default, PartialEq, Clone)] pub struct TargetLevel { /// The target (module name). pub target: String, diff --git a/crates/wick/wick-settings/src/settings.rs b/crates/wick/wick-settings/src/settings.rs index 737828ad..2d53a65f 100644 --- a/crates/wick/wick-settings/src/settings.rs +++ b/crates/wick/wick-settings/src/settings.rs @@ -49,10 +49,8 @@ pub struct TraceSettings { #[serde(deny_unknown_fields)] /// Log filter settings pub struct LogSettings { - /// Inclusion filter. - pub include: Option, - /// Exclusion filter. - pub exclude: Option, + /// Log event filter. + pub filter: Option, } #[derive(Debug, Deserialize, Serialize)]