From 24cd19d65702b304164a071a4c5b89d45ff714e1 Mon Sep 17 00:00:00 2001 From: "Alexander V. Nikolaev" Date: Thu, 29 Aug 2024 17:54:29 +0300 Subject: [PATCH 1/3] Use str references instead of String Signed-off-by: Alexander V. Nikolaev --- src/admin/registry.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/admin/registry.rs b/src/admin/registry.rs index d1c215f..a1806d3 100644 --- a/src/admin/registry.rs +++ b/src/admin/registry.rs @@ -37,7 +37,7 @@ impl Registry { self.send_event(event) } - pub fn deregister(&self, name: &String) -> anyhow::Result<()> { + pub fn deregister(&self, name: &str) -> anyhow::Result<()> { let mut state = self.map.lock().unwrap(); match state.remove(name) { Some(entry) => { @@ -49,7 +49,7 @@ impl Registry { } } - pub fn by_name(&self, name: &String) -> anyhow::Result { + pub fn by_name(&self, name: &str) -> anyhow::Result { let state = self.map.lock().unwrap(); state .get(name) @@ -57,11 +57,11 @@ impl Registry { .ok_or_else(|| anyhow!("Service {name} not registered")) } - pub fn find_names(&self, name: &String) -> anyhow::Result> { + pub fn find_names(&self, name: &str) -> anyhow::Result> { let state = self.map.lock().unwrap(); let list: Vec = state .keys() - .filter(|x| x.starts_with(name.as_str())) + .filter(|x| x.starts_with(name)) .cloned() .collect(); if list.len() == 0 { @@ -85,12 +85,12 @@ impl Registry { } } - pub fn contains(&self, name: &String) -> bool { + pub fn contains(&self, name: &str) -> bool { let state = self.map.lock().unwrap(); state.contains_key(name) } - pub fn create_unique_entry_name(&self, name: &String) -> String { + pub fn create_unique_entry_name(&self, name: &str) -> String { let state = self.map.lock().unwrap(); let mut counter = 0; loop { @@ -107,7 +107,7 @@ impl Registry { state.values().filter(|x| x.watch).cloned().collect() } - pub fn update_state(&self, name: &String, status: UnitStatus) -> anyhow::Result<()> { + pub fn update_state(&self, name: &str, status: UnitStatus) -> anyhow::Result<()> { let mut state = self.map.lock().unwrap(); state .get_mut(name) From 506436a3230598a58be1a8937709b15d0a63599e Mon Sep 17 00:00:00 2001 From: "Alexander V. Nikolaev" Date: Thu, 29 Aug 2024 19:45:33 +0300 Subject: [PATCH 2/3] Improve logging setup Signed-off-by: Alexander V. Nikolaev --- Cargo.lock | 58 ++++++++++++++++++++++++++++++++++++++++--- Cargo.toml | 3 ++- src/admin/server.rs | 7 +++--- src/bin/givc-admin.rs | 6 ++--- src/bin/givc-agent.rs | 2 +- src/bin/givc-cli.rs | 2 +- src/lib.rs | 38 ++++++++++++++++++++++++++-- 7 files changed, 101 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 89dcd31..5d5462d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -535,6 +535,7 @@ dependencies = [ "tonic-types", "tower", "tracing", + "tracing-journald", "tracing-subscriber", "x509-parser", ] @@ -762,6 +763,15 @@ version = "0.4.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" +[[package]] +name = "matchers" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8263075bb86c5a1b1427b5ae862e8889656f126e9f77c484496e8b47cf5c5558" +dependencies = [ + "regex-automata 0.1.10", +] + [[package]] name = "matchit" version = "0.7.3" @@ -876,6 +886,15 @@ dependencies = [ "libc", ] +[[package]] +name = "num_threads" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c7398b9c8b70908f6371f47ed36737907c87c52af34c268fed0bf0ceb92ead9" +dependencies = [ + "libc", +] + [[package]] name = "object" version = "0.32.2" @@ -1091,8 +1110,17 @@ checksum = "c117dbdfde9c8308975b6a18d71f3f385c89461f7b3fb054288ecf2a2058ba4c" dependencies = [ "aho-corasick", "memchr", - "regex-automata", - "regex-syntax", + "regex-automata 0.4.6", + "regex-syntax 0.8.3", +] + +[[package]] +name = "regex-automata" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" +dependencies = [ + "regex-syntax 0.6.29", ] [[package]] @@ -1103,9 +1131,15 @@ checksum = "86b83b8b9847f9bf95ef68afb0b8e6cdb80f498442f5179a29fad448fcc1eaea" dependencies = [ "aho-corasick", "memchr", - "regex-syntax", + "regex-syntax 0.8.3", ] +[[package]] +name = "regex-syntax" +version = "0.6.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" + [[package]] name = "regex-syntax" version = "0.8.3" @@ -1413,7 +1447,9 @@ checksum = "5dfd88e563464686c916c7e46e623e520ddc6d79fa6641390f2e3fa86e83e885" dependencies = [ "deranged", "itoa", + "libc", "num-conv", + "num_threads", "powerfmt", "serde", "time-core", @@ -1641,6 +1677,17 @@ dependencies = [ "valuable", ] +[[package]] +name = "tracing-journald" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ba49f4829f4e95702943ec6b2fad8936b369d20fa27036caf01329fb230e460" +dependencies = [ + "libc", + "tracing-core", + "tracing-subscriber", +] + [[package]] name = "tracing-log" version = "0.2.0" @@ -1658,10 +1705,15 @@ version = "0.3.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad0f048c97dbd9faa9b7df56362b8ebcaa52adb06b498c050d2f4e32f90a7a8b" dependencies = [ + "matchers", "nu-ansi-term", + "once_cell", + "regex", "sharded-slab", "smallvec", "thread_local", + "time", + "tracing", "tracing-core", "tracing-log", ] diff --git a/Cargo.toml b/Cargo.toml index df0a0ac..335d00d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,8 @@ tonic-types = {version="0.11.0"} tonic-reflection = {version="0.11.0"} tower = {version = "0.4"} tracing = "0.1" -tracing-subscriber = {version = "0.3"} +tracing-subscriber = {version = "0.3", features = ["env-filter", "tracing-log", "time", "local-time"]} +tracing-journald = {version =" 0.2.0"} serde = { version = "1.0.202", features = ["derive"]} serde_json = "1.0.120" x509-parser = { version = "0.16" } diff --git a/src/admin/server.rs b/src/admin/server.rs index 6401af8..94fd39a 100644 --- a/src/admin/server.rs +++ b/src/admin/server.rs @@ -7,7 +7,7 @@ use std::pin::Pin; use std::sync::Arc; use std::time::Duration; use tonic::{Code, Response, Status}; -use tracing::{error, info}; +use tracing::{debug, error, info}; pub use pb::admin_service_server::AdminServiceServer; @@ -177,7 +177,7 @@ impl AdminServiceImpl { async fn monitor_routine(&self) -> anyhow::Result<()> { let watch_list = self.registry.watch_list(); for entry in watch_list { - info!("Monitoring {}...", &entry.name); + debug!("Monitoring {}...", &entry.name); match self.get_remote_status(&entry).await { Err(err) => { error!( @@ -199,7 +199,7 @@ impl AdminServiceImpl { ) }; - info!("Status of {} is {:#?} (updated)", &entry.name, status); + debug!("Status of {} is {:#?} (updated)", &entry.name, status); // We have immutable copy of entry here, but need update _in registry_ copy self.registry.update_state(&entry.name, status)?; @@ -219,7 +219,6 @@ impl AdminServiceImpl { watch.tick().await; // First tick fires instantly loop { watch.tick().await; - info!("Monitoring..."); if let Err(err) = self.monitor_routine().await { error!("Error during watch: {}", err); } diff --git a/src/bin/givc-admin.rs b/src/bin/givc-admin.rs index 5bad5fd..fb19945 100644 --- a/src/bin/givc-admin.rs +++ b/src/bin/givc-admin.rs @@ -5,7 +5,7 @@ use givc_common::pb::reflection::ADMIN_DESCRIPTOR; use std::net::SocketAddr; use std::path::PathBuf; use tonic::transport::Server; -use tracing::info; +use tracing::debug; #[derive(Debug, Parser)] // requires `derive` feature #[command(name = "givc-admin")] @@ -39,10 +39,10 @@ struct Cli { #[tokio::main] async fn main() -> std::result::Result<(), Box> { - givc::trace_init(); + givc::trace_init()?; let cli = Cli::parse(); - info!("CLI is {:#?}", cli); + debug!("CLI is {:#?}", cli); let addr = SocketAddr::new(cli.addr.parse().unwrap(), cli.port); diff --git a/src/bin/givc-agent.rs b/src/bin/givc-agent.rs index a571b6e..1b565f7 100644 --- a/src/bin/givc-agent.rs +++ b/src/bin/givc-agent.rs @@ -60,7 +60,7 @@ struct Cli { #[tokio::main] async fn main() -> std::result::Result<(), Box> { - givc::trace_init(); + givc::trace_init()?; let cli = Cli::parse(); info!("CLI is {:#?}", cli); diff --git a/src/bin/givc-cli.rs b/src/bin/givc-cli.rs index 96aecf2..f9e41ac 100644 --- a/src/bin/givc-cli.rs +++ b/src/bin/givc-cli.rs @@ -110,7 +110,7 @@ async fn test_subcommands(test: Test, admin: AdminClient) -> anyhow::Result<()> #[tokio::main] async fn main() -> std::result::Result<(), Box> { - givc::trace_init(); + givc::trace_init()?; let cli = Cli::parse(); info!("CLI is {:#?}", cli); diff --git a/src/lib.rs b/src/lib.rs index 3c69a6a..406ac0d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,40 @@ pub mod pb { pub use givc_client::endpoint; pub use givc_common::types; -pub fn trace_init() { - tracing_subscriber::fmt::init(); +pub fn trace_init() -> anyhow::Result<()> { + use std::env; + use tracing::Level; + use tracing_subscriber::{filter::LevelFilter, layer::SubscriberExt, EnvFilter, Layer}; + + let env_filter = + EnvFilter::try_from_env("GIVC_LOG").unwrap_or_else(|_| EnvFilter::from("info")); + let is_debug_log_level = env_filter + .max_level_hint() + .map_or_else(|| false, |level| level >= Level::DEBUG); + + let stdout = tracing_subscriber::fmt::layer() + .with_target(is_debug_log_level) + .with_file(is_debug_log_level) + .with_line_number(is_debug_log_level) + .with_thread_ids(is_debug_log_level); + + let stdout = if is_debug_log_level { + stdout.pretty().boxed() + } else { + stdout.boxed() + }; + + // enable journald logging only on release to avoid log spam on dev machines + let journald = match env::var("INVOCATION_ID") { + Err(_) => None, + Ok(_) => tracing_journald::layer().ok(), + }; + + let subscriber = tracing_subscriber::registry() + .with(journald.with_filter(LevelFilter::INFO)) + .with(stdout.with_filter(env_filter)); + + tracing::subscriber::set_global_default(subscriber) + .expect("tracing shouldn't already have been set up"); + Ok(()) } From 46a389f743629e76a4b16def9e612cf75d1c4ee4 Mon Sep 17 00:00:00 2001 From: "Alexander V. Nikolaev" Date: Thu, 29 Aug 2024 20:29:16 +0300 Subject: [PATCH 3/3] Control logs in admin module Signed-off-by: Alexander V. Nikolaev --- nixos/modules/admin.nix | 5 +++++ nixos/tests/admin.nix | 1 + 2 files changed, 6 insertions(+) diff --git a/nixos/modules/admin.nix b/nixos/modules/admin.nix index f69a700..1ec42bc 100644 --- a/nixos/modules/admin.nix +++ b/nixos/modules/admin.nix @@ -23,6 +23,7 @@ in { options.givc.admin = { enable = mkEnableOption "Enable givc-admin module."; + debug = mkEnableOption "Enable givc-admin debug logging."; name = mkOption { description = "Host name (without domain)."; @@ -134,6 +135,10 @@ in "CA_CERT" = "${cfg.tls.caCertPath}"; "HOST_CERT" = "${cfg.tls.certPath}"; "HOST_KEY" = "${cfg.tls.keyPath}"; + } + // attrsets.optionalAttrs cfg.debug { + "RUST_BACKTRACE" = "1"; + "GIVC_LOG" = "debug"; }; }; networking.firewall.allowedTCPPorts = diff --git a/nixos/tests/admin.nix b/nixos/tests/admin.nix index 21e416c..b783882 100644 --- a/nixos/tests/admin.nix +++ b/nixos/tests/admin.nix @@ -44,6 +44,7 @@ in ]; givc.admin = { enable = true; + debug = true; name = "admin-vm"; addr = addrs.adminvm; port = "9001";