This change adds the writing of the remote execution log to a file behind an experimental flag. It also adds a logging handler for Execute calls so that they are logged.

PiperOrigin-RevId: 190991493

7 files changed, 114 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD
index 42bf402027..18e38edb9e 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD
@@ -31,6 +31,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/exec/local:options",
         "//src/main/java/com/google/devtools/build/lib/remote/blobstore",
         "//src/main/java/com/google/devtools/build/lib/remote/blobstore/http",
+        "//src/main/java/com/google/devtools/build/lib/remote/logging",
         "//src/main/java/com/google/devtools/build/lib/remote/util",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/common/options",
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 dbb0e063b4..805f8f24ae 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
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.exec.ExecutorBuilder;
+import com.google.devtools.build.lib.remote.logging.LoggingInterceptor;
 import com.google.devtools.build.lib.remote.util.DigestUtil;
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.Command;
@@ -29,6 +30,7 @@ import com.google.devtools.build.lib.runtime.CommandEnvironment;
 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.util.io.AsynchronousFileOutputStream;
 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;
@@ -36,12 +38,14 @@ import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsProvider;
 import com.google.devtools.remoteexecution.v1test.Digest;
 import io.grpc.Channel;
+import io.grpc.ClientInterceptors;
 import java.io.IOException;
 import java.util.logging.Logger;
 
 /** RemoteModule provides distributed cache and remote execution for Bazel. */
 public final class RemoteModule extends BlazeModule {
   private static final Logger logger = Logger.getLogger(RemoteModule.class.getName());
+  private AsynchronousFileOutputStream rpcLogFile;
 
   @VisibleForTesting
   static final class CasPathConverter implements PathConverter {
@@ -126,6 +130,10 @@ public final class RemoteModule extends BlazeModule {
       boolean remoteOrLocalCache = SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions);
       boolean grpcCache = GrpcRemoteCache.isRemoteCacheOptions(remoteOptions);
 
+      if (!remoteOptions.experimentalRemoteGrpcLog.isEmpty()) {
+        rpcLogFile = new AsynchronousFileOutputStream(remoteOptions.experimentalRemoteGrpcLog);
+      }
+
       RemoteRetrier retrier =
           new RemoteRetrier(
               remoteOptions, RemoteRetrier.RETRIABLE_GRPC_ERRORS, Retrier.ALLOW_ALL_CALLS);
@@ -157,9 +165,13 @@ public final class RemoteModule extends BlazeModule {
 
       final GrpcRemoteExecutor executor;
       if (remoteOptions.remoteExecutor != null) {
+        Channel ch = GoogleAuthUtils.newChannel(remoteOptions.remoteExecutor, authAndTlsOptions);
+        if (rpcLogFile != null) {
+          ch = ClientInterceptors.intercept(ch, new LoggingInterceptor(rpcLogFile));
+        }
         executor =
             new GrpcRemoteExecutor(
-                GoogleAuthUtils.newChannel(remoteOptions.remoteExecutor, authAndTlsOptions),
+                ch,
                 GoogleAuthUtils.newCallCredentials(authAndTlsOptions),
                 remoteOptions.remoteTimeout,
                 retrier);
@@ -176,6 +188,19 @@ public final class RemoteModule extends BlazeModule {
   }
 
   @Override
+  public void afterCommand() {
+    if (rpcLogFile != null) {
+      try {
+        rpcLogFile.close();
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      } finally {
+        rpcLogFile = null;
+      }
+    }
+  }
+
+  @Override
   public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
     if (actionContextProvider != null) {
       builder.addActionContextProvider(actionContextProvider);
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
index e6493f38de..dd4f5a0d04 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
@@ -206,4 +206,20 @@ public final class RemoteOptions extends OptionsBase {
             + "writing of files, which could cause false positives."
   )
   public boolean experimentalGuardAgainstConcurrentChanges;
+
+  @Option(
+    name = "experimental_remote_grpc_log",
+    defaultValue = "",
+    category = "remote",
+    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    help =
+        "If specified, a path to a file to log gRPC call related details. This log consists "
+            + "of a sequence of serialized "
+            + "com.google.devtools.build.lib.remote.logging.RemoteExecutionLog.LogEntry "
+            + "protobufs with each message prefixed by a varint denoting the size of the following "
+            + "serialized protobuf message, as performed by the method "
+            + "LogEntry.writeDelimitedTo(OutputStream)."
+  )
+  public String experimentalRemoteGrpcLog;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/logging/BUILD b/src/main/java/com/google/devtools/build/lib/remote/logging/BUILD
index 9d8af5875d..cd688e1439 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/logging/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/logging/BUILD
@@ -11,10 +11,12 @@ java_library(
     srcs = glob(["*.java"]),
     tags = ["bazel"],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib:io",
         "//src/main/java/com/google/devtools/build/lib/remote/util",
         "//src/main/protobuf:remote_execution_log_java_proto",
         "//third_party:guava",
         "//third_party/grpc:grpc-jar",
+        "@googleapis//:google_devtools_remoteexecution_v1test_remote_execution_java_grpc",
         "@googleapis//:google_devtools_remoteexecution_v1test_remote_execution_java_proto",
         "@googleapis//:google_longrunning_operations_java_proto",
         "@googleapis//:google_rpc_status_java_proto",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/logging/ExecuteHandler.java b/src/main/java/com/google/devtools/build/lib/remote/logging/ExecuteHandler.java
new file mode 100644
index 0000000000..52ad59c9d4
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/remote/logging/ExecuteHandler.java
@@ -0,0 +1,45 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.remote.logging;
+
+import com.google.devtools.build.lib.remote.logging.RemoteExecutionLog.ExecuteDetails;
+import com.google.devtools.build.lib.remote.logging.RemoteExecutionLog.RpcCallDetails;
+import com.google.devtools.remoteexecution.v1test.ExecuteRequest;
+import com.google.longrunning.Operation;
+
+/** LoggingHandler for google.devtools.remoteexecution.v1test.Execution.Execute gRPC call. */
+public class ExecuteHandler implements LoggingHandler<ExecuteRequest, Operation> {
+
+  private final ExecuteDetails.Builder builder;
+
+  public ExecuteHandler() {
+    builder = ExecuteDetails.newBuilder();
+  }
+
+  @Override
+  public void handleReq(ExecuteRequest message) {
+    builder.setRequest(message);
+  }
+
+  @Override
+  public void handleResp(Operation message) {
+    builder.setResponse(message);
+  }
+
+  @Override
+  public RpcCallDetails getDetails() {
+    return RpcCallDetails.newBuilder().setExecute(builder).build();
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingHandler.java b/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingHandler.java
index fb23d6ca4b..d9653a834e 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingHandler.java
@@ -14,11 +14,10 @@
 
 package com.google.devtools.build.lib.remote.logging;
 
-import com.google.devtools.build.lib.remote.logging.RemoteExecutionLog.LogEntry;
+import com.google.devtools.build.lib.remote.logging.RemoteExecutionLog.RpcCallDetails;
 
 /**
- * An interface for building {@link LogEntry}s specialized for a specific gRPC call with specific
- * request and response types.
+ * An interface for building {@link RpcCallDetails}s specialized for a specific gRPC call.
  *
  * @param <ReqT> request type of the gRPC call
  * @param <RespT> response type of the gRPC call
@@ -39,6 +38,8 @@ public interface LoggingHandler<ReqT, RespT> {
    */
   void handleResp(RespT message);
 
-  /** Returns a {@link LogEntry} based on the requests and responses handled by this handler * */
-  LogEntry getEntry();
+  /**
+   * Returns a {@link RpcCallDetails} based on the requests and responses handled by this handler.
+   */
+  RpcCallDetails getDetails();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingInterceptor.java b/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingInterceptor.java
index 37629d7ae1..b46d4e090c 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingInterceptor.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/logging/LoggingInterceptor.java
@@ -16,6 +16,8 @@ package com.google.devtools.build.lib.remote.logging;
 
 import com.google.devtools.build.lib.remote.logging.RemoteExecutionLog.LogEntry;
 import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
+import com.google.devtools.build.lib.util.io.AsynchronousFileOutputStream;
+import com.google.devtools.remoteexecution.v1test.ExecutionGrpc;
 import com.google.devtools.remoteexecution.v1test.RequestMetadata;
 import io.grpc.CallOptions;
 import io.grpc.Channel;
@@ -30,6 +32,12 @@ import javax.annotation.Nullable;
 
 /** Client interceptor for logging details of certain gRPC calls. */
 public class LoggingInterceptor implements ClientInterceptor {
+  AsynchronousFileOutputStream rpcLogFile;
+
+  /** Constructs a LoggingInterceptor which logs RPC calls to the given file. */
+  public LoggingInterceptor(AsynchronousFileOutputStream rpcLogFile) {
+    this.rpcLogFile = rpcLogFile;
+  }
 
   /**
    * Returns a {@link LoggingHandler} to handle logging details for the specified method. If there
@@ -37,9 +45,12 @@ public class LoggingInterceptor implements ClientInterceptor {
    *
    * @param method Method to return handler for.
    */
-  protected <ReqT, RespT> @Nullable LoggingHandler<ReqT, RespT> selectHandler(
+  @SuppressWarnings("rawtypes")
+  protected <ReqT, RespT> @Nullable LoggingHandler selectHandler(
       MethodDescriptor<ReqT, RespT> method) {
-    // TODO(cdlee): add handlers for methods
+    if (method == ExecutionGrpc.getExecuteMethod()) {
+      return new ExecuteHandler();
+    }
     return null;
   }
 
@@ -56,9 +67,10 @@ public class LoggingInterceptor implements ClientInterceptor {
   }
 
   /**
-   * Wraps client call to log call details by building a {@link LogEntry} and writing it to a log.
+   * Wraps client call to log call details by building a {@link LogEntry} and writing it to the RPC
+   * log file.
    */
-  private static class LoggingForwardingCall<ReqT, RespT>
+  private class LoggingForwardingCall<ReqT, RespT>
       extends ForwardingClientCall.SimpleForwardingClientCall<ReqT, RespT> {
     private final LoggingHandler<ReqT, RespT> handler;
     private final LogEntry.Builder entryBuilder;
@@ -90,8 +102,8 @@ public class LoggingInterceptor implements ClientInterceptor {
             @Override
             public void onClose(Status status, Metadata trailers) {
               entryBuilder.setStatus(makeStatusProto(status));
-              // TODO(cdlee): Actually store this and log the entry.
-              entryBuilder.mergeFrom(handler.getEntry()).build();
+              entryBuilder.setDetails(handler.getDetails());
+              rpcLogFile.write(entryBuilder.build());
               super.onClose(status, trailers);
             }
           },