Migrating to fluent logging (green).
I did some minor clean-ups of the resulting logging statements (mostly moving/adding exceptions as causes that were missed), and a few other drive-bys.
PiperOrigin-RevId: 306429419
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
index 27f6691..9128729 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
@@ -16,6 +16,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.Iterables;
+import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
@@ -29,7 +30,6 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.logging.Logger;
import javax.annotation.Nullable;
/**
@@ -38,8 +38,7 @@
*/
public abstract class AbstractContainerizingSandboxedSpawn implements SandboxedSpawn {
- private static final Logger logger =
- Logger.getLogger(AbstractContainerizingSandboxedSpawn.class.getName());
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
private static final AtomicBoolean warnedAboutMovesBeingCopies = new AtomicBoolean(false);
@@ -193,13 +192,11 @@
target.getParentDirectory().createDirectoryAndParents();
if (FileSystemUtils.moveFile(source, target).equals(MoveResult.FILE_COPIED)) {
if (warnedAboutMovesBeingCopies.compareAndSet(false, true)) {
- logger.warning(
- "Moving files out of the sandbox (e.g. from "
- + source
- + " to "
- + target
+ logger.atWarning().log(
+ "Moving files out of the sandbox (e.g. from %s to %s"
+ ") had to be done with a file copy, which is detrimental to performance; are "
- + " the two trees in different file systems?");
+ + " the two trees in different file systems?",
+ source, target);
}
}
} else if (source.isDirectory()) {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD
index b7cac33..dff9604 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/BUILD
@@ -37,6 +37,7 @@
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/common/options",
"//third_party:auto_value",
+ "//third_party:flogger",
"//third_party:gson",
"//third_party:guava",
"//third_party:jsr305",
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/RealSandboxfsProcess.java b/src/main/java/com/google/devtools/build/lib/sandbox/RealSandboxfsProcess.java
index 8972a6d..2616a94 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/RealSandboxfsProcess.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/RealSandboxfsProcess.java
@@ -17,6 +17,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.shell.Subprocess;
import com.google.devtools.build.lib.shell.SubprocessBuilder;
import com.google.devtools.build.lib.util.OS;
@@ -26,7 +27,6 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
-import java.util.logging.Logger;
import javax.annotation.Nullable;
/**
@@ -39,7 +39,7 @@
*/
abstract class RealSandboxfsProcess implements SandboxfsProcess {
- private static final Logger log = Logger.getLogger(RealSandboxfsProcess.class.getName());
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
/**
* Contains the {@code --allow} flag to pass to sandboxfs.
@@ -81,8 +81,9 @@
try {
this.destroy();
} catch (Exception e) {
- log.warning("Failed to destroy running sandboxfs instance; mount point may have "
- + "been left behind: " + e);
+ logger.atWarning().withCause(e).log(
+ "Failed to destroy running sandboxfs instance; mount point may have "
+ + "been left behind");
}
});
Runtime.getRuntime().addShutdownHook(shutdownHook);
@@ -105,7 +106,7 @@
*/
static SandboxfsProcess mount(PathFragment binary, Path mountPoint, Path logFile)
throws IOException {
- log.info("Mounting sandboxfs (" + binary + ") onto " + mountPoint);
+ logger.atInfo().log("Mounting sandboxfs (%s) onto %s", binary, mountPoint);
GnuVersionParser<SemVer> parser = new GnuVersionParser<>("sandboxfs", SemVer::parse);
SemVer version;
@@ -172,13 +173,13 @@
try {
process.getOutputStream().close();
} catch (IOException e) {
- log.warning("Failed to close sandboxfs's stdin pipe: " + e);
+ logger.atWarning().withCause(e).log("Failed to close sandboxfs's stdin pipe");
}
try {
process.getInputStream().close();
} catch (IOException e) {
- log.warning("Failed to close sandboxfs's stdout pipe: " + e);
+ logger.atWarning().withCause(e).log("Failed to close sandboxfs's stdout pipe");
}
process.destroyAndWait();
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java
index f7a454e..6ec3fc6 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java
@@ -18,6 +18,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Joiner;
+import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.exec.TreeDeleter;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs;
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
@@ -31,7 +32,6 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
-import java.util.logging.Logger;
import javax.annotation.Nullable;
/**
@@ -39,7 +39,7 @@
* FUSE filesystem on the provided path.
*/
class SandboxfsSandboxedSpawn implements SandboxedSpawn {
- private static final Logger log = Logger.getLogger(SandboxfsSandboxedSpawn.class.getName());
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
/** Sequence number to assign a unique subtree to each action within the mount point. */
private static final AtomicInteger lastId = new AtomicInteger();
@@ -216,7 +216,7 @@
// We use independent subdirectories for each action, so a failure to unmap one, while
// annoying, is not a big deal. The sandboxfs instance will be unmounted anyway after
// the build, which will cause these to go away anyway.
- log.warning("Cannot unmap " + sandboxName + ": " + e);
+ logger.atWarning().withCause(e).log("Cannot unmap %s", sandboxName);
}
sandboxIsMapped = false;
}
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java
index 93a0696..19d742e 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxUtil.java
@@ -17,6 +17,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.GoogleLogger;
import com.google.common.io.ByteStreams;
import com.google.devtools.build.lib.shell.Subprocess;
import com.google.devtools.build.lib.shell.SubprocessBuilder;
@@ -32,11 +33,10 @@
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
-import java.util.logging.Logger;
/** Utility functions for the {@code windows-sandbox}. */
public final class WindowsSandboxUtil {
- private static final Logger log = Logger.getLogger(WindowsSandboxUtil.class.getName());
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
/**
* Checks if the given Windows sandbox binary is available and is valid.
@@ -55,7 +55,8 @@
.setWorkingDirectory(new File("."))
.start();
} catch (IOException e) {
- log.warning("Windows sandbox binary at " + binary + " seems to be missing; got error: " + e);
+ logger.atWarning().withCause(e).log(
+ "Windows sandbox binary at %s seems to be missing", binary);
return false;
}
@@ -79,13 +80,9 @@
// the DottedVersion logic from the Apple rules.
return true;
} else {
- log.warning(
- "Windows sandbox binary at "
- + binary
- + " returned non-zero exit code "
- + exitCode
- + " and output "
- + outErr);
+ logger.atWarning().log(
+ "Windows sandbox binary at %s returned non-zero exit code %d and output %s",
+ binary, exitCode, outErr);
return false;
}
}