Skip to content

Commit

Permalink
feat: use tracing config
Browse files Browse the repository at this point in the history
  • Loading branch information
karlem committed Jul 18, 2024
1 parent 0e7c1f1 commit b348fe0
Show file tree
Hide file tree
Showing 14 changed files with 185 additions and 295 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

115 changes: 0 additions & 115 deletions fendermint/app/options/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use clap::{Args, Parser, Subcommand};
use config::ConfigArgs;
use debug::DebugArgs;
use fvm_shared::address::Network;
use ipc_observability::traces::FileLayerConfig;
use lazy_static::lazy_static;
use tracing_subscriber::EnvFilter;

use self::{
eth::EthArgs, genesis::GenesisArgs, key::KeyArgs, materializer::MaterializerArgs, rpc::RpcArgs,
Expand All @@ -25,10 +23,8 @@ pub mod materializer;
pub mod rpc;
pub mod run;

mod log;
mod parse;

use log::{parse_log_level, parse_rotation_kind, LogLevel, RotationKind};
use parse::parse_network;

lazy_static! {
Expand Down Expand Up @@ -103,70 +99,10 @@ pub struct Options {
#[arg(long, env = "FM_CONFIG_DIR")]
config_dir: Option<PathBuf>,

// TODO Karel - move all FM_LOG_FILE* flags to a configuration file instead

// Enable logging to a file.
#[arg(long, env = "FM_LOG_FILE_ENABLED")]
pub log_file_enabled: Option<bool>,

/// Set a custom directory for ipc log files.
#[arg(long, env = "FM_LOG_FILE_DIR")]
pub log_dir: Option<PathBuf>,

#[arg(long, env = "FM_LOG_FILE_MAX_FILES")]
pub max_log_files: Option<usize>,

#[arg(
long,
default_value = "daily",
value_enum,
env = "FM_LOG_FILE_ROTATION",
help = "The kind of rotation to use for log files. Options: minutely, hourly, daily, never.",
value_parser = parse_rotation_kind,
)]
pub log_files_rotation: Option<RotationKind>,

#[arg(
long,
env = "FM_LOG_FILE_DOMAINS_FILTER",
help = "Filter log events by domains. Only events from the specified domains will be logged. Comma separated.",
value_delimiter = ','
)]
pub domains_filter: Option<Vec<String>>,

#[arg(
long,
env = "FM_LOG_FILE_EVENTS_FILTER",
help = "Filter log events by name. Only events with the specified names will be logged. Comma separated.",
value_delimiter = ','
)]
pub events_filter: Option<Vec<String>>,

/// Optionally override the default configuration.
#[arg(short, long, default_value = "dev")]
pub mode: String,

/// Set the logging level of the console.
#[arg(
short = 'l',
long,
default_value = "info",
value_enum,
env = "FM_LOG_LEVEL",
help = "Standard log levels, or a comma separated list of filters, e.g. 'debug,tower_abci=warn,libp2p::gossipsub=info'",
value_parser = parse_log_level,
)]
log_level: LogLevel,

/// Set the logging level of the log file. If missing, it defaults to the same level as the console.
#[arg(
long,
value_enum,
env = "FM_LOG_FILE_LEVEL",
value_parser = parse_log_level,
)]
log_file_level: Option<LogLevel>,

/// Global options repeated here for discoverability, so they show up in `--help` among the others.
#[command(flatten)]
pub global: GlobalArgs,
Expand All @@ -176,35 +112,6 @@ pub struct Options {
}

impl Options {
/// Tracing filter for the console.
///
/// Coalescing everything into a filter instead of either a level or a filter
/// because the `tracing_subscriber` setup methods like `with_filter` and `with_level`
/// produce different static types and it's not obvious how to use them as alternatives.
pub fn log_console_filter(&self) -> anyhow::Result<EnvFilter> {
self.log_level.to_filter()
}

/// Tracing filter for the log file.
pub fn log_file_filter(&self) -> anyhow::Result<EnvFilter> {
if let Some(ref level) = self.log_file_level {
level.to_filter()
} else {
self.log_console_filter()
}
}

pub fn log_file_config(&self) -> FileLayerConfig {
FileLayerConfig {
enabled: self.log_file_enabled.unwrap_or(false),
directory: self.log_dir.clone(),
max_log_files: self.max_log_files,
rotation: self.log_files_rotation.clone(),
domain_filter: self.domains_filter.clone(),
events_filter: self.events_filter.clone(),
}
}

/// Path to the configuration directories.
///
/// If not specified then returns the default under the home directory.
Expand Down Expand Up @@ -248,7 +155,6 @@ mod tests {
use crate::*;
use clap::Parser;
use fvm_shared::address::Network;
use tracing::level_filters::LevelFilter;

/// Set some env vars, run a fallible piece of code, then unset the variables otherwise they would affect the next test.
pub fn with_env_vars<F, T>(vars: &[(&str, &str)], f: F) -> T
Expand Down Expand Up @@ -318,27 +224,6 @@ mod tests {
assert!(e.to_string().contains("Usage:"), "unexpected help: {e}");
}

#[test]
fn parse_log_level() {
let parse_filter = |cmd: &str| {
let opts: Options = Options::parse_from(cmd.split_ascii_whitespace());
opts.log_console_filter().expect("filter should parse")
};

let assert_level = |cmd: &str, level: LevelFilter| {
let filter = parse_filter(cmd);
assert_eq!(filter.max_level_hint(), Some(level))
};

assert_level("fendermint --log-level debug run", LevelFilter::DEBUG);
assert_level("fendermint --log-level off run", LevelFilter::OFF);
assert_level(
"fendermint --log-level libp2p=warn,error run",
LevelFilter::WARN,
);
assert_level("fendermint --log-level info run", LevelFilter::INFO);
}

#[test]
fn parse_invalid_log_level() {
// NOTE: `nonsense` in itself is interpreted as a target. Maybe we should mandate at least `=` in it?
Expand Down
93 changes: 0 additions & 93 deletions fendermint/app/options/src/log.rs

This file was deleted.

1 change: 1 addition & 0 deletions fendermint/app/settings/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ fvm_shared = { workspace = true }
fvm_ipld_encoding = { workspace = true }
ipc-api = { workspace = true }
ipc-provider = { workspace = true }
ipc-observability = { workspace = true }
tracing = { workspace = true }

fendermint_vm_encoding = { path = "../../vm/encoding" }
Expand Down
3 changes: 3 additions & 0 deletions fendermint/app/settings/src/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use serde::Deserialize;
use serde_with::{serde_as, DurationSeconds};
use std::time::Duration;

use ipc_observability::traces_settings::TracesSettings;

use crate::{IsHumanReadable, MetricsSettings, SocketAddress};

/// Ethereum API facade settings.
Expand All @@ -19,6 +21,7 @@ pub struct EthSettings {
pub gas: GasOpt,
pub max_nonce_gap: u64,
pub metrics: MetricsSettings,
pub tracing: TracesSettings,
}

#[serde_as]
Expand Down
2 changes: 2 additions & 0 deletions fendermint/app/settings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use fendermint_vm_topdown::BlockHeight;
use self::eth::EthSettings;
use self::fvm::FvmSettings;
use self::resolver::ResolverSettings;
use ipc_observability::traces_settings::TracesSettings;
use ipc_provider::config::deserialize::deserialize_eth_address_from_str;

pub mod eth;
Expand Down Expand Up @@ -286,6 +287,7 @@ pub struct Settings {
pub broadcast: BroadcastSettings,
pub ipc: IpcSettings,
pub testing: Option<TestingSettings>,
pub tracing: TracesSettings,
}

impl Settings {
Expand Down
3 changes: 3 additions & 0 deletions fendermint/app/src/cmd/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::time::Duration;

use anyhow::Context;
use fendermint_eth_api::HybridClient;
use ipc_observability::traces::set_global_tracing_subscriber;
use tracing::info;

use crate::{
Expand Down Expand Up @@ -34,6 +35,8 @@ cmd! {

/// Run the Ethereum API facade.
async fn run(settings: EthSettings, client: HybridClient) -> anyhow::Result<()> {
// TOOO kare - set up tracing

if settings.metrics.enabled {
info!("metrics enabled");

Expand Down
14 changes: 10 additions & 4 deletions fendermint/app/src/cmd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ use crate::{
use anyhow::{anyhow, Context};
use async_trait::async_trait;

use ipc_observability::traces::create_subscriber;
use tracing::subscriber;

pub mod config;
pub mod debug;
pub mod eth;
Expand Down Expand Up @@ -81,10 +84,13 @@ fn settings(opts: &Options) -> anyhow::Result<Settings> {
d => d,
};

tracing::info!(
path = config_dir.to_string_lossy().into_owned(),
"reading configuration"
);
subscriber::with_default(create_subscriber(), || {
tracing::info!(
path = config_dir.to_string_lossy().into_owned(),
"reading configuration"
)
});

let settings =
Settings::new(&config_dir, &opts.home_dir, &opts.mode).context("error parsing settings")?;

Expand Down
4 changes: 4 additions & 0 deletions fendermint/app/src/cmd/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ use tracing::info;
use crate::cmd::key::read_secret_key;
use crate::{cmd, options::run::RunArgs, settings::Settings};

use ipc_observability::traces::set_global_tracing_subscriber;

cmd! {
RunArgs(self, settings) {
run(settings).await
Expand All @@ -59,6 +61,8 @@ namespaces! {
///
/// This method acts as our composition root.
async fn run(settings: Settings) -> anyhow::Result<()> {
let _work_guard = set_global_tracing_subscriber(&settings.tracing)?;

let tendermint_rpc_url = settings.tendermint_rpc_url()?;
tracing::info!("Connecting to Tendermint at {tendermint_rpc_url}");

Expand Down
Loading

0 comments on commit b348fe0

Please sign in to comment.