Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions app/buck2_action_impl/src/actions/impls/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ enum ExecuteResult {
executor_preference: ExecutorPreference,
action_and_blobs: ActionDigestAndBlobs,
input_files_bytes: u64,
request: CommandExecutionRequest,
},
}

Expand Down Expand Up @@ -1131,6 +1132,7 @@ impl RunAction {
executor_preference: req.executor_preference,
action_and_blobs,
input_files_bytes: req.paths().input_files_bytes(),
request: req,
})
}

Expand Down Expand Up @@ -1446,6 +1448,7 @@ impl Action for RunAction {
executor_preference,
action_and_blobs,
input_files_bytes,
request,
) = match self.execute_inner(ctx).await? {
ExecuteResult::LocalDepFileHit(outputs, metadata) => {
return Ok((outputs, metadata));
Expand All @@ -1456,12 +1459,14 @@ impl Action for RunAction {
executor_preference,
action_and_blobs,
input_files_bytes,
request,
} => (
result,
dep_file_bundle,
executor_preference,
action_and_blobs,
input_files_bytes,
request,
),
};

Expand Down Expand Up @@ -1489,6 +1494,7 @@ impl Action for RunAction {
let re_result = result.action_result.take();
let upload_result = ctx
.cache_upload(
&request,
&action_and_blobs,
&result,
re_result,
Expand Down
1 change: 1 addition & 0 deletions app/buck2_build_api/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ pub trait ActionExecutionCtx: Send + Sync {

async fn cache_upload(
&mut self,
request: &CommandExecutionRequest,
action: &ActionDigestAndBlobs,
execution_result: &CommandExecutionResult,
re_result: Option<TActionResult2>,
Expand Down
18 changes: 18 additions & 0 deletions app/buck2_build_api/src/actions/execute/action_execution_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,22 @@ impl CommandExecutionTarget for ActionExecutionTarget<'_> {
identifier: self.action.identifier().unwrap_or("").to_owned(),
}
}

fn action_mnemonic(&self) -> Option<String> {
Some(self.action.category().as_str().to_owned())
}

fn target_label(&self) -> Option<String> {
self.action
.owner()
.unpack_target_label()
.map(ToString::to_string)
}

fn configuration_hash(&self) -> Option<String> {
self.action
.owner()
.unpack_target_label()
.map(|label| label.cfg().output_hash().as_str().to_owned())
}
}
2 changes: 2 additions & 0 deletions app/buck2_build_api/src/actions/execute/action_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ impl ActionExecutionCtx for BuckActionExecutionContext<'_> {

async fn cache_upload(
&mut self,
request: &CommandExecutionRequest,
action_digest_and_blobs: &ActionDigestAndBlobs,
execution_result: &CommandExecutionResult,
re_result: Option<TActionResult2>,
Expand All @@ -608,6 +609,7 @@ impl ActionExecutionCtx for BuckActionExecutionContext<'_> {
digest_config: self.digest_config(),
mergebase: self.mergebase().0.as_ref(),
re_platform: self.re_platform(),
paths: request.paths(),
},
execution_result,
re_result,
Expand Down
7 changes: 6 additions & 1 deletion app/buck2_build_api/src/materialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use buck2_common::legacy_configs::key::BuckconfigKeyRef;
use buck2_common::legacy_configs::view::LegacyBuckConfigView;
use buck2_core::execution_types::executor_config::RemoteExecutorUseCase;
use buck2_core::fs::project_rel_path::ProjectRelativePath;
use buck2_directory::directory::fingerprinted_directory::FingerprintedDirectory;
use buck2_error::BuckErrorContext;
use buck2_execute::artifact::artifact_dyn::ArtifactDyn;
use buck2_execute::artifact_utils::ArtifactValueBuilder;
Expand All @@ -27,6 +28,7 @@ use buck2_execute::digest_config::HasDigestConfig;
use buck2_execute::directory::ActionDirectoryBuilder;
use buck2_execute::execute::blobs::ActionBlobs;
use buck2_execute::materialize::materializer::HasMaterializer;
use buck2_execute::re::action_identity::ReActionIdentity;
use dashmap::DashSet;
use dice::DiceComputations;
use dice::UserComputationData;
Expand Down Expand Up @@ -190,7 +192,10 @@ async fn ensure_uploaded(
&ActionBlobs::new(digest_config),
ProjectRelativePath::empty(),
&dir,
None,
&ReActionIdentity::minimal(
dir.fingerprint().raw_digest().to_string(),
Some(dir.fingerprint().raw_digest().to_string()),
),
digest_config,
ctx.per_transaction_data()
.get_run_action_knobs()
Expand Down
2 changes: 2 additions & 0 deletions app/buck2_execute/src/execute/cache_uploader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use remote_execution::TActionResult2;
use crate::digest_config::DigestConfig;
use crate::execute::action_digest_and_blobs::ActionDigestAndBlobs;
use crate::execute::dep_file_digest::DepFileDigest;
use crate::execute::request::CommandExecutionPaths;
use crate::execute::result::CommandExecutionResult;
use crate::execute::target::CommandExecutionTarget;
use crate::materialize::materializer::Materializer;
Expand All @@ -26,6 +27,7 @@ pub struct CacheUploadInfo<'a> {
pub digest_config: DigestConfig,
pub mergebase: &'a Option<String>,
pub re_platform: &'a remote_execution::Platform,
pub paths: &'a CommandExecutionPaths,
}

#[async_trait]
Expand Down
15 changes: 15 additions & 0 deletions app/buck2_execute/src/execute/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,19 @@ pub trait CommandExecutionTarget: Send + Sync + Debug {
fn as_proto_action_key(&self) -> buck2_data::ActionKey;

fn as_proto_action_name(&self) -> buck2_data::ActionName;

/// Optional mnemonic describing the action kind (e.g. `CxxCompile`).
fn action_mnemonic(&self) -> Option<String> {
None
}

/// Optional configured target label this action belongs to.
fn target_label(&self) -> Option<String> {
None
}

/// Optional hash identifying the build configuration of the target.
fn configuration_hash(&self) -> Option<String> {
None
}
}
40 changes: 36 additions & 4 deletions app/buck2_execute/src/re/action_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::execute::target::CommandExecutionTarget;
pub struct ReActionIdentity<'a> {
/// This is currently unused, but historically it has been useful to add logging in the RE
/// client, so it's worth keeping around.
_target: &'a dyn CommandExecutionTarget,
_target: Option<&'a dyn CommandExecutionTarget>,

/// Actions with the same action key share e.g. memory requirements learnt by RE.
pub action_key: String,
Expand All @@ -26,7 +26,15 @@ pub struct ReActionIdentity<'a> {
pub affinity_key: String,

/// Details about the action collected while uploading
pub paths: &'a CommandExecutionPaths,
pub paths: Option<&'a CommandExecutionPaths>,

/// Optional action id (usually the action digest hash) used for request metadata.
pub action_id: Option<String>,

/// Optional action mnemonic, target label and configuration hash used for OSS RE metadata.
pub action_mnemonic: Option<String>,
pub target_label: Option<String>,
pub configuration_hash: Option<String>,

//// Trace ID which started the execution of this action, to be added on the RE side
pub trace_id: TraceId,
Expand All @@ -37,20 +45,44 @@ impl<'a> ReActionIdentity<'a> {
target: &'a dyn CommandExecutionTarget,
executor_action_key: Option<&str>,
paths: &'a CommandExecutionPaths,
action_id: Option<String>,
) -> Self {
let mut action_key = target.re_action_key();
if let Some(executor_action_key) = executor_action_key {
action_key = format!("{executor_action_key} {action_key}");
}

let trace_id = get_dispatcher().trace_id().to_owned();
let action_mnemonic = target.action_mnemonic();
let target_label = target.target_label();
let configuration_hash = target.configuration_hash();

Self {
_target: target,
_target: Some(target),
action_key,
affinity_key: target.re_affinity_key(),
paths,
paths: Some(paths),
action_id,
action_mnemonic,
target_label,
configuration_hash,
trace_id,
}
}

/// Create a minimal identity for operations that don't have a full action context,
/// such as permission checks.
pub fn minimal(action_key: String, action_id: Option<String>) -> Self {
Self {
_target: None,
action_key,
affinity_key: String::new(),
paths: None,
action_id,
action_mnemonic: None,
target_label: None,
configuration_hash: None,
trace_id: get_dispatcher().trace_id().to_owned(),
}
}
}
33 changes: 27 additions & 6 deletions app/buck2_execute/src/re/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ impl RemoteExecutionClient {
dir_path: &ProjectRelativePath,
input_dir: &ActionImmutableDirectory,
use_case: RemoteExecutorUseCase,
identity: Option<&ReActionIdentity<'_>>,
identity: &ReActionIdentity<'_>,
digest_config: DigestConfig,
deduplicate_get_digests_ttl_calls: bool,
) -> buck2_error::Result<UploadStats> {
Expand Down Expand Up @@ -268,6 +268,7 @@ impl RemoteExecutionClient {
directories: Vec<remote_execution::Path>,
inlined_blobs_with_digest: Vec<InlinedBlobWithDigest>,
use_case: RemoteExecutorUseCase,
identity: &ReActionIdentity<'_>,
) -> buck2_error::Result<()> {
self.data
.uploads
Expand All @@ -276,6 +277,7 @@ impl RemoteExecutionClient {
directories,
inlined_blobs_with_digest,
use_case,
identity,
))
.await
}
Expand Down Expand Up @@ -923,13 +925,16 @@ impl RemoteExecutionClientImpl {
action_digest: ActionDigest,
use_case: RemoteExecutorUseCase,
) -> buck2_error::Result<Option<ActionResultResponse>> {
let mut metadata = use_case.metadata(None);
metadata.action_id = Some(action_digest.raw_digest().to_string());

let res = with_error_handler(
"action_cache",
self.get_session_id(),
self.client()
.get_action_cache_client()
.get_action_result(
use_case.metadata(None),
metadata,
ActionResultRequest {
digest: action_digest.to_re(),
..Default::default()
Expand Down Expand Up @@ -957,14 +962,19 @@ impl RemoteExecutionClientImpl {
directories: Vec<remote_execution::Path>,
inlined_blobs_with_digest: Vec<InlinedBlobWithDigest>,
use_case: RemoteExecutorUseCase,
identity: &ReActionIdentity<'_>,
) -> buck2_error::Result<()> {
let mut metadata = use_case.metadata(Some(identity));
if let Some(action_id) = identity.action_id.as_ref() {
metadata.action_id = Some(action_id.clone());
}
with_error_handler(
"upload_files_and_directories",
self.get_session_id(),
self.client()
.get_cas_client()
.upload(
use_case.metadata(None),
metadata,
UploadRequest {
files_with_digest: Some(files_with_digest),
inlined_blobs_with_digest: Some(inlined_blobs_with_digest),
Expand Down Expand Up @@ -1298,6 +1308,7 @@ impl RemoteExecutionClientImpl {
..Default::default()
}),
respect_file_symlinks: Some(self.respect_file_symlinks),
action_id: Some(action_digest.raw_digest().to_string()),
..use_case.metadata(Some(identity))
};

Expand Down Expand Up @@ -1325,7 +1336,10 @@ impl RemoteExecutionClientImpl {
host_runtime_requirements: THostRuntimeRequirements {
platform: re_platform(platform),
host_resource_requirements: THostResourceRequirements {
input_files_bytes: identity.paths.input_files_bytes() as i64,
input_files_bytes: identity
.paths
.map(|p| p.input_files_bytes() as i64)
.unwrap_or(0),
resource_units: re_resource_units.unwrap_or_default(),
..Default::default()
},
Expand Down Expand Up @@ -1376,13 +1390,17 @@ impl RemoteExecutionClientImpl {
return Ok((Vec::new(), TLocalCacheStats::default()));
}
let expected_blobs = digests.len();
let mut metadata = use_case.metadata(identity);
metadata.action_id = identity
.and_then(|id| id.action_id.clone())
.or(metadata.action_id);
let response = with_error_handler(
"download_typed_blobs",
self.get_session_id(),
self.client()
.get_cas_client()
.download(
use_case.metadata(identity),
metadata,
DownloadRequest {
inlined_digests: Some(digests),
..Default::default()
Expand Down Expand Up @@ -1420,13 +1438,15 @@ impl RemoteExecutionClientImpl {
use_case: RemoteExecutorUseCase,
) -> buck2_error::Result<(Vec<u8>, TLocalCacheStats)> {
let re_action = format!("download_blob for digest {digest}");
let mut metadata = use_case.metadata(None);
metadata.action_id = Some(digest.hash.clone());
let response = with_error_handler(
re_action.as_str(),
self.get_session_id(),
self.client()
.get_cas_client()
.download(
use_case.metadata(None),
metadata,
DownloadRequest {
inlined_digests: Some(vec![digest.clone()]),
..Default::default()
Expand Down Expand Up @@ -1601,6 +1621,7 @@ impl RemoteExecutionClientImpl {
attributes,
..Default::default()
}),
action_id: Some(digest.raw_digest().to_string()),
..use_case.metadata(None)
},
WriteActionResultRequest {
Expand Down
4 changes: 3 additions & 1 deletion app/buck2_execute/src/re/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ impl ManagedRemoteExecutionClient {
blobs: &ActionBlobs,
dir_path: &ProjectRelativePath,
input_dir: &ActionImmutableDirectory,
identity: Option<&ReActionIdentity<'_>>,
identity: &ReActionIdentity<'_>,
digest_config: DigestConfig,
deduplicate_get_digests_ttl_calls: bool,
) -> buck2_error::Result<UploadStats> {
Expand All @@ -405,6 +405,7 @@ impl ManagedRemoteExecutionClient {
files_with_digest: Vec<NamedDigest>,
directories: Vec<remote_execution::Path>,
inlined_blobs_with_digest: Vec<InlinedBlobWithDigest>,
identity: &ReActionIdentity<'_>,
) -> buck2_error::Result<()> {
self.lock()?
.get()
Expand All @@ -414,6 +415,7 @@ impl ManagedRemoteExecutionClient {
directories,
inlined_blobs_with_digest,
self.use_case,
identity,
)
.await
}
Expand Down
Loading
Loading