From ba03f55629cc815dead17ff199d7e82c2783c97c Mon Sep 17 00:00:00 2001 From: Austin Seipp Date: Sun, 14 Jul 2024 17:35:46 -0500 Subject: [PATCH] buck2_client: download traces via `buck2.log_url` config key When doing a command like `buck2 log replay --trace-id ...` it helps to be able to download previously created logs from users, automated CI, etc. This functionality exists within Meta, as logs are available for download through the "Manifest" system, but it isn't usable in OSS, despite being useful for diagnostics and getting help. The only missing thing to really get things working is a download mechanism for log files, and a way to know where they should come from. With this patch, if a user configures the `buck2.log_url` key, which is expected to look something like: [buck2] log_url = https://example.com Then, upon executing a `log` command with a `--trace-id` flag, this server will be queried with a: GET /v1/get/{uuid} request, which is expected to return the raw zst-encoded protobuf file. So, buck2 will do that and download the trace, and then use it like normal. The request is done with `curl`, though in principle it could be done within Buck itself. This shares as much code as possible with the existing infrastructure and tries to only insert a small key set of `#[cfg(fbcode_build)]` directives. Notably, the path Meta uses for their "Manifold" client is still fully available in the OSS version; `fbcode_build` is only used to gate what the default choice is. How the server gets access to these files and how they are uploaded is another question. But for now, this can be done several ways outside of buck2 core, so this is good enough. GitHub Issue: https://github.com/facebook/buck2/issues/441 Signed-off-by: Austin Seipp --- app/buck2_client/src/commands/log/options.rs | 82 ++++++++++++++------ app/buck2_client_ctx/src/client_ctx.rs | 9 +++ app/buck2_common/src/init.rs | 51 ++++++++++++ 3 files changed, 119 insertions(+), 23 deletions(-) diff --git a/app/buck2_client/src/commands/log/options.rs b/app/buck2_client/src/commands/log/options.rs index c284e58c7cfe..19c9a298b014 100644 --- a/app/buck2_client/src/commands/log/options.rs +++ b/app/buck2_client/src/commands/log/options.rs @@ -12,6 +12,7 @@ use std::process::Stdio; use anyhow::Context; use buck2_client_ctx::client_ctx::ClientCommandContext; use buck2_client_ctx::path_arg::PathArg; +use buck2_common::init::LogDownloadMethod; use buck2_common::temp_path::TempPath; use buck2_core::fs::fs_util; use buck2_core::fs::paths::abs_path::AbsPathBuf; @@ -29,8 +30,8 @@ use rand::Rng; #[derive(Debug, buck2_error::Error)] enum EventLogOptionsError { - #[error("Manifold failed; stderr:\n{}", indent(" ", _0))] - ManifoldFailed(String), + #[error("{0} failed; stderr:\n{}", indent(" ", _1))] + DownloadFailed(String, String), #[error("Log not found locally by trace id `{0}`")] LogNotFoundLocally(TraceId), } @@ -95,7 +96,7 @@ impl EventLogOptions { trace_id: &TraceId, ctx: &ClientCommandContext<'_>, ) -> anyhow::Result { - let manifold_file_name = FileNameBuf::try_from(format!( + let log_file_name = FileNameBuf::try_from(format!( "{}{}", trace_id, // TODO(nga): hardcoded default, should at least use the same default buck2 uses, @@ -106,7 +107,7 @@ impl EventLogOptions { let log_path = ctx .paths()? .log_dir() - .join(FileName::new(&format!("dl-{}", manifold_file_name))?); + .join(FileName::new(&format!("dl-{}", log_file_name))?); if fs_util::try_exists(&log_path)? { return Ok(log_path.into_abs_path_buf()); @@ -116,34 +117,69 @@ impl EventLogOptions { fs_util::create_dir_all(&tmp_dir)?; let temp_path = tmp_dir.join(FileName::new(&format!( "dl.{}.{}.tmp", - manifold_file_name, + log_file_name, Self::random_string() ))?); // Delete the file on failure. let temp_path = TempPath::new_path(temp_path); - - let args = [ - "get", - &format!("buck2_logs/flat/{}", manifold_file_name), - temp_path - .path() - .as_os_str() - .to_str() - .context("temp_path is not valid UTF-8")?, - ]; - buck2_client_ctx::eprintln!("Spawning: manifold {}", args.join(" "))?; - let command = async_background_command("manifold") - .args(args) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::piped()) - .spawn()?; + let (command_name, command) = match ctx.log_download_method() { + LogDownloadMethod::Manifold => { + let args = [ + "get", + &format!("buck2_logs/flat/{}", log_file_name), + temp_path + .path() + .as_os_str() + .to_str() + .context("temp_path is not valid UTF-8")?, + ]; + buck2_client_ctx::eprintln!("Spawning: manifold {}", args.join(" "))?; + ( + "Manifold", + async_background_command("manifold") + .args(args) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn()?, + ) + } + LogDownloadMethod::Curl(log_url) => { + let log_url = log_url.trim_end_matches('/'); + + let args = [ + "--fail", + "-L", + &format!("{}/v1/logs/get/{}", log_url, trace_id), + "-o", + temp_path + .path() + .as_os_str() + .to_str() + .context("temp_path is not valid UTF-8")?, + ]; + buck2_client_ctx::eprintln!("Spawning: curl {}", args.join(" "))?; + ( + "Curl", + async_background_command("curl") + .args(&args) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn()?, + ) + } + LogDownloadMethod::None => { + return Err(EventLogOptionsError::LogNotFoundLocally(trace_id.dupe()).into()); + } + }; // No timeout here, just press Ctrl-C if you want it to cancel. let result = command.wait_with_output().await?; if !result.status.success() { - return Err(EventLogOptionsError::ManifoldFailed( + return Err(EventLogOptionsError::DownloadFailed( + command_name.to_owned(), String::from_utf8_lossy(&result.stderr).into_owned(), ) .into()); diff --git a/app/buck2_client_ctx/src/client_ctx.rs b/app/buck2_client_ctx/src/client_ctx.rs index b8972a0b05a9..11707b41b7b2 100644 --- a/app/buck2_client_ctx/src/client_ctx.rs +++ b/app/buck2_client_ctx/src/client_ctx.rs @@ -15,6 +15,7 @@ use buck2_cli_proto::client_context::HostPlatformOverride as GrpcHostPlatformOve use buck2_cli_proto::client_context::PreemptibleWhen as GrpcPreemptibleWhen; use buck2_cli_proto::ClientContext; use buck2_common::argv::Argv; +use buck2_common::init::LogDownloadMethod; use buck2_common::invocation_paths::InvocationPaths; use buck2_core::error::buck2_hard_error_env; use buck2_core::fs::working_dir::WorkingDir; @@ -251,4 +252,12 @@ impl<'a> ClientCommandContext<'a> { pub fn allow_vpnless(&self) -> anyhow::Result { Ok(self.immediate_config.daemon_startup_config()?.allow_vpnless) } + + pub fn log_download_method(&self) -> LogDownloadMethod { + self.immediate_config + .daemon_startup_config() + .unwrap() + .log_download_method + .clone() + } } diff --git a/app/buck2_common/src/init.rs b/app/buck2_common/src/init.rs index 8baf276105f9..40126319dc74 100644 --- a/app/buck2_common/src/init.rs +++ b/app/buck2_common/src/init.rs @@ -268,6 +268,13 @@ impl ResourceControlConfig { } } +#[derive(Allocative, Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +pub enum LogDownloadMethod { + Manifold, + Curl(String), + None, +} + /// Configurations that are used at startup by the daemon. Those are actually read by the client, /// and passed on to the daemon. /// @@ -288,6 +295,7 @@ pub struct DaemonStartupConfig { pub materializations: Option, pub http: HttpConfig, pub resource_control: ResourceControlConfig, + pub log_download_method: LogDownloadMethod, } impl DaemonStartupConfig { @@ -300,6 +308,44 @@ impl DaemonStartupConfig { })? .unwrap_or(true); + let log_download_method = { + // Determine the log download method to use. This rather annoyingly + // long bit of logic is here so that there is the smallest amount of + // #[cfg]'d code possible, and so that fbcode and non-fbcode builds + // can have the same codepath for manifold/curl, with manifold being + // the fbcode default and curl being the OSS default. + + #[cfg(fbcode_build)] + let use_manifold_default = true; + #[cfg(not(fbcode_build))] + let use_manifold_default = false; + + let use_manifold = config + .parse(BuckconfigKeyRef { + section: "buck2", + property: "log_use_manifold", + })? + .unwrap_or(use_manifold_default); + + if use_manifold { + Ok(LogDownloadMethod::Manifold) + } else { + let log_url = config.get(BuckconfigKeyRef { + section: "buck2", + property: "log_url", + }); + if let Some(log_url) = log_url { + if log_url.is_empty() { + Err(anyhow::anyhow!("Empty log_url")) + } else { + Ok(LogDownloadMethod::Curl(log_url.to_owned())) + } + } else { + Ok(LogDownloadMethod::None) + } + } + }?; + Ok(Self { daemon_buster: config .get(BuckconfigKeyRef { @@ -329,6 +375,7 @@ impl DaemonStartupConfig { .map(ToOwned::to_owned), http: HttpConfig::from_config(config)?, resource_control: ResourceControlConfig::from_config(config)?, + log_download_method, }) } @@ -350,6 +397,10 @@ impl DaemonStartupConfig { materializations: None, http: HttpConfig::default(), resource_control: ResourceControlConfig::default(), + #[cfg(fbcode_build)] + log_download_method: LogDownloadMethod::Manifold, + #[cfg(not(fbcode_build))] + log_download_method: LogDownloadMethod::None, } } }