feat: support redirecting the logs to the terminal

Signed-off-by: GFX9 <[email protected]>

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 = [

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<PathBuf>, 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
             )
         );

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<InitialClusterState, ConfigParseError> {
     }
 }
 
+/// 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<PathBuf, ConfigParseError> {
+    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());
+    }
 }

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<PathBuf>,
     /// Log rotate strategy, eg: never, hourly, daily
     #[clap(long, value_parser = parse_rotation, default_value_t = default_rotation())]
     log_rotate: RotationConfig,

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<dyn std::io::Write + Send> {
+    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<Option<WorkerGuard>> {
-    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))
 }

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}"
+
+
 }

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)."

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
 }