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.

Sign up for GitHub

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

buck2_client: download traces via buck2.log_url config key #770

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

buck2_client: download traces via buck2.log_url config key #770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
82 changes: 59 additions & 23 deletions app/buck2_client/src/commands/log/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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),
}
Expand Down Expand Up @@ -95,7 +96,7 @@ impl EventLogOptions {
trace_id: &TraceId,
ctx: &ClientCommandContext<'_>,
) -> anyhow::Result<AbsPathBuf> {
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,
Expand All @@ -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());
Expand All @@ -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());
Expand Down
9 changes: 9 additions & 0 deletions app/buck2_client_ctx/src/client_ctx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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_common::invocation_paths_result::InvocationPathsResult;
use buck2_core::error::buck2_hard_error_env;
Expand Down Expand Up @@ -286,4 +287,12 @@ impl<'a> ClientCommandContext<'a> {
pub fn async_cleanup_context(&self) -> &AsyncCleanupContext<'a> {
&self.async_cleanup
}

pub fn log_download_method(&self) -> LogDownloadMethod {
self.immediate_config
.daemon_startup_config()
.unwrap()
.log_download_method
.clone()
}
}
52 changes: 52 additions & 0 deletions app/buck2_common/src/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,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.
///
Expand All @@ -332,11 +339,51 @@ pub struct DaemonStartupConfig {
pub materializations: Option<String>,
pub http: HttpConfig,
pub resource_control: ResourceControlConfig,
pub log_download_method: LogDownloadMethod,
}

impl DaemonStartupConfig {
pub fn new(config: &LegacyBuckConfig) -> buck2_error::Result<Self> {
// Intepreted client side because we need the value here.

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 {
Expand Down Expand Up @@ -365,6 +412,7 @@ impl DaemonStartupConfig {
.map(ToOwned::to_owned),
http: HttpConfig::from_config(config)?,
resource_control: ResourceControlConfig::from_config(config)?,
log_download_method,
})
}

Expand All @@ -385,6 +433,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,
}
}
}
Loading