Anchor input fetches to source action id
Provide the specific action id via RequestMetadata which provided an
action input artifact when using remote_download_minimal. This replaces
the unattributable "fetch-remote-inputs" identifier populated for each
input via a nested context.
Closes #11236.
PiperOrigin-RevId: 310170811
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
index 8900a7a..27a205f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java
@@ -111,6 +111,15 @@
return 0;
}
+ /**
+ * Remote action source identifier for the file.
+ *
+ * <p>"" indicates that a remote action output was not the source of this artifact.
+ */
+ public String getActionId() {
+ return "";
+ }
+
/** Returns {@code true} if this is a special marker as opposed to a representing a real file. */
public boolean isMarkerValue() {
return this instanceof Singleton;
@@ -544,11 +553,17 @@
private final byte[] digest;
private final long size;
private final int locationIndex;
+ private final String actionId;
- public RemoteFileArtifactValue(byte[] digest, long size, int locationIndex) {
+ public RemoteFileArtifactValue(byte[] digest, long size, int locationIndex, String actionId) {
this.digest = digest;
this.size = size;
this.locationIndex = locationIndex;
+ this.actionId = actionId;
+ }
+
+ public RemoteFileArtifactValue(byte[] digest, long size, int locationIndex) {
+ this(digest, size, locationIndex, /* actionId= */ "");
}
@Override
@@ -589,6 +604,11 @@
}
@Override
+ public String getActionId() {
+ return actionId;
+ }
+
+ @Override
public long getModifiedTime() {
throw new UnsupportedOperationException(
"RemoteFileArifactValue doesn't support getModifiedTime");
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java
index 78ed389..ee95136 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java
@@ -31,8 +31,10 @@
* @param digest the digest of the file.
* @param size the size of the file in bytes.
* @param locationIndex is only used in Blaze.
+ * @param actionId the id of the action that produced this file.
*/
- void injectRemoteFile(Artifact output, byte[] digest, long size, int locationIndex);
+ void injectRemoteFile(
+ Artifact output, byte[] digest, long size, int locationIndex, String actionId);
/**
* Inject the metadata of a tree artifact whose contents are stored remotely.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
index 519b7f4..4352d26 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.remote;
import build.bazel.remote.execution.v2.Digest;
+import build.bazel.remote.execution.v2.RequestMetadata;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
@@ -32,6 +33,7 @@
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
import com.google.devtools.build.lib.remote.util.DigestUtil;
+import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
import com.google.devtools.build.lib.vfs.Path;
import io.grpc.Context;
import java.io.IOException;
@@ -65,12 +67,13 @@
private final RemoteCache remoteCache;
private final Path execRoot;
- private final Context ctx;
+ private final RequestMetadata requestMetadata;
- RemoteActionInputFetcher(RemoteCache remoteCache, Path execRoot, Context ctx) {
+ RemoteActionInputFetcher(
+ RemoteCache remoteCache, Path execRoot, RequestMetadata requestMetadata) {
this.remoteCache = Preconditions.checkNotNull(remoteCache);
this.execRoot = Preconditions.checkNotNull(execRoot);
- this.ctx = Preconditions.checkNotNull(ctx);
+ this.requestMetadata = Preconditions.checkNotNull(requestMetadata);
}
/**
@@ -164,6 +167,9 @@
ListenableFuture<Void> download = downloadsInProgress.get(path);
if (download == null) {
+ Context ctx =
+ TracingMetadataUtils.contextWithMetadata(
+ requestMetadata.toBuilder().setActionId(metadata.getActionId()).build());
Context prevCtx = ctx.attach();
try {
Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
index 95df362..157d5d4 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -528,6 +528,7 @@
*/
@Nullable
public InMemoryOutput downloadMinimal(
+ String actionId,
ActionResult result,
Collection<? extends ActionInput> outputs,
@Nullable PathFragment inMemoryOutputPath,
@@ -570,7 +571,7 @@
inMemoryOutput = output;
}
if (output instanceof Artifact) {
- injectRemoteArtifact((Artifact) output, metadata, execRoot, metadataInjector);
+ injectRemoteArtifact((Artifact) output, metadata, execRoot, metadataInjector, actionId);
}
}
@@ -594,7 +595,8 @@
Artifact output,
ActionResultMetadata metadata,
Path execRoot,
- MetadataInjector metadataInjector)
+ MetadataInjector metadataInjector,
+ String actionId)
throws IOException {
if (output.isTreeArtifact()) {
DirectoryMetadata directory =
@@ -617,7 +619,8 @@
new RemoteFileArtifactValue(
DigestUtil.toBinaryDigest(file.digest()),
file.digest().getSizeBytes(),
- /* locationIndex= */ 1);
+ /* locationIndex= */ 1,
+ actionId);
childMetadata.put(p, r);
}
metadataInjector.injectRemoteDirectory(
@@ -633,7 +636,8 @@
output,
DigestUtil.toBinaryDigest(outputMetadata.digest()),
outputMetadata.digest().getSizeBytes(),
- /* locationIndex= */ 1);
+ /* locationIndex= */ 1,
+ actionId);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 62e519d..1d480a8 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.remote;
import build.bazel.remote.execution.v2.DigestFunction;
+import build.bazel.remote.execution.v2.RequestMetadata;
import build.bazel.remote.execution.v2.ServerCapabilities;
import com.google.auth.Credentials;
import com.google.common.base.Preconditions;
@@ -662,12 +663,14 @@
env.getOptions().getOptions(RemoteOptions.class), "RemoteOptions");
RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode;
if (!remoteOutputsMode.downloadAllOutputs()) {
- Context ctx =
- TracingMetadataUtils.contextWithMetadata(
- env.getBuildRequestId(), env.getCommandId().toString(), "fetch-remote-inputs");
+ RequestMetadata requestMetadata =
+ RequestMetadata.newBuilder()
+ .setCorrelatedInvocationsId(env.getBuildRequestId())
+ .setToolInvocationId(env.getCommandId().toString())
+ .build();
actionInputFetcher =
new RemoteActionInputFetcher(
- actionContextProvider.getRemoteCache(), env.getExecRoot(), ctx);
+ actionContextProvider.getRemoteCache(), env.getExecRoot(), requestMetadata);
builder.setActionInputPrefetcher(actionInputFetcher);
remoteOutputService.setActionInputFetcher(actionInputFetcher);
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index 4c3a617..b33338a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -186,6 +186,7 @@
prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs minimal")) {
inMemoryOutput =
remoteCache.downloadMinimal(
+ actionKey.getDigest().getHash(),
result,
spawn.getOutputFiles(),
inMemoryOutputPath,
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 2b5a893..bd0e378 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -262,6 +262,7 @@
} else {
try {
return downloadAndFinalizeSpawnResult(
+ actionKey.getDigest().getHash(),
cachedResult,
/* cacheHit= */ true,
spawn,
@@ -341,6 +342,7 @@
try {
return downloadAndFinalizeSpawnResult(
+ actionKey.getDigest().getHash(),
actionResult,
reply.getCachedResult(),
spawn,
@@ -413,6 +415,7 @@
}
private SpawnResult downloadAndFinalizeSpawnResult(
+ String actionId,
ActionResult actionResult,
boolean cacheHit,
Spawn spawn,
@@ -441,6 +444,7 @@
Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs minimal")) {
inMemoryOutput =
remoteCache.downloadMinimal(
+ actionId,
actionResult,
spawn.getOutputFiles(),
inMemoryOutputPath,
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java b/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java
index a4b5511..79a0cbb 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java
@@ -70,18 +70,21 @@
Preconditions.checkNotNull(buildRequestId);
Preconditions.checkNotNull(commandId);
Preconditions.checkNotNull(actionId);
- RequestMetadata.Builder metadata =
+ RequestMetadata metadata =
RequestMetadata.newBuilder()
.setCorrelatedInvocationsId(buildRequestId)
- .setToolInvocationId(commandId);
- metadata.setActionId(actionId);
- metadata
- .setToolDetails(
- ToolDetails.newBuilder()
- .setToolName("bazel")
- .setToolVersion(BlazeVersionInfo.instance().getVersion()))
- .build();
- return Context.current().withValue(CONTEXT_KEY, metadata.build());
+ .setToolInvocationId(commandId)
+ .setActionId(actionId)
+ .setToolDetails(
+ ToolDetails.newBuilder()
+ .setToolName("bazel")
+ .setToolVersion(BlazeVersionInfo.instance().getVersion()))
+ .build();
+ return contextWithMetadata(metadata);
+ }
+
+ public static Context contextWithMetadata(RequestMetadata metadata) {
+ return Context.current().withValue(CONTEXT_KEY, metadata);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 347c5f5..568650f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -542,7 +542,8 @@
}
@Override
- public void injectRemoteFile(Artifact output, byte[] digest, long size, int locationIndex) {
+ public void injectRemoteFile(
+ Artifact output, byte[] digest, long size, int locationIndex, String actionId) {
Preconditions.checkArgument(
isKnownOutput(output), output + " is not a declared output of this action");
Preconditions.checkArgument(
@@ -551,7 +552,7 @@
output);
Preconditions.checkState(
executionMode.get(), "Tried to inject %s outside of execution", output);
- store.injectRemoteFile(output, digest, size, locationIndex);
+ store.injectRemoteFile(output, digest, size, locationIndex, actionId);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java b/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java
index bc75258..914b8b5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/OutputStore.java
@@ -95,9 +95,11 @@
treeArtifactContents.computeIfAbsent(artifact, a -> Sets.newConcurrentHashSet()).add(contents);
}
- void injectRemoteFile(Artifact output, byte[] digest, long size, int locationIndex) {
+ void injectRemoteFile(
+ Artifact output, byte[] digest, long size, int locationIndex, String actionId) {
injectOutputData(
- output, new FileArtifactValue.RemoteFileArtifactValue(digest, size, locationIndex));
+ output,
+ new FileArtifactValue.RemoteFileArtifactValue(digest, size, locationIndex, actionId));
}
final void injectOutputData(Artifact output, FileArtifactValue artifactValue) {
