diff --git a/Cargo.lock b/Cargo.lock index a1efe2a4..e96b17ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5195,6 +5195,7 @@ dependencies = [ "tokio", "tokio-util", "tracing", + "tracing-subscriber", "url", "wiremock", ] diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 26a00608..2f1534a7 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -49,6 +49,7 @@ sha2.workspace = true [dev-dependencies] tempfile.workspace = true +tracing-subscriber.workspace = true test-case.workspace = true wiremock.workspace = true pluto-cluster = { workspace = true, features = ["test-cluster"] } diff --git a/crates/cli/README.md b/crates/cli/README.md index 4efcee5c..0e32001e 100644 --- a/crates/cli/README.md +++ b/crates/cli/README.md @@ -69,7 +69,7 @@ Starts a libp2p circuit relay that charon clients can use to discover and connec - `--data-dir `: The directory where pluto will store all its internal data. (default: `.charon`) - `--http-address `: Listening address (ip and port) for the relay http server serving runtime ENR. (default: `127.0.0.1:3640`) - `--auto-p2pkey`: Automatically generate and persist a p2p key if one does not exist. Always on: it defaults to true and cannot be switched off on the command line (`--auto-p2pkey=false` is rejected); set `CHARON_AUTO_P2PKEY=false` to require an existing key. - - `--p2p-relay-loglevel `: **[IGNORED]** Parsed but never applied. + - `--p2p-relay-loglevel `: Log level for the upstream `libp2p_relay` crate, letting its logs be quieted (`--p2p-relay-loglevel=error`) without lowering pluto's own verbosity. Takes the same values as `--log-level`; when unset the relay crate follows `--log-level`. - `--p2p-max-reservations `: Updates max circuit reservations per peer (each valid for 1 hour). (default: `512`) - `--p2p-max-connections `: Currently applied as the relay's total reservation limit; it does not cap inbound connections. (default: `16384`) - `--p2p-advertise-private-addresses`: Enable advertising of libp2p auto-detected private addresses. @@ -269,7 +269,7 @@ Shared by `run`, `relay`, `dkg` and `alpha test peers`. Shared by `run`, `relay` and `dkg`. - `--log-format `: **[IGNORED]** Accepted but not yet applied — output is always console-formatted. (default: `console`) -- `--log-level `: Log level; `debug`, `info`, `warn` or `error`. (default: `info`) +- `--log-level `: Log level; `off`, `trace`, `debug`, `info`, `warn` or `error`. (default: `info`) - `--log-color `: Log color; `auto`, `force` or `disable`. (default: `auto`) - `--log-output-path `: **[IGNORED]** Accepted but not yet applied — no log file is written. diff --git a/crates/cli/src/commands/common.rs b/crates/cli/src/commands/common.rs index 9a430943..fd2b7cb2 100644 --- a/crates/cli/src/commands/common.rs +++ b/crates/cli/src/commands/common.rs @@ -1,5 +1,7 @@ //! Shared helpers for CLI commands. +use std::fmt; + use pluto_p2p::config::RelayAddr; use tracing::warn; @@ -24,6 +26,33 @@ pub enum ConsoleColor { Disable, } +/// The log levels `tracing_subscriber`'s `EnvFilter` understands. +/// +/// `Display` renders the directive spelling, so these compose into a filter +/// string that always parses. +#[derive(clap::ValueEnum, Clone, Copy, Debug, PartialEq, Eq)] +pub enum LogLevel { + Off, + Error, + Warn, + Info, + Debug, + Trace, +} + +impl fmt::Display for LogLevel { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::Off => "off", + Self::Error => "error", + Self::Warn => "warn", + Self::Info => "info", + Self::Debug => "debug", + Self::Trace => "trace", + }) + } +} + /// Builds a tracing configuration for CLI commands, optionally enabling Loki. /// /// `loki` is `Some` when the caller wants events forwarded to a Loki endpoint diff --git a/crates/cli/src/commands/relay.rs b/crates/cli/src/commands/relay.rs index 44e9d795..49001f16 100644 --- a/crates/cli/src/commands/relay.rs +++ b/crates/cli/src/commands/relay.rs @@ -1,5 +1,7 @@ use crate::{ - commands::common::{ConsoleColor, LICENSE, build_console_tracing_config, parse_relay_addrs}, + commands::common::{ + ConsoleColor, LICENSE, LogLevel, build_console_tracing_config, parse_relay_addrs, + }, error::CliError, }; use pluto_p2p::k1; @@ -11,6 +13,15 @@ use tracing::{error, info}; /// once `BackgroundTaskController::shutdown` has been signalled. const LOKI_FLUSH_TIMEOUT: Duration = Duration::from_secs(3); +/// Adds a `libp2p_relay` directive to the `base` env filter, which `EnvFilter` +/// prefix-matches against every `libp2p_relay::*` target. +fn relay_filter(base: LogLevel, relay_level: Option) -> String { + match relay_level { + Some(level) => format!("{base},libp2p_relay={level}"), + None => base.to_string(), + } +} + /// Arguments for the relay command. #[derive(clap::Args, Clone)] pub struct RelayArgs { @@ -91,14 +102,16 @@ impl TryInto for RelayArgs { } }; - let log_config = - build_console_tracing_config(self.log.level.clone(), &self.log.color, loki_config); + let log_config = build_console_tracing_config( + relay_filter(self.log.level, self.relay.p2p_relay_log_level), + &self.log.color, + loki_config, + ); let builder = pluto_relay_server::config::Config::builder() .data_dir(self.data_dir.data_dir) .http_addr(self.relay.http_address) .auto_p2p_key(self.relay.auto_p2p_key) - .libp2p_log_level(self.relay.p2p_relay_log_level) .max_res_per_peer(self.relay.max_res_per_peer) .max_conns(self.relay.max_conns) // Invert p2p-advertise-private-addresses flag boolean: @@ -146,10 +159,10 @@ pub struct RelayRelayArgs { #[arg( long = "p2p-relay-loglevel", env = "CHARON_P2P_RELAY_LOGLEVEL", - default_value = "", - help = "Libp2p circuit relay log level. E.g., debug, info, warn, error." + ignore_case = true, + help = "Libp2p circuit relay log level. Defaults to --log-level." )] - pub p2p_relay_log_level: String, + pub p2p_relay_log_level: Option, // TODO: Check if https://github.com/libp2p/go-libp2p/issues/1713 is relevant for the Rust libp2p implementation // If so, decrease defaults after this has been addressed @@ -259,9 +272,10 @@ pub struct RelayLogFlags { long = "log-level", env = "CHARON_LOG_LEVEL", default_value = "info", - help = "Log level; debug, info, warn or error" + ignore_case = true, + help = "Log level" )] - pub level: String, + pub level: LogLevel, #[arg(long = "log-color", default_value = "auto", help = "Log color")] pub color: ConsoleColor, @@ -387,6 +401,7 @@ fn load_or_create_key( #[cfg(test)] mod tests { + use clap::{Parser as _, ValueEnum as _}; use std::{ net::{Ipv4Addr, SocketAddr}, path::Path, @@ -396,6 +411,10 @@ mod tests { }; use tokio::{net, task::JoinHandle}; use tokio_util::sync::CancellationToken; + use tracing::{Level, enabled}; + use tracing_subscriber::{EnvFilter, layer::SubscriberExt as _}; + + use crate::cli::Cli; /// Args mirroring the clap defaults (notably `debug_addr: Some("")`), /// plus a TCP address so the baseline conversion succeeds. @@ -407,7 +426,7 @@ mod tests { relay: super::RelayRelayArgs { http_address: "127.0.0.1:3640".into(), auto_p2p_key: true, - p2p_relay_log_level: "info".into(), + p2p_relay_log_level: None, max_res_per_peer: 512, max_conns: 16384, advertise_priv: false, @@ -426,7 +445,7 @@ mod tests { }, log: super::RelayLogFlags { format: "console".into(), - level: "error".into(), + level: super::LogLevel::Error, color: super::ConsoleColor::Disable, log_output_path: None, }, @@ -761,7 +780,7 @@ mod tests { relay: super::RelayRelayArgs { http_address: ANY_ADDR.into(), auto_p2p_key: true, - p2p_relay_log_level: "info".into(), + p2p_relay_log_level: None, max_res_per_peer: 0, max_conns: 0, advertise_priv: true, @@ -780,7 +799,7 @@ mod tests { }, log: super::RelayLogFlags { format: "console".into(), - level: "error".into(), + level: super::LogLevel::Error, color: super::ConsoleColor::Disable, log_output_path: None, }, @@ -925,4 +944,46 @@ mod tests { let addr = listener.local_addr().unwrap().to_string(); (listener, addr) } + + /// Runs `f` with a subscriber that only lets `filter` through. + fn with_filter(filter: &str, f: impl FnOnce()) { + let filter = EnvFilter::from_str(filter).expect("relay filter should be a valid EnvFilter"); + tracing::subscriber::with_default(tracing_subscriber::registry().with(filter), f); + } + + #[test] + fn relay_filter_scopes_upstream_relay_logs() { + // An unset relay level leaves the base filter alone. + with_filter(&super::relay_filter(super::LogLevel::Info, None), || { + assert!(enabled!(target: "libp2p_relay::behaviour::handler", Level::WARN)); + }); + + // A relay level silences the upstream relay crate but not our own logs. + with_filter( + &super::relay_filter(super::LogLevel::Info, Some(super::LogLevel::Error)), + || { + assert!(!enabled!(target: "libp2p_relay::behaviour::handler", Level::WARN)); + assert!(enabled!(target: "pluto_relay_server::p2p", Level::INFO)); + }, + ); + } + + #[test] + fn every_log_level_composes_into_a_valid_filter() { + for base in super::LogLevel::value_variants() { + for relay in super::LogLevel::value_variants() { + let filter = super::relay_filter(*base, Some(*relay)); + EnvFilter::from_str(&filter).unwrap_or_else(|e| panic!("{filter:?}: {e}")); + } + } + } + #[test] + fn unknown_log_level_is_rejected() { + let err = match Cli::try_parse_from(["pluto", "relay", "--p2p-relay-loglevel=fatal"]) { + Ok(_) => panic!("`fatal` is not an EnvFilter level"), + Err(err) => err, + }; + + assert_eq!(err.kind(), clap::error::ErrorKind::InvalidValue); + } } diff --git a/crates/relay-server/src/config.rs b/crates/relay-server/src/config.rs index a919015b..245eb9fe 100644 --- a/crates/relay-server/src/config.rs +++ b/crates/relay-server/src/config.rs @@ -49,9 +49,6 @@ pub struct Config { /// Whether to filter private addresses. #[builder(default = false)] pub filter_private_addrs: bool, - /// LibP2PLogLevel. - #[builder(default = "Info".to_string())] - pub libp2p_log_level: String, } pub(crate) fn create_relay_config(config: &Config) -> relay::Config {