From 602ea0804f2921834a27876096e575f34a0c04e3 Mon Sep 17 00:00:00 2001
From: GFX9 <xwdtoni@126.com>
Date: Wed, 1 May 2024 23:09:21 +0800
Subject: [PATCH] feat: Use Option<PathBuf> && write log to stdout by default

Signed-off-by: GFX9 <xwdtoni@126.com>
---
 crates/utils/src/config.rs      |  8 ++++----
 crates/utils/src/parser.rs      | 21 ++++++++++++++++++++-
 crates/xline/src/utils/args.rs  |  8 +++-----
 crates/xline/src/utils/trace.rs |  5 ++---
 scripts/quick_start.sh          |  3 +--
 5 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/crates/utils/src/config.rs b/crates/utils/src/config.rs
index e6c484672..88c628165 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<PathBuf>, rotation: RotationConfig, level: LevelConfig) -> Self {
         Self {
-            path: Some(path),
+            path: path,
             rotation,
             level,
         }
@@ -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,7 +1452,7 @@ mod tests {
         assert_eq!(
             config.log,
             LogConfig::new(
-                PathBuf::from("/var/log/xline"),
+                Some(PathBuf::from("/var/log/xline")),
                 RotationConfig::Daily,
                 LevelConfig::INFO
             )
diff --git a/crates/utils/src/parser.rs b/crates/utils/src/parser.rs
index ba7186074..6ff010122 100644
--- a/crates/utils/src/parser.rs
+++ b/crates/utils/src/parser.rs
@@ -1,4 +1,4 @@
-use std::{collections::HashMap, time::Duration};
+use std::{collections::HashMap, path::PathBuf, time::Duration};
 
 use clippy_utilities::OverflowArithmetic;
 use thiserror::Error;
@@ -162,6 +162,25 @@ 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<Option<PathBuf>, ConfigParseError> {
+    if s.is_empty() {
+        return Ok(None);
+    }
+    let path = PathBuf::from(s);
+    if path.exists() {
+        Ok(Some(path))
+    } else {
+        Err(ConfigParseError::InvalidValue(format!(
+            "Log file path is illegal: {}",
+            s
+        )))
+    }
+}
+
 /// Parse `LevelConfig` from string
 /// # Errors
 /// Return error when parsing the given string to `LevelConfig` failed
diff --git a/crates/xline/src/utils/args.rs b/crates/xline/src/utils/args.rs
index 2ec9de836..689e4fc3b 100644
--- a/crates/xline/src/utils/args.rs
+++ b/crates/xline/src/utils/args.rs
@@ -19,9 +19,7 @@ use utils::{
         EngineConfig, InitialClusterState, LevelConfig, LogConfig, MetricsConfig,
         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 +90,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 = "/stdout")]
-    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,
diff --git a/crates/xline/src/utils/trace.rs b/crates/xline/src/utils/trace.rs
index 775b2256c..6406b90a9 100644
--- a/crates/xline/src/utils/trace.rs
+++ b/crates/xline/src/utils/trace.rs
@@ -9,14 +9,13 @@ use utils::config::{default_rotation, file_appender, LogConfig, TraceConfig};
 /// Return a Box trait from the config
 fn generate_writer(name: &str, log_config: &LogConfig) -> Box<dyn std::io::Write + Send> {
     match *log_config.path() {
-        Some(ref file_path) if file_path.to_string_lossy() == "/stdout" => {
+        Some(ref file_path) => Box::new(file_appender(*log_config.rotation(), file_path, name)),
+        None => {
             if *log_config.rotation() != default_rotation() {
                 warn!("The log is output to the terminal, so the rotation parameter is ignored.");
             }
             Box::new(std::io::stdout())
         }
-        Some(ref file_path) => Box::new(file_appender(*log_config.rotation(), file_path, name)),
-        None => unreachable!("the path of log cannot be empty in config"),
     }
 }
 
diff --git a/scripts/quick_start.sh b/scripts/quick_start.sh
index cc91578e2..c0c1de528 100755
--- a/scripts/quick_start.sh
+++ b/scripts/quick_start.sh
@@ -64,13 +64,12 @@ run_xline() {
     log::info "run_xline node${1}: docker run -d -it --rm --name=node${1} -d --net=xline_net \
         --ip=${SERVERS[$1]} --cap-add=NET_ADMIN --cpu-shares=1024 \
         -m=512M ${mount_point} ${image} \
+        -e RUST_LOG=debug
         ${cmd}"
     log::info xline${1} started
 }
 
 # run etcdctl
-# args:
-#   $1: size of cluster
 run_etcdctl() {
     log::info etcdctl starting