diff --git a/Cargo.lock b/Cargo.lock index 687c5fa88..d1d39456f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3360,6 +3360,7 @@ dependencies = [ "pbkdf2", "petgraph", "rand", + "regex", "serde", "test-macros", "thiserror", diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index 82bfb6992..ae1f5cd23 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -29,6 +29,7 @@ parking_lot = { version = "0.12.3", optional = true } pbkdf2 = { version = "0.12.2", features = ["simple"] } petgraph = "0.6.4" rand = "0.8.5" +regex = "1.10.4" serde = { version = "1.0.203", features = ["derive"] } thiserror = "1.0.61" tokio = { version = "0.2.25", package = "madsim-tokio", features = [ diff --git a/crates/utils/src/config.rs b/crates/utils/src/config.rs index dcf7e547d..d37b3c088 100644 --- a/crates/utils/src/config.rs +++ b/crates/utils/src/config.rs @@ -801,9 +801,9 @@ impl LogConfig { /// Generate a new `LogConfig` object #[must_use] #[inline] - pub fn new(path: PathBuf, rotation: RotationConfig, level: LevelConfig) -> Self { + pub fn new(path: Option, rotation: RotationConfig, level: LevelConfig) -> Self { Self { - path: Some(path), + path, rotation, level, } @@ -857,7 +857,7 @@ pub mod rotation_format { #[must_use] #[inline] pub const fn default_rotation() -> RotationConfig { - RotationConfig::Daily + RotationConfig::Never } /// Generates a `RollingFileAppender` from the given `RotationConfig` and `name` @@ -1326,7 +1326,7 @@ mod tests { assert_eq!( config.log, LogConfig::new( - PathBuf::from("/var/log/xline"), + Some(PathBuf::from("/var/log/xline")), RotationConfig::Daily, LevelConfig::INFO ) @@ -1452,8 +1452,8 @@ mod tests { assert_eq!( config.log, LogConfig::new( - PathBuf::from("/var/log/xline"), - RotationConfig::Daily, + Some(PathBuf::from("/var/log/xline")), + RotationConfig::Never, LevelConfig::INFO ) ); diff --git a/crates/utils/src/parser.rs b/crates/utils/src/parser.rs index ba7186074..e7e4e8520 100644 --- a/crates/utils/src/parser.rs +++ b/crates/utils/src/parser.rs @@ -1,6 +1,7 @@ -use std::{collections::HashMap, time::Duration}; +use std::{collections::HashMap, path::PathBuf, time::Duration}; use clippy_utilities::OverflowArithmetic; +use regex::Regex; use thiserror::Error; use crate::config::{ @@ -162,6 +163,37 @@ pub fn parse_state(s: &str) -> Result { } } +/// Parse `LOG_PATH` from string +/// # Errors +/// Return error when parsing the given string to `PathBuf` failed +#[inline] +pub fn parse_log_file(s: &str) -> Result { + if s.is_empty() { + return Err(ConfigParseError::InvalidValue( + "the log path should not be empty".to_owned(), + )); + } + // This regular expression matches file paths in a specific format. + // Explanation of the regex pattern: + // ^(\.|\.\.)? - Matches an optional prefix consisting of a dot (.), two dots (..), or a tilde (~). + // (/[a-zA-Z0-9._-]+)+/? - Matches one or more occurrences of a forward slash (/) followed by one or more alphanumeric characters, dots (.), underscores (_), or hyphens (-). + // /?$ - Matches an optional trailing forward slash (/) at the end of the string. + // Overall, this regex pattern is used to validate file paths that follow a specific format commonly used in Linux filesystem + let re = Regex::new(r"^(\.|\.\.)?(/[a-zA-Z0-9._-]+)+/?$").unwrap_or_else(|err| { + unreachable!( + r"regex expression (^(\.|\.\.)?(/[a-zA-Z0-9._-]+)+/?$) should not return Error: {}", + err.to_string() + ) + }); + if re.is_match(s) { + Ok(PathBuf::from(s)) + } else { + Err(ConfigParseError::InvalidValue(format!( + "Invalid log path {s}" + ))) + } +} + /// Parse `LevelConfig` from string /// # Errors /// Return error when parsing the given string to `LevelConfig` failed @@ -366,4 +398,30 @@ mod test { ); assert!(parse_metrics_push_protocol("thrift").is_err()); } + + #[test] + fn test_parse_log_file() { + // Test case 1: Valid log file path + assert!(parse_log_file("/path/to/log/file.log").is_ok()); + + // Test case 2: Empty log file path + assert!(parse_log_file("").is_err()); + + // Test case 4: Log file path with spaces + assert!(parse_log_file("/path/with spaces/log file.log").is_err()); + + // Test case 5: Log file path with special character + assert!(parse_log_file("/path/with@spaces/log?!file.log").is_err()); + + // Test case 6: Log file path with special character + assert!(parse_log_file("/path/with-spaces/log_file.log-123.456-789").is_ok()); + + // Test case 7: Log file path starts with ., .. or ~ + assert!(parse_log_file("./path/with-spaces/log_file.log-123.456-789").is_ok()); + assert!(parse_log_file("../path/with-spaces/log_file.log-123.456-789").is_ok()); + assert!(parse_log_file("~/path/with-spaces/log_file.log-123.456-789").is_err()); + + assert!(parse_log_file(".../path/with-spaces/log_file.log-123.456-789").is_err()); + assert!(parse_log_file("~~/path/with-spaces/log_file.log-123.456-789").is_err()); + } } diff --git a/crates/xline/src/utils/args.rs b/crates/xline/src/utils/args.rs index 199d26b7e..f16c55002 100644 --- a/crates/xline/src/utils/args.rs +++ b/crates/xline/src/utils/args.rs @@ -20,8 +20,8 @@ use utils::{ MetricsPushProtocol, RotationConfig, ServerTimeout, StorageConfig, TlsConfig, TraceConfig, XlineServerConfig, }, - parse_batch_bytes, parse_duration, parse_log_level, parse_members, parse_metrics_push_protocol, - parse_rotation, parse_state, ConfigFileError, + parse_batch_bytes, parse_duration, parse_log_file, parse_log_level, parse_members, + parse_metrics_push_protocol, parse_rotation, parse_state, ConfigFileError, }; /// Xline server config path env name @@ -92,8 +92,8 @@ pub struct ServerArgs { #[clap(long, value_parser = parse_metrics_push_protocol, default_value_t = default_metrics_push_protocol())] metrics_push_protocol: MetricsPushProtocol, /// Log file path - #[clap(long, default_value = "/var/log/xline")] - log_file: PathBuf, + #[clap(long, value_parser = parse_log_file, default_value = None)] + log_file: Option, /// Log rotate strategy, eg: never, hourly, daily #[clap(long, value_parser = parse_rotation, default_value_t = default_rotation())] log_rotate: RotationConfig, diff --git a/crates/xline/src/utils/trace.rs b/crates/xline/src/utils/trace.rs index 3b3fda98c..9384626a1 100644 --- a/crates/xline/src/utils/trace.rs +++ b/crates/xline/src/utils/trace.rs @@ -1,9 +1,22 @@ -use anyhow::Result; +use anyhow::{Ok, Result}; use opentelemetry_contrib::trace::exporter::jaeger_json::JaegerJsonExporter; use opentelemetry_sdk::runtime::Tokio; +use tracing::warn; use tracing_appender::non_blocking::WorkerGuard; use tracing_subscriber::{fmt::format, layer::SubscriberExt, util::SubscriberInitExt, Layer}; -use utils::config::{file_appender, LogConfig, TraceConfig}; +use utils::config::{file_appender, LogConfig, RotationConfig, TraceConfig}; + +/// Return a Box trait from the config +fn generate_writer(name: &str, log_config: &LogConfig) -> Box { + if let Some(ref file_path) = *log_config.path() { + Box::new(file_appender(*log_config.rotation(), file_path, name)) + } else { + if matches!(*log_config.rotation(), RotationConfig::Never) { + warn!("The log is output to the terminal, so the rotation parameter is ignored."); + } + Box::new(std::io::stdout()) + } +} /// init tracing subscriber /// # Errors @@ -14,19 +27,6 @@ pub fn init_subscriber( log_config: &LogConfig, trace_config: &TraceConfig, ) -> Result> { - let mut guard = None; - let log_file_layer = log_config.path().as_ref().map(|log_path| { - let file_appender = file_appender(*log_config.rotation(), log_path, name); - // `WorkerGuard` should be assigned in the `main` function or whatever the entrypoint of the program is. - let (non_blocking, guard_inner) = tracing_appender::non_blocking(file_appender); - guard = Some(guard_inner); - tracing_subscriber::fmt::layer() - .event_format(format().compact()) - .with_writer(non_blocking) - .with_ansi(false) - .with_filter(*log_config.level()) - }); - let jaeger_level = *trace_config.jaeger_level(); let jaeger_online_layer = trace_config .jaeger_online() @@ -55,15 +55,20 @@ pub fn init_subscriber( .install_batch(), ) }); - let jaeger_fmt_layer = tracing_subscriber::fmt::layer() .with_filter(tracing_subscriber::EnvFilter::from_default_env()); - + let writer = generate_writer(name, log_config); + let (non_blocking, guard) = tracing_appender::non_blocking(writer); + let log_layer = tracing_subscriber::fmt::layer() + .event_format(format().compact()) + .with_writer(non_blocking) + .with_ansi(false) + .with_filter(*log_config.level()); tracing_subscriber::registry() - .with(log_file_layer) .with(jaeger_fmt_layer) .with(jaeger_online_layer) .with(jaeger_offline_layer) + .with(log_layer) .try_init()?; - Ok(guard) + Ok(Some(guard)) } diff --git a/scripts/common.sh b/scripts/common.sh index ee894edea..cd7bb555c 100644 --- a/scripts/common.sh +++ b/scripts/common.sh @@ -7,21 +7,6 @@ NEWMEMBERS="${MEMBERS},node4=${SERVERS[4]}:2380,${SERVERS[4]}:2381" XLINE_IMAGE="ghcr.io/xline-kv/xline:latest" ETCDCTL_IMAGE="ghcr.io/xline-kv/etcdctl:v3.5.9" -function common::run_container() { - ith=${1} - mount_point="-v ${DIR}:/mnt" - if [ -n "$LOG_PATH" ]; then - mkdir -p ${LOG_PATH}/node${ith} - mount_point="${mount_point} -v ${LOG_PATH}/node${ith}:/var/log/xline" - fi - log::info starting container node${ith} ... - docker run -d -it --rm --name=node${ith} --net=xline_net \ - --ip=${SERVERS[$ith]} --cap-add=NET_ADMIN --cpu-shares=1024 \ - -m=512M ${mount_point} ${XLINE_IMAGE} bash & - wait $! - log::info container node${ith} started -} - function common::stop_container() { image_name=$1 log::info stopping container ${image_name} ... @@ -49,6 +34,14 @@ function common::run_etcd_client() { # $2: members # $3: initial cluster state function common::run_xline() { + ith=${1} + mount_point="-v ${DIR}:/mnt" + if [ -n "$LOG_PATH" ]; then + mkdir -p ${DIR}/logs/node${ith} + mount_point="${mount_point} -v ${DIR}/logs/node${ith}:${LOG_PATH}" + fi + + log::info starting container node${ith} ... cmd="/usr/local/bin/xline \ --name node${1} \ --members ${2} \ @@ -63,7 +56,7 @@ function common::run_xline() { --initial-cluster-state=${3}" if [ -n "${LOG_PATH}" ]; then - cmd="${cmd} --log-file ${LOG_PATH}/node${1}" + cmd="${cmd} --log-file ${LOG_PATH}" fi if [ -n "$LOG_LEVEL" ]; then @@ -74,6 +67,18 @@ function common::run_xline() { cmd="${cmd} --is-leader" fi - docker exec -e RUST_LOG=info -d node${1} ${cmd} - log::info "command is: docker exec -e RUST_LOG=debug -d node${1} ${cmd}" + if [ -n "$RUST_LOG" ]; then + debug_env="-e RUST_LOG=${RUST_LOG}" + fi + + docker_cmd="docker run ${debug_env} -d -it --rm --name=node${ith} --net=xline_net \ + --ip=${SERVERS[$ith]} --cap-add=NET_ADMIN --cpu-shares=1024 \ + -m=512M ${mount_point} ${XLINE_IMAGE} ${cmd}" + + eval ${docker_cmd} + wait $! + + log::info "${docker_cmd}" + + } diff --git a/scripts/quick_start.sh b/scripts/quick_start.sh index 6fac8b89d..406171b88 100755 --- a/scripts/quick_start.sh +++ b/scripts/quick_start.sh @@ -11,11 +11,7 @@ source $DIR/log.sh stop_all() { log::info stopping for name in "node1" "node2" "node3" "client"; do - docker_id=$(docker ps -qf "name=${name}") - if [ -n "$docker_id" ]; then - docker exec $docker_id rm -rf $LOG_PATH/$name - docker stop $docker_id -t 1 - fi + common::stop_container ${name} done docker network rm xline_net >/dev/null 2>&1 docker stop "prometheus" > /dev/null 2>&1 @@ -29,22 +25,11 @@ run_cluster() { common::run_xline 1 ${MEMBERS} new common::run_xline 2 ${MEMBERS} new common::run_xline 3 ${MEMBERS} new + common::run_etcd_client wait log::info cluster started } -# run container of xline/etcd use specified image -# args: -# $1: size of cluster -run_container() { - size=${1} - for ((i = 1; i <= ${size}; i++)); do - common::run_container ${i} - done - common::run_etcd_client -} - - # run prometheus run_prometheus() { docker run -d -it --rm --name=prometheus --net=xline_net -p 9090:9090 \ @@ -56,7 +41,6 @@ if [ -z "$1" ]; then stop_all docker network create --subnet=172.20.0.0/24 xline_net >/dev/null 2>&1 log::warn "A Docker network named 'xline_net' is created for communication among various xline nodes. You can use the command 'docker network rm xline_net' to remove it after use." - run_container 3 run_cluster run_prometheus "172.20.0.6" echo "Prometheus starts on http://172.20.0.6:9090/graph and http://127.0.0.1:9090/graph (if you are using Docker Desktop)." diff --git a/scripts/validation_test.sh b/scripts/validation_test.sh index d454d4775..f53a8d9c7 100755 --- a/scripts/validation_test.sh +++ b/scripts/validation_test.sh @@ -6,7 +6,7 @@ source $DIR/log.sh QUICK_START="${DIR}/quick_start.sh" ETCDCTL="docker exec -i client etcdctl --endpoints=http://172.20.0.3:2379,http://172.20.0.4:2379" LOCK_CLIENT="docker exec -i client /mnt/validation_lock_client --endpoints=http://172.20.0.3:2379,http://172.20.0.4:2379,http://172.20.0.5:2379" -export LOG_PATH=/mnt/logs +export LOG_PATH=/var/log/xline export LOG_LEVEL=debug bash ${QUICK_START} @@ -30,7 +30,6 @@ function run_new_member() { if [ -n "$docker_id" ]; then docker stop $docker_id fi - common::run_container 4 common::run_xline 4 ${NEWMEMBERS} existing }