From e5bacef2ded89f76ddcfe7ab22305c7ea3689575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Fri, 1 Nov 2024 02:09:35 -0700 Subject: [PATCH] Convert log commands to record InvocationRecord Summary: Log commands don't have any InvocationRecord generated. I would like to track details about who/how invokes log commands. Let's ensure they are written to buck2_builds table with no event log option (so scrapped from scribe?). Reviewed By: iguridi Differential Revision: D64044944 fbshipit-source-id: d4ec4a9fe3cc12224f8344dea218abe96a49223c --- app/buck2_client/src/commands/clean.rs | 1 + app/buck2_client/src/commands/kill.rs | 1 + app/buck2_client/src/commands/killall.rs | 1 + .../src/commands/log/critical_path.rs | 7 +++--- .../commands/log/diff/action_divergence.rs | 7 +++--- app/buck2_client/src/commands/log/path_log.rs | 7 +++--- app/buck2_client/src/commands/log/replay.rs | 9 +++---- app/buck2_client/src/commands/log/show_log.rs | 6 ++--- .../src/commands/log/show_user_log.rs | 6 ++--- app/buck2_client/src/commands/log/summary.rs | 7 +++--- app/buck2_client/src/commands/log/what_cmd.rs | 6 ++--- .../src/commands/log/what_materialized.rs | 5 ++-- app/buck2_client/src/commands/log/what_ran.rs | 8 +++---- app/buck2_client/src/commands/log/what_up.rs | 2 +- .../src/commands/log/what_uploaded.rs | 5 ++-- app/buck2_client_ctx/src/client_ctx.rs | 24 +++++++++++++++++-- 16 files changed, 59 insertions(+), 43 deletions(-) diff --git a/app/buck2_client/src/commands/clean.rs b/app/buck2_client/src/commands/clean.rs index 5a8ca78196ef..77d9b39a8e52 100644 --- a/app/buck2_client/src/commands/clean.rs +++ b/app/buck2_client/src/commands/clean.rs @@ -114,6 +114,7 @@ impl CleanCommand { clean(buck_out_dir, daemon_dir, console, Some(&lifecycle_lock)).await }, ) + .into() } pub fn sanitize_argv(&self, argv: Argv) -> SanitizedArgv { diff --git a/app/buck2_client/src/commands/kill.rs b/app/buck2_client/src/commands/kill.rs index cd19d1208e5b..18d53a50f29b 100644 --- a/app/buck2_client/src/commands/kill.rs +++ b/app/buck2_client/src/commands/kill.rs @@ -49,6 +49,7 @@ impl KillCommand { ) .await }) + .into() } pub fn sanitize_argv(&self, argv: Argv) -> SanitizedArgv { diff --git a/app/buck2_client/src/commands/killall.rs b/app/buck2_client/src/commands/killall.rs index ad905a99b654..f696ac8e7dfb 100644 --- a/app/buck2_client/src/commands/killall.rs +++ b/app/buck2_client/src/commands/killall.rs @@ -30,6 +30,7 @@ impl KillallCommand { .then_some(()) .ok_or(anyhow::anyhow!("Killall command failed")) }) + .into() } pub fn sanitize_argv(&self, argv: Argv) -> SanitizedArgv { diff --git a/app/buck2_client/src/commands/log/critical_path.rs b/app/buck2_client/src/commands/log/critical_path.rs index 9ddcda3de840..fe1f06b77ef5 100644 --- a/app/buck2_client/src/commands/log/critical_path.rs +++ b/app/buck2_client/src/commands/log/critical_path.rs @@ -51,7 +51,7 @@ impl CriticalPathCommand { pub fn exec(self, _matches: &clap::ArgMatches, ctx: ClientCommandContext<'_>) -> ExitResult { let Self { event_log, format } = self; - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-critical-path", |ctx| async move { let log_path = event_log.get(&ctx).await?; let (invocation, mut events) = log_path.unpack_stream().await?; @@ -80,9 +80,8 @@ impl CriticalPathCommand { } anyhow::Ok(()) - })?; - - ExitResult::success() + }) + .into() } } diff --git a/app/buck2_client/src/commands/log/diff/action_divergence.rs b/app/buck2_client/src/commands/log/diff/action_divergence.rs index d74b50ea6dd6..d91a2932b7ca 100644 --- a/app/buck2_client/src/commands/log/diff/action_divergence.rs +++ b/app/buck2_client/src/commands/log/diff/action_divergence.rs @@ -147,7 +147,7 @@ fn print_divergence_msg( impl ActionDivergenceCommand { pub fn exec(self, _matches: &clap::ArgMatches, ctx: ClientCommandContext<'_>) -> ExitResult { - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-diff-action-divergence", |ctx| async move { let options1 = EventLogOptions { recent: self.recent1, path: self.path1, @@ -197,8 +197,7 @@ impl ActionDivergenceCommand { buck2_client_ctx::println!("No divergent actions found.")?; } anyhow::Ok(()) - })?; - - ExitResult::success() + }) + .into() } } diff --git a/app/buck2_client/src/commands/log/path_log.rs b/app/buck2_client/src/commands/log/path_log.rs index 983c0af64e1d..2e6cc94cdbef 100644 --- a/app/buck2_client/src/commands/log/path_log.rs +++ b/app/buck2_client/src/commands/log/path_log.rs @@ -33,7 +33,7 @@ impl PathLogCommand { all, } = self; - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-path", |ctx| async move { let paths = if all { retrieve_all_logs(ctx.paths().context("Error identifying log dir")?)? } else { @@ -43,8 +43,7 @@ impl PathLogCommand { buck2_client_ctx::println!("{}", path.path().display())?; } anyhow::Ok(()) - })?; - - ExitResult::success() + }) + .into() } } diff --git a/app/buck2_client/src/commands/log/replay.rs b/app/buck2_client/src/commands/log/replay.rs index 2b41445da16a..635137ac3f88 100644 --- a/app/buck2_client/src/commands/log/replay.rs +++ b/app/buck2_client/src/commands/log/replay.rs @@ -11,12 +11,12 @@ use buck2_client_ctx::client_ctx::ClientCommandContext; use buck2_client_ctx::common::ui::CommonConsoleOptions; use buck2_client_ctx::daemon::client::NoPartialResultHandler; use buck2_client_ctx::events_ctx::EventsCtx; -use buck2_client_ctx::exit_result::ExitCode; use buck2_client_ctx::exit_result::ExitResult; use buck2_client_ctx::replayer::Replayer; use buck2_client_ctx::signal_handler::with_simple_sigint_handler; use buck2_client_ctx::subscribers::get::get_console_with_root; use buck2_client_ctx::subscribers::subscribers::EventSubscribers; +use buck2_error::buck2_error_anyhow; use crate::commands::log::options::EventLogOptions; @@ -57,7 +57,7 @@ impl ReplayCommand { override_args: _, } = self; - ctx.with_runtime(|mut ctx| async move { + ctx.instant_command_no_log("log-replay", |mut ctx| async move { let work = async { let (replayer, invocation) = Replayer::new(event_log.get(&ctx).await?, speed, preload).await?; @@ -100,13 +100,14 @@ impl ReplayCommand { } // FIXME(JakobDegen)(easy): This should probably return failures if there were errors - ExitResult::success() + Ok(()) }; with_simple_sigint_handler(work) .await - .unwrap_or_else(|| ExitResult::status(ExitCode::SignalInterrupt)) + .unwrap_or_else(|| Err(buck2_error_anyhow!([], "Signal Interrupted"))) }) + .into() } } diff --git a/app/buck2_client/src/commands/log/show_log.rs b/app/buck2_client/src/commands/log/show_log.rs index bb76bbc62686..43ba0f935a03 100644 --- a/app/buck2_client/src/commands/log/show_log.rs +++ b/app/buck2_client/src/commands/log/show_log.rs @@ -25,7 +25,7 @@ impl ShowLogCommand { pub fn exec(self, _matches: &clap::ArgMatches, ctx: ClientCommandContext<'_>) -> ExitResult { let Self { event_log } = self; - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-show", |ctx| async move { let log_path = event_log.get(&ctx).await?; let (invocation, mut events) = log_path.unpack_stream().await?; @@ -44,7 +44,7 @@ impl ShowLogCommand { } anyhow::Ok(()) - })?; - ExitResult::success() + }) + .into() } } diff --git a/app/buck2_client/src/commands/log/show_user_log.rs b/app/buck2_client/src/commands/log/show_user_log.rs index 8e0b4342b554..2e6c92f9ac3d 100644 --- a/app/buck2_client/src/commands/log/show_user_log.rs +++ b/app/buck2_client/src/commands/log/show_user_log.rs @@ -26,7 +26,7 @@ impl ShowUserLogCommand { pub fn exec(self, _matches: &clap::ArgMatches, ctx: ClientCommandContext<'_>) -> ExitResult { let Self { event_log } = self; - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-show-user", |ctx| async move { let log_path = event_log.get(&ctx).await?; let (invocation, mut events) = log_path.unpack_stream().await?; @@ -48,7 +48,7 @@ impl ShowUserLogCommand { } anyhow::Ok(()) - })?; - ExitResult::success() + }) + .into() } } diff --git a/app/buck2_client/src/commands/log/summary.rs b/app/buck2_client/src/commands/log/summary.rs index 9c0289bd6e66..e29e1bc38c1d 100644 --- a/app/buck2_client/src/commands/log/summary.rs +++ b/app/buck2_client/src/commands/log/summary.rs @@ -243,7 +243,7 @@ pub struct SummaryCommand { impl SummaryCommand { pub fn exec(self, _matches: &clap::ArgMatches, ctx: ClientCommandContext<'_>) -> ExitResult { - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-summary", |ctx| async move { let log_path = self.event_log.get(&ctx).await?; let (invocation, mut events) = log_path.unpack_stream().await?; @@ -276,8 +276,7 @@ impl SummaryCommand { } buck2_client_ctx::eprintln!("{}", stats)?; anyhow::Ok(()) - })?; - - ExitResult::success() + }) + .into() } } diff --git a/app/buck2_client/src/commands/log/what_cmd.rs b/app/buck2_client/src/commands/log/what_cmd.rs index ccb8e2dc412c..4d98a32a8bcb 100644 --- a/app/buck2_client/src/commands/log/what_cmd.rs +++ b/app/buck2_client/src/commands/log/what_cmd.rs @@ -30,7 +30,7 @@ impl WhatCmdCommand { pub(crate) fn exec(self, _matches: &clap::ArgMatches, ctx: ClientCommandContext) -> ExitResult { let WhatCmdCommand { event_log, expand } = self; - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-what-cmd", |ctx| async move { let log_path = event_log.get(&ctx).await?; let (invocation, _events) = log_path.unpack_stream().await?; @@ -40,8 +40,8 @@ impl WhatCmdCommand { } else { buck2_client_ctx::println!("{}", invocation.display_command_line())?; } - - ExitResult::success() + Ok(()) }) + .into() } } diff --git a/app/buck2_client/src/commands/log/what_materialized.rs b/app/buck2_client/src/commands/log/what_materialized.rs index 016da9291716..4d844df09319 100644 --- a/app/buck2_client/src/commands/log/what_materialized.rs +++ b/app/buck2_client/src/commands/log/what_materialized.rs @@ -176,7 +176,7 @@ impl WhatMaterializedCommand { } = self; buck2_client_ctx::stdio::print_with_writer::(|w| { let mut output = transform_format(output, w); - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-what-materialized", |ctx| async move { let log_path = event_log.get(&ctx).await?; let (invocation, mut events) = log_path.unpack_stream().await?; @@ -223,8 +223,7 @@ impl WhatMaterializedCommand { } anyhow::Ok(()) - })?; - anyhow::Ok(()) + }) })?; ExitResult::success() } diff --git a/app/buck2_client/src/commands/log/what_ran.rs b/app/buck2_client/src/commands/log/what_ran.rs index 892f9cd2bfda..abe205170c32 100644 --- a/app/buck2_client/src/commands/log/what_ran.rs +++ b/app/buck2_client/src/commands/log/what_ran.rs @@ -132,7 +132,7 @@ impl WhatRanCommand { include_std_err: show_std_err, omit_empty_std_err, }; - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-what-ran", |ctx| async move { let log_path = event_log.get(&ctx).await?; let (invocation, events) = log_path.unpack_stream().await?; @@ -153,10 +153,8 @@ impl WhatRanCommand { incomplete, }; WhatRanCommandState::execute(events, &mut output, &options).await?; - - anyhow::Ok(()) - })?; - anyhow::Ok(()) + Ok(()) + }) })?; ExitResult::success() } diff --git a/app/buck2_client/src/commands/log/what_up.rs b/app/buck2_client/src/commands/log/what_up.rs index f1541469f0b9..51f855965bc9 100644 --- a/app/buck2_client/src/commands/log/what_up.rs +++ b/app/buck2_client/src/commands/log/what_up.rs @@ -52,7 +52,7 @@ impl WhatUpCommand { let Self { event_log, after } = self; let cutoff_time = after.map(Duration::from_millis); - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-what-up", |ctx| async move { let log_path = event_log.get(&ctx).await?; // Get events diff --git a/app/buck2_client/src/commands/log/what_uploaded.rs b/app/buck2_client/src/commands/log/what_uploaded.rs index 3470fd7c6aef..e8af856c3b69 100644 --- a/app/buck2_client/src/commands/log/what_uploaded.rs +++ b/app/buck2_client/src/commands/log/what_uploaded.rs @@ -161,7 +161,7 @@ impl WhatUploadedCommand { buck2_client_ctx::stdio::print_with_writer::(|w| { let mut output = transform_format(output, w); - ctx.with_runtime(|ctx| async move { + ctx.instant_command_no_log("log-what-uploaded", |ctx| async move { let log_path = event_log.get(&ctx).await?; let (invocation, mut events) = log_path.unpack_stream().await?; @@ -232,8 +232,7 @@ impl WhatUploadedCommand { } anyhow::Ok(()) - })?; - anyhow::Ok(()) + }) })?; ExitResult::success() } diff --git a/app/buck2_client_ctx/src/client_ctx.rs b/app/buck2_client_ctx/src/client_ctx.rs index 3a62687f0b89..3e155e3dda11 100644 --- a/app/buck2_client_ctx/src/client_ctx.rs +++ b/app/buck2_client_ctx/src/client_ctx.rs @@ -34,7 +34,6 @@ use crate::common::PreemptibleWhen; use crate::daemon::client::connect::BuckdConnectOptions; use crate::daemon::client::BuckdClientConnector; use crate::daemon_constraints::get_possibly_nested_invocation_daemon_uuid; -use crate::exit_result::ExitResult; use crate::immediate_config::ImmediateConfigContext; use crate::restarter::Restarter; use crate::stdin::Stdin; @@ -137,7 +136,7 @@ impl<'a> ClientCommandContext<'a> { command_name: &'static str, event_log_opts: &CommonEventLogOptions, func: F, - ) -> ExitResult + ) -> anyhow::Result<()> where Fut: Future> + 'a, F: FnOnce(ClientCommandContext<'a>) -> Fut, @@ -158,6 +157,27 @@ impl<'a> ClientCommandContext<'a> { result.into() } + /// Invoke a command without writing event log. + /// (For example, we don't write logs in `buck2 log` command.) + pub fn instant_command_no_log( + self, + command_name: &'static str, + func: F, + ) -> anyhow::Result<()> + where + Fut: Future> + 'a, + F: FnOnce(ClientCommandContext<'a>) -> Fut, + { + self.instant_command( + command_name, + &CommonEventLogOptions { + no_event_log: true, + ..CommonEventLogOptions::default() + }, + func, + ) + } + pub fn stdin(&mut self) -> &mut Stdin { self.stdin }