Propagating and printing server logs on demand.

WANT_LGTM=all
TESTED=RBE, unit tests
RELNOTES: None
PiperOrigin-RevId: 189938345

4 files changed, 58 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
index 2828424d83..be81ad6cfe 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
@@ -210,7 +210,7 @@ public abstract class AbstractRemoteActionCache implements AutoCloseable {
    * Download a file (that is not a directory). If the {@code content} is not given, the content is
    * fetched from the digest.
    */
-  protected void downloadFile(
+  public void downloadFile(
       Path path, Digest digest, boolean isExecutable, @Nullable ByteString content)
       throws IOException, InterruptedException {
     FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index f3a2baec31..a4f074a9b9 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.exec.local.WindowsLocalEnvProvider;
 import com.google.devtools.build.lib.remote.util.DigestUtil;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.vfs.Path;
 import javax.annotation.Nullable;
 
 /**
@@ -40,16 +41,19 @@ final class RemoteActionContextProvider extends ActionContextProvider {
   private final AbstractRemoteActionCache cache;
   private final GrpcRemoteExecutor executor;
   private final DigestUtil digestUtil;
+  private final Path logDir;
 
   RemoteActionContextProvider(
       CommandEnvironment env,
       @Nullable AbstractRemoteActionCache cache,
       @Nullable GrpcRemoteExecutor executor,
-      DigestUtil digestUtil) {
+      DigestUtil digestUtil,
+      Path logDir) {
     this.env = env;
     this.executor = executor;
     this.cache = cache;
     this.digestUtil = digestUtil;
+    this.logDir = logDir;
   }
 
   @Override
@@ -84,7 +88,8 @@ final class RemoteActionContextProvider extends ActionContextProvider {
               commandId,
               cache,
               executor,
-              digestUtil);
+              digestUtil,
+              logDir);
       return ImmutableList.of(new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner));
     }
   }
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 72c58d312b..dbb0e063b4 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
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.runtime.ServerBuilder;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.vfs.FileSystem.HashFunction;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsProvider;
@@ -97,6 +98,18 @@ public final class RemoteModule extends BlazeModule {
     String buildRequestId = env.getBuildRequestId().toString();
     String commandId = env.getCommandId().toString();
     logger.info("Command: buildRequestId = " + buildRequestId + ", commandId = " + commandId);
+    Path logDir =
+        env.getOutputBase().getRelative(env.getRuntime().getProductName() + "-remote-logs");
+    try {
+      // Clean out old logs files.
+      if (logDir.exists()) {
+        FileSystemUtils.deleteTree(logDir);
+      }
+      logDir.createDirectory();
+    } catch (IOException e) {
+      env.getReporter()
+          .handle(Event.error("Could not create base directory for remote logs: " + logDir));
+    }
     RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
     AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
     HashFunction hashFn = env.getRuntime().getFileSystem().getDigestFunction();
@@ -154,7 +167,8 @@ public final class RemoteModule extends BlazeModule {
         executor = null;
       }
 
-      actionContextProvider = new RemoteActionContextProvider(env, cache, executor, digestUtil);
+      actionContextProvider =
+          new RemoteActionContextProvider(env, cache, executor, digestUtil, logDir);
     } catch (IOException e) {
       env.getReporter().handle(Event.error(e.getMessage()));
       env.getBlazeModuleEnvironment().exit(new AbruptExitException(ExitCode.COMMAND_LINE_ERROR));
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 289fe8b4a9..fb9724bac9 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
@@ -49,6 +49,7 @@ import com.google.devtools.remoteexecution.v1test.Command;
 import com.google.devtools.remoteexecution.v1test.Digest;
 import com.google.devtools.remoteexecution.v1test.ExecuteRequest;
 import com.google.devtools.remoteexecution.v1test.ExecuteResponse;
+import com.google.devtools.remoteexecution.v1test.LogFile;
 import com.google.devtools.remoteexecution.v1test.Platform;
 import com.google.protobuf.TextFormat;
 import com.google.protobuf.TextFormat.ParseException;
@@ -83,6 +84,7 @@ class RemoteSpawnRunner implements SpawnRunner {
   private final String buildRequestId;
   private final String commandId;
   private final DigestUtil digestUtil;
+  private final Path logDir;
 
   // Used to ensure that a warning is reported only once.
   private final AtomicBoolean warningReported = new AtomicBoolean();
@@ -97,7 +99,8 @@ class RemoteSpawnRunner implements SpawnRunner {
       String commandId,
       @Nullable AbstractRemoteActionCache remoteCache,
       @Nullable GrpcRemoteExecutor remoteExecutor,
-      DigestUtil digestUtil) {
+      DigestUtil digestUtil,
+      Path logDir) {
     this.execRoot = execRoot;
     this.options = options;
     this.fallbackRunner = fallbackRunner;
@@ -108,6 +111,7 @@ class RemoteSpawnRunner implements SpawnRunner {
     this.buildRequestId = buildRequestId;
     this.commandId = commandId;
     this.digestUtil = digestUtil;
+    this.logDir = logDir;
   }
 
   @Override
@@ -196,6 +200,7 @@ class RemoteSpawnRunner implements SpawnRunner {
                 .setAction(action)
                 .setSkipCacheLookup(!acceptCachedResult);
         ExecuteResponse reply = remoteExecutor.executeRemotely(request.build());
+        maybeDownloadServerLogs(reply, actionKey);
         result = reply.getResult();
         remoteCacheHit = reply.getCachedResult();
       } catch (IOException e) {
@@ -215,6 +220,32 @@ class RemoteSpawnRunner implements SpawnRunner {
     }
   }
 
+  private void maybeDownloadServerLogs(ExecuteResponse resp, ActionKey actionKey)
+      throws InterruptedException {
+    ActionResult result = resp.getResult();
+    if (resp.getServerLogsCount() > 0
+        && (result.getExitCode() != 0 || resp.getStatus().getCode() != Code.OK.value())) {
+      Path parent = logDir.getRelative(actionKey.getDigest().getHash());
+      Path logPath = null;
+      int logCount = 0;
+      for (Map.Entry<String, LogFile> e : resp.getServerLogsMap().entrySet()) {
+        if (e.getValue().getHumanReadable()) {
+          logPath = parent.getRelative(e.getKey());
+          logCount++;
+          try {
+            remoteCache.downloadFile(logPath, e.getValue().getDigest(), false, null);
+          } catch (IOException ex) {
+            reportOnce(Event.warn("Failed downloading server logs from the remote cache."));
+          }
+        }
+      }
+      if (logCount > 0 && verboseFailures) {
+        report(
+            Event.info("Server logs of failing action:\n   " + (logCount > 1 ? parent : logPath)));
+      }
+    }
+  }
+
   private SpawnResult.Builder downloadRemoteResults(ActionResult result, FileOutErr outErr)
       throws ExecException, IOException, InterruptedException {
     remoteCache.download(result, execRoot, outErr);
@@ -242,16 +273,17 @@ class RemoteSpawnRunner implements SpawnRunner {
             && RemoteRetrierUtils.causedByExecTimeout((RetryException) cause))) {
       return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey);
     }
-    return handleError(cause, policy.getFileOutErr());
+    return handleError(cause, policy.getFileOutErr(), actionKey);
   }
 
-  private SpawnResult handleError(IOException exception, FileOutErr outErr)
+  private SpawnResult handleError(IOException exception, FileOutErr outErr, ActionKey actionKey)
       throws ExecException, InterruptedException, IOException {
     final Throwable cause = exception.getCause();
     if (cause instanceof ExecutionStatusException) {
       ExecutionStatusException e = (ExecutionStatusException) cause;
       if (e.getResponse() != null) {
         ExecuteResponse resp = e.getResponse();
+        maybeDownloadServerLogs(resp, actionKey);
         if (resp.hasResult()) {
           // We try to download all (partial) results even on server error, for debuggability.
           remoteCache.download(resp.getResult(), execRoot, outErr);