From fa2fa1b846f8203e0543b1a5e033b38347a4a31f Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 15 Jan 2021 21:22:29 +0100 Subject: [PATCH] Add new logic to limit local log output --- CHANGELOG.md | 13 ++- Cargo.lock | 7 ++ Cargo.toml | 1 + client/cli.rs | 16 +++- client/client.rs | 9 +- client/display/log.rs | 130 +++++++++++++++++++------- client/display/mod.rs | 2 + daemon/network/message_handler/log.rs | 1 + shared/log.rs | 16 ---- shared/network/message.rs | 2 + 10 files changed, 140 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 881c370..b0c4b82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.10.3] - +## [0.11.0] - ### Added -- Add the `--successful-only` flag to the `clean` subcommand. - This let's keep you all important logs of failed tasks, while freeing up some screen space. +- Add the `--lines` flag to the `log` subcommand. + This is used to only show the last X lines of each task's stdout and stderr. +- Add the `--full` flag to the `log` subcommand. + This is used to show the whole logfile of each task's stdout and stderr. + +### Changed + +- If multiple tasks are selected, `log` now only shows the last few lines for each log. + You can use the new `--full` option to get the old behavior. ### Fixed diff --git a/Cargo.lock b/Cargo.lock index e638110..a558df8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1197,6 +1197,7 @@ dependencies = [ "psutil", "rand 0.8.2", "rcgen", + "rev_lines", "rustls", "serde", "serde_derive", @@ -1385,6 +1386,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "rev_lines" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18eb52b6664d331053136fcac7e4883bdc6f5fc04a6aab3b0f75eafb80ab88b3" + [[package]] name = "ring" version = "0.16.19" diff --git a/Cargo.toml b/Cargo.toml index db8a2e2..5322e42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,7 @@ async-std = { version = "1", features = ["attributes", "std"] } async-tls = "0.11" async-trait = "0.1" rustls = "0.19" +rev_lines = "0.2" rcgen = "0.8" byteorder = "^1" snap = "1" diff --git a/client/cli.rs b/client/cli.rs index 0eb9d8e..ef07c5a 100644 --- a/client/cli.rs +++ b/client/cli.rs @@ -261,13 +261,25 @@ pub enum SubCommand { /// Display the log output of finished tasks. /// Prints either all logs or only the logs of specified tasks. + /// + /// When looking at multiple logs, only the last 20 lines will be shown Log { /// View the task output of these specific tasks. task_ids: Vec, - /// Print the current state as json. - /// Includes EVERYTHING. + /// Print the resulting tasks and output as json. + /// Can be very large! #[clap(short, long)] json: bool, + + /// Only print the last X lines of each task's output. + /// This is done by default if you're looking at multiple tasks. + #[clap(short, long, conflicts_with = "full")] + lines: Option, + + /// Show the whole std_out and std_err output. + /// This is the default if only a single task is being looked at. + #[clap(short, long)] + full: bool, }, /// Follow the output of a currently running task. diff --git a/client/client.rs b/client/client.rs index 2a2c988..9d1e92d 100644 --- a/client/client.rs +++ b/client/client.rs @@ -388,10 +388,17 @@ impl Client { Ok(Message::Group(message)) } SubCommand::Status { .. } => Ok(Message::Status), - SubCommand::Log { task_ids, .. } => { + SubCommand::Log { + task_ids, + lines, + full, + .. + } => { let message = LogRequestMessage { task_ids: task_ids.clone(), send_logs: !self.settings.client.read_local_logs, + lines: lines.clone(), + full: *full, }; Ok(Message::Log(message)) } diff --git a/client/display/log.rs b/client/display/log.rs index d2f1efa..dffc966 100644 --- a/client/display/log.rs +++ b/client/display/log.rs @@ -1,5 +1,8 @@ -use std::collections::BTreeMap; -use std::io; +use std::{collections::BTreeMap, io::BufReader}; +use std::{ + fs::File, + io::{self, Stdout}, +}; use anyhow::Result; use comfy_table::*; @@ -21,18 +24,30 @@ pub fn print_logs( cli_command: &SubCommand, settings: &Settings, ) { - let (json, task_ids) = match cli_command { - SubCommand::Log { json, task_ids } => (*json, task_ids.clone()), + // Get actual commandline options. + // This is necessary to know how we should display/return the log information. + let (json, task_ids, lines, full) = match cli_command { + SubCommand::Log { + json, + task_ids, + lines, + full, + } => (*json, task_ids.clone(), lines.clone(), *full), _ => panic!( "Got wrong Subcommand {:?} in print_log. This shouldn't happen", cli_command ), }; + + // Return the server response in json representation. + // TODO: This only works properly if we get the logs from remote. + // TODO: However, this still doesn't work, since the logs are still compressed. if json { println!("{}", serde_json::to_string(&task_logs).unwrap()); return; } + // Check some early return conditions if task_ids.is_empty() && task_logs.is_empty() { println!("There are no finished tasks"); return; @@ -43,9 +58,26 @@ pub fn print_logs( return; } + // Determine, whether we should draw everything or only a part of the log output. + // None implicates that all lines are printed + let lines = if full { + None + } else if let Some(lines) = lines { + Some(lines) + } else { + // By default only some lines are shown per task, if multiple tasks exist. + // For a single task, the whole log output is shown. + if task_logs.len() > 1 { + Some(15) + } else { + None + } + }; + + // Do the actual log printing let mut task_iter = task_logs.iter_mut().peekable(); while let Some((_, mut task_log)) = task_iter.next() { - print_log(&mut task_log, settings); + print_log(&mut task_log, settings, lines); // Add a newline if there is another task that's going to be printed. if let Some((_, task_log)) = task_iter.peek() { @@ -59,8 +91,14 @@ pub fn print_logs( } /// Print the log of a single task. -pub fn print_log(task_log: &mut TaskLogMessage, settings: &Settings) { - let task = &task_log.task; +/// +/// message: The message returned by the daemon. This message includes all +/// requested tasks and the tasks' logs, if we don't read local logs. +/// lines: Whether we should reduce the log output of each task to a specific number of lines. +/// `None` implicates that everything should be printed. +/// This is only important, if we read local lines. +pub fn print_log(message: &mut TaskLogMessage, settings: &Settings, lines: Option) { + let task = &message.task; // We only show logs of finished or running tasks. if !vec![TaskStatus::Done, TaskStatus::Running, TaskStatus::Paused].contains(&task.status) { return; @@ -94,9 +132,9 @@ pub fn print_log(task_log: &mut TaskLogMessage, settings: &Settings) { } if settings.client.read_local_logs { - print_local_log_output(task_log.task.id, settings); - } else if task_log.stdout.is_some() && task_log.stderr.is_some() { - print_task_output_from_daemon(task_log); + print_local_log(message.task.id, settings, lines); + } else if message.stdout.is_some() && message.stderr.is_some() { + print_remote_log(message); } else { println!("Logs requested from pueue daemon, but none received. Please report this bug."); } @@ -104,7 +142,7 @@ pub fn print_log(task_log: &mut TaskLogMessage, settings: &Settings) { /// The daemon didn't send any log output, thereby we didn't request any. /// If that's the case, read the log files from the local pueue directory -pub fn print_local_log_output(task_id: usize, settings: &Settings) { +pub fn print_local_log(task_id: usize, settings: &Settings, lines: Option) { let (mut stdout_log, mut stderr_log) = match get_log_file_handles(task_id, &settings.shared.pueue_directory) { Ok((stdout, stderr)) => (stdout, stderr), @@ -117,29 +155,51 @@ pub fn print_local_log_output(task_id: usize, settings: &Settings) { // without having to load anything into memory. let mut stdout = io::stdout(); - if let Ok(metadata) = stdout_log.metadata() { + print_local_file( + &mut stdout, + &mut stdout_log, + &lines, + style_text("stdout:", Some(Color::Green), Some(Attribute::Bold)), + ); + + print_local_file( + &mut stdout, + &mut stderr_log, + &lines, + style_text("stderr:", Some(Color::Red), Some(Attribute::Bold)), + ); +} + +/// Print a local log file. +/// This is usually either the stdout or the stderr +pub fn print_local_file(stdout: &mut Stdout, file: &mut File, lines: &Option, text: String) { + if let Ok(metadata) = file.metadata() { if metadata.len() != 0 { - println!( - "\n{}", - style_text("stdout:", Some(Color::Green), Some(Attribute::Bold)) - ); + println!("\n{}", text); + // Only print the last lines if requested + if let Some(lines) = lines { + // TODO: This is super imperformant, but works as long as we don't use the last + // 1000 lines. It would be cleaner to seek to the beginning of the requested + // position and simply stream the content to stdout. + let last_lines: Vec = rev_lines::RevLines::new(BufReader::new(file)) + .expect("Failed to read last lines of file") + .take(*lines) + .collect(); - if let Err(err) = io::copy(&mut stdout_log, &mut stdout) { - println!("Failed reading local stdout log file: {}", err); - }; - } - } + println!( + "{}", + last_lines + .into_iter() + .rev() + .collect::>() + .join("\n") + ); + return; + } - if let Ok(metadata) = stderr_log.metadata() { - if metadata.len() != 0 { - // Add a spacer line between stdout and stderr - println!( - "\n{}", - style_text("stderr:", Some(Color::Red), Some(Attribute::Bold)) - ); - - if let Err(err) = io::copy(&mut stderr_log, &mut stdout) { - println!("Failed reading local stderr log file: {}", err); + // Print everything + if let Err(err) = io::copy(file, stdout) { + println!("Failed reading local log file: {}", err); }; } } @@ -148,23 +208,23 @@ pub fn print_local_log_output(task_id: usize, settings: &Settings) { /// Prints log output received from the daemon. /// We can safely call .unwrap() on stdout and stderr in here, since this /// branch is always called after ensuring that both are `Some`. -pub fn print_task_output_from_daemon(task_log: &TaskLogMessage) { +pub fn print_remote_log(task_log: &TaskLogMessage) { // Save whether stdout was printed, so we can add a newline between outputs. if !task_log.stdout.as_ref().unwrap().is_empty() { - if let Err(err) = print_remote_task_output(&task_log, true) { + if let Err(err) = print_remote_task_log(&task_log, true) { println!("Error while parsing stdout: {}", err); } } if !task_log.stderr.as_ref().unwrap().is_empty() { - if let Err(err) = print_remote_task_output(&task_log, false) { + if let Err(err) = print_remote_task_log(&task_log, false) { println!("Error while parsing stderr: {}", err); }; } } /// Print log output of a finished process. -pub fn print_remote_task_output(task_log: &TaskLogMessage, stdout: bool) -> Result<()> { +pub fn print_remote_task_log(task_log: &TaskLogMessage, stdout: bool) -> Result<()> { let (pre_text, color, bytes) = if stdout { ("stdout: ", Color::Green, task_log.stdout.as_ref().unwrap()) } else { diff --git a/client/display/mod.rs b/client/display/mod.rs index fd36c30..d5a4a03 100644 --- a/client/display/mod.rs +++ b/client/display/mod.rs @@ -14,10 +14,12 @@ pub use self::group::print_groups; pub use self::log::print_logs; pub use self::state::print_state; +/// Used to style any generic success message from the daemon. pub fn print_success(message: &str) { println!("{}", message); } +/// Used to style any generic failure message from the daemon. pub fn print_error(message: &str) { let styled = style_text(message, Some(Color::Red), None); println!("{}", styled); diff --git a/daemon/network/message_handler/log.rs b/daemon/network/message_handler/log.rs index 90360e6..1331a95 100644 --- a/daemon/network/message_handler/log.rs +++ b/daemon/network/message_handler/log.rs @@ -26,6 +26,7 @@ pub fn get_log(message: LogRequestMessage, state: &SharedState) -> Message { { Ok((stdout, stderr)) => (Some(stdout), Some(stderr)), Err(err) => { + // Fail early if there's some problem with getting the log output return create_failure_message(format!( "Failed reading process output file: {:?}", err diff --git a/shared/log.rs b/shared/log.rs index 9d70280..5ed82fe 100644 --- a/shared/log.rs +++ b/shared/log.rs @@ -1,6 +1,5 @@ use std::fs::{read_dir, remove_file, File}; use std::io; -use std::io::prelude::*; use std::path::PathBuf; use anyhow::{bail, Result}; @@ -33,21 +32,6 @@ pub fn get_log_file_handles(task_id: usize, path: &PathBuf) -> Result<(File, Fil Ok((stdout, stderr)) } -/// Return the content of temporary stdout and stderr files for a task. -pub fn read_log_files(task_id: usize, path: &PathBuf) -> Result<(String, String)> { - let (mut stdout_handle, mut stderr_handle) = get_log_file_handles(task_id, path)?; - let mut stdout_buffer = Vec::new(); - let mut stderr_buffer = Vec::new(); - - stdout_handle.read_to_end(&mut stdout_buffer)?; - stderr_handle.read_to_end(&mut stderr_buffer)?; - - let stdout = String::from_utf8_lossy(&stdout_buffer); - let stderr = String::from_utf8_lossy(&stderr_buffer); - - Ok((stdout.to_string(), stderr.to_string())) -} - /// Remove temporary stdout and stderr files for a task. pub fn clean_log_handles(task_id: usize, path: &PathBuf) { let (out_path, err_path) = get_log_paths(task_id, path); diff --git a/shared/network/message.rs b/shared/network/message.rs index f1d2a58..eb1f998 100644 --- a/shared/network/message.rs +++ b/shared/network/message.rs @@ -166,6 +166,8 @@ pub struct StreamRequestMessage { pub struct LogRequestMessage { pub task_ids: Vec, pub send_logs: bool, + pub lines: Option, + pub full: bool, } /// Helper struct for sending tasks and their log output to the client.