Add flag --experimental_remote_build_event_upload

src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java

@@ -31,6 +31,7 @@
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
 import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.CachePolicy;
+import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode;
 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;
@@ -52,12 +53,14 @@
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /** A {@link BuildEventArtifactUploader} backed by {@link RemoteCache}. */
 class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
     implements BuildEventArtifactUploader {
+  private static final Pattern TEST_LOG_PATTERN = Pattern.compile(".*/bazel-out/[^/]*/testlogs/.*");
 
   private final Executor executor;
   private final ExtendedEventHandler reporter;
@@ -73,6 +76,8 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
   private final Set<PathFragment> omittedFiles = Sets.newConcurrentHashSet();
   private final Set<PathFragment> omittedTreeRoots = Sets.newConcurrentHashSet();
   private final XattrProvider xattrProvider;
+  private final RemoteBuildEventUploadMode remoteBuildEventUploadMode;
+  private final Path profilePath;
 
   ByteStreamBuildEventArtifactUploader(
       Executor executor,
@@ -82,7 +87,9 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
       String remoteServerInstanceName,
       String buildRequestId,
       String commandId,
-      XattrProvider xattrProvider) {
+      XattrProvider xattrProvider,
+      RemoteBuildEventUploadMode remoteBuildEventUploadMode,
+      Path profilePath) {
     this.executor = executor;
     this.reporter = reporter;
     this.verboseFailures = verboseFailures;
@@ -92,6 +99,8 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
     this.remoteServerInstanceName = remoteServerInstanceName;
     this.scheduler = Schedulers.from(executor);
     this.xattrProvider = xattrProvider;
+    this.remoteBuildEventUploadMode = remoteBuildEventUploadMode;
+    this.profilePath = profilePath;
   }
 
   public void omitFile(Path file) {
@@ -197,8 +206,23 @@ private static void processQueryResult(
     }
   }
 
-  private static boolean shouldUpload(PathMetadata path) {
-    return path.getDigest() != null && !path.isRemote() && !path.isDirectory() && !path.isOmitted();
+  private boolean shouldUpload(PathMetadata path) {
+    boolean result =
+        path.getDigest() != null && !path.isRemote() && !path.isDirectory() && !path.isOmitted();
+
+    if (remoteBuildEventUploadMode == RemoteBuildEventUploadMode.MINIMAL) {
+      result = result && (isTestLog(path) || isProfile(path));
+    }
+
+    return result;
+  }
+
+  private boolean isTestLog(PathMetadata path) {
+    return TEST_LOG_PATTERN.matcher(path.getPath().getPathString()).matches();
+  }
+
+  private boolean isProfile(PathMetadata path) {
+    return path.getPath().equals(profilePath);
   }
 
   private Single<List<PathMetadata>> queryRemoteCache(
@@ -207,8 +231,7 @@ private Single<List<PathMetadata>> queryRemoteCache(
     List<PathMetadata> filesToQuery = new ArrayList<>();
     Set<Digest> digestsToQuery = new HashSet<>();
     for (PathMetadata path : paths) {
-      // Query remote cache for files even if omitted from uploading
-      if (shouldUpload(path) || path.isOmitted()) {
+      if (path.getDigest() != null && !path.isRemote() && !path.isDirectory()) {
         filesToQuery.add(path);
         digestsToQuery.add(path.getDigest());
       } else {
@@ -256,17 +279,18 @@ private Single<List<PathMetadata>> uploadLocalFiles(
               return toCompletable(
                       () -> remoteCache.uploadFile(context, path.getDigest(), path.getPath()),
                       executor)
-                  .toSingleDefault(path)
+                  .toSingle(
+                      () ->
+                          new PathMetadata(
+                              path.getPath(),
+                              path.getDigest(),
+                              path.isDirectory(),
+                              /*remote=*/ true,
+                              path.isOmitted()))
                   .onErrorResumeNext(
                       error -> {
                         reporterUploadError(error);
-                        return Single.just(
-                            new PathMetadata(
-                                path.getPath(),
-                                /*digest=*/ null,
-                                path.isDirectory(),
-                                path.isRemote(),
-                                path.isOmitted()));
+                        return Single.just(path);
                       });
             })
         .collect(Collectors.toList());
@@ -347,7 +371,7 @@ private static class PathConverterImpl implements PathConverter {
       for (PathMetadata pair : uploads) {
         Path path = pair.getPath();
         Digest digest = pair.getDigest();
-        if (!pair.isRemote() && pair.isOmitted()) {
+        if (!pair.isRemote()) {
           localPaths.add(path);
         } else if (digest != null) {
           pathToDigest.put(path, digest);

src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java

@@ -17,8 +17,11 @@
 
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode;
 import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.runtime.CommonCommandOptions;
+import com.google.devtools.build.lib.vfs.Path;
 import java.util.concurrent.Executor;
 import javax.annotation.Nullable;
 
@@ -32,6 +35,7 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU
   private final String remoteServerInstanceName;
   private final String buildRequestId;
   private final String commandId;
+  private final RemoteBuildEventUploadMode remoteBuildEventUploadMode;
 
   @Nullable private ByteStreamBuildEventArtifactUploader uploader;
 
@@ -42,19 +46,29 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU
       RemoteCache remoteCache,
       String remoteServerInstanceName,
       String buildRequestId,
-      String commandId) {
+      String commandId,
+      RemoteBuildEventUploadMode remoteBuildEventUploadMode) {
     this.executor = executor;
     this.reporter = reporter;
     this.verboseFailures = verboseFailures;
     this.remoteCache = remoteCache;
     this.remoteServerInstanceName = remoteServerInstanceName;
     this.buildRequestId = buildRequestId;
     this.commandId = commandId;
+    this.remoteBuildEventUploadMode = remoteBuildEventUploadMode;
   }
 
   @Override
   public BuildEventArtifactUploader create(CommandEnvironment env) {
     checkState(uploader == null, "Already created");
+
+    Path profilePath;
+    CommonCommandOptions options = env.getOptions().getOptions(CommonCommandOptions.class);
+    if (options != null && options.profilePath != null) {
+      profilePath = env.getWorkspace().getRelative(options.profilePath);
+    } else {
+      profilePath = env.getDirectories().getOutputBase().getRelative("command.profile.gz");
+    }
     uploader =
         new ByteStreamBuildEventArtifactUploader(
             executor,
@@ -64,7 +78,9 @@ public BuildEventArtifactUploader create(CommandEnvironment env) {
             remoteServerInstanceName,
             buildRequestId,
             commandId,
-            env.getXattrProvider());
+            env.getXattrProvider(),
+            remoteBuildEventUploadMode,
+            profilePath);
     return uploader;
   }
 

src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java

@@ -66,6 +66,7 @@
 import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
 import com.google.devtools.build.lib.remote.downloader.GrpcRemoteDownloader;
 import com.google.devtools.build.lib.remote.logging.LoggingInterceptor;
+import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
 import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
 import com.google.devtools.build.lib.remote.util.DigestUtil;
@@ -634,7 +635,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
             actionContextProvider.getRemoteCache(),
             remoteBytestreamUriPrefix,
             buildRequestId,
-            invocationId));
+            invocationId,
+            remoteOptions.remoteBuildEventUploadMode));
 
     if (enableRemoteDownloader) {
       remoteDownloaderSupplier.set(
@@ -732,7 +734,9 @@ public void afterAnalysis(
       actionContextProvider.setFilesToDownload(ImmutableSet.copyOf(filesToDownload));
     }
 
-    if (remoteOptions != null && remoteOptions.incompatibleRemoteBuildEventUploadRespectNoCache) {
+    if (remoteOptions != null
+        && remoteOptions.remoteBuildEventUploadMode == RemoteBuildEventUploadMode.ALL
+        && remoteOptions.incompatibleRemoteBuildEventUploadRespectNoCache) {
       parseNoCacheOutputs(analysisResult);
     }
   }

src/main/java/com/google/devtools/build/lib/remote/options/RemoteBuildEventUploadMode.java

@@ -0,0 +1,6 @@
+package com.google.devtools.build.lib.remote.options;
+
+public enum RemoteBuildEventUploadMode {
+  ALL,
+  MINIMAL,
+}

src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java

@@ -268,16 +268,44 @@ public String getTypeDescription() {
       help = "Whether to upload locally executed action results to the remote cache.")
   public boolean remoteUploadLocalResults;
 
+  @Deprecated
   @Option(
       name = "incompatible_remote_build_event_upload_respect_no_cache",
       defaultValue = "false",
       documentationCategory = OptionDocumentationCategory.REMOTE,
       effectTags = {OptionEffectTag.UNKNOWN},
+      deprecationWarning =
+          "--incompatible_remote_build_event_upload_respect_no_cache has been deprecated in favor"
+              + " of --experimental_remote_build_event_upload=minimal.",
       help =
           "If set to true, outputs referenced by BEP are not uploaded to remote cache if the"
               + " generating action cannot be cached remotely.")
   public boolean incompatibleRemoteBuildEventUploadRespectNoCache;
 
+  @Option(
+      name = "experimental_remote_build_event_upload",
+      defaultValue = "all",
+      documentationCategory = OptionDocumentationCategory.REMOTE,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      converter = RemoteBuildEventUploadModeConverter.class,
+      help =
+          "If set to 'all', all local outputs referenced by BEP are uploaded to remote cache.\n"
+              + "If set to 'minimal', local outputs referenced by BEP are not uploaded to the"
+              + " remote cache, except for files that are important to the consumers of BEP (e.g."
+              + " test logs and timing profile).\n"
+              + "file:// scheme is used for the paths of local files and bytestream:// scheme is"
+              + " used for the paths of (already) uploaded files.\n"
+              + "Default to 'all'.")
+  public RemoteBuildEventUploadMode remoteBuildEventUploadMode;
+
+  /** Build event upload mode flag parser */
+  public static class RemoteBuildEventUploadModeConverter
+      extends EnumConverter<RemoteBuildEventUploadMode> {
+    public RemoteBuildEventUploadModeConverter() {
+      super(RemoteBuildEventUploadMode.class, "remote build event upload");
+    }
+  }
+
   @Option(
       name = "incompatible_remote_results_ignore_disk",
       defaultValue = "false",

src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java

@@ -53,6 +53,7 @@
 import com.google.devtools.build.lib.remote.common.MissingDigestsFinder;
 import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
 import com.google.devtools.build.lib.remote.grpc.ChannelConnectionFactory;
+import com.google.devtools.build.lib.remote.options.RemoteBuildEventUploadMode;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
 import com.google.devtools.build.lib.remote.util.DigestUtil;
 import com.google.devtools.build.lib.remote.util.RxNoGlobalErrorsRule;
@@ -481,7 +482,9 @@ private ByteStreamBuildEventArtifactUploader newArtifactUploader(RemoteCache rem
         /*remoteServerInstanceName=*/ "localhost/instance",
         /*buildRequestId=*/ "none",
         /*commandId=*/ "none",
-        SyscallCache.NO_CACHE);
+        SyscallCache.NO_CACHE,
+        RemoteBuildEventUploadMode.ALL,
+        /*profilePath=*/ fs.getPath("command.profile.gz"));
   }
 
   private static class StaticMissingDigestsFinder implements MissingDigestsFinder {

src/test/shell/bazel/remote/BUILD

@@ -117,3 +117,14 @@ sh_test(
         "@bazel_tools//tools/bash/runfiles",
     ],
 )
+
+sh_test(
+    name = "remote_build_event_uploader_test",
+    srcs = ["remote_build_event_uploader_test.sh"],
+    data = [
+        ":remote_utils",
+        "//src/test/shell/bazel:test-deps",
+        "//src/tools/remote:worker",
+        "@bazel_tools//tools/bash/runfiles",
+    ],
+)