Migrate remote worker to Flogger. This wasn't strictly necessary for Bazel's Flogger clean-up, but I think worth it anyway. PiperOrigin-RevId: 313609431
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java index a9e5d83..35c67c4 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java
@@ -14,20 +14,19 @@ package com.google.devtools.build.remote.worker; -import static java.util.logging.Level.WARNING; import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheImplBase; import build.bazel.remote.execution.v2.ActionResult; import build.bazel.remote.execution.v2.GetActionResultRequest; import build.bazel.remote.execution.v2.UpdateActionResultRequest; +import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.util.DigestUtil; import io.grpc.stub.StreamObserver; -import java.util.logging.Logger; /** A basic implementation of an {@link ActionCacheImplBase} service. */ final class ActionCacheServer extends ActionCacheImplBase { - private static final Logger logger = Logger.getLogger(ActionCacheImplBase.class.getName()); + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private final OnDiskBlobStoreCache cache; private final DigestUtil digestUtil; @@ -52,7 +51,7 @@ responseObserver.onNext(result); responseObserver.onCompleted(); } catch (Exception e) { - logger.log(WARNING, "getActionResult request failed.", e); + logger.atWarning().withCause(e).log("getActionResult request failed"); responseObserver.onError(StatusUtils.internalError(e)); } } @@ -66,7 +65,7 @@ responseObserver.onNext(request.getActionResult()); responseObserver.onCompleted(); } catch (Exception e) { - logger.log(WARNING, "updateActionResult request failed.", e); + logger.atWarning().withCause(e).log("updateActionResult request failed"); responseObserver.onError(StatusUtils.internalError(e)); } }
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD index 5fb5522..a265b4c 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD
@@ -34,6 +34,7 @@ "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", "//src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/http", + "//third_party:flogger", "//third_party:guava", "//third_party:netty", "//third_party/grpc:grpc-jar",
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java index 4a5b30b..39b4fd2 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java
@@ -15,8 +15,6 @@ package com.google.devtools.build.remote.worker; import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; -import static java.util.logging.Level.SEVERE; -import static java.util.logging.Level.WARNING; import build.bazel.remote.execution.v2.Digest; import com.google.bytestream.ByteStreamGrpc.ByteStreamImplBase; @@ -24,6 +22,7 @@ import com.google.bytestream.ByteStreamProto.ReadResponse; import com.google.bytestream.ByteStreamProto.WriteRequest; import com.google.bytestream.ByteStreamProto.WriteResponse; +import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.remote.Chunker; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.util.DigestUtil; @@ -35,12 +34,11 @@ import java.io.IOException; import java.io.OutputStream; import java.util.UUID; -import java.util.logging.Logger; import javax.annotation.Nullable; /** A basic implementation of a {@link ByteStreamImplBase} service. */ final class ByteStreamServer extends ByteStreamImplBase { - private static final Logger logger = Logger.getLogger(ByteStreamServer.class.getName()); + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private final OnDiskBlobStoreCache cache; private final Path workPath; private final DigestUtil digestUtil; @@ -89,7 +87,7 @@ } catch (CacheNotFoundException e) { responseObserver.onError(StatusUtils.notFoundError(digest)); } catch (Exception e) { - logger.log(WARNING, "Read request failed.", e); + logger.atWarning().withCause(e).log("Read request failed"); responseObserver.onError(StatusUtils.internalError(e)); } } @@ -101,7 +99,7 @@ FileSystemUtils.createDirectoryAndParents(temp.getParentDirectory()); FileSystemUtils.createEmptyFile(temp); } catch (IOException e) { - logger.log(SEVERE, "Failed to create temporary file for upload", e); + logger.atSevere().withCause(e).log("Failed to create temporary file for upload"); responseObserver.onError(StatusUtils.internalError(e)); // We need to make sure that subsequent onNext or onCompleted calls don't make any further // calls on the responseObserver after the onError above, so we return a no-op observer. @@ -190,13 +188,13 @@ @Override public void onError(Throwable t) { if (Status.fromThrowable(t).getCode() != Status.Code.CANCELLED) { - logger.log(WARNING, "Write request failed remotely.", t); + logger.atWarning().withCause(t).log("Write request failed remotely"); } closed = true; try { temp.delete(); } catch (IOException e) { - logger.log(WARNING, "Could not delete temp file.", e); + logger.atWarning().withCause(e).log("Could not delete temp file"); } } @@ -223,7 +221,7 @@ try { temp.delete(); } catch (IOException e) { - logger.log(WARNING, "Could not delete temp file.", e); + logger.atWarning().withCause(e).log("Could not delete temp file"); } if (!d.equals(digest)) { @@ -238,7 +236,7 @@ responseObserver.onNext(WriteResponse.newBuilder().setCommittedSize(offset).build()); responseObserver.onCompleted(); } catch (Exception e) { - logger.log(WARNING, "Write request failed.", e); + logger.atWarning().withCause(e).log("Write request failed"); responseObserver.onError(StatusUtils.internalError(e)); closed = true; }
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java index e6634a3..e05e843 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.remote.worker; import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; -import static java.util.logging.Level.WARNING; import build.bazel.remote.execution.v2.BatchUpdateBlobsRequest; import build.bazel.remote.execution.v2.BatchUpdateBlobsResponse; @@ -27,6 +26,7 @@ import build.bazel.remote.execution.v2.FindMissingBlobsResponse; import build.bazel.remote.execution.v2.GetTreeRequest; import build.bazel.remote.execution.v2.GetTreeResponse; +import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.protobuf.InvalidProtocolBufferException; import com.google.rpc.Code; @@ -35,11 +35,10 @@ import java.util.Deque; import java.util.HashSet; import java.util.Set; -import java.util.logging.Logger; /** A basic implementation of a {@link ContentAddressableStorageImplBase} service. */ final class CasServer extends ContentAddressableStorageImplBase { - private static final Logger logger = Logger.getLogger(CasServer.class.getName()); + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); static final long MAX_BATCH_SIZE_BYTES = 1024 * 1024 * 4; private final OnDiskBlobStoreCache cache; @@ -106,7 +105,7 @@ responseObserver.onError(StatusUtils.interruptedError(digest)); return; } catch (Exception e) { - logger.log(WARNING, "Read request failed.", e); + logger.atWarning().withCause(e).log("Read request failed"); responseObserver.onError(StatusUtils.internalError(e)); return; } @@ -114,7 +113,7 @@ try { directory = Directory.parseFrom(directoryBytes); } catch (InvalidProtocolBufferException e) { - logger.log(WARNING, "Failed to parse directory in tree.", e); + logger.atWarning().withCause(e).log("Failed to parse directory in tree"); responseObserver.onError(StatusUtils.internalError(e)); return; }
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java index 5ad5d33..9b7ff5a 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
@@ -15,10 +15,6 @@ package com.google.devtools.build.remote.worker; import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; -import static java.util.logging.Level.FINE; -import static java.util.logging.Level.INFO; -import static java.util.logging.Level.SEVERE; -import static java.util.logging.Level.WARNING; import build.bazel.remote.execution.v2.Action; import build.bazel.remote.execution.v2.ActionResult; @@ -33,6 +29,7 @@ import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.WaitExecutionRequest; import com.google.common.base.Throwables; +import com.google.common.flogger.GoogleLogger; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; @@ -75,13 +72,11 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.logging.Level; -import java.util.logging.Logger; import javax.annotation.Nullable; /** A basic implementation of an {@link ExecutionImplBase} service. */ final class ExecutionServer extends ExecutionImplBase { - private static final Logger logger = Logger.getLogger(ExecutionServer.class.getName()); + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); // The name of the container image entry in the Platform proto // (see third_party/googleapis/devtools/remoteexecution/*/remote_execution.proto and @@ -178,7 +173,7 @@ if (e instanceof ExecutionStatusException) { resp = ((ExecutionStatusException) e).getResponse(); } else { - logger.log(Level.SEVERE, "Work failed: " + opName, e); + logger.atSevere().withCause(e).log("Work failed: %s", opName); resp = ExecuteResponse.newBuilder() .setStatus(StatusUtils.internalErrorStatus(e)) @@ -213,6 +208,7 @@ waitExecution(opName, future, responseObserver); } + @SuppressWarnings("LogAndThrow") private ActionResult execute(ExecuteRequest request, String id) throws IOException, InterruptedException, StatusException { Path tempRoot = workPath.getRelative("build-" + id); @@ -224,25 +220,21 @@ String.format( "build-request-id: %s command-id: %s action-id: %s", meta.getCorrelatedInvocationsId(), meta.getToolInvocationId(), meta.getActionId()); - logger.log(FINE, "Received work for: {0}", workDetails); + logger.atFine().log("Received work for: %s", workDetails); ActionResult result = execute(request.getActionDigest(), tempRoot); - logger.log(FINE, "Completed {0}.", workDetails); + logger.atFine().log("Completed %s", workDetails); return result; } catch (Exception e) { - logger.log(Level.SEVERE, "Work failed: {0} {1}.", new Object[] {workDetails, e}); + logger.atSevere().withCause(e).log("Work failed: %s", workDetails); throw e; } finally { if (workerOptions.debug) { - logger.log(INFO, "Preserving work directory {0}.", tempRoot); + logger.atInfo().log("Preserving work directory %s", tempRoot); } else { try { tempRoot.deleteTree(); } catch (IOException e) { - logger.log( - SEVERE, - String.format( - "Failed to delete tmp directory %s: %s", - tempRoot, Throwables.getStackTraceAsString(e))); + logger.atSevere().withCause(e).log("Failed to delete tmp directory %s", tempRoot); } } } @@ -320,7 +312,7 @@ String.format( "Command:\n%s\nexceeded deadline of %f seconds.", Arrays.toString(command.getArgumentsList().toArray()), timeoutMillis / 1000.0); - logger.warning(errMessage); + logger.atWarning().log(errMessage); errStatus = Status.newBuilder() .setCode(Code.DEADLINE_EXCEEDED.getNumber()) @@ -397,8 +389,8 @@ cmd.execute(stdout, stderr); return Long.parseLong(stdout.toString().trim()); } catch (CommandException | NumberFormatException e) { - logger.log( - WARNING, "Could not get UID for passing to Docker container. Proceeding without it.", e); + logger.atWarning().withCause(e).log( + "Could not get UID for passing to Docker container. Proceeding without it"); return -1; } }
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java index 0bd3791..c28e086 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java
@@ -16,8 +16,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.logging.Level.FINE; -import static java.util.logging.Level.INFO; -import static java.util.logging.Level.SEVERE; import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheImplBase; import build.bazel.remote.execution.v2.ActionResult; @@ -27,6 +25,7 @@ import com.google.bytestream.ByteStreamGrpc.ByteStreamImplBase; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.flogger.GoogleLogger; import com.google.common.io.ByteStreams; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningScheduledExecutorService; @@ -88,7 +87,7 @@ // collected, which would cause us to loose their configuration. private static final Logger rootLogger = Logger.getLogger(""); private static final Logger nettyLogger = Logger.getLogger("io.grpc.netty"); - private static final Logger logger = Logger.getLogger(RemoteWorker.class.getName()); + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private final RemoteWorkerOptions workerOptions; private final ActionCacheImplBase actionCacheServer; @@ -166,11 +165,11 @@ if (execServer != null) { b.addService(ServerInterceptors.intercept(execServer, headersInterceptor)); } else { - logger.info("Execution disabled, only serving cache requests."); + logger.atInfo().log("Execution disabled, only serving cache requests"); } Server server = b.build(); - logger.log(INFO, "Starting gRPC server on port {0,number,#}.", workerOptions.listenPort); + logger.atInfo().log("Starting gRPC server on port %d", workerOptions.listenPort); server.start(); return server; @@ -251,7 +250,7 @@ if (remoteWorkerOptions.casPath == null || (!PathFragment.create(remoteWorkerOptions.casPath).isAbsolute() || !fs.getPath(remoteWorkerOptions.casPath).exists())) { - logger.severe("--cas_path must be specified and refer to an exiting absolut path."); + logger.atSevere().log("--cas_path must be specified and refer to an exiting absolute path"); System.exit(1); return; } @@ -279,11 +278,10 @@ .handler(new LoggingHandler(LogLevel.INFO)) .childHandler(new HttpCacheServerInitializer()); ch = b.bind(remoteWorkerOptions.httpListenPort).sync().channel(); - logger.log( - INFO, - "Started HTTP cache server on port " + remoteWorkerOptions.httpListenPort); + logger.atInfo().log( + "Started HTTP cache server on port %d", remoteWorkerOptions.httpListenPort); } else { - logger.log(INFO, "Not starting HTTP cache server"); + logger.atInfo().log("Not starting HTTP cache server"); } worker.createPidFile(); @@ -304,20 +302,20 @@ private static Path prepareSandboxRunner(FileSystem fs, RemoteWorkerOptions remoteWorkerOptions) { if (OS.getCurrent() != OS.LINUX) { - logger.severe("Sandboxing requested, but it is currently only available on Linux."); + logger.atSevere().log("Sandboxing requested, but it is currently only available on Linux"); System.exit(1); } if (remoteWorkerOptions.workPath == null) { - logger.severe("Sandboxing requested, but --work_path was not specified."); + logger.atSevere().log("Sandboxing requested, but --work_path was not specified"); System.exit(1); } InputStream sandbox = RemoteWorker.class.getResourceAsStream("/main/tools/linux-sandbox"); if (sandbox == null) { - logger.severe( + logger.atSevere().log( "Sandboxing requested, but could not find bundled linux-sandbox binary. " - + "Please rebuild a worker_deploy.jar on Linux to make this work."); + + "Please rebuild a worker_deploy.jar on Linux to make this work"); System.exit(1); } @@ -329,7 +327,8 @@ } sandboxPath.setExecutable(true); } catch (IOException e) { - logger.log(SEVERE, "Could not extract the bundled linux-sandbox binary to " + sandboxPath, e); + logger.atSevere().withCause(e).log( + "Could not extract the bundled linux-sandbox binary to %s", sandboxPath); System.exit(1); } @@ -344,11 +343,9 @@ try { cmdResult = cmd.execute(); } catch (CommandException e) { - logger.log( - SEVERE, - "Sandboxing requested, but it failed to execute 'true' as a self-check: " - + new String(cmdResult.getStderr(), UTF_8), - e); + logger.atSevere().withCause(e).log( + "Sandboxing requested, but it failed to execute 'true' as a self-check: %s", + new String(cmdResult.getStderr(), UTF_8)); System.exit(1); }
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorkerOptions.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorkerOptions.java index 78a040f..e219a0b 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorkerOptions.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorkerOptions.java
@@ -14,6 +14,7 @@ package com.google.devtools.build.remote.worker; +import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.actions.LocalHostCapacity; import com.google.devtools.build.lib.util.ResourceConverter; import com.google.devtools.common.options.Option; @@ -22,11 +23,10 @@ import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParsingException; import java.util.List; -import java.util.logging.Logger; /** Options for remote worker. */ public class RemoteWorkerOptions extends OptionsBase { - private static final Logger logger = Logger.getLogger(RemoteWorkerOptions.class.getName()); + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); @Option( name = "listen_port", @@ -197,12 +197,11 @@ String.format("Value '(%d)' must be at least %d.", value, minValue)); } if (value > maxValue) { - logger.warning( - String.format( - "Flag remoteWorker \"jobs\" ('%d') was set too high. " - + "This is a result of passing large values to --local_resources or --jobs. " - + "Using '%d' jobs", - value, maxValue)); + logger.atWarning().log( + "Flag remoteWorker \"jobs\" ('%d') was set too high. " + + "This is a result of passing large values to --local_resources or --jobs. " + + "Using '%d' jobs", + value, maxValue); value = maxValue; } return value;