From cb952ace3acd9e07e38d9ea5df7c95e8a37a17e4 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Fri, 18 Aug 2023 15:53:24 +0300 Subject: [PATCH 01/20] add backend --- Cargo.toml | 9 +++++++++ backend/Cargo.toml | 1 + cli/Cargo.toml | 1 + 3 files changed, 11 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 5dc6f159..3eca40a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,11 +12,20 @@ default-members = [ "frontend", "backend"] resolver = "2" [profile.release] +# Link Time optimization, causes a bit longer compilation +# lto = true + # Optimize for size # opt-level = "s" # Optimize for speed opt-level = 3 +# Strip symbols from binary; Turn off for cargo bloat command +strip = true + +# Maximize size reduction optimization, causes longer compilation +codegen-units = 1 + # Slightly increase perfomance and reduce binary size panic = "abort" diff --git a/backend/Cargo.toml b/backend/Cargo.toml index ecb7a738..8a1af9cc 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -45,3 +45,4 @@ cli = { path = "../cli" } [features] default = [ "swagger" ] swagger = [ "dep:utoipa", "dep:utoipa-swagger-ui" , "dep:utoipa-redoc", "dep:utoipa-rapidoc" ] + diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 0e880854..f2b26d52 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -25,3 +25,4 @@ clap = { version = "4.4", features = ["derive", "cargo"] } # Meta-info build-time = "0.1" lazy_static="1.4" + From f201390662b1ddbb679052099eb9a4381fa1ea9a Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Thu, 7 Sep 2023 10:14:09 +0300 Subject: [PATCH 02/20] post review fixes pt.1 --- Cargo.toml | 9 --------- backend/src/lib.rs | 1 - build.rs | 2 ++ 3 files changed, 2 insertions(+), 10 deletions(-) create mode 100644 build.rs diff --git a/Cargo.toml b/Cargo.toml index 3eca40a8..5dc6f159 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,20 +12,11 @@ default-members = [ "frontend", "backend"] resolver = "2" [profile.release] -# Link Time optimization, causes a bit longer compilation -# lto = true - # Optimize for size # opt-level = "s" # Optimize for speed opt-level = 3 -# Strip symbols from binary; Turn off for cargo bloat command -strip = true - -# Maximize size reduction optimization, causes longer compilation -codegen-units = 1 - # Slightly increase perfomance and reduce binary size panic = "abort" diff --git a/backend/src/lib.rs b/backend/src/lib.rs index 9cadb2ec..ceea80ae 100644 --- a/backend/src/lib.rs +++ b/backend/src/lib.rs @@ -4,7 +4,6 @@ use axum::{routing::get, Router}; use utoipa::OpenApi; - pub mod config; pub mod connector; pub mod error; diff --git a/build.rs b/build.rs new file mode 100644 index 00000000..cd52f649 --- /dev/null +++ b/build.rs @@ -0,0 +1,2 @@ +fn main() {} + From 8ad4e43f596038a8b07a8716d301f685376b6842 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sun, 10 Sep 2023 16:44:15 +0300 Subject: [PATCH 03/20] post review fixes pt.2 --- build.rs | 1 - cli/src/config.rs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/build.rs b/build.rs index cd52f649..f328e4d9 100644 --- a/build.rs +++ b/build.rs @@ -1,2 +1 @@ fn main() {} - diff --git a/cli/src/config.rs b/cli/src/config.rs index 10568eaf..73f9cd32 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -48,6 +48,7 @@ pub struct LoggerConfig { pub trace_level: tracing::Level, } + impl Default for Config { fn default() -> Self { Self { From b858a016f611269812f8e1865d007ea0f9f5a656 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sun, 10 Sep 2023 17:34:22 +0300 Subject: [PATCH 04/20] remove wrappers --- cli/src/config.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/src/config.rs b/cli/src/config.rs index 73f9cd32..10568eaf 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -48,7 +48,6 @@ pub struct LoggerConfig { pub trace_level: tracing::Level, } - impl Default for Config { fn default() -> Self { Self { From 2932ae860fbd1851fcf053d31a8a4208e15e5b2d Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sun, 17 Sep 2023 17:58:34 +0300 Subject: [PATCH 05/20] post review fixes (WIP) --- cli/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/Cargo.toml b/cli/Cargo.toml index f2b26d52..0e880854 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -25,4 +25,3 @@ clap = { version = "4.4", features = ["derive", "cargo"] } # Meta-info build-time = "0.1" lazy_static="1.4" - From 47a7769d3eadd8f3d2a656db9687487b397c0389 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Tue, 19 Sep 2023 12:00:15 +0300 Subject: [PATCH 06/20] change build flow + error handling --- frontend/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/Cargo.toml b/frontend/Cargo.toml index 71019618..20b05c78 100644 --- a/frontend/Cargo.toml +++ b/frontend/Cargo.toml @@ -19,4 +19,3 @@ tsync = "2" [dependencies] tsync = "2" - From f6d9b15ba4f8130717b23cc6b3d18533a407e32c Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Wed, 20 Sep 2023 22:51:24 +0300 Subject: [PATCH 07/20] log4rs add (WIP) --- backend/Cargo.toml | 2 ++ backend/src/config.rs | 62 +++++++++++++++++++++++++++++++++++++- backend/src/main.rs | 12 +++++--- cli/Cargo.toml | 1 + cli/src/config.rs | 6 ++-- frontend/package-lock.json | 6 ++++ 6 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 frontend/package-lock.json diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 8a1af9cc..2092e625 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -23,6 +23,8 @@ tower-http = { version = "0.4", features = ["cors", "fs"] } ## Logging tracing = "0.1" tracing-subscriber = "0.3" +log4rs = { version = "1.2", features = ["rolling_file_appender", "delete_roller"] } +log = "0.4" ## Error Handling error-stack = "0.4" diff --git a/backend/src/config.rs b/backend/src/config.rs index 9cce6bd5..2dead9af 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -1,4 +1,18 @@ -use cli::Config; +use cli::{Config, LoggerConfig}; +use log4rs::{ + append::rolling_file::{ + policy::compound::{ + roll::{delete::DeleteRoller, fixed_window::FixedWindowRoller}, + trigger::size::SizeTrigger, + CompoundPolicy, + }, + RollingFileAppender, RollingFileAppenderBuilder, + }, + config::{Appender, Root}, + encode::pattern::PatternEncoder, + filter::threshold::ThresholdFilter, + Handle, +}; use tower_http::cors::CorsLayer; pub trait ConfigExt { @@ -7,6 +21,11 @@ pub trait ConfigExt { fn get_cors_configuration(&self) -> CorsLayer; } +pub trait LoggerExt { + /// Return [`tracing_appender`] instance based on [`LoggerConfig`]'s `rotation_frequency` field + fn get_tracing_appender(&self) -> Result; +} + impl ConfigExt for Config { fn get_cors_configuration(&self) -> CorsLayer { self.cors_allow_all @@ -14,3 +33,44 @@ impl ConfigExt for Config { .unwrap_or_default() } } +impl LoggerExt for LoggerConfig { + fn get_tracing_appender(&self) -> Result { + // let logfile = RollingFileAppender::builder().build(self.log_file, CompoundPolicy::new(SizeTrigger::new(self.log_size), DeleteRoller::new())); + // let config = log4rs::Config::builder().appender(Appender::builder().) + let window_size = 3; // log0, log1, log2 + let fixed_window_roller = FixedWindowRoller::builder() + .build("log{}", window_size) + .unwrap(); + let size_limit = 5 * 1024; // 5KB as max log file size to roll + let size_trigger = SizeTrigger::new(size_limit); + let compound_policy = + CompoundPolicy::new(Box::new(size_trigger), Box::new(fixed_window_roller)); + let config = log4rs::Config::builder() + .appender( + Appender::builder() + .filter(Box::new(ThresholdFilter::new(log::LevelFilter::Debug))) + .build( + "logfile", + Box::new( + RollingFileAppender::builder() + .encoder(Box::new(PatternEncoder::new("{d} {l}::{m}{n}"))) + .build("logfile", Box::new(compound_policy)) + .unwrap(), + ), + ), + ) + .build( + Root::builder() + .appender("logfile") + .build(log::LevelFilter::Debug), + ) + .unwrap(); + + Ok(log4rs::init_config(config).unwrap()) + } +} + +#[derive(Debug)] +pub enum LoggerError { + InitError, +} diff --git a/backend/src/main.rs b/backend/src/main.rs index 5ec70d1c..b1f4f8c8 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -28,7 +28,7 @@ async fn main() -> Result<(), AppError> { let logger = &config.logger; - init_tracer(&logger.log_file, logger.trace_level); + // init_tracer(logger.clone(), logger.trace_level); tracing::info!("Logger: {logger:?}"); let cors: CorsLayer = config.get_cors_configuration(); @@ -50,10 +50,12 @@ async fn main() -> Result<(), AppError> { Ok(()) } -fn init_tracer(_log_file: &Option, trace_level: Level) { - let subscriber = tracing_subscriber::fmt().with_max_level(trace_level); - subscriber.init(); -} +// fn init_tracer(log_file: LoggerConfig, trace_level: Level) { +// let handle = log_file.get_tracing_appender().unwrap(); +// // let sub = tracing_subscriber::registry(). +// let subscriber = tracing_subscriber::fmt().with_max_level(trace_level); +// subscriber.init(); +// } fn router(cors: CorsLayer) -> Router { let mut frontend = env::current_exe().expect("Couldn't get current executable path."); diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 0e880854..376909ce 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -14,6 +14,7 @@ serde_yaml = "0.9" serde_with = "3.3" humantime-serde = "1.1" tracing = "0.1" +tracing-appender = "0.2" # Error Handling error-stack = "0.4" diff --git a/cli/src/config.rs b/cli/src/config.rs index 10568eaf..3b2eb0f6 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -31,14 +31,14 @@ pub struct Config { #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct LoggerConfig { - /// [Stub] File to save logs + /// File to save logs pub log_file: Option, - /// [Stub] Number of log files + /// Number of log files #[serde(default = "LoggerConfig::default_log_amount")] pub log_amount: usize, - /// [Stub] Max size of a single log file, in bytes + /// Max size of a single log file, in bytes #[serde(default = "LoggerConfig::default_log_size")] pub log_size: u64, diff --git a/frontend/package-lock.json b/frontend/package-lock.json new file mode 100644 index 00000000..aba25f73 --- /dev/null +++ b/frontend/package-lock.json @@ -0,0 +1,6 @@ +{ + "name": "frontend", + "lockfileVersion": 3, + "requires": true, + "packages": {} +} From 654d24aecc8c0645cd7e90238e544d587adf776a Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sat, 23 Sep 2023 13:24:14 +0300 Subject: [PATCH 08/20] add loggers --- backend/Cargo.toml | 4 +- backend/src/config.rs | 129 ++++++++++++++++++++++++------------- backend/src/main.rs | 26 ++++---- cli/src/config.rs | 103 ++++++++++++++++++++++++----- config.yaml | 6 +- frontend/package-lock.json | 6 -- 6 files changed, 191 insertions(+), 83 deletions(-) delete mode 100644 frontend/package-lock.json diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 2092e625..c76feeee 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -22,9 +22,9 @@ tower-http = { version = "0.4", features = ["cors", "fs"] } ## Logging tracing = "0.1" +file-rotate = "0.7" +tracing-appender = "0.2" tracing-subscriber = "0.3" -log4rs = { version = "1.2", features = ["rolling_file_appender", "delete_roller"] } -log = "0.4" ## Error Handling error-stack = "0.4" diff --git a/backend/src/config.rs b/backend/src/config.rs index 2dead9af..8b3837e8 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -1,20 +1,13 @@ use cli::{Config, LoggerConfig}; -use log4rs::{ - append::rolling_file::{ - policy::compound::{ - roll::{delete::DeleteRoller, fixed_window::FixedWindowRoller}, - trigger::size::SizeTrigger, - CompoundPolicy, - }, - RollingFileAppender, RollingFileAppenderBuilder, - }, - config::{Appender, Root}, - encode::pattern::PatternEncoder, - filter::threshold::ThresholdFilter, - Handle, -}; +use core::fmt; +use error_stack::Context; +use file_rotate::{suffix::AppendTimestamp, ContentLimit, FileRotate}; +use std::fmt::Display; use tower_http::cors::CorsLayer; +use tracing_appender::non_blocking::WorkerGuard; +use tracing_subscriber::{filter::LevelFilter, prelude::*}; +#[allow(clippy::module_name_repetitions)] pub trait ConfigExt { /// Return either very permissive [`CORS`](`CorsLayer`) configuration /// or empty one based on `cors_allow_all` field @@ -22,8 +15,22 @@ pub trait ConfigExt { } pub trait LoggerExt { - /// Return [`tracing_appender`] instance based on [`LoggerConfig`]'s `rotation_frequency` field - fn get_tracing_appender(&self) -> Result; + /// Initialize logger. + /// + /// Returns [`WorkerGuard`]s for off-thread writers. + /// Should not be dropped. + /// + /// # Errors + /// + /// Function returns error if `init_file_rotate` fails + fn init_logger(&self) -> Result, LoggerError>; + + /// Returns [`std:io::Write`] object that rotates files on write + /// + /// # Errors + /// + /// Function returns error if `log_file` is not specified + fn init_file_rotate(&self) -> Result, LoggerError>; } impl ConfigExt for Config { @@ -33,44 +40,74 @@ impl ConfigExt for Config { .unwrap_or_default() } } + impl LoggerExt for LoggerConfig { - fn get_tracing_appender(&self) -> Result { - // let logfile = RollingFileAppender::builder().build(self.log_file, CompoundPolicy::new(SizeTrigger::new(self.log_size), DeleteRoller::new())); - // let config = log4rs::Config::builder().appender(Appender::builder().) - let window_size = 3; // log0, log1, log2 - let fixed_window_roller = FixedWindowRoller::builder() - .build("log{}", window_size) - .unwrap(); - let size_limit = 5 * 1024; // 5KB as max log file size to roll - let size_trigger = SizeTrigger::new(size_limit); - let compound_policy = - CompoundPolicy::new(Box::new(size_trigger), Box::new(fixed_window_roller)); - let config = log4rs::Config::builder() - .appender( - Appender::builder() - .filter(Box::new(ThresholdFilter::new(log::LevelFilter::Debug))) - .build( - "logfile", - Box::new( - RollingFileAppender::builder() - .encoder(Box::new(PatternEncoder::new("{d} {l}::{m}{n}"))) - .build("logfile", Box::new(compound_policy)) - .unwrap(), - ), + fn init_logger(&self) -> Result, LoggerError> { + let (file_writer, file_guard) = if self.file.trace_level.is_some() { + tracing_appender::non_blocking(self.init_file_rotate()?) + } else { + tracing_appender::non_blocking(std::io::sink()) + }; + let (std_out_writer, stdout_guard) = tracing_appender::non_blocking(std::io::stdout()); + let (std_err_writer, stderr_guard) = tracing_appender::non_blocking(std::io::stderr()); + + tracing_subscriber::registry() + .with( + tracing_subscriber::fmt::layer() + .with_writer(file_writer) + .with_filter( + self.file + .trace_level + .map_or(LevelFilter::OFF, LevelFilter::from_level), ), ) - .build( - Root::builder() - .appender("logfile") - .build(log::LevelFilter::Debug), + .with( + tracing_subscriber::fmt::layer() + .with_writer(std_out_writer) + .with_filter( + self.stdout + .trace_level + .map_or(LevelFilter::OFF, LevelFilter::from_level), + ), + ) + .with( + tracing_subscriber::fmt::layer() + .with_writer(std_err_writer) + .with_filter( + self.stderr + .trace_level + .map_or(LevelFilter::OFF, LevelFilter::from_level), + ), ) - .unwrap(); + .init(); - Ok(log4rs::init_config(config).unwrap()) + Ok(vec![file_guard, stdout_guard, stderr_guard]) + } + + fn init_file_rotate(&self) -> Result, LoggerError> { + Ok(FileRotate::new( + self.file.log_file.as_ref().ok_or(LoggerError::NoFileName)?, + AppendTimestamp::default(file_rotate::suffix::FileLimit::MaxFiles( + self.file.log_amount, + )), + ContentLimit::BytesSurpassed(self.file.log_size), + file_rotate::compression::Compression::OnRotate(1), + None, + )) } } #[derive(Debug)] pub enum LoggerError { - InitError, + NoFileName, +} + +impl Display for LoggerError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::NoFileName => "No filename specified", + }) + } } + +impl Context for LoggerError {} diff --git a/backend/src/main.rs b/backend/src/main.rs index b1f4f8c8..372b7318 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -1,26 +1,29 @@ -#![allow(clippy::multiple_crate_versions)] +#![allow( + clippy::multiple_crate_versions, + clippy::unwrap_used, + clippy::expect_used +)] use axum::Router; use bob_management::{ - config::ConfigExt, + config::{ConfigExt, LoggerExt}, prelude::*, root, router::{ApiV1, ApiVersion, NoApi, RouterApiExt}, services::api_router_v1, ApiDoc, }; -use cli::Parser; +use cli::{LoggerConfig, Parser}; use error_stack::{Result, ResultExt}; use hyper::Method; -use std::{env, path::PathBuf}; +use std::env; use tower::ServiceBuilder; use tower_http::{cors::CorsLayer, services::ServeDir}; -use tracing::Level; +use tracing_appender::non_blocking::WorkerGuard; const FRONTEND_FOLDER: &str = "frontend"; #[tokio::main] -#[allow(clippy::unwrap_used, clippy::expect_used)] async fn main() -> Result<(), AppError> { let config = cli::Config::try_from(cli::Args::parse()) .change_context(AppError::InitializationError) @@ -28,7 +31,7 @@ async fn main() -> Result<(), AppError> { let logger = &config.logger; - // init_tracer(logger.clone(), logger.trace_level); + let _guard = init_tracer(logger); tracing::info!("Logger: {logger:?}"); let cors: CorsLayer = config.get_cors_configuration(); @@ -50,12 +53,9 @@ async fn main() -> Result<(), AppError> { Ok(()) } -// fn init_tracer(log_file: LoggerConfig, trace_level: Level) { -// let handle = log_file.get_tracing_appender().unwrap(); -// // let sub = tracing_subscriber::registry(). -// let subscriber = tracing_subscriber::fmt().with_max_level(trace_level); -// subscriber.init(); -// } +fn init_tracer(log_file: &LoggerConfig) -> Vec { + log_file.init_logger().unwrap() +} fn router(cors: CorsLayer) -> Router { let mut frontend = env::current_exe().expect("Couldn't get current executable path."); diff --git a/cli/src/config.rs b/cli/src/config.rs index 3b2eb0f6..19f53b8a 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -26,26 +26,64 @@ pub struct Config { } /// Logger Configuration passed on initialization -#[allow(clippy::module_name_repetitions)] +#[derive(Clone, Debug, Deserialize, Default)] +#[serde(rename_all = "kebab-case")] +pub struct LoggerConfig { + /// Rolling file logger config + #[serde(default)] + pub file: FileLogger, + + /// Stdout logger config + #[serde(default)] + pub stdout: StdoutLogger, + + /// Stderr logger config + #[serde(default)] + pub stderr: StderrLogger, +} + +/// File Logger Configuration for writing logs to files #[serde_as] #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] -pub struct LoggerConfig { +pub struct FileLogger { /// File to save logs pub log_file: Option, /// Number of log files - #[serde(default = "LoggerConfig::default_log_amount")] + #[serde(default = "FileLogger::default_log_amount")] pub log_amount: usize, /// Max size of a single log file, in bytes - #[serde(default = "LoggerConfig::default_log_size")] - pub log_size: u64, + #[serde(default = "FileLogger::default_log_size")] + pub log_size: usize, /// Tracing Level - #[serde(default = "LoggerConfig::tracing_default")] - #[serde_as(as = "DisplayFromStr")] - pub trace_level: tracing::Level, + #[serde(default = "FileLogger::level_default")] + #[serde_as(as = "Option")] + pub trace_level: Option, +} + +/// Stdout Logger Configuration for printing logs into stdout +#[serde_as] +#[derive(Clone, Debug, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct StdoutLogger { + /// Tracing Level + #[serde(default = "StdoutLogger::level_default")] + #[serde_as(as = "Option")] + pub trace_level: Option, +} + +/// Stdout Logger Configuration for printing logs into stderr +#[serde_as] +#[derive(Clone, Debug, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct StderrLogger { + /// Tracing Level + #[serde(default = "StderrLogger::level_default")] + #[serde_as(as = "Option")] + pub trace_level: Option, } impl Default for Config { @@ -60,36 +98,71 @@ impl Default for Config { } impl Config { + #[must_use] pub const fn default_cors() -> bool { false } + #[must_use] pub const fn default_timeout() -> Duration { Duration::from_millis(5000) } } -impl LoggerConfig { - pub const fn tracing_default() -> tracing::Level { - tracing::Level::INFO +impl FileLogger { + #[must_use] + pub const fn level_default() -> Option { + Some(tracing::Level::TRACE) } + #[must_use] pub const fn default_log_amount() -> usize { 5 } - pub const fn default_log_size() -> u64 { - 10u64.pow(6) + #[must_use] + pub const fn default_log_size() -> usize { + 10usize.pow(6) + } +} + +impl StdoutLogger { + #[must_use] + pub const fn level_default() -> Option { + Some(tracing::Level::INFO) + } +} + +impl StderrLogger { + #[must_use] + pub const fn level_default() -> Option { + Some(tracing::Level::WARN) + } +} + +impl Default for StderrLogger { + fn default() -> Self { + Self { + trace_level: Self::level_default(), + } + } +} + +impl Default for StdoutLogger { + fn default() -> Self { + Self { + trace_level: Self::level_default(), + } } } -impl Default for LoggerConfig { +impl Default for FileLogger { fn default() -> Self { Self { log_file: None, log_amount: Self::default_log_amount(), log_size: Self::default_log_size(), - trace_level: Self::tracing_default(), + trace_level: Self::level_default(), } } } diff --git a/config.yaml b/config.yaml index db7be989..b0713767 100644 --- a/config.yaml +++ b/config.yaml @@ -1,3 +1,7 @@ address: 0.0.0.0:9000 logger: - trace-level: INFO + file: + trace-level: ~ + log-file: /tmp/bob.log + stdout: + trace-level: INFO diff --git a/frontend/package-lock.json b/frontend/package-lock.json deleted file mode 100644 index aba25f73..00000000 --- a/frontend/package-lock.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "name": "frontend", - "lockfileVersion": 3, - "requires": true, - "packages": {} -} From c3d512ea33c0822cbe9122e6aff88b08b67f1ad1 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sat, 23 Sep 2023 13:34:01 +0300 Subject: [PATCH 09/20] clean up --- CHANGELOG.md | 1 + backend/src/main.rs | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d2bf5af..b64e6ba2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,3 +9,4 @@ Bob Management GUI changelog - Initial project structure, backend only (#9) - Initial project stricture, frontend (#10) - Dockerfile and Docker-Compose to simplify deployment (#5) +- Logger Initialization (#14) diff --git a/backend/src/main.rs b/backend/src/main.rs index 372b7318..77e6e9f0 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -13,13 +13,12 @@ use bob_management::{ services::api_router_v1, ApiDoc, }; -use cli::{LoggerConfig, Parser}; +use cli::Parser; use error_stack::{Result, ResultExt}; use hyper::Method; use std::env; use tower::ServiceBuilder; use tower_http::{cors::CorsLayer, services::ServeDir}; -use tracing_appender::non_blocking::WorkerGuard; const FRONTEND_FOLDER: &str = "frontend"; @@ -31,7 +30,7 @@ async fn main() -> Result<(), AppError> { let logger = &config.logger; - let _guard = init_tracer(logger); + let _guard = logger.init_logger().unwrap(); tracing::info!("Logger: {logger:?}"); let cors: CorsLayer = config.get_cors_configuration(); @@ -53,10 +52,6 @@ async fn main() -> Result<(), AppError> { Ok(()) } -fn init_tracer(log_file: &LoggerConfig) -> Vec { - log_file.init_logger().unwrap() -} - fn router(cors: CorsLayer) -> Router { let mut frontend = env::current_exe().expect("Couldn't get current executable path."); frontend.pop(); From ec05d75c59e70f0ed43e1f8f5183338e81ce26be Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sat, 23 Sep 2023 13:40:06 +0300 Subject: [PATCH 10/20] remove dep --- cli/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 376909ce..0e880854 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -14,7 +14,6 @@ serde_yaml = "0.9" serde_with = "3.3" humantime-serde = "1.1" tracing = "0.1" -tracing-appender = "0.2" # Error Handling error-stack = "0.4" From d2f95ebf7e9fbf224921f847885419f4dc010c0f Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Wed, 27 Sep 2023 16:06:58 +0300 Subject: [PATCH 11/20] remove stderr target --- backend/src/config.rs | 52 ++++++++++------------- cli/src/config.rs | 85 ++++++++++++-------------------------- config.yaml | 5 +-- frontend/package-lock.json | 6 +++ 4 files changed, 56 insertions(+), 92 deletions(-) create mode 100644 frontend/package-lock.json diff --git a/backend/src/config.rs b/backend/src/config.rs index 8b3837e8..90b26a8f 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -43,54 +43,44 @@ impl ConfigExt for Config { impl LoggerExt for LoggerConfig { fn init_logger(&self) -> Result, LoggerError> { - let (file_writer, file_guard) = if self.file.trace_level.is_some() { - tracing_appender::non_blocking(self.init_file_rotate()?) + let mut guards = Vec::with_capacity(2); + let file_writer = if self.file.is_some() { + let (writer, guard) = tracing_appender::non_blocking(self.init_file_rotate()?); + guards.push(guard); + writer } else { - tracing_appender::non_blocking(std::io::sink()) + tracing_appender::non_blocking(std::io::sink()).0 + }; + let std_out_writer = if self.stdout.is_some() { + let (writer, guard) = tracing_appender::non_blocking(std::io::stdout()); + guards.push(guard); + writer + } else { + tracing_appender::non_blocking(std::io::sink()).0 }; - let (std_out_writer, stdout_guard) = tracing_appender::non_blocking(std::io::stdout()); - let (std_err_writer, stderr_guard) = tracing_appender::non_blocking(std::io::stderr()); tracing_subscriber::registry() .with( tracing_subscriber::fmt::layer() .with_writer(file_writer) - .with_filter( - self.file - .trace_level - .map_or(LevelFilter::OFF, LevelFilter::from_level), - ), + .with_filter(LevelFilter::from_level(self.trace_level)), ) .with( tracing_subscriber::fmt::layer() .with_writer(std_out_writer) - .with_filter( - self.stdout - .trace_level - .map_or(LevelFilter::OFF, LevelFilter::from_level), - ), - ) - .with( - tracing_subscriber::fmt::layer() - .with_writer(std_err_writer) - .with_filter( - self.stderr - .trace_level - .map_or(LevelFilter::OFF, LevelFilter::from_level), - ), + .with_filter(LevelFilter::from_level(self.trace_level)), ) .init(); - Ok(vec![file_guard, stdout_guard, stderr_guard]) + Ok(guards) } fn init_file_rotate(&self) -> Result, LoggerError> { + let config = self.file.as_ref().ok_or(LoggerError::EmptyConfig)?; Ok(FileRotate::new( - self.file.log_file.as_ref().ok_or(LoggerError::NoFileName)?, - AppendTimestamp::default(file_rotate::suffix::FileLimit::MaxFiles( - self.file.log_amount, - )), - ContentLimit::BytesSurpassed(self.file.log_size), + config.log_file.as_ref().ok_or(LoggerError::NoFileName)?, + AppendTimestamp::default(file_rotate::suffix::FileLimit::MaxFiles(config.log_amount)), + ContentLimit::BytesSurpassed(config.log_size), file_rotate::compression::Compression::OnRotate(1), None, )) @@ -99,12 +89,14 @@ impl LoggerExt for LoggerConfig { #[derive(Debug)] pub enum LoggerError { + EmptyConfig, NoFileName, } impl Display for LoggerError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str(match self { + Self::EmptyConfig => "Empty logger configuration", Self::NoFileName => "No filename specified", }) } diff --git a/cli/src/config.rs b/cli/src/config.rs index 19f53b8a..841471bf 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -26,20 +26,22 @@ pub struct Config { } /// Logger Configuration passed on initialization -#[derive(Clone, Debug, Deserialize, Default)] +#[serde_as] +#[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct LoggerConfig { /// Rolling file logger config #[serde(default)] - pub file: FileLogger, + pub file: Option, /// Stdout logger config #[serde(default)] - pub stdout: StdoutLogger, + pub stdout: Option, - /// Stderr logger config - #[serde(default)] - pub stderr: StderrLogger, + /// Tracing Level + #[serde(default = "LoggerConfig::level_default")] + #[serde_as(as = "DisplayFromStr")] + pub trace_level: tracing::Level, } /// File Logger Configuration for writing logs to files @@ -57,34 +59,13 @@ pub struct FileLogger { /// Max size of a single log file, in bytes #[serde(default = "FileLogger::default_log_size")] pub log_size: usize, - - /// Tracing Level - #[serde(default = "FileLogger::level_default")] - #[serde_as(as = "Option")] - pub trace_level: Option, } /// Stdout Logger Configuration for printing logs into stdout #[serde_as] #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] -pub struct StdoutLogger { - /// Tracing Level - #[serde(default = "StdoutLogger::level_default")] - #[serde_as(as = "Option")] - pub trace_level: Option, -} - -/// Stdout Logger Configuration for printing logs into stderr -#[serde_as] -#[derive(Clone, Debug, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub struct StderrLogger { - /// Tracing Level - #[serde(default = "StderrLogger::level_default")] - #[serde_as(as = "Option")] - pub trace_level: Option, -} +pub struct StdoutLogger; impl Default for Config { fn default() -> Self { @@ -97,6 +78,16 @@ impl Default for Config { } } +impl Default for LoggerConfig { + fn default() -> Self { + Self { + file: None, + stdout: None, + trace_level: Self::level_default(), + } + } +} + impl Config { #[must_use] pub const fn default_cors() -> bool { @@ -109,12 +100,14 @@ impl Config { } } -impl FileLogger { +impl LoggerConfig { #[must_use] - pub const fn level_default() -> Option { - Some(tracing::Level::TRACE) + pub const fn level_default() -> tracing::Level { + tracing::Level::TRACE } +} +impl FileLogger { #[must_use] pub const fn default_log_amount() -> usize { 5 @@ -124,35 +117,10 @@ impl FileLogger { pub const fn default_log_size() -> usize { 10usize.pow(6) } -} - -impl StdoutLogger { - #[must_use] - pub const fn level_default() -> Option { - Some(tracing::Level::INFO) - } -} -impl StderrLogger { #[must_use] - pub const fn level_default() -> Option { - Some(tracing::Level::WARN) - } -} - -impl Default for StderrLogger { - fn default() -> Self { - Self { - trace_level: Self::level_default(), - } - } -} - -impl Default for StdoutLogger { - fn default() -> Self { - Self { - trace_level: Self::level_default(), - } + pub const fn enabled_default() -> bool { + false } } @@ -162,7 +130,6 @@ impl Default for FileLogger { log_file: None, log_amount: Self::default_log_amount(), log_size: Self::default_log_size(), - trace_level: Self::level_default(), } } } diff --git a/config.yaml b/config.yaml index b0713767..41a0bcbd 100644 --- a/config.yaml +++ b/config.yaml @@ -1,7 +1,6 @@ address: 0.0.0.0:9000 logger: + trace-level: INFO file: - trace-level: ~ log-file: /tmp/bob.log - stdout: - trace-level: INFO + stdout: ~ diff --git a/frontend/package-lock.json b/frontend/package-lock.json new file mode 100644 index 00000000..aba25f73 --- /dev/null +++ b/frontend/package-lock.json @@ -0,0 +1,6 @@ +{ + "name": "frontend", + "lockfileVersion": 3, + "requires": true, + "packages": {} +} From 4e09e192ed4fcbd11aa481d3349709c682959286 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Tue, 3 Oct 2023 16:41:35 +0300 Subject: [PATCH 12/20] delete stderr target + unify trace-level --- backend/src/config.rs | 38 ++++++++++++++++++++++++-------------- cli/src/config.rs | 28 +++++++++++++++++++++++++++- cli/src/lib.rs | 3 +-- config.yaml | 4 +++- 4 files changed, 55 insertions(+), 18 deletions(-) diff --git a/backend/src/config.rs b/backend/src/config.rs index 90b26a8f..b7e4c09c 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -44,20 +44,30 @@ impl ConfigExt for Config { impl LoggerExt for LoggerConfig { fn init_logger(&self) -> Result, LoggerError> { let mut guards = Vec::with_capacity(2); - let file_writer = if self.file.is_some() { - let (writer, guard) = tracing_appender::non_blocking(self.init_file_rotate()?); - guards.push(guard); - writer - } else { - tracing_appender::non_blocking(std::io::sink()).0 - }; - let std_out_writer = if self.stdout.is_some() { - let (writer, guard) = tracing_appender::non_blocking(std::io::stdout()); - guards.push(guard); - writer - } else { - tracing_appender::non_blocking(std::io::sink()).0 - }; + let file_writer = self.file.as_ref().map_or_else( + || Ok(tracing_appender::non_blocking(std::io::sink()).0), + |config| { + Ok(if config.enabled { + let (writer, guard) = tracing_appender::non_blocking(self.init_file_rotate()?); + guards.push(guard); + writer + } else { + tracing_appender::non_blocking(std::io::sink()).0 + }) + }, + )?; + let std_out_writer = self.stdout.as_ref().map_or_else( + || tracing_appender::non_blocking(std::io::sink()).0, + |config| { + if config.enabled { + let (writer, guard) = tracing_appender::non_blocking(std::io::stdout()); + guards.push(guard); + writer + } else { + tracing_appender::non_blocking(std::io::sink()).0 + } + }, + ); tracing_subscriber::registry() .with( diff --git a/cli/src/config.rs b/cli/src/config.rs index 841471bf..6c6e0c1c 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -49,6 +49,9 @@ pub struct LoggerConfig { #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct FileLogger { + /// Enable log output to file + pub enabled: bool, + /// File to save logs pub log_file: Option, @@ -65,7 +68,10 @@ pub struct FileLogger { #[serde_as] #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] -pub struct StdoutLogger; +pub struct StdoutLogger { + /// Enable log output to stdout + pub enabled: bool, +} impl Default for Config { fn default() -> Self { @@ -108,6 +114,11 @@ impl LoggerConfig { } impl FileLogger { + #[must_use] + pub const fn default_enabled() -> bool { + false + } + #[must_use] pub const fn default_log_amount() -> usize { 5 @@ -124,16 +135,31 @@ impl FileLogger { } } +impl StdoutLogger { + #[must_use] + pub const fn default_enabled() -> bool { + false + } +} + impl Default for FileLogger { fn default() -> Self { Self { log_file: None, log_amount: Self::default_log_amount(), log_size: Self::default_log_size(), + enabled: Self::default_enabled(), } } } +impl Default for StdoutLogger { + fn default() -> Self { + Self { + enabled: Self::default_enabled(), + } + } +} pub trait FromFile { /// Parses the file spcified in `path` /// diff --git a/cli/src/lib.rs b/cli/src/lib.rs index 39a1438d..1c055388 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -3,5 +3,4 @@ mod config; pub use clap::Parser; pub use cli::Args; -pub use config::{Config, FromFile, LoggerConfig}; - +pub use config::{Config, FileLogger, FromFile, LoggerConfig, StdoutLogger}; diff --git a/config.yaml b/config.yaml index 41a0bcbd..6768d5d1 100644 --- a/config.yaml +++ b/config.yaml @@ -2,5 +2,7 @@ address: 0.0.0.0:9000 logger: trace-level: INFO file: + enabled: true log-file: /tmp/bob.log - stdout: ~ + stdout: + enabled: true From 11a1fcdcb275e137b063ff27cbfbc887854a77eb Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Tue, 3 Oct 2023 16:46:22 +0300 Subject: [PATCH 13/20] delete redundant lock file --- frontend/package-lock.json | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 frontend/package-lock.json diff --git a/frontend/package-lock.json b/frontend/package-lock.json deleted file mode 100644 index aba25f73..00000000 --- a/frontend/package-lock.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "name": "frontend", - "lockfileVersion": 3, - "requires": true, - "packages": {} -} From c62143318adba49ddf506f71cfe7ef7d172ce62b Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Mon, 13 Nov 2023 17:50:36 +0300 Subject: [PATCH 14/20] clean up --- backend/src/config.rs | 16 ++++------------ build.rs | 1 - 2 files changed, 4 insertions(+), 13 deletions(-) delete mode 100644 build.rs diff --git a/backend/src/config.rs b/backend/src/config.rs index b7e4c09c..9c7d6950 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -3,6 +3,7 @@ use core::fmt; use error_stack::Context; use file_rotate::{suffix::AppendTimestamp, ContentLimit, FileRotate}; use std::fmt::Display; +use thiserror::Error; use tower_http::cors::CorsLayer; use tracing_appender::non_blocking::WorkerGuard; use tracing_subscriber::{filter::LevelFilter, prelude::*}; @@ -97,19 +98,10 @@ impl LoggerExt for LoggerConfig { } } -#[derive(Debug)] +#[derive(Debug, Error)] pub enum LoggerError { + #[error("Empty logger configuration")] EmptyConfig, + #[error("No filename specified")] NoFileName, } - -impl Display for LoggerError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(match self { - Self::EmptyConfig => "Empty logger configuration", - Self::NoFileName => "No filename specified", - }) - } -} - -impl Context for LoggerError {} diff --git a/build.rs b/build.rs deleted file mode 100644 index f328e4d9..00000000 --- a/build.rs +++ /dev/null @@ -1 +0,0 @@ -fn main() {} From 64263a294c9c19bb0ef21cbed37c70e681b39db6 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Mon, 27 Nov 2023 19:50:59 +0300 Subject: [PATCH 15/20] logger init rework --- backend/src/config.rs | 113 +++++++++++++++++++++++++++--------------- cli/src/config.rs | 5 -- 2 files changed, 72 insertions(+), 46 deletions(-) diff --git a/backend/src/config.rs b/backend/src/config.rs index 9c7d6950..d6f3e20a 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -1,12 +1,9 @@ use cli::{Config, LoggerConfig}; -use core::fmt; -use error_stack::Context; use file_rotate::{suffix::AppendTimestamp, ContentLimit, FileRotate}; -use std::fmt::Display; use thiserror::Error; use tower_http::cors::CorsLayer; -use tracing_appender::non_blocking::WorkerGuard; -use tracing_subscriber::{filter::LevelFilter, prelude::*}; +use tracing_appender::non_blocking::{NonBlocking, WorkerGuard}; +use tracing_subscriber::{filter::LevelFilter, prelude::*, util::SubscriberInitExt}; #[allow(clippy::module_name_repetitions)] pub trait ConfigExt { @@ -32,6 +29,28 @@ pub trait LoggerExt { /// /// Function returns error if `log_file` is not specified fn init_file_rotate(&self) -> Result, LoggerError>; + + /// Returns non-blocking file writer + /// + /// Also returns [`WorkerGuard`] for off-thread writing. + /// Should not be dropped. + /// + /// # Errors + /// + /// This function will return an error if the file logger configuration is empty, file logging + /// is disabled or logs filename is not specified + fn non_blocking_file_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError>; + + /// Returns non-blocking stdout writer + /// + /// Also returns [`WorkerGuard`] for off-thread writing. + /// Should not be dropped. + /// + /// # Errors + /// + /// This function will return an error if the stdout logger configuration is empty or stdout logging + /// is disabled + fn non_blocking_stdout_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError>; } impl ConfigExt for Config { @@ -45,43 +64,27 @@ impl ConfigExt for Config { impl LoggerExt for LoggerConfig { fn init_logger(&self) -> Result, LoggerError> { let mut guards = Vec::with_capacity(2); - let file_writer = self.file.as_ref().map_or_else( - || Ok(tracing_appender::non_blocking(std::io::sink()).0), - |config| { - Ok(if config.enabled { - let (writer, guard) = tracing_appender::non_blocking(self.init_file_rotate()?); - guards.push(guard); - writer - } else { - tracing_appender::non_blocking(std::io::sink()).0 - }) - }, - )?; - let std_out_writer = self.stdout.as_ref().map_or_else( - || tracing_appender::non_blocking(std::io::sink()).0, - |config| { - if config.enabled { - let (writer, guard) = tracing_appender::non_blocking(std::io::stdout()); - guards.push(guard); - writer - } else { - tracing_appender::non_blocking(std::io::sink()).0 - } - }, - ); - tracing_subscriber::registry() - .with( - tracing_subscriber::fmt::layer() - .with_writer(file_writer) - .with_filter(LevelFilter::from_level(self.trace_level)), - ) - .with( - tracing_subscriber::fmt::layer() - .with_writer(std_out_writer) - .with_filter(LevelFilter::from_level(self.trace_level)), - ) - .init(); + let mut layers_iter = [ + self.non_blocking_file_writer().ok(), + self.non_blocking_stdout_writer().ok(), + ] + .into_iter() + .flatten() + .map(|(writer, guard)| { + guards.push(guard); + tracing_subscriber::fmt::layer() + .with_writer(writer) + .with_filter(LevelFilter::from_level(self.trace_level)) + }); + + if let Some(first_layer) = layers_iter.next() { + tracing_subscriber::registry() + .with(layers_iter.fold(first_layer.boxed(), |layer, next_layer| { + layer.and_then(next_layer).boxed() + })) + .init(); + }; Ok(guards) } @@ -96,6 +99,32 @@ impl LoggerExt for LoggerConfig { None, )) } + + fn non_blocking_file_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError> { + self.file.as_ref().map_or_else( + || Err(LoggerError::EmptyConfig), + |config| { + if config.enabled { + Ok(tracing_appender::non_blocking(self.init_file_rotate()?)) + } else { + Err(LoggerError::NotEnabled) + } + }, + ) + } + + fn non_blocking_stdout_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError> { + self.stdout.as_ref().map_or_else( + || Err(LoggerError::EmptyConfig), + |config| { + if config.enabled { + Ok(tracing_appender::non_blocking(std::io::stdout())) + } else { + Err(LoggerError::NotEnabled) + } + }, + ) + } } #[derive(Debug, Error)] @@ -104,4 +133,6 @@ pub enum LoggerError { EmptyConfig, #[error("No filename specified")] NoFileName, + #[error("This logger is not enabled")] + NotEnabled, } diff --git a/cli/src/config.rs b/cli/src/config.rs index 6c6e0c1c..78678ef2 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -128,11 +128,6 @@ impl FileLogger { pub const fn default_log_size() -> usize { 10usize.pow(6) } - - #[must_use] - pub const fn enabled_default() -> bool { - false - } } impl StdoutLogger { From 0b2506c27bc7290859bf2a1ac5f6ba94c4b056f4 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sat, 16 Dec 2023 16:50:18 +0300 Subject: [PATCH 16/20] post review fixes + error_stack errors --- backend/src/config.rs | 66 +++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/backend/src/config.rs b/backend/src/config.rs index d6f3e20a..00e195e2 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -1,3 +1,4 @@ +use crate::prelude::*; use cli::{Config, LoggerConfig}; use file_rotate::{suffix::AppendTimestamp, ContentLimit, FileRotate}; use thiserror::Error; @@ -35,22 +36,27 @@ pub trait LoggerExt { /// Also returns [`WorkerGuard`] for off-thread writing. /// Should not be dropped. /// + /// Returns `None` if logger is not enabled + /// /// # Errors /// /// This function will return an error if the file logger configuration is empty, file logging /// is disabled or logs filename is not specified - fn non_blocking_file_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError>; + fn non_blocking_file_writer(&self) -> Result, LoggerError>; /// Returns non-blocking stdout writer /// /// Also returns [`WorkerGuard`] for off-thread writing. /// Should not be dropped. /// + /// Returns `None` if logger is not enabled + /// /// # Errors /// /// This function will return an error if the stdout logger configuration is empty or stdout logging /// is disabled - fn non_blocking_stdout_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError>; + fn non_blocking_stdout_writer(&self) + -> Result, LoggerError>; } impl ConfigExt for Config { @@ -66,8 +72,8 @@ impl LoggerExt for LoggerConfig { let mut guards = Vec::with_capacity(2); let mut layers_iter = [ - self.non_blocking_file_writer().ok(), - self.non_blocking_stdout_writer().ok(), + self.non_blocking_file_writer()?, + self.non_blocking_stdout_writer()?, ] .into_iter() .flatten() @@ -91,37 +97,43 @@ impl LoggerExt for LoggerConfig { fn init_file_rotate(&self) -> Result, LoggerError> { let config = self.file.as_ref().ok_or(LoggerError::EmptyConfig)?; - Ok(FileRotate::new( - config.log_file.as_ref().ok_or(LoggerError::NoFileName)?, - AppendTimestamp::default(file_rotate::suffix::FileLimit::MaxFiles(config.log_amount)), - ContentLimit::BytesSurpassed(config.log_size), - file_rotate::compression::Compression::OnRotate(1), - None, - )) + let log_file = config.log_file.as_ref().ok_or(LoggerError::NoFileName)?; + log_file + .is_file() + .then(|| { + FileRotate::new( + log_file, + AppendTimestamp::default(file_rotate::suffix::FileLimit::MaxFiles( + config.log_amount, + )), + ContentLimit::BytesSurpassed(config.log_size), + file_rotate::compression::Compression::OnRotate(1), + None, + ) + }) + .ok_or_else(|| LoggerError::IncorrectFileName.into()) } - fn non_blocking_file_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError> { + fn non_blocking_file_writer(&self) -> Result, LoggerError> { self.file.as_ref().map_or_else( - || Err(LoggerError::EmptyConfig), + || Err(LoggerError::EmptyConfig.into()), |config| { - if config.enabled { - Ok(tracing_appender::non_blocking(self.init_file_rotate()?)) - } else { - Err(LoggerError::NotEnabled) - } + Ok(config + .enabled + .then_some(tracing_appender::non_blocking(self.init_file_rotate()?))) }, ) } - fn non_blocking_stdout_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError> { + fn non_blocking_stdout_writer( + &self, + ) -> Result, LoggerError> { self.stdout.as_ref().map_or_else( - || Err(LoggerError::EmptyConfig), + || Err(LoggerError::EmptyConfig.into()), |config| { - if config.enabled { - Ok(tracing_appender::non_blocking(std::io::stdout())) - } else { - Err(LoggerError::NotEnabled) - } + Ok(config + .enabled + .then(|| tracing_appender::non_blocking(std::io::stdout()))) }, ) } @@ -133,6 +145,6 @@ pub enum LoggerError { EmptyConfig, #[error("No filename specified")] NoFileName, - #[error("This logger is not enabled")] - NotEnabled, + #[error("Incorrect filename specified")] + IncorrectFileName, } From 41801c81da849baa3fdcc6a91aed66deb8cdf79a Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Sun, 17 Dec 2023 08:48:32 +0300 Subject: [PATCH 17/20] adjust fixes --- backend/src/config.rs | 102 +++++++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 45 deletions(-) diff --git a/backend/src/config.rs b/backend/src/config.rs index 00e195e2..5405c0c3 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -36,27 +36,22 @@ pub trait LoggerExt { /// Also returns [`WorkerGuard`] for off-thread writing. /// Should not be dropped. /// - /// Returns `None` if logger is not enabled - /// /// # Errors /// /// This function will return an error if the file logger configuration is empty, file logging /// is disabled or logs filename is not specified - fn non_blocking_file_writer(&self) -> Result, LoggerError>; + fn non_blocking_file_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError>; /// Returns non-blocking stdout writer /// /// Also returns [`WorkerGuard`] for off-thread writing. /// Should not be dropped. /// - /// Returns `None` if logger is not enabled - /// /// # Errors /// /// This function will return an error if the stdout logger configuration is empty or stdout logging /// is disabled - fn non_blocking_stdout_writer(&self) - -> Result, LoggerError>; + fn non_blocking_stdout_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError>; } impl ConfigExt for Config { @@ -71,18 +66,19 @@ impl LoggerExt for LoggerConfig { fn init_logger(&self) -> Result, LoggerError> { let mut guards = Vec::with_capacity(2); - let mut layers_iter = [ - self.non_blocking_file_writer()?, - self.non_blocking_stdout_writer()?, - ] - .into_iter() - .flatten() - .map(|(writer, guard)| { - guards.push(guard); - tracing_subscriber::fmt::layer() - .with_writer(writer) - .with_filter(LevelFilter::from_level(self.trace_level)) - }); + let file_writer = disable_on_error(self.non_blocking_file_writer())?; + let stdout_writer = disable_on_error(self.non_blocking_stdout_writer())?; + + let mut layers_iter = + [file_writer, stdout_writer] + .into_iter() + .flatten() + .map(|(writer, guard)| { + guards.push(guard); + tracing_subscriber::fmt::layer() + .with_writer(writer) + .with_filter(LevelFilter::from_level(self.trace_level)) + }); if let Some(first_layer) = layers_iter.next() { tracing_subscriber::registry() @@ -98,42 +94,43 @@ impl LoggerExt for LoggerConfig { fn init_file_rotate(&self) -> Result, LoggerError> { let config = self.file.as_ref().ok_or(LoggerError::EmptyConfig)?; let log_file = config.log_file.as_ref().ok_or(LoggerError::NoFileName)?; - log_file - .is_file() - .then(|| { - FileRotate::new( - log_file, - AppendTimestamp::default(file_rotate::suffix::FileLimit::MaxFiles( - config.log_amount, - )), - ContentLimit::BytesSurpassed(config.log_size), - file_rotate::compression::Compression::OnRotate(1), - None, - ) - }) - .ok_or_else(|| LoggerError::IncorrectFileName.into()) + std::fs::OpenOptions::new() + .create(true) + .write(true) + .open(log_file) + .change_context(LoggerError::CantWrite)?; + + Ok(FileRotate::new( + log_file, + AppendTimestamp::default(file_rotate::suffix::FileLimit::MaxFiles(config.log_amount)), + ContentLimit::BytesSurpassed(config.log_size), + file_rotate::compression::Compression::OnRotate(1), + None, + )) } - fn non_blocking_file_writer(&self) -> Result, LoggerError> { + fn non_blocking_file_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError> { self.file.as_ref().map_or_else( || Err(LoggerError::EmptyConfig.into()), |config| { - Ok(config - .enabled - .then_some(tracing_appender::non_blocking(self.init_file_rotate()?))) + if config.enabled { + Ok(tracing_appender::non_blocking(self.init_file_rotate()?)) + } else { + Err(LoggerError::NotEnabled.into()) + } }, ) } - fn non_blocking_stdout_writer( - &self, - ) -> Result, LoggerError> { + fn non_blocking_stdout_writer(&self) -> Result<(NonBlocking, WorkerGuard), LoggerError> { self.stdout.as_ref().map_or_else( || Err(LoggerError::EmptyConfig.into()), |config| { - Ok(config - .enabled - .then(|| tracing_appender::non_blocking(std::io::stdout()))) + if config.enabled { + Ok(tracing_appender::non_blocking(std::io::stdout())) + } else { + Err(LoggerError::NotEnabled.into()) + } }, ) } @@ -145,6 +142,21 @@ pub enum LoggerError { EmptyConfig, #[error("No filename specified")] NoFileName, - #[error("Incorrect filename specified")] - IncorrectFileName, + #[error("Can't write logs in file")] + CantWrite, + #[error("This logger is not enabled")] + NotEnabled, +} + +/// Consume some errors to produce empty logger +fn disable_on_error( + logger: Result<(NonBlocking, WorkerGuard), LoggerError>, +) -> Result, LoggerError> { + Ok(match logger { + Ok(writer) => Some(writer), + Err(e) => match e.current_context() { + LoggerError::NotEnabled | LoggerError::EmptyConfig => None, + _ => return Err(e), + }, + }) } From 83252562f8e8603cbb743135298b33cc3d584070 Mon Sep 17 00:00:00 2001 From: Simeon Romanov Date: Fri, 22 Dec 2023 09:25:50 +0300 Subject: [PATCH 18/20] replace write check with is_empty check --- backend/src/config.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/backend/src/config.rs b/backend/src/config.rs index 5405c0c3..3fc014ce 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -94,11 +94,11 @@ impl LoggerExt for LoggerConfig { fn init_file_rotate(&self) -> Result, LoggerError> { let config = self.file.as_ref().ok_or(LoggerError::EmptyConfig)?; let log_file = config.log_file.as_ref().ok_or(LoggerError::NoFileName)?; - std::fs::OpenOptions::new() - .create(true) - .write(true) - .open(log_file) - .change_context(LoggerError::CantWrite)?; + log_file + .as_os_str() + .is_empty() + .then(|| LoggerError::NoFileName) + .map_or(Ok(()), Err)?; Ok(FileRotate::new( log_file, @@ -142,8 +142,6 @@ pub enum LoggerError { EmptyConfig, #[error("No filename specified")] NoFileName, - #[error("Can't write logs in file")] - CantWrite, #[error("This logger is not enabled")] NotEnabled, } From 434d2fa58e4f4fe2dbd1ad88ce0d46e0a3dfe180 Mon Sep 17 00:00:00 2001 From: ikopylov Date: Fri, 22 Dec 2023 17:24:33 +0100 Subject: [PATCH 19/20] Update backend/src/config.rs --- backend/src/config.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/backend/src/config.rs b/backend/src/config.rs index 3fc014ce..112cd1c0 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -94,11 +94,9 @@ impl LoggerExt for LoggerConfig { fn init_file_rotate(&self) -> Result, LoggerError> { let config = self.file.as_ref().ok_or(LoggerError::EmptyConfig)?; let log_file = config.log_file.as_ref().ok_or(LoggerError::NoFileName)?; - log_file - .as_os_str() - .is_empty() - .then(|| LoggerError::NoFileName) - .map_or(Ok(()), Err)?; + if log_file.as_os_str().is_empty() { + return Err(LoggerError::NoFileName); + } Ok(FileRotate::new( log_file, From 6dc178771f1b1ea66bcce3040fb64b64db2956c4 Mon Sep 17 00:00:00 2001 From: ikopylov Date: Fri, 22 Dec 2023 17:28:58 +0100 Subject: [PATCH 20/20] Update backend/src/config.rs --- backend/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/config.rs b/backend/src/config.rs index 112cd1c0..8beec0f7 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -95,7 +95,7 @@ impl LoggerExt for LoggerConfig { let config = self.file.as_ref().ok_or(LoggerError::EmptyConfig)?; let log_file = config.log_file.as_ref().ok_or(LoggerError::NoFileName)?; if log_file.as_os_str().is_empty() { - return Err(LoggerError::NoFileName); + return Err(LoggerError::NoFileName.into()); } Ok(FileRotate::new(