fix(cli): install the tracing subscriber before flag validation

crates/cli/README.md

@@ -6,7 +6,7 @@ Pluto enables the operation of Ethereum validators in a fault tolerant manner by
 
 ## Commands (current)
 
-Most flags below also read a `CHARON_*` environment variable (for example `--beacon-node-endpoints` reads `CHARON_BEACON_NODE_ENDPOINTS`), mirroring charon's environment surface. Exceptions: none of the `alpha test` flags have env bindings, nor do `enr --verbose`, `version --verbose` and `relay --log-color`. A variable that is set but empty is treated as unset. Run `pluto <COMMAND> --help` for the authoritative list — flags with a binding show an `[env: ...]` line.
+Most flags below also read a `CHARON_*` environment variable (for example `--beacon-node-endpoints` reads `CHARON_BEACON_NODE_ENDPOINTS`), mirroring charon's environment surface. A variable that is set but empty is treated as unset. Run `pluto <COMMAND> --help` for the authoritative list — flags with a binding show an `[env: ...]` line.
 
 Some flags are accepted for charon compatibility but are not yet wired up. They are marked below as **[IGNORED]** (parsed, then dropped — `pluto run` logs a warning for most of them) or **[UNSUPPORTED]** (setting the flag makes the command exit at startup with an error).
 
@@ -59,7 +59,7 @@ Starts the long-running Pluto middleware process to perform distributed validato
   - `--testnet-genesis-timestamp <TIMESTAMP>`: Genesis timestamp of the custom test network.
   - `--testnet-capella-hard-fork <VERSION>`: Capella hard fork version of the custom test network.
   - The custom network is only registered when the testnet flags are fully specified; a partial set is silently ignored and pluto falls back to the built-in network registry.
-- Plus the [common P2P flags](#common-p2p-flags) and [common logging flags](#common-logging-flags) (including the Loki flags).
+- Plus the [common P2P flags](#common-p2p-flags) and [common logging flags](#common-logging-flags).
 
 ### `pluto relay`
 
@@ -69,13 +69,12 @@ Starts a libp2p circuit relay that charon clients can use to discover and connec
   - `--data-dir <PATH>`: The directory where pluto will store all its internal data. (default: `.charon`)
   - `--http-address <ADDR>`: 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 <LEVEL>`: 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 <N>`: Updates max circuit reservations per peer (each valid for 1 hour). (default: `512`)
   - `--p2p-max-connections <N>`: 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.
   - `--monitoring-address <ADDR>`: Listening address (ip and port) for the monitoring API (prometheus).
   - `--debug-address <ADDR>`: **[IGNORED]** Parsed but no debug listener is started (no warning is emitted).
-- Plus the [common P2P flags](#common-p2p-flags) and [common logging flags](#common-logging-flags) (including the Loki flags). Note that `--p2p-relays` is accepted but unused by the relay itself.
+- Plus the [common P2P flags](#common-p2p-flags) and [common logging flags](#common-logging-flags). Note that `--p2p-relays` is accepted but unused by the relay itself.
 
 ### `pluto dkg`
 
@@ -94,7 +93,7 @@ Participate in a distributed key generation ceremony for a specific cluster defi
   - `--publish-address <URL>`: The URL to publish the cluster to. (default: `https://api.obol.tech/v1`)
   - `--publish-timeout <DURATION>`: Timeout for publishing a cluster; increase for clusters with more than 200 validators. (default: `30s`)
   - `--zipped`: Create a tar archive compressed with gzip of the target directory after creation.
-- Plus the [common P2P flags](#common-p2p-flags) and the [common logging flags](#common-logging-flags) (Loki flags are **not** available on `dkg`).
+- Plus the [common P2P flags](#common-p2p-flags) and the [common logging flags](#common-logging-flags).
 
 ### `pluto enr`
 
@@ -266,17 +265,19 @@ Shared by `run`, `relay`, `dkg` and `alpha test peers`.
 
 ### Common logging flags
 
-Shared by `run`, `relay` and `dkg`.
+Global: accepted by every command, and parsed identically before or after the subcommand (`pluto --log-level=debug run` and `pluto run --log-level=debug` are equivalent).
 
-- `--log-format <FORMAT>`: **[IGNORED]** Accepted but not yet applied — output is always console-formatted. (default: `console`)
-- `--log-level <LEVEL>`: Log level; `off`, `trace`, `debug`, `info`, `warn` or `error`. (default: `info`)
-- `--log-color <COLOR>`: Log color; `auto`, `force` or `disable`. (default: `auto`)
-- `--log-output-path <PATH>`: **[IGNORED]** Accepted but not yet applied — no log file is written.
+All log output goes to stderr, leaving each command's stdout free for its own data.
 
-`run` and `relay` additionally support Loki output:
+`RUST_LOG` is not consulted; `--log-level` (or its default) always decides the filter.
 
+- `--log-format <FORMAT>`: **[IGNORED]** Accepted but not yet applied — output is always console-formatted. (default: `console`)
+- `--log-level <LEVEL>`: Log level; `off`, `trace`, `debug`, `info`, `warn` or `error`. Charon accepts only the last four; the two extra levels are what `tracing`'s `EnvFilter` understands. (default: `info`)
+- `--log-color <COLOR>`: Log color; `auto`, `force` or `disable`. `auto` means "unless `NO_COLOR` is set", not TTY detection. (default: `auto`)
+- `--log-output-path <PATH>`: **[IGNORED]** Accepted but not yet applied — no log file is written.
 - `--loki-addresses <ADDRS>`: Enables sending of logfmt structured logs to a Loki log aggregation server, in addition to normal stderr logs. Only the first address is used; extra entries are ignored with a warning (charon fans out to every address).
 - `--loki-service <NAME>`: Service label sent with logs to Loki. (default: `pluto`)
+- `--p2p-relay-loglevel <LEVEL>`: 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`. Charon scopes this to `relay`.
 
 ## Example
 

crates/cli/src/cli.rs

@@ -3,6 +3,7 @@
 use clap::{Parser, Subcommand};
 
 use crate::commands::{
+    common::TracingArgs,
     create_cluster::CreateClusterArgs,
     create_dkg::CreateDkgArgs,
     create_enr::CreateEnrArgs,
@@ -29,6 +30,10 @@ pub struct Cli {
     /// The subcommand to execute.
     #[command(subcommand)]
     pub command: Commands,
+
+    /// Logging flags, shared by every subcommand.
+    #[command(flatten)]
+    pub tracing: TracingArgs,
 }
 
 /// Available commands.

crates/cli/src/commands/common.rs

@@ -1,6 +1,6 @@
 //! Shared helpers for CLI commands.
 
-use std::fmt;
+use std::{collections::HashMap, fmt, path::PathBuf};
 
 use pluto_p2p::config::RelayAddr;
 use tracing::warn;
@@ -53,32 +53,156 @@ impl fmt::Display for LogLevel {
     }
 }
 
-/// Builds a tracing configuration for CLI commands, optionally enabling Loki.
+/// 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<LogLevel>) -> String {
+    match relay_level {
+        Some(level) => format!("{base},libp2p_relay={level}"),
+        None => base.to_string(),
+    }
+}
+
+/// Log output encoding
+#[derive(clap::ValueEnum, Clone, Copy, Debug, Default, PartialEq, Eq)]
+pub enum LogFormat {
+    /// Human-readable, optionally colored.
+    #[default]
+    Console,
+    /// Flat `key=value` pairs.
+    Logfmt,
+    /// One JSON object per event.
+    Json,
+}
+
+/// Logging and Loki flags, accepted by every subcommand.
 ///
-/// `loki` is `Some` when the caller wants events forwarded to a Loki endpoint
-/// (e.g. via `--loki-addresses`), and `None` for commands that only need
-/// console output.
+/// These are `global`, so they parse identically before or after the
+/// subcommand and are readable from the root [`crate::cli::Cli`] before any
+/// command-specific config conversion runs. That ordering is what lets
+/// `main` install the subscriber before validation starts.
 // TODO: wire `log-output-path` (file output) and `log-format` (logfmt/json)
 // into the tracing layers. `pluto_tracing` supports console + Loki only, so
-// `run`/`dkg`/`relay` accept these flags but do not yet apply them.
-pub fn build_console_tracing_config(
-    level: impl Into<String>,
-    color: &ConsoleColor,
-    loki: Option<pluto_tracing::LokiConfig>,
-) -> pluto_tracing::TracingConfig {
-    let mut builder = pluto_tracing::TracingConfig::builder().with_default_console();
-
-    builder = match color {
-        ConsoleColor::Auto => builder.console_with_ansi(std::env::var("NO_COLOR").is_err()),
-        ConsoleColor::Force => builder.console_with_ansi(true),
-        ConsoleColor::Disable => builder.console_with_ansi(false),
-    };
-
-    if let Some(loki) = loki {
-        builder = builder.loki(loki);
+// these flags are accepted but not yet applied.
+#[derive(clap::Args, Clone, Debug)]
+#[command(next_help_heading = "Logging")]
+pub struct TracingArgs {
+    #[arg(
+        long = "log-format",
+        env = "CHARON_LOG_FORMAT",
+        default_value = "console",
+        global = true,
+        ignore_case = true,
+        display_order = 1000,
+        help = "Log format; console, logfmt or json"
+    )]
+    pub log_format: LogFormat,
+
+    #[arg(
+        long = "log-level",
+        env = "CHARON_LOG_LEVEL",
+        default_value = "info",
+        global = true,
+        ignore_case = true,
+        display_order = 1001,
+        help = "Log level"
+    )]
+    pub log_level: LogLevel,
+
+    #[arg(
+        long = "log-color",
+        env = "CHARON_LOG_COLOR",
+        default_value = "auto",
+        global = true,
+        ignore_case = true,
+        display_order = 1002,
+        help = "Log color; auto, force, disable."
+    )]
+    pub log_color: ConsoleColor,
+
+    #[arg(
+        long = "log-output-path",
+        env = "CHARON_LOG_OUTPUT_PATH",
+        global = true,
+        display_order = 1003,
+        help = "Path in which to write on-disk logs."
+    )]
+    pub log_output_path: Option<PathBuf>,
+
+    #[arg(
+        long = "loki-addresses",
+        env = "CHARON_LOKI_ADDRESSES",
+        value_delimiter = ',',
+        global = true,
+        display_order = 1004,
+        help = "Enables sending of logfmt structured logs to these Loki log aggregation server addresses. This is in addition to normal stderr logs."
+    )]
+    pub loki_addresses: Vec<String>,
+
+    #[arg(
+        long = "loki-service",
+        env = "CHARON_LOKI_SERVICE",
+        default_value = "pluto",
+        global = true,
+        display_order = 1005,
+        help = "Service label sent with logs to Loki."
+    )]
+    pub loki_service: String,
+
+    #[arg(
+        long = "p2p-relay-loglevel",
+        env = "CHARON_P2P_RELAY_LOGLEVEL",
+        global = true,
+        ignore_case = true,
+        display_order = 1006,
+        help = "Libp2p circuit relay log level. Defaults to --log-level."
+    )]
+    pub p2p_relay_log_level: Option<LogLevel>,
+}
+
+impl TracingArgs {
+    /// Builds the subscriber configuration.
+    ///
+    /// Emits nothing: this runs before the subscriber exists, so any diagnostic
+    /// it produced would be dropped. Deferred warnings live in
+    /// [`TracingArgs::warn_unused`].
+    pub fn tracing_config(&self) -> pluto_tracing::TracingConfig {
+        let ansi = match self.log_color {
+            ConsoleColor::Auto => std::env::var_os("NO_COLOR").is_none(),
+            ConsoleColor::Force => true,
+            ConsoleColor::Disable => false,
+        };
+
+        let mut builder = pluto_tracing::TracingConfig::builder()
+            .with_default_console()
+            .console_with_ansi(ansi)
+            .override_env_filter(relay_filter(self.log_level, self.p2p_relay_log_level));
+
+        // Only the first address is used; see `warn_unused`.
+        if let Some(loki_url) = self.loki_addresses.first() {
+            builder = builder.loki(pluto_tracing::LokiConfig {
+                loki_url: loki_url.clone(),
+                labels: HashMap::from([("service".to_string(), self.loki_service.clone())]),
+                extra_fields: HashMap::new(),
+            });
+        }
+
+        builder.build()
     }
 
-    builder.override_env_filter(level.into()).build()
+    /// Reports flag values that were accepted but not applied.
+    ///
+    /// Call once the subscriber is installed.
+    pub fn warn_unused(&self) {
+        // Charon fans logs out to every entry in `loki-addresses`, but
+        // `pluto_tracing::TracingConfig` supports a single Loki layer today.
+        let ignored = self.loki_addresses.len().saturating_sub(1);
+        if ignored > 0 {
+            warn!(
+                ignored,
+                "Additional --loki-addresses ignored; only the first is used"
+            );
+        }
+    }
 }
 
 /// Parses the configured relay addresses, warning about insecure ones.
@@ -120,6 +244,122 @@ pub fn parse_relay_addrs(relays: &[String]) -> std::result::Result<Vec<RelayAddr
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::cli::Cli;
+    use clap::ValueEnum as _;
+    use std::str::FromStr as _;
+    use tracing::{Level, enabled};
+    use tracing_subscriber::{EnvFilter, layer::SubscriberExt as _};
+
+    #[test]
+    fn log_flags_accept_any_casing() {
+        // Charon takes these from env as often as from the command line, where
+        // `CHARON_LOG_LEVEL=INFO` is idiomatic.
+        for level in ["debug", "DEBUG", "Debug"] {
+            let cli = <Cli as clap::Parser>::try_parse_from([
+                "pluto",
+                "enr",
+                &format!("--log-level={level}"),
+            ])
+            .unwrap_or_else(|err| panic!("--log-level={level} should parse: {err}"));
+
+            assert_eq!(
+                cli.tracing.tracing_config().override_env_filter.as_deref(),
+                Some("debug")
+            );
+        }
+
+        for color in ["disable", "DISABLE", "Disable"] {
+            let cli = <Cli as clap::Parser>::try_parse_from([
+                "pluto",
+                "enr",
+                &format!("--log-color={color}"),
+            ])
+            .unwrap_or_else(|err| panic!("--log-color={color} should parse: {err}"));
+
+            assert!(
+                !cli.tracing
+                    .tracing_config()
+                    .console
+                    .expect("console")
+                    .with_ansi
+            );
+        }
+
+        for format in ["logfmt", "LOGFMT", "Logfmt"] {
+            let cli = <Cli as clap::Parser>::try_parse_from([
+                "pluto",
+                "enr",
+                &format!("--log-format={format}"),
+            ])
+            .unwrap_or_else(|err| panic!("--log-format={format} should parse: {err}"));
+
+            assert_eq!(cli.tracing.log_format, LogFormat::Logfmt);
+        }
+    }
+
+    #[test]
+    fn log_flags_reject_unknown_values() {
+        for flag in [
+            "--log-level=nonsense",
+            "--log-format=nonsense",
+            "--p2p-relay-loglevel=fatal",
+        ] {
+            let err = match <Cli as clap::Parser>::try_parse_from(["pluto", "enr", flag]) {
+                Ok(_) => panic!("{flag} should be rejected"),
+                Err(err) => err,
+            };
+
+            assert_eq!(err.kind(), clap::error::ErrorKind::InvalidValue);
+        }
+    }
+
+    /// 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(&relay_filter(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(&relay_filter(LogLevel::Info, Some(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 LogLevel::value_variants() {
+            for relay in LogLevel::value_variants() {
+                let filter = relay_filter(*base, Some(*relay));
+                EnvFilter::from_str(&filter).unwrap_or_else(|e| panic!("{filter:?}: {e}"));
+            }
+        }
+    }
+
+    #[test]
+    fn p2p_relay_loglevel_reaches_the_env_filter() {
+        // The flag is global, so it composes with `--log-level` from the root
+        // rather than from the `relay` subcommand that used to own it.
+        let cli = <Cli as clap::Parser>::try_parse_from([
+            "pluto",
+            "relay",
+            "--log-level=info",
+            "--p2p-relay-loglevel=error",
+        ])
+        .expect("relay args should parse");
+
+        assert_eq!(
+            cli.tracing.tracing_config().override_env_filter.as_deref(),
+            Some("info,libp2p_relay=error")
+        );
+    }
 
     // Per-address parsing is covered by `RelayAddr`'s own tests; what is left
     // to check here is the empty-value contract and the error wrapping.