Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
node: Refactor logging initialization
◌ CI pending Lorenz Leutgeb committed 7 months ago
commit db057d1d17dc6a609e353f5ba6da64d7b38dd8af
parent 32b7ee9d2ac31868384bd32a6e733c436d3b66b8
1 pending (1 total) View logs
2 files changed +130 -100
modified crates/radicle-node/src/main.rs
@@ -15,21 +15,68 @@ use radicle_node::{Runtime, VERSION};
#[cfg(unix)]
use radicle_signals as signals;

-
/// The log level to use before reading any other value
-
/// from configuration.
-
///
-
/// Note that this is different from the default value
-
/// of the command line argument `--log`, as it is valid
-
/// *even before that argument is parsed*.
-
/// It ensures that we log the errors parsing the
-
/// command line arguments, such as `--log`.
-
const LOG_LEVEL_DEFAULT: &log::Level = &log::Level::Warn;
-

-
#[derive(Debug, Default, Clone)]
-
enum LogFormat {
-
    #[default]
+
const HELP_MSG: &str = r#"
+
Usage
+

+
   radicle-node [<option>...]
+

+
   If you're running a public seed node, make sure to use `--listen` to bind a listening socket to
+
   eg. `0.0.0.0:8776`, and add your external addresses in your configuration.
+

+
Options
+

+
    --config      <path>                            Config file to use
+
                  (default: ~/.radicle/config.json)
+
    --force                                         Force start even if an existing control socket
+
                                                      is found
+
    --listen      <address>                         Address to listen on
+
    --log-level   <level>                           Set log level
+
                  (default: info)
+
    --log-logger  (radicle | structured | systemd)  Set logger implementation
+
                  (default: radicle)
+
    --log-format  json                              Set log format for logger implementation
+
    --version                                       Print program version
+
    --help                                          Print help
+
"#;
+

+
#[derive(Debug, Clone)]
+
enum Logger {
    Radicle,
    #[cfg(feature = "structured-logger")]
+
    Structured,
+
    #[cfg(all(feature = "systemd", target_os = "linux"))]
+
    Systemd,
+
}
+

+
impl Default for Logger {
+
    fn default() -> Self {
+
        #[cfg(all(feature = "systemd", target_os = "linux"))]
+
        if radicle_systemd::journal::connected() {
+
            Logger::Systemd
+
        } else {
+
            Logger::Radicle
+
        }
+
    }
+
}
+

+
impl FromStr for Logger {
+
    type Err = &'static str;
+

+
    fn from_str(s: &str) -> Result<Self, Self::Err> {
+
        match s {
+
            "radicle" => Ok(Logger::Radicle),
+
            #[cfg(feature = "structured-logger")]
+
            "structured" => Ok(Logger::Structured),
+
            #[cfg(all(feature = "systemd", target_os = "linux"))]
+
            "systemd" => Ok(Logger::Systemd),
+
            _ => Err("unknown logger"),
+
        }
+
    }
+
}
+

+
#[derive(Debug, Clone)]
+
enum LogFormat {
+
    #[cfg(feature = "structured-logger")]
    Json,
}

@@ -40,36 +87,21 @@ impl FromStr for LogFormat {
        match s {
            #[cfg(feature = "structured-logger")]
            "json" => Ok(LogFormat::Json),
-
            "radicle" => Ok(LogFormat::Radicle),
            _ => Err("unknown log format"),
        }
    }
}

-
pub const HELP_MSG: &str = r#"
-
Usage
-

-
   radicle-node [<option>...]
-

-
   If you're running a public seed node, make sure to use `--listen` to bind a listening socket to
-
   eg. `0.0.0.0:8776`, and add your external addresses in your configuration.
-

-
Options
-

-
    --config             <path>         Config file to use (default ~/.radicle/config.json)
-
    --force                             Force start even if an existing control socket is found
-
    --listen             <address>      Address to listen on
-
    --log                <level>        Set log level (default: info)
-
    --log-format         <format>       Set log format (default: radicle)
-
    --version                           Print program version
-
    --help                              Print help
-
"#;
+
struct LogOptions {
+
    level: Option<log::Level>,
+
    logger: Logger,
+
    format: Option<LogFormat>,
+
}

struct Options {
    config: Option<PathBuf>,
    listen: Vec<SocketAddr>,
-
    log: Option<log::Level>,
-
    log_format: Option<LogFormat>,
+
    log: LogOptions,
    force: bool,
}

@@ -81,7 +113,8 @@ fn parse_options() -> Result<Options, lexopt::Error> {
    let mut listen = Vec::new();
    let mut config = None;
    let mut force = false;
-
    let mut log = None;
+
    let mut log_level = None;
+
    let mut log_logger = Logger::default();
    let mut log_format = None;

    while let Some(arg) = parser.next()? {
@@ -96,8 +129,21 @@ fn parse_options() -> Result<Options, lexopt::Error> {
                let addr = parser.value()?.parse_with(SocketAddr::from_str)?;
                listen.push(addr);
            }
-
            Long("log") => {
-
                log = Some(parser.value()?.parse_with(log::Level::from_str)?);
+
            Long("log") | Long("log-level") => {
+
                if matches!(arg, Long("log")) {
+
                    eprintln!("Warning: The option `--log` is deprecated and will be removed. Please use `--log-level` instead.");
+
                }
+
                log_level = Some(parser.value()?.parse_with(log::Level::from_str)?);
+
            }
+
            Long("log-logger") => {
+
                let parsed = parser.value()?.parse_with(Logger::from_str)?;
+
                if matches!(parsed, Logger::Radicle) {
+
                    return Err(lexopt::Error::Custom(
+
                        "choice of this logger explicitly is forbidden, because it is deprecated"
+
                            .into(),
+
                    ));
+
                }
+
                log_logger = parsed;
            }
            Long("log-format") => {
                log_format = Some(parser.value()?.parse_with(LogFormat::from_str)?);
@@ -119,9 +165,12 @@ fn parse_options() -> Result<Options, lexopt::Error> {
    Ok(Options {
        force,
        listen,
-
        log,
-
        log_format,
        config,
+
        log: LogOptions {
+
            level: log_level,
+
            logger: log_logger,
+
            format: log_format,
+
        },
    })
}

@@ -144,14 +193,14 @@ fn execute(options: Options) -> Result<(), ExecutionError> {
    // The first thing we do after reading command line options is
    // to set the log level, as this influences logging during
    // configuration loading.
-
    if let Some(level) = options.log {
+
    if let Some(level) = options.log.level {
        log::set_max_level(level.to_level_filter());
    }

    let config = options.config.unwrap_or_else(|| home.config());
    let mut config = profile::Config::load(&config)?;

-
    if options.log.is_none() {
+
    if options.log.level.is_none() {
        log::set_max_level(log::Level::from(config.node.log).to_level_filter());
    } else {
        // It might seem counter-intuitive at first, as there
@@ -207,68 +256,48 @@ fn execute(options: Options) -> Result<(), ExecutionError> {
    Ok(())
}

-
fn initialize_logging(log_format: LogFormat) {
-
    let level = *LOG_LEVEL_DEFAULT;
-

-
    //  - We are compiling conditionally, so cannot depend
-
    //    on the concrete type of the logger(s).
-
    //  - We are dealing with `Option`, so we need `Box: Sized`.
-
    //  - We want to provide a `Box` to `log::set_boxed_logger`.
-
    //  - We also want to keep around any errors along the way.
-
    type Logger = Box<dyn log::Log>;
-
    type Error = Box<dyn std::error::Error>;
+
fn initialize_logging(options: &LogOptions) -> Result<(), Box<dyn std::error::Error>> {
+
    let level = options.level.unwrap_or(log::Level::Info);

-
    let logger: Result<Option<Logger>, Error> = {
-
        match log_format {
+
    let logger: Box<dyn log::Log> = {
+
        match options.logger {
            #[cfg(feature = "structured-logger")]
-
            LogFormat::Json => {
+
            Logger::Structured => {
                use structured_logger::{json, Builder};

-
                Ok(Some(Box::new(
-
                    Builder::new()
-
                        .with_default_writer(json::new_writer(io::stdout()))
-
                        .build(),
-
                )))
+
                let writer = match options.format {
+
                    Some(LogFormat::Json) | None => json::new_writer(io::stdout()),
+
                };
+

+
                Box::new(Builder::new().with_default_writer(writer).build())
            }
-
            _ => {
-
                #[cfg(all(feature = "systemd", target_os = "linux"))]
-
                {
-
                    use thiserror::Error;
-

-
                    #[derive(Error, Debug)]
-
                    #[error("Error connecting to systemd journal: {0}")]
-
                    struct JournalError(io::Error);
-

-
                    radicle_systemd::journal::logger::<&str, &str, _>(
-
                        "radicle-node".to_string(),
-
                        [],
-
                    )
-
                    .map_err(|err| Box::new(JournalError(err)) as Error)
+
            #[cfg(all(feature = "systemd", target_os = "linux"))]
+
            Logger::Systemd => {
+
                use radicle_systemd::journal::*;
+
                use thiserror::Error;
+

+
                #[derive(Error, Debug)]
+
                enum JournalError {
+
                    #[error("journald not connected")]
+
                    NotConnected,
+
                    #[error("journald i/o: {0}")]
+
                    Io(#[from] io::Error),
                }
-
                #[cfg(not(all(feature = "systemd", target_os = "linux")))]
-
                {
-
                    // This is constant, and `rustc` will hopefully use it to
-
                    // optimize away the `match` below.
-
                    Ok(None)
+

+
                if !connected() {
+
                    return Err(Box::new(JournalError::NotConnected));
                }
+

+
                logger::<&str, &str, _>("radicle-node".to_string(), []).map_err(Box::new)?
            }
+
            Logger::Radicle => Box::new(radicle::logger::Logger::new(level)),
        }
    };

-
    let (logger, err) = match logger {
-
        Ok(Some(logger)) => (logger, None),
-
        otherwise => (
-
            Box::new(radicle::logger::Logger::new(level)) as Logger,
-
            otherwise.err(),
-
        ),
-
    };
-

    log::set_boxed_logger(logger).expect("no other logger should have been set already");
    log::set_max_level(level.to_level_filter());

-
    if let Some(err) = err {
-
        log::warn!(target: "node", "Error initializing logger (fell back to default): {err}");
-
    }
+
    Ok(())
}

fn main() {
@@ -281,14 +310,15 @@ fn main() {
    }

    let options = parse_options().unwrap_or_else(|err| {
-
        // Fallback to default logging format if not able to parse options.
-
        initialize_logging(LogFormat::Radicle);
        // The lexopt errors read nicely with a comma.
-
        log::error!(target: "node", "Failed to parse options, {err:#}");
+
        eprintln!("Failed to parse options, {err:#}");
        exit(2);
    });

-
    initialize_logging(options.log_format.clone().unwrap_or_default());
+
    initialize_logging(&options.log).unwrap_or_else(|err| {
+
        eprintln!("Failed to initialize logging: {err:#}");
+
        exit(3);
+
    });

    if let Err(err) = execute(options) {
        log::error!(target: "node", "{err:#}");
modified crates/radicle-systemd/src/journal.rs
@@ -5,19 +5,19 @@ use systemd_journal_logger::{connected_to_journal, current_exe_identifier, Journ
pub fn logger<K, V, I>(
    default_identifier: String,
    extra_fields: I,
-
) -> std::io::Result<Option<Box<dyn log::Log>>>
+
) -> std::io::Result<Box<dyn log::Log>>
where
    I: IntoIterator<Item = (K, V)>,
    K: AsRef<str>,
    V: AsRef<[u8]>,
{
-
    if !connected_to_journal() {
-
        return Ok(None);
-
    }
-

-
    Ok(Some(Box::new(
+
    Ok(Box::new(
        JournalLog::new()?
            .with_syslog_identifier(current_exe_identifier().unwrap_or(default_identifier))
            .with_extra_fields(extra_fields),
-
    )))
+
    ))
+
}
+

+
pub fn connected() -> bool {
+
    connected_to_journal()
}