Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hermes: replace default JSON with human-readable output #796

Merged
merged 19 commits into from
Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
- [ibc-relayer-cli]
- Clarified success path for updating a client that is already up-to-date ([#734])
- Added `create` and `update` wrappers for client raw commands ([#772])
- Output by default is human-readable, and JSON is optional ([#805])

### BUG FIXES

Expand Down Expand Up @@ -102,6 +103,7 @@
[#793]: https://github.com/informalsystems/ibc-rs/pull/793
[#798]: https://github.com/informalsystems/ibc-rs/issues/798
[#801]: https://github.com/informalsystems/ibc-rs/issues/801
[#805]: https://github.com/informalsystems/ibc-rs/issues/805


## v0.1.1
Expand Down
2 changes: 1 addition & 1 deletion e2e/e2e/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def to_cmd(self) -> str:
return f"{self.name} {' '.join(self.args())}"

def run(self, config: Config, retries: int = 0) -> CmdResult[T]:
full_cmd = f'{config.relayer_cmd} -c {config.config_file}'.split(' ')
full_cmd = f'{config.relayer_cmd} -c {config.config_file} --json'.split(' ')
full_cmd.extend(self.name.split(' '))
full_cmd.extend(self.args())
l.debug(' '.join(full_cmd))
Expand Down
1 change: 0 additions & 1 deletion relayer-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ authors = [

description = """
Implementation of `hermes`, an IBC Relayer developed in Rust.
This crate is a CLI wrapper over the `ibc-relayer` library.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

"""

[[bin]]
Expand Down
7 changes: 0 additions & 7 deletions relayer-cli/build.rs

This file was deleted.

51 changes: 24 additions & 27 deletions relayer-cli/src/application.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
//! Cli Abscissa Application

use crate::components::Tracing;
use crate::{commands::CliCmd, config::Config};

use abscissa_core::terminal::component::Terminal;
use abscissa_core::{
application::{self, AppCell},
component::Component,
config, trace, Application, Configurable, EntryPoint, FrameworkError, StandardPaths,
config, Application, Configurable, FrameworkError, StandardPaths,
};

use crate::components::{JsonTracing, PrettyTracing};
use crate::entry::EntryPoint;
use crate::{commands::CliCmd, config::Config};

/// Application state
pub static APPLICATION: AppCell<CliApp> = AppCell::new();

Expand Down Expand Up @@ -40,6 +41,9 @@ pub struct CliApp {

/// Application state.
state: application::State<Self>,

/// Toggle json output on/off. Changed with the global config option `-j` / `--json`.
json_output: bool,
}

/// Initialize a new application instance.
Expand All @@ -51,10 +55,18 @@ impl Default for CliApp {
Self {
config: None,
state: application::State::default(),
json_output: false,
}
}
}

impl CliApp {
/// Whether or not JSON output is enabled
pub fn json_output(&self) -> bool {
self.json_output
}
}

impl Application for CliApp {
/// Entrypoint command for this application.
type Cmd = EntryPoint<CliCmd>;
Expand Down Expand Up @@ -116,32 +128,17 @@ impl Application for CliApp {
.transpose()?
.unwrap_or_default();

// For `start` and `start-multi` commands exclusively we disable JSON; otherwise output is JSON-only
let json_on = if let Some(c) = &command.command {
!matches!(c, CliCmd::Start(_) | CliCmd::StartMulti(_))
} else {
true
};
// Update the `json_output` flag used by `conclude::Output`
self.json_output = command.json;

if json_on {
let tracing = Tracing::new(config.global)?;
if command.json {
// Enable JSON by using the crate-level `Tracing`
let tracing = JsonTracing::new(config.global)?;
Ok(vec![Box::new(terminal), Box::new(tracing)])
} else {
let alt_tracing = abscissa_core::trace::Tracing::new(
abscissa_core::trace::Config::from(config.global.log_level),
abscissa_core::terminal::ColorChoice::Auto,
)
.unwrap();
Ok(vec![Box::new(terminal), Box::new(alt_tracing)])
}
}

/// Get tracing configuration from command-line options
fn tracing_config(&self, command: &EntryPoint<CliCmd>) -> trace::Config {
if command.verbose {
trace::Config::verbose()
} else {
trace::Config::default()
// Use abscissa's tracing, which pretty-prints to the terminal obeying log levels
let tracing = PrettyTracing::new(config.global)?;
Ok(vec![Box::new(terminal), Box::new(tracing)])
}
}
}
2 changes: 2 additions & 0 deletions relayer-cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use std::path::PathBuf;

use abscissa_core::{Command, Configurable, FrameworkError, Help, Options, Runnable};
use tracing::info;

use crate::config::Config;

Expand All @@ -29,6 +30,7 @@ mod version;

/// Default configuration file path
pub fn default_config_file() -> Option<PathBuf> {
info!("Using default configuration from: '.hermes/config.toml'");
dirs_next::home_dir().map(|home| home.join(".hermes/config.toml"))
}

Expand Down
6 changes: 3 additions & 3 deletions relayer-cli/src/commands/keys/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ibc_relayer::keys::add::{add_key, KeysAddOptions};

use crate::application::app_config;
use crate::conclude::Output;
use crate::error::{Error, Kind};
use crate::error::Kind;

#[derive(Clone, Command, Debug, Options)]
pub struct KeysAddCmd {
Expand Down Expand Up @@ -45,10 +45,10 @@ impl Runnable for KeysAddCmd {
Ok(result) => result,
};

let res: Result<String, Error> = add_key(opts).map_err(|e| Kind::Keys.context(e).into());
let res = add_key(opts).map_err(|e| Kind::Keys.context(e));

match res {
Ok(r) => Output::success(r).exit(),
Ok(r) => Output::success_msg(r).exit(),
Err(e) => Output::error(format!("{}", e)).exit(),
}
}
Expand Down
6 changes: 3 additions & 3 deletions relayer-cli/src/commands/keys/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ibc_relayer::keys::list::{list_keys, KeysListOptions};

use crate::application::app_config;
use crate::conclude::Output;
use crate::error::{Error, Kind};
use crate::error::Kind;

#[derive(Clone, Command, Debug, Options)]
pub struct KeysListCmd {
Expand Down Expand Up @@ -35,10 +35,10 @@ impl Runnable for KeysListCmd {
Ok(result) => result,
};

let res: Result<String, Error> = list_keys(opts).map_err(|e| Kind::Keys.context(e).into());
let res = list_keys(opts).map_err(|e| Kind::Keys.context(e));

match res {
Ok(r) => Output::success(r).exit(),
Ok(r) => Output::success_msg(r).exit(),
Err(e) => Output::error(format!("{}", e)).exit(),
}
}
Expand Down
4 changes: 2 additions & 2 deletions relayer-cli/src/commands/start_multi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn start_specified(
let supervisor = Supervisor::spawn(chain_a, chain_b)?;
supervisor.run()?;

Ok(Output::success("ok"))
Ok(Output::success_msg("ok"))
}

fn start_all_connections(config: &Config) -> Result<Output, BoxError> {
Expand Down Expand Up @@ -98,7 +98,7 @@ fn start_all_connections(config: &Config) -> Result<Output, BoxError> {
});

match result {
Ok(Ok(())) => Ok(Output::success("ok")),
Ok(Ok(())) => Ok(Output::success_msg("ok")),
Ok(Err(e)) => Err(e),
Err(e) => std::panic::resume_unwind(e),
}
Expand Down
86 changes: 70 additions & 16 deletions relayer-cli/src/components.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,51 @@
use std::io;

use abscissa_core::{Component, FrameworkError};
use ibc_relayer::config::GlobalConfig;
use tracing_subscriber::fmt::{
format::{Format, Json, JsonFields},
time::SystemTime,
Formatter,
use tracing_subscriber::{
fmt::{
format::{Format, Json, JsonFields},
time::SystemTime,
Formatter as TracingFormatter,
},
reload::Handle,
util::SubscriberInitExt,
EnvFilter, FmtSubscriber,
};
use tracing_subscriber::util::SubscriberInitExt;
use tracing_subscriber::{reload::Handle, EnvFilter, FmtSubscriber};

/// Abscissa component for initializing the `tracing` subsystem
use ibc_relayer::config::GlobalConfig;
use tracing_subscriber::fmt::format::{DefaultFields, Full};

/// Custom types to simplify the `Tracing` definition below
type JsonFormatter = TracingFormatter<JsonFields, Format<Json, SystemTime>, StdWriter>;
type PrettyFormatter = TracingFormatter<DefaultFields, Format<Full, SystemTime>, StdWriter>;
type StdWriter = fn() -> io::Stderr;

/// A custom component for parametrizing `tracing` in the relayer.
/// Primarily used for:
///
/// - Customizing the log output level, for filtering the output produced via tracing macros
/// (`debug!`, `info!`, etc.) or abscissa macros (`status_err`, `status_info`, etc.).
/// - Enabling JSON-formatted output without coloring
#[derive(Component, Debug)]
pub struct Tracing {
filter_handle: Handle<EnvFilter, Formatter<JsonFields, Format<Json, SystemTime>>>,
pub struct JsonTracing {
filter_handle: Handle<EnvFilter, JsonFormatter>,
}

/// A custom component for parametrizing `tracing` in the relayer. Primarily used for:
/// - customizes the log output level, for filtering the output produced via tracing macros
/// (`debug!`, `info!`, etc.) or abscissa macros (`status_err`, `status_info`, etc.).
/// - enables JSON-formatted output
impl Tracing {
impl JsonTracing {
/// Creates a new [`Tracing`] component
#[allow(trivial_casts)]
pub fn new(cfg: GlobalConfig) -> Result<Self, FrameworkError> {
let filter = cfg.log_level;
let filter = build_tracing_filter(cfg.log_level);
let use_color = false;

// Construct a tracing subscriber with the supplied filter and enable reloading.
let builder = FmtSubscriber::builder()
.with_env_filter(filter)
.with_writer(std::io::stderr as StdWriter)
.with_ansi(use_color)
.json()
.with_filter_reloading();

let filter_handle = builder.reload_handle();

let subscriber = builder.finish();
Expand All @@ -38,3 +54,41 @@ impl Tracing {
Ok(Self { filter_handle })
}
}

#[derive(Component, Debug)]
pub struct PrettyTracing {
filter_handle: Handle<EnvFilter, PrettyFormatter>,
}

impl PrettyTracing {
/// Creates a new [`Tracing`] component
#[allow(trivial_casts)]
pub fn new(cfg: GlobalConfig) -> Result<Self, FrameworkError> {
let filter = build_tracing_filter(cfg.log_level);
let use_color = false;

// Construct a tracing subscriber with the supplied filter and enable reloading.
let builder = FmtSubscriber::builder()
.with_env_filter(filter)
.with_writer(std::io::stderr as StdWriter)
.with_ansi(use_color)
.with_filter_reloading();

let filter_handle = builder.reload_handle();

let subscriber = builder.finish();
subscriber.init();

Ok(Self { filter_handle })
}
}

fn build_tracing_filter(log_level: String) -> String {
let target_crates = ["ibc_relayer", "ibc_relayer_cli"];

target_crates
.iter()
.map(|&c| format!("{}={}", c, log_level))
.reduce(|a, b| format!("{},{}", a, b))
.unwrap()
}
Loading