From a0c6b80e1e1f6f0eda090f5a08897f63a29761d7 Mon Sep 17 00:00:00 2001 From: Emmanuel Bosquet Date: Wed, 24 Apr 2024 15:48:22 +0200 Subject: [PATCH] propagate logging setup errors --- bin/src/command/mod.rs | 6 ++++-- bin/src/ctl/command.rs | 6 +++++- bin/src/ctl/mod.rs | 6 ++++-- bin/src/upgrade.rs | 6 ++++-- bin/src/worker.rs | 7 +++++-- command/examples/bench_logger.rs | 4 +++- command/src/config.rs | 1 + command/src/logging/logs.rs | 23 +++++++++++++---------- e2e/src/sozu/worker.rs | 4 +++- e2e/src/tests/tests.rs | 8 ++++++-- lib/examples/http.rs | 2 +- lib/examples/https.rs | 2 +- lib/examples/tcp.rs | 6 ++++-- lib/src/lib.rs | 2 +- 14 files changed, 55 insertions(+), 28 deletions(-) diff --git a/bin/src/command/mod.rs b/bin/src/command/mod.rs index e811235b6..ec9134206 100644 --- a/bin/src/command/mod.rs +++ b/bin/src/command/mod.rs @@ -11,7 +11,7 @@ use mio::net::UnixListener; use sozu_command_lib::{ config::{Config, ConfigError}, - logging::setup_logging_with_config, + logging::{setup_logging_with_config, LogError}, }; use crate::{ @@ -52,6 +52,8 @@ pub enum StartError { SetPermissions(IoError), #[error("could not launch new worker: {0}")] LaunchWorker(ServerError), + #[error("could not setup the logger: {0}")] + SetupLogging(LogError), } pub fn begin_main_process(args: &Args) -> Result<(), StartError> { @@ -59,7 +61,7 @@ pub fn begin_main_process(args: &Args) -> Result<(), StartError> { let config = Config::load_from_path(config_file_path).map_err(StartError::LoadConfig)?; - setup_logging_with_config(&config, "MAIN"); + setup_logging_with_config(&config, "MAIN").map_err(StartError::SetupLogging)?; info!("Starting up"); setup_metrics(&config).map_err(StartError::SetupMetrics)?; write_pid_file(&config).map_err(StartError::WritePidFile)?; diff --git a/bin/src/ctl/command.rs b/bin/src/ctl/command.rs index 59e0b07a0..30d05574b 100644 --- a/bin/src/ctl/command.rs +++ b/bin/src/ctl/command.rs @@ -154,7 +154,11 @@ impl CommandManager { let config = self.config.clone(); upgrade_jobs.push(std::thread::spawn(move || { - setup_logging_with_config(&config, &format!("UPGRADE-WRK-{}", worker.id)); + if let Err(e) = + setup_logging_with_config(&config, &format!("UPGRADE-WRK-{}", worker.id)) + { + error!("Could not setup logging: {}", e); + } info!("creating channel to upgrade worker {}", worker.id); let channel = match create_channel(&config) { diff --git a/bin/src/ctl/mod.rs b/bin/src/ctl/mod.rs index d89808266..4817b20d5 100644 --- a/bin/src/ctl/mod.rs +++ b/bin/src/ctl/mod.rs @@ -7,7 +7,7 @@ use sozu_command_lib::{ certificate::CertificateError, channel::{Channel, ChannelError}, config::{Config, ConfigError}, - logging::setup_logging_with_config, + logging::{setup_logging_with_config, LogError}, proto::{ command::{Request, Response}, DisplayError, @@ -53,6 +53,8 @@ pub enum CtlError { NeedClusterDomain, #[error("wrong response from Sōzu: {0:?}")] WrongResponse(Response), + #[error("could not setup the logger: {0}")] + SetupLogging(LogError), } pub struct CommandManager { @@ -70,7 +72,7 @@ pub fn ctl(args: cli::Args) -> Result<(), CtlError> { // prevent logging for json responses for a clean output if !args.json { - setup_logging_with_config(&config, "CTL"); + setup_logging_with_config(&config, "CTL").map_err(CtlError::SetupLogging)?; } // If the command is `config check` then exit because if we are here, the configuration is valid diff --git a/bin/src/upgrade.rs b/bin/src/upgrade.rs index aeefee09b..a8a5d9405 100644 --- a/bin/src/upgrade.rs +++ b/bin/src/upgrade.rs @@ -18,7 +18,7 @@ use tempfile::tempfile; use sozu_command_lib::{ channel::{Channel, ChannelError}, - logging::setup_logging_with_config, + logging::{setup_logging_with_config, LogError}, }; use crate::{ @@ -69,6 +69,8 @@ pub enum UpgradeError { CreateHub(HubError), #[error("could not enable cloexec after upgrade: {0}")] EnableCloexec(ServerError), + #[error("could not setup the logger: {0}")] + SetupLogging(LogError), } /// unix-forks the main process @@ -180,7 +182,7 @@ pub fn begin_new_main_process( println!("Setting up logging"); - setup_logging_with_config(&config, "MAIN"); + setup_logging_with_config(&config, "MAIN").map_err(UpgradeError::SetupLogging)?; util::setup_metrics(&config).map_err(UpgradeError::SetupMetrics)?; let mut command_hub = diff --git a/bin/src/worker.rs b/bin/src/worker.rs index 7d5b0394d..e0fcfee29 100644 --- a/bin/src/worker.rs +++ b/bin/src/worker.rs @@ -22,7 +22,7 @@ use tempfile::tempfile; use sozu_command_lib::{ channel::{Channel, ChannelError}, config::Config, - logging::{setup_logging, AccessLogFormat}, + logging::{setup_logging, AccessLogFormat, LogError}, proto::command::{ServerConfig, WorkerRequest, WorkerResponse}, ready::Ready, request::{read_initial_state_from_file, RequestError}, @@ -74,6 +74,8 @@ pub enum WorkerError { state: String, channel_err: ChannelError, }, + #[error("could not setup the logger: {0}")] + SetupLogging(LogError), } /// called within a worker process, this starts the actual proxy @@ -117,7 +119,8 @@ pub fn begin_worker_process( Some(worker_config.log_colored), &worker_config.log_level, &worker_id, - ); + ) + .map_err(WorkerError::SetupLogging)?; trace!( "Creating worker {} with config: {:#?}", diff --git a/command/examples/bench_logger.rs b/command/examples/bench_logger.rs index 0519ab329..f84118877 100644 --- a/command/examples/bench_logger.rs +++ b/command/examples/bench_logger.rs @@ -53,7 +53,9 @@ fn main() { eprintln!( "n={n}, pre_generate={pre_generate}, target={target}, colored={colored}, filter={filter}" ); - setup_logging(&target, colored, None, None, None, &filter, "WRK-01"); + if let Err(e) = setup_logging(&target, colored, None, None, None, &filter, "WRK-01") { + println!("could not setup logging: {}", e); + } let mut pre_generated_log_iterator; let mut log_iterator = std::iter::repeat(()) diff --git a/command/src/config.rs b/command/src/config.rs index 3ab949f65..40de91e73 100644 --- a/command/src/config.rs +++ b/command/src/config.rs @@ -172,6 +172,7 @@ pub const MAX_LOOP_ITERATIONS: usize = 100000; /// with little influence on performance. Defaults to 4. pub const DEFAULT_SEND_TLS_13_TICKETS: u64 = 4; +/// for both logs and access logs pub const DEFAULT_LOG_TARGET: &str = "stdout"; #[derive(Debug)] diff --git a/command/src/logging/logs.rs b/command/src/logging/logs.rs index 52527c33b..c2dc206ae 100644 --- a/command/src/logging/logs.rs +++ b/command/src/logging/logs.rs @@ -4,7 +4,7 @@ use std::{ fmt::Arguments, fs::{File, OpenOptions}, io::{stdout, Error as IoError, ErrorKind as IoErrorKind, Stdout, Write}, - net::{SocketAddr, TcpStream, ToSocketAddrs, UdpSocket}, + net::{SocketAddr, TcpStream, UdpSocket}, ops::{Deref, DerefMut}, path::Path, str::FromStr, @@ -131,14 +131,12 @@ impl Logger { access_logs_target: Option<&str>, access_format: Option, access_colored: Option, - ) { + ) -> Result<(), LogError> { println!("setting log target to {log_target}"); let backend = target_or_default(log_target); println!("setting target of access logs to {access_logs_target:?}"); - let access_backend = access_logs_target.map(|target| { - target_to_backend(target).expect("could not setup logger for access logs") - }); + let access_backend = access_logs_target.map(target_to_backend).transpose()?; let (directives, _errors) = parse_logging_spec(spec); LOGGER.with(|logger| { @@ -170,6 +168,7 @@ impl Logger { log::set_max_level(log::LevelFilter::Info); } }); + Ok(()) } pub fn set_directives(&mut self, directives: Vec) { @@ -616,12 +615,16 @@ pub fn parse_logging_spec(spec: &str) -> (Vec, Vec Result<(), LogError> { setup_logging("stdout", log_colored, None, None, None, log_level, tag) } /// start the logger from config (takes RUST_LOG into account) -pub fn setup_logging_with_config(config: &Config, tag: &str) { +pub fn setup_logging_with_config(config: &Config, tag: &str) -> Result<(), LogError> { setup_logging( &config.log_target, config.log_colored, @@ -645,7 +648,7 @@ pub fn setup_logging( access_logs_colored: Option, log_level: &str, tag: &str, -) { +) -> Result<(), LogError> { let log_level = env::var("RUST_LOG").unwrap_or(log_level.to_string()); Logger::init( @@ -656,13 +659,13 @@ pub fn setup_logging( access_logs_target, access_logs_format, access_logs_colored, - ); + ) } /// defaults to stdout if the log target is unparseable fn target_or_default(target: &str) -> LoggerBackend { match target_to_backend(target) { - Ok(backend) => return backend, + Ok(backend) => backend, Err(target_error) => { eprintln!("{target_error}, defaulting to stdout"); LoggerBackend::Stdout(stdout()) diff --git a/e2e/src/sozu/worker.rs b/e2e/src/sozu/worker.rs index 6cde49d80..a989fc03d 100644 --- a/e2e/src/sozu/worker.rs +++ b/e2e/src/sozu/worker.rs @@ -133,7 +133,9 @@ impl Worker { println!("Setting up logging"); let server_job = thread::spawn(move || { - setup_default_logging(false, "error", &thread_name); + if let Err(e) = setup_default_logging(false, "error", &thread_name) { + println!("could not setup logging: {e}"); + } let mut server = Server::try_new_from_config( cmd_worker_to_main, thread_scm_worker_to_main, diff --git a/e2e/src/tests/tests.rs b/e2e/src/tests/tests.rs index 26ef0818c..36825b3a4 100644 --- a/e2e/src/tests/tests.rs +++ b/e2e/src/tests/tests.rs @@ -629,7 +629,9 @@ pub fn try_hard_or_soft_stop(soft: bool) -> State { } fn try_http_behaviors() -> State { - setup_default_logging(false, "debug", "BEHAVE-OUT"); + if let Err(e) = setup_default_logging(false, "debug", "BEHAVE-OUT") { + println!("could not setup default logging: {e}"); + } info!("starting up"); @@ -1121,7 +1123,9 @@ pub fn try_blue_geen() -> State { } pub fn try_keep_alive() -> State { - setup_default_logging(false, "debug", "KA-OUT"); + if let Err(e) = setup_default_logging(false, "debug", "KA-OUT") { + println!("could not setup default logging: {e}"); + } let front_address = create_local_address(); diff --git a/lib/examples/http.rs b/lib/examples/http.rs index 460f6c4b4..68ec35a03 100644 --- a/lib/examples/http.rs +++ b/lib/examples/http.rs @@ -15,7 +15,7 @@ use sozu_command_lib::{ }; fn main() -> anyhow::Result<()> { - setup_default_logging(true, "info", "EXAMPLE"); + setup_default_logging(true, "info", "EXAMPLE").with_context(|| "could not setup logging")?; info!("starting up"); diff --git a/lib/examples/https.rs b/lib/examples/https.rs index 281de4723..5f9bde1c8 100644 --- a/lib/examples/https.rs +++ b/lib/examples/https.rs @@ -18,7 +18,7 @@ use sozu_command_lib::{ }; fn main() -> anyhow::Result<()> { - setup_default_logging(true, "info", "EXAMPLE"); + setup_default_logging(true, "info", "EXAMPLE").with_context(|| "could not setup logging")?; info!("MAIN\tstarting up"); diff --git a/lib/examples/tcp.rs b/lib/examples/tcp.rs index acad9ec04..baff2661f 100644 --- a/lib/examples/tcp.rs +++ b/lib/examples/tcp.rs @@ -16,7 +16,7 @@ use sozu_command_lib::{ }; fn main() -> anyhow::Result<()> { - setup_default_logging(true, "info", "EXAMPLE"); + setup_default_logging(true, "info", "EXAMPLE").with_context(|| "could not setup logging")?; info!("starting up"); @@ -30,7 +30,9 @@ fn main() -> anyhow::Result<()> { address: SocketAddress::new_v4(127, 0, 0, 1, 8080), ..Default::default() }; - setup_default_logging(true, "debug", "TCP"); + if let Err(e) = setup_default_logging(true, "debug", "TCP") { + println!("could not setup logging: {e}"); + } sozu_lib::tcp::testing::start_tcp_worker(listener, max_buffers, buffer_size, channel); }); diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 9cd8f4097..eeb4d3144 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -226,7 +226,7 @@ //! }; //! //! fn main() -> anyhow::Result<()> { -//! setup_default_logging(true, "info", "EXAMPLE"); +//! setup_default_logging(true, "info", "EXAMPLE").with_context(|| "could not setup logging")?; //! //! info!("starting up"); //!