From f362a79042ef3ba7cf0fdde9232418f20b0cbec1 Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Wed, 13 Dec 2023 15:34:51 +0100 Subject: [PATCH 1/7] Fix: use only `snake_case` --- mithril-client-cli/src/utils/progress_reporter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mithril-client-cli/src/utils/progress_reporter.rs b/mithril-client-cli/src/utils/progress_reporter.rs index b6490bb8031..15141538e8f 100644 --- a/mithril-client-cli/src/utils/progress_reporter.rs +++ b/mithril-client-cli/src/utils/progress_reporter.rs @@ -99,7 +99,7 @@ impl DownloadProgressReporter { if should_report { println!( - r#"{{ "bytesDownloaded": {}, "bytesTotal": {}, "secondsLeft": {}.{}, "secondsElapsed": {}.{} }}"#, + r#"{{ "bytes_downloaded": {}, "bytes_total": {}, "seconds_left": {}.{}, "seconds_elapsed": {}.{} }}"#, self.progress_bar.position(), self.progress_bar.length().unwrap_or(0), self.progress_bar.eta().as_secs(), From 293e63951cd8ab1ecbfbf15e6ef23b60d7c0c0e6 Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Wed, 13 Dec 2023 16:52:25 +0100 Subject: [PATCH 2/7] Add timestamp in JSON logs --- mithril-client-cli/src/commands/snapshot/download.rs | 4 +++- mithril-client-cli/src/utils/progress_reporter.rs | 9 ++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/mithril-client-cli/src/commands/snapshot/download.rs b/mithril-client-cli/src/commands/snapshot/download.rs index 5d455647871..9bd95e63cb8 100644 --- a/mithril-client-cli/src/commands/snapshot/download.rs +++ b/mithril-client-cli/src/commands/snapshot/download.rs @@ -1,4 +1,5 @@ use anyhow::{anyhow, Context}; +use chrono::Utc; use clap::Parser; use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind}; use slog_scope::{debug, logger, warn}; @@ -183,7 +184,8 @@ impl SnapshotDownloadCommand { if self.json { println!( - r#"{{"db_directory": "{}"}}"#, + r#"{{"timestamp": "{}", "db_directory": "{}"}}"#, + Utc::now().to_rfc3339(), canonicalized_filepath.display() ); } else { diff --git a/mithril-client-cli/src/utils/progress_reporter.rs b/mithril-client-cli/src/utils/progress_reporter.rs index 15141538e8f..87969557253 100644 --- a/mithril-client-cli/src/utils/progress_reporter.rs +++ b/mithril-client-cli/src/utils/progress_reporter.rs @@ -1,3 +1,4 @@ +use chrono::Utc; use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget}; use mithril_common::StdResult; use slog_scope::warn; @@ -49,8 +50,9 @@ impl ProgressPrinter { pub fn report_step(&self, step_number: u16, text: &str) -> StdResult<()> { match self.output_type { ProgressOutputType::JsonReporter => println!( - r#"{{"step_num": {step_number}, "total_steps": {}, "message": "{text}"}}"#, - self.number_of_steps + r#"{{"timestamp": "{timestamp}", "step_num": {step_number}, "total_steps": {number_of_steps}, "message": "{text}"}}"#, + timestamp = Utc::now().to_rfc3339(), + number_of_steps = self.number_of_steps, ), ProgressOutputType::TTY => self .multi_progress @@ -99,7 +101,8 @@ impl DownloadProgressReporter { if should_report { println!( - r#"{{ "bytes_downloaded": {}, "bytes_total": {}, "seconds_left": {}.{}, "seconds_elapsed": {}.{} }}"#, + r#"{{ "timestamp": "{}", "bytes_downloaded": {}, "bytes_total": {}, "seconds_left": {}.{}, "seconds_elapsed": {}.{} }}"#, + Utc::now().to_rfc3339(), self.progress_bar.position(), self.progress_bar.length().unwrap_or(0), self.progress_bar.eta().as_secs(), From fafe5efb7ee26f606fcd7507d626300615a89ba0 Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Thu, 14 Dec 2023 12:27:30 +0100 Subject: [PATCH 3/7] Add `ProgressBarJsonFormatter` This makes reported JSON logs easier to test --- .../src/utils/progress_reporter.rs | 100 ++++++++++++++++-- 1 file changed, 90 insertions(+), 10 deletions(-) diff --git a/mithril-client-cli/src/utils/progress_reporter.rs b/mithril-client-cli/src/utils/progress_reporter.rs index 87969557253..56d4efbfdac 100644 --- a/mithril-client-cli/src/utils/progress_reporter.rs +++ b/mithril-client-cli/src/utils/progress_reporter.rs @@ -72,6 +72,23 @@ impl Deref for ProgressPrinter { } } +pub struct ProgressBarJsonFormatter; + +impl ProgressBarJsonFormatter { + pub fn format(progress_bar: &ProgressBar) -> String { + format!( + r#"{{"timestamp": "{}", "bytes_downloaded": {}, "bytes_total": {}, "seconds_left": {}.{:0>3}, "seconds_elapsed": {}.{:0>3}}}"#, + Utc::now().to_rfc3339(), + progress_bar.position(), + progress_bar.length().unwrap_or(0), + progress_bar.eta().as_secs(), + progress_bar.eta().subsec_millis(), + progress_bar.elapsed().as_secs(), + progress_bar.elapsed().subsec_millis(), + ) + } +} + /// Wrapper of a indicatif [ProgressBar] to allow reporting to json. pub struct DownloadProgressReporter { progress_bar: ProgressBar, @@ -100,16 +117,7 @@ impl DownloadProgressReporter { }; if should_report { - println!( - r#"{{ "timestamp": "{}", "bytes_downloaded": {}, "bytes_total": {}, "seconds_left": {}.{}, "seconds_elapsed": {}.{} }}"#, - Utc::now().to_rfc3339(), - self.progress_bar.position(), - self.progress_bar.length().unwrap_or(0), - self.progress_bar.eta().as_secs(), - self.progress_bar.eta().subsec_millis(), - self.progress_bar.elapsed().as_secs(), - self.progress_bar.elapsed().subsec_millis(), - ); + println!("{}", ProgressBarJsonFormatter::format(&self.progress_bar)); match self.last_json_report_instant.write() { Ok(mut instant) => *instant = Some(Instant::now()), @@ -132,3 +140,75 @@ impl DownloadProgressReporter { } } } + +#[cfg(test)] +mod tests { + use std::thread::sleep; + + use super::*; + use indicatif::ProgressBar; + + #[test] + fn check_seconds_elapsed_in_json_report_with_more_than_100_milliseconds() { + let progress_bar = ProgressBar::new(10).with_elapsed(Duration::from_millis(5124)); + + let json_string = ProgressBarJsonFormatter::format(&progress_bar); + + assert!( + json_string.contains(r#""seconds_elapsed": 5.124"#), + "Not expected value in json output: {}", + json_string + ); + } + + #[test] + fn check_seconds_elapsed_in_json_report_with_less_than_100_milliseconds() { + let progress_bar = ProgressBar::new(10).with_elapsed(Duration::from_millis(5004)); + + let json_string = ProgressBarJsonFormatter::format(&progress_bar); + + assert!( + json_string.contains(r#""seconds_elapsed": 5.004"#), + "Not expected value in json output: {}", + json_string + ); + } + + #[test] + fn check_seconds_left_in_json_report_with_more_than_100_milliseconds() { + let half_position = 5; + let progress_bar = ProgressBar::new(half_position * 2); + sleep(Duration::from_millis(123)); + progress_bar.set_position(half_position); + let json_string = ProgressBarJsonFormatter::format(&progress_bar); + + let milliseconds = progress_bar.eta().subsec_millis(); + assert!(milliseconds > 100); + assert!( + json_string.contains(&format!(r#""seconds_left": 0.{}"#, milliseconds)), + "Not expected value in json output: {}", + json_string + ); + } + + #[test] + fn check_seconds_left_in_json_report_with_less_than_100_milliseconds() { + let half_position = 5; + let progress_bar = ProgressBar::new(half_position * 2); + sleep(Duration::from_millis(1)); + progress_bar.set_position(half_position); + let json_string = ProgressBarJsonFormatter::format(&progress_bar); + + assert!( + json_string.contains(r#""seconds_left": 0.0"#), + "Not expected value in json output: {}", + json_string + ); + + assert!( + !json_string.contains(r#""seconds_left": 0.000"#), + "Not expected value in json output: {}", + json_string + ); + } +} From f0eec8ad19b21ce69ae0dde9b342311aa747915f Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Fri, 15 Dec 2023 11:45:02 +0100 Subject: [PATCH 4/7] Add `--log-format-json` argument in commands --- Cargo.lock | 1 + mithril-client-cli/Cargo.toml | 1 + mithril-client-cli/src/main.rs | 23 +++++++++++++++++++---- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 154b3261991..a4fa3d27e69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3303,6 +3303,7 @@ dependencies = [ "serde_json", "slog", "slog-async", + "slog-bunyan", "slog-scope", "slog-term", "thiserror", diff --git a/mithril-client-cli/Cargo.toml b/mithril-client-cli/Cargo.toml index 349fc68e17d..628c4dccf5d 100644 --- a/mithril-client-cli/Cargo.toml +++ b/mithril-client-cli/Cargo.toml @@ -46,6 +46,7 @@ slog = { version = "2.7.0", features = [ "release_max_level_debug", ] } slog-async = "2.8.0" +slog-bunyan = "2.4.0" slog-scope = "4.4.0" slog-term = "2.9.0" thiserror = "1.0.49" diff --git a/mithril-client-cli/src/main.rs b/mithril-client-cli/src/main.rs index d5c4795fb5b..f0b6f6da86b 100644 --- a/mithril-client-cli/src/main.rs +++ b/mithril-client-cli/src/main.rs @@ -43,6 +43,10 @@ pub struct Args { /// Override configuration Aggregator endpoint URL. #[clap(long, env = "AGGREGATOR_ENDPOINT")] aggregator_endpoint: Option, + + /// Enable JSON output for logs displayed according to verbosity level + #[clap(long)] + log_format_json: bool, } impl Args { @@ -69,10 +73,21 @@ impl Args { } fn build_logger(&self) -> Logger { - let decorator = slog_term::TermDecorator::new().build(); - let drain = slog_term::CompactFormat::new(decorator).build().fuse(); - let drain = slog::LevelFilter::new(drain, self.log_level()).fuse(); - let drain = slog_async::Async::new(drain).build().fuse(); + let drain = if self.log_format_json { + let drain = slog_bunyan::new(std::io::stdout()) + .set_pretty(false) + .build() + .fuse(); + let drain = slog::LevelFilter::new(drain, self.log_level()).fuse(); + + slog_async::Async::new(drain).build().fuse() + } else { + let decorator = slog_term::TermDecorator::new().build(); + let drain = slog_term::CompactFormat::new(decorator).build().fuse(); + let drain = slog::LevelFilter::new(drain, self.log_level()).fuse(); + + slog_async::Async::new(drain).build().fuse() + }; Logger::root(Arc::new(drain), slog::o!()) } From 26f24b531c2fd2a43330685d7275c37026f5344e Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Fri, 15 Dec 2023 17:26:40 +0100 Subject: [PATCH 5/7] Add `--log-output` argument in commands --- mithril-client-cli/src/main.rs | 72 ++++++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/mithril-client-cli/src/main.rs b/mithril-client-cli/src/main.rs index f0b6f6da86b..03c6766d778 100644 --- a/mithril-client-cli/src/main.rs +++ b/mithril-client-cli/src/main.rs @@ -2,13 +2,16 @@ mod commands; -use std::path::PathBuf; -use std::sync::Arc; - +use anyhow::Context; use clap::{Parser, Subcommand}; use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind}; -use slog::{Drain, Level, Logger}; +use slog::{Drain, Fuse, Level, Logger}; +use slog_async::Async; use slog_scope::debug; +use slog_term::Decorator; +use std::io::Write; +use std::sync::Arc; +use std::{fs::File, path::PathBuf}; use mithril_client_cli::common::StdResult; @@ -16,6 +19,25 @@ use commands::{ mithril_stake_distribution::MithrilStakeDistributionCommands, snapshot::SnapshotCommands, }; +enum LogOutputType { + Stdout, + File(String), +} + +impl LogOutputType { + fn get_writer(&self) -> StdResult> { + let writer: Box = match self { + LogOutputType::Stdout => Box::new(std::io::stdout()), + LogOutputType::File(filepath) => Box::new( + File::create(filepath) + .with_context(|| format!("Can not create output log file: {}", filepath))?, + ), + }; + + Ok(writer) + } +} + #[derive(Parser, Debug, Clone)] #[clap(name = "mithril-client")] #[clap( @@ -47,6 +69,10 @@ pub struct Args { /// Enable JSON output for logs displayed according to verbosity level #[clap(long)] log_format_json: bool, + + /// Redirect the logs to a file + #[clap(long, alias("o"))] + log_output: Option, } impl Args { @@ -72,24 +98,38 @@ impl Args { } } - fn build_logger(&self) -> Logger { + fn get_log_output_type(&self) -> LogOutputType { + if let Some(output_filepath) = &self.log_output { + LogOutputType::File(output_filepath.to_string()) + } else { + LogOutputType::Stdout + } + } + + fn wrap_drain(&self, decorator: D) -> Fuse { + let drain = slog_term::CompactFormat::new(decorator).build().fuse(); + let drain = slog::LevelFilter::new(drain, self.log_level()).fuse(); + + slog_async::Async::new(drain).build().fuse() + } + + fn build_logger(&self) -> StdResult { + let log_output_type = self.get_log_output_type(); + let writer = log_output_type.get_writer()?; + let drain = if self.log_format_json { - let drain = slog_bunyan::new(std::io::stdout()) - .set_pretty(false) - .build() - .fuse(); + let drain = slog_bunyan::new(writer).set_pretty(false).build().fuse(); let drain = slog::LevelFilter::new(drain, self.log_level()).fuse(); slog_async::Async::new(drain).build().fuse() } else { - let decorator = slog_term::TermDecorator::new().build(); - let drain = slog_term::CompactFormat::new(decorator).build().fuse(); - let drain = slog::LevelFilter::new(drain, self.log_level()).fuse(); - - slog_async::Async::new(drain).build().fuse() + match log_output_type { + LogOutputType::Stdout => self.wrap_drain(slog_term::TermDecorator::new().build()), + LogOutputType::File(_) => self.wrap_drain(slog_term::PlainDecorator::new(writer)), + } }; - Logger::root(Arc::new(drain), slog::o!()) + Ok(Logger::root(Arc::new(drain), slog::o!())) } } @@ -135,7 +175,7 @@ impl ArtifactCommands { async fn main() -> StdResult<()> { // Load args let args = Args::parse(); - let _guard = slog_scope::set_global_logger(args.build_logger()); + let _guard = slog_scope::set_global_logger(args.build_logger()?); #[cfg(feature = "bundle_openssl")] openssl_probe::init_ssl_cert_env_vars(); From 0eb6882acc6f97b2d47d1f06357b99173fe50a88 Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Fri, 15 Dec 2023 14:31:15 +0100 Subject: [PATCH 6/7] Update `mithril-client` documentation - Add new parameter `json_output` at a global level - Move `--json` parameter to commands level --- .../developer-docs/nodes/mithril-client.md | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/docs/website/root/manual/developer-docs/nodes/mithril-client.md b/docs/website/root/manual/developer-docs/nodes/mithril-client.md index 9a99906e911..9646844ef43 100644 --- a/docs/website/root/manual/developer-docs/nodes/mithril-client.md +++ b/docs/website/root/manual/developer-docs/nodes/mithril-client.md @@ -137,6 +137,10 @@ Options: Directory where configuration file is located [default: ./config] --aggregator-endpoint Override configuration Aggregator endpoint URL + --log-format-json + Enable JSON output for logs displayed according to verbosity level + --log-output + Redirect the logs to a file -h, --help Print help -V, --version @@ -296,13 +300,21 @@ Here is a list of the available parameters: | `network` | - | - | `NETWORK` | Cardano network | - | `testnet` or `mainnet` or `devnet` | :heavy_check_mark: | | `aggregator_endpoint` | `--aggregator-endpoint` | - | `AGGREGATOR_ENDPOINT` | Aggregator node endpoint | - | `https://aggregator.pre-release-preview.api.mithril.network/aggregator` | :heavy_check_mark: | | `genesis_verification_key` | - | - | `GENESIS_VERIFICATION_KEY` | Genesis verification key | - | - | :heavy_check_mark: | -| `json_output` | `--json` | `-j` | - | Enable JSON output | no | - | - | +| `log_format_json` | `--log-format-json` | - | - | Enable JSON output for logs | - | - | - | +| `log_output` | `--log-output` | `-o` | - | Redirect the logs to a file | - | `./mithril-client.log` | - | `snapshot show` command: | Parameter | Command line (long) | Command line (short) | Environment variable | Description | Default value | Example | Mandatory | |-----------|---------------------|:---------------------:|----------------------|-------------|---------------|---------|:---------:| | `digest` | `--digest` | - | `DIGEST` | Snapshot digest or `latest` for the latest digest | - | - | :heavy_check_mark: | +| `json` | `--json` | - | - | Enable JSON output for command results | - | - | - | + +`snapshot list` command: + +| Parameter | Command line (long) | Command line (short) | Environment variable | Description | Default value | Example | Mandatory | +|-----------|---------------------|:---------------------:|----------------------|-------------|---------------|---------|:---------:| +| `json` | `--json` | - | - | Enable JSON output for command results | - | - | - | `snapshot download` command: @@ -310,6 +322,13 @@ Here is a list of the available parameters: |-----------|---------------------|:---------------------:|----------------------|-------------|---------------|---------|:---------:| | `digest` | `--digest` | - | `DIGEST` | Snapshot digest or `latest` for the latest digest | - | - | :heavy_check_mark: | | `download_dir` | `--download-dir` | - | - | Directory where the snapshot will be downloaded | . | - | - | +| `json` | `--json` | - | - | Enable JSON output for progress logs | - | - | - | + +`mithril-stake-distribution list` command: + +| Parameter | Command line (long) | Command line (short) | Environment variable | Description | Default value | Example | Mandatory | +|-----------|---------------------|:---------------------:|----------------------|-------------|---------------|---------|:---------:| +| `json` | `--json` | - | - | Enable JSON output for command results | - | - | - | `mithril-stake-distribution download` command: From 9f17b503e113c3c7299123526daa2f71613b9c0a Mon Sep 17 00:00:00 2001 From: Damien LACHAUME / PALO-IT Date: Mon, 18 Dec 2023 12:27:42 +0100 Subject: [PATCH 7/7] Update crate version --- Cargo.lock | 2 +- mithril-client-cli/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a4fa3d27e69..84a6b5d0447 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3280,7 +3280,7 @@ dependencies = [ [[package]] name = "mithril-client-cli" -version = "0.5.11" +version = "0.5.12" dependencies = [ "anyhow", "async-recursion", diff --git a/mithril-client-cli/Cargo.toml b/mithril-client-cli/Cargo.toml index 628c4dccf5d..9af9c9368ab 100644 --- a/mithril-client-cli/Cargo.toml +++ b/mithril-client-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-client-cli" -version = "0.5.11" +version = "0.5.12" description = "A Mithril Client" authors = { workspace = true } edition = { workspace = true }