From 8fd53fba63971f58beab889dd4f3d46726dde672 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 29 Mar 2018 14:59:43 -0700 Subject: 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 --- .../com/google/devtools/build/lib/remote/BUILD | 1 + .../devtools/build/lib/remote/RemoteModule.java | 27 ++++++++++++- .../devtools/build/lib/remote/RemoteOptions.java | 16 ++++++++ .../google/devtools/build/lib/remote/logging/BUILD | 2 + .../build/lib/remote/logging/ExecuteHandler.java | 45 ++++++++++++++++++++++ .../build/lib/remote/logging/LoggingHandler.java | 11 +++--- .../lib/remote/logging/LoggingInterceptor.java | 24 +++++++++--- 7 files changed, 114 insertions(+), 12 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/logging/ExecuteHandler.java (limited to 'src/main/java') 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); @@ -175,6 +187,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) { 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 { + + 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 request type of the gRPC call * @param response type of the gRPC call @@ -39,6 +38,8 @@ public interface LoggingHandler { */ 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 @Nullable LoggingHandler selectHandler( + @SuppressWarnings("rawtypes") + protected @Nullable LoggingHandler selectHandler( MethodDescriptor 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 + private class LoggingForwardingCall extends ForwardingClientCall.SimpleForwardingClientCall { private final LoggingHandler 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); } }, -- cgit v1.2.3