Skip to content

Commit

Permalink
refactor(rust): add log format for user-facing terminal logs
Browse files Browse the repository at this point in the history
  • Loading branch information
adrianbenavides committed Feb 23, 2025
1 parent c460690 commit 8f3f6d6
Show file tree
Hide file tree
Showing 17 changed files with 239 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::path::PathBuf;
use tracing_core::Level;
use tracing_subscriber::EnvFilter;

use super::{Colored, LoggingEnabled};
use super::{Colored, LoggingEnabled, OckamUserLogFormat};
use crate::logs::LogFormat;

/// List of all the configuration parameters relevant for configuring the logs
Expand All @@ -26,9 +26,9 @@ pub struct LoggingConfiguration {
/// This parameter specifies if the log output is colored (typically in terminals supporting it)
colored: Colored,
/// Director where log files must be created.
/// If no directory is defined then log messages appear on the console
/// If no directory is defined, then log messages appear on the console
log_dir: Option<PathBuf>,
/// List of create for which we want to keep log messages
/// List of crates for which we want to keep log messages
crates: Option<Vec<String>>,
}

Expand Down Expand Up @@ -207,18 +207,20 @@ pub fn logging_configuration(
level_and_crates: LogLevelWithCratesFilter,
log_dir: Option<PathBuf>,
colored: Colored,
default_log_format: LogFormat,
enabled: LoggingEnabled,
) -> ockam_core::Result<LoggingConfiguration> {
let enabled = if level_and_crates.explicit_verbose_flag {
LoggingEnabled::On
} else {
logging_enabled()?
enabled
};
Ok(LoggingConfiguration::new(
enabled,
level_and_crates.level,
log_max_size_bytes()?,
log_max_files()?,
log_format()?,
get_env_with_default(OCKAM_LOG_FORMAT, default_log_format)?,
colored,
log_dir,
level_and_crates.crates_filter.clone(),
Expand Down Expand Up @@ -353,6 +355,7 @@ impl CratesFilter {
CratesFilter::Basic => Some(vec![
"ockam_api::ui::terminal".to_string(),
"ockam_command".to_string(),
OckamUserLogFormat::TARGET.to_string(),
]),
CratesFilter::Core => Some(vec![
"ockam".to_string(),
Expand All @@ -363,6 +366,7 @@ impl CratesFilter {
"ockam_transport_tcp".to_string(),
"ockam_api".to_string(),
"ockam_command".to_string(),
OckamUserLogFormat::TARGET.to_string(),
]),
CratesFilter::Selected(list) => Some(list.clone()),
}
Expand Down
37 changes: 37 additions & 0 deletions implementations/rust/ockam/ockam_api/src/logs/logging_options.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::fmt_log;
use nu_ansi_term::{Color, Style};
use ockam_core::env::FromString;
use std::fmt::{Debug, Display, Formatter};
Expand Down Expand Up @@ -43,13 +44,15 @@ pub enum Colored {
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum LogFormat {
Default,
User,
Pretty,
Json,
}

impl FromString for LogFormat {
fn from_string(s: &str) -> ockam_core::Result<Self> {
match s {
"user" => Ok(LogFormat::User),
"pretty" => Ok(LogFormat::Pretty),
"json" => Ok(LogFormat::Json),
_ => Ok(LogFormat::Default),
Expand All @@ -61,6 +64,7 @@ impl Display for LogFormat {
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
match self {
LogFormat::Default => write!(f, "default"),
LogFormat::User => write!(f, "user"),
LogFormat::Pretty => write!(f, "pretty"),
LogFormat::Json => write!(f, "json"),
}
Expand Down Expand Up @@ -174,6 +178,39 @@ where
}
}

#[derive(Default)]
pub struct OckamUserLogFormat {}

impl OckamUserLogFormat {
pub const TARGET: &'static str = "ockam_command::user";

pub fn new() -> Self {
Self {}
}
}

impl<S, N> FormatEvent<S, N> for OckamUserLogFormat
where
S: Subscriber + for<'a> LookupSpan<'a>,
N: for<'a> FormatFields<'a> + 'static,
{
fn format_event(
&self,
ctx: &FmtContext<'_, S, N>,
mut writer: Writer<'_>,
event: &Event<'_>,
) -> std::fmt::Result {
// Padding
writer.write_str(&fmt_log!(""))?;

// Event
ctx.format_fields(writer.by_ref(), event)?;
writer.write_char(' ')?;

writeln!(writer)
}
}

struct FmtLevel<'a> {
level: &'a Level,
ansi: bool,
Expand Down
8 changes: 7 additions & 1 deletion implementations/rust/ockam/ockam_api/src/logs/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::logs::secure_client_service::SecureClientService;
use crate::logs::tracing_guard::TracingGuard;
use crate::logs::{
ExportingConfiguration, LoggingConfiguration, OckamLogExporter, OckamLogFormat,
TelemetryEndpoint,
OckamUserLogFormat, TelemetryEndpoint,
};
use crate::logs::{LogFormat, OckamSpanExporter};
use gethostname::gethostname;
Expand Down Expand Up @@ -131,6 +131,9 @@ impl LoggingTracing {
LogFormat::Default => layers
.with(appender.event_format(OckamLogFormat::new()))
.try_init(),
LogFormat::User => layers
.with(appender.event_format(OckamUserLogFormat::new()))
.try_init(),
};
result.expect("Failed to initialize tracing subscriber");

Expand All @@ -152,6 +155,9 @@ impl LoggingTracing {
LogFormat::Default => layers
.with(appender.event_format(OckamLogFormat::new()))
.try_init(),
LogFormat::User => layers
.with(appender.event_format(OckamUserLogFormat::new()))
.try_init(),
};
result.expect("Failed to initialize tracing subscriber");
};
Expand Down
10 changes: 8 additions & 2 deletions implementations/rust/ockam/ockam_api/src/nodes/service/relay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ impl NodeManager {
node_manager: Arc::downgrade(self),
context: ctx.try_clone()?,
addr: address.clone(),
alias: alias.clone(),
relay_address: relay_address.clone(),
connection: None,
relay_worker_address: None,
Expand Down Expand Up @@ -288,6 +289,7 @@ impl InMemoryNode {
struct RelaySessionReplacer {
node_manager: Weak<NodeManager>,
context: Context,
alias: String,
relay_address: Option<String>,

// current status
Expand Down Expand Up @@ -379,7 +381,9 @@ impl SessionReplacer for RelaySessionReplacer {
if let Some(node_manager) = self.node_manager.upgrade() {
node_manager.cli_state.notify_message(
fmt_warn!(
"The Node lost the connection to the Relay at {}\n",
"The Node {} lost the connection to the Relay {} listening at {}\n",
color_primary(&node_manager.node_name),
color_primary(&self.alias),
color_primary(&self.addr)
) + &fmt_info!("Attempting to reconnect...\n"),
);
Expand All @@ -389,7 +393,9 @@ impl SessionReplacer for RelaySessionReplacer {
async fn on_session_replaced(&self) {
if let Some(node_manager) = self.node_manager.upgrade() {
node_manager.cli_state.notify_message(fmt_ok!(
"The Node has restored the connection to the Relay at {}\n",
"The Node {} has restored the connection to the Relay {} listening at {}\n",
color_primary(&node_manager.node_name),
color_primary(&self.alias),
color_primary(&self.addr)
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@ impl SessionReplacer for InletSessionReplacer {
if let Some(node_manager) = self.node_manager.upgrade() {
node_manager.cli_state.notify_message(
fmt_warn!(
"The TCP Inlet at {} lost the connection to the TCP Outlet at {}\n",
"The TCP Inlet {} listening at {} lost the connection to the TCP Outlet at {}\n",
color_primary(&self.resource.resource_name),
color_primary(&self.listen_addr),
color_primary(&self.outlet_addr)
) + &fmt_info!("Attempting to reconnect...\n"),
Expand All @@ -313,7 +314,8 @@ impl SessionReplacer for InletSessionReplacer {
async fn on_session_replaced(&self) {
if let Some(node_manager) = self.node_manager.upgrade() {
node_manager.cli_state.notify_message(fmt_ok!(
"The TCP Inlet at {} has restored the connection to the TCP Outlet at {}\n",
"The TCP Inlet {} listening at {} has restored the connection to the TCP Outlet at {}\n",
color_primary(&self.resource.resource_name),
color_primary(&self.listen_addr),
color_primary(&self.outlet_addr)
));
Expand Down
45 changes: 18 additions & 27 deletions implementations/rust/ockam/ockam_api/src/ui/terminal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ use tracing::warn;
pub struct Terminal<T: TerminalWriter + Debug, WriteMode = ToStdErr> {
stdout: T,
stderr: T,
logging_enabled: bool,
logging_goes_to_file: bool,
logging_options: LoggingOptions,
quiet: bool,
no_input: bool,
output_format: OutputFormat,
Expand All @@ -47,7 +46,7 @@ impl<T: TerminalWriter + Debug, W> Terminal<T, W> {
}

fn log_msg(&self, msg: &str) {
if !self.logging_enabled {
if !self.logging_options.enabled {
return;
}
for line in msg.lines() {
Expand All @@ -72,6 +71,13 @@ impl<T: TerminalWriter + Debug, W> Terminal<T, W> {
}
}

#[derive(Clone, Debug)]
pub struct LoggingOptions {
pub enabled: bool,
pub logging_to_file: bool,
pub with_user_format: bool,
}

/// A small wrapper around the `Write` trait, enriched with CLI
/// attributes to facilitate output handling.
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -109,8 +115,7 @@ pub trait TerminalWriter: Clone {
impl<W: TerminalWriter + Debug> Terminal<W> {
#[allow(clippy::too_many_arguments)]
pub fn new(
logging_enabled: bool,
logging_goes_to_file: bool,
logging_options: LoggingOptions,
quiet: bool,
no_color: bool,
no_input: bool,
Expand All @@ -125,8 +130,7 @@ impl<W: TerminalWriter + Debug> Terminal<W> {
Self {
stdout,
stderr,
logging_enabled,
logging_goes_to_file,
logging_options,
quiet,
no_input,
output_format,
Expand All @@ -136,18 +140,6 @@ impl<W: TerminalWriter + Debug> Terminal<W> {
}
}

pub fn quiet(logging_enabled: bool, logging_goes_to_file: bool) -> Self {
Self::new(
logging_enabled,
logging_goes_to_file,
true,
false,
false,
OutputFormat::Plain,
OutputBranding::default(),
)
}

/// Prompt the user for a confirmation.
pub fn confirm(&self, msg: impl AsRef<str>) -> Result<ConfirmResult> {
if !self.can_ask_for_user_input() {
Expand Down Expand Up @@ -245,16 +237,16 @@ impl<W: TerminalWriter + Debug> Terminal<W, ToStdErr> {
}

/// Return true if log messages are emitted to the console
fn logging_to_console_only(&self) -> bool {
self.logging_enabled && !self.logging_goes_to_file
fn logging_to_console(&self) -> bool {
self.logging_options.enabled && !self.logging_options.logging_to_file
}

/// Return true if we can write to stderr
/// We can write to stderr unless:
/// - all the messages are logged to the console
/// - or quiet is true
fn can_write_to_stderr(&self) -> bool {
!self.logging_to_console_only() && !self.is_quiet()
self.logging_options.with_user_format || (!self.logging_to_console() && !self.is_quiet())
}

pub fn write(&self, msg: impl AsRef<str>) -> Result<()> {
Expand Down Expand Up @@ -312,8 +304,7 @@ impl<W: TerminalWriter + Debug> Terminal<W, ToStdErr> {
Terminal {
stdout: self.stdout,
stderr: self.stderr,
logging_enabled: self.logging_enabled,
logging_goes_to_file: self.logging_goes_to_file,
logging_options: self.logging_options,
quiet: self.quiet,
no_input: self.no_input,
output_format: self.output_format,
Expand Down Expand Up @@ -354,14 +345,14 @@ impl<W: TerminalWriter + Debug> Terminal<W, ToStdOut> {
}

/// Return true if log messages are emitted to the console
fn logging_to_console_only(&self) -> bool {
self.logging_enabled && !self.logging_goes_to_file
fn logging_to_console(&self) -> bool {
self.logging_options.enabled && !self.logging_options.logging_to_file
}

/// Return true if we can write to stdout
/// We can write to stdout unless all the messages are logged to the console
fn can_write_to_stdout(&self) -> bool {
!self.logging_to_console_only()
self.logging_options.with_user_format || !self.logging_to_console()
}

pub fn write_line(mut self) -> Result<()> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl<T: TerminalWriter + Debug + Send + 'static> NotificationHandler<T> {
let stop = Arc::new(AtomicBool::new(false));
let _self = NotificationHandler {
rx: cli_state.subscribe_to_notifications(),
terminal: terminal.clone(),
terminal,
progress_bar: None,
stop: stop.clone(),
};
Expand Down
9 changes: 6 additions & 3 deletions implementations/rust/ockam/ockam_api/src/ui/terminal/term.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,16 @@ mod tests {
use dialoguer::console::Term;

use crate::output::{OutputBranding, OutputFormat};
use crate::terminal::{Terminal, TerminalStream};
use crate::terminal::{LoggingOptions, Terminal, TerminalStream};

#[test]
fn test_write() {
let sut: Terminal<TerminalStream<Term>> = Terminal::new(
false,
false,
LoggingOptions {
enabled: false,
logging_to_file: false,
with_user_format: false,
},
false,
false,
false,
Expand Down
13 changes: 10 additions & 3 deletions implementations/rust/ockam/ockam_app_lib/src/log.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::state::{AppState, NODE_NAME};
use ockam_api::logs::{
logging_configuration, Colored, ExportingConfiguration, LogLevelWithCratesFilter,
LoggingTracing,
logging_configuration, logging_enabled, Colored, ExportingConfiguration, LogFormat,
LogLevelWithCratesFilter, LoggingTracing,
};
use ockam_core::TryClone;
use ockam_node::Context;
Expand Down Expand Up @@ -30,7 +30,14 @@ impl AppState {
.unwrap()
.add_crates(vec!["ockam_app_lib"]);
let tracing_guard = LoggingTracing::setup(
&logging_configuration(level_and_crates, Some(node_dir), Colored::Off).unwrap(),
&logging_configuration(
level_and_crates,
Some(node_dir),
Colored::Off,
LogFormat::Default,
logging_enabled().unwrap(),
)
.unwrap(),
&ExportingConfiguration::foreground(&state, ctx)
.await
.unwrap(),
Expand Down
Loading

0 comments on commit 8f3f6d6

Please sign in to comment.