Migrate pgm classes to Flogger

This is the next part of the migration to Flogger. This change migrates
all classes of the 'pgm' module to Flogger. Classes of most other
modules have blready een migrated by predecessor changes. Some remaining
modules continue to use slf4j. They should be migrated by follow-up
changes.

During this migration we try to make the log statements more consistent:
- avoid string concatenation
- avoid usage of String.format(...)

Change-Id: I51374131bbd97359ac4552df9878ff7f74c64e21
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2018-05-08 09:09:58 +02:00
parent 95bd9c2022
commit a8fec0e227
15 changed files with 88 additions and 80 deletions

View File

@ -51,11 +51,11 @@ java_library(
"//lib:servlet-api-3_1-without-neverlink",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
"//lib/guice:guice-servlet",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/log:api",
"//lib/log:jsonevent-layout",
"//lib/log:log4j",
"//lib/prolog:cafeteria",

View File

@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.elasticsearch.ElasticIndexModule;
import com.google.gerrit.extensions.client.AuthType;
@ -127,12 +128,10 @@ import javax.servlet.http.HttpServletRequest;
import org.eclipse.jgit.lib.Config;
import org.kohsuke.args4j.Option;
import org.kohsuke.args4j.spi.ExplicitBooleanOptionHandler;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Run SSH daemon portions of Gerrit. */
public class Daemon extends SiteProgram {
private static final Logger log = LoggerFactory.getLogger(Daemon.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Option(name = "--enable-httpd", usage = "Enable the internal HTTP daemon")
private Boolean httpd;
@ -249,7 +248,7 @@ public class Daemon extends SiteProgram {
new UncaughtExceptionHandler() {
@Override
public void uncaughtException(Thread t, Throwable e) {
log.error("Thread " + t.getName() + " threw exception", e);
logger.atSevere().withCause(e).log("Thread %s threw exception", t.getName());
}
});
@ -269,17 +268,17 @@ public class Daemon extends SiteProgram {
start();
RuntimeShutdown.add(
() -> {
log.info("caught shutdown, cleaning up");
logger.atInfo().log("caught shutdown, cleaning up");
stop();
});
log.info("Gerrit Code Review " + myVersion() + " ready");
logger.atInfo().log("Gerrit Code Review %s ready", myVersion());
if (runId != null) {
try {
Files.write(runFile, (runId + "\n").getBytes(UTF_8));
runFile.toFile().setReadable(true, false);
} catch (IOException err) {
log.warn("Cannot write --run-id to " + runFile, err);
logger.atWarning().withCause(err).log("Cannot write --run-id to %s", runFile);
}
}
@ -299,7 +298,7 @@ public class Daemon extends SiteProgram {
}
return 0;
} catch (Throwable err) {
log.error("Unable to start daemon", err);
logger.atSevere().withCause(err).log("Unable to start daemon");
return 1;
}
}
@ -366,7 +365,7 @@ public class Daemon extends SiteProgram {
try {
Files.delete(runFile);
} catch (IOException err) {
log.warn("failed to delete " + runFile, err);
logger.atWarning().withCause(err).log("failed to delete %s", runFile);
}
}
manager.stop();

View File

@ -14,6 +14,7 @@
package com.google.gerrit.pgm;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.launcher.GerritLauncher;
import java.io.File;
import java.io.IOException;
@ -24,11 +25,10 @@ import java.net.URL;
import java.net.URLClassLoader;
import java.util.ArrayList;
import java.util.Properties;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class JythonShell {
private static final Logger log = LoggerFactory.getLogger(JythonShell.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String STARTUP_RESOURCE = "com/google/gerrit/pgm/Startup.py";
private static final String STARTUP_FILE = "Startup.py";
@ -79,7 +79,7 @@ public class JythonShell {
try {
shell = console.getConstructor(new Class<?>[] {}).newInstance();
log.info("Jython shell instance created.");
logger.atInfo().log("Jython shell instance created.");
} catch (InstantiationException
| IllegalAccessException
| IllegalArgumentException
@ -170,10 +170,10 @@ public class JythonShell {
if (in != null) {
execStream(in, "resource " + p);
} else {
log.error("Cannot load resource " + p);
logger.atSevere().log("Cannot load resource %s", p);
}
} catch (IOException e) {
log.error(e.getMessage(), e);
logger.atSevere().withCause(e).log(e.getMessage());
}
}
@ -188,15 +188,16 @@ public class JythonShell {
new Class<?>[] {String.class},
new Object[] {script.getAbsolutePath()});
} else {
log.info(
"User initialization file "
+ script.getAbsolutePath()
+ " is not found or not executable");
logger
.atInfo()
.log(
"User initialization file %s is not found or not executable",
script.getAbsolutePath());
}
} catch (InvocationTargetException e) {
log.error("Exception occurred while loading file " + p + " : ", e);
logger.atSevere().withCause(e).log("Exception occurred while loading file %s", p);
} catch (SecurityException e) {
log.error("SecurityException occurred while loading file " + p + " : ", e);
logger.atSevere().withCause(e).log("SecurityException occurred while loading file %s", p);
}
}
@ -209,7 +210,7 @@ public class JythonShell {
new Class<?>[] {InputStream.class, String.class},
new Object[] {in, p});
} catch (InvocationTargetException e) {
log.error("Exception occurred while loading " + p + " : ", e);
logger.atSevere().withCause(e).log("Exception occurred while loading %s", p);
}
}

View File

@ -19,6 +19,7 @@ import static com.google.gerrit.server.schema.DataSourceProvider.Context.SINGLE_
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.IoUtil;
import com.google.gerrit.common.SiteLibraryLoaderUtil;
import com.google.gerrit.pgm.util.SiteProgram;
@ -40,10 +41,10 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.kohsuke.args4j.Option;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class SwitchSecureStore extends SiteProgram {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static String getSecureStoreClassFromGerritConfig(SitePaths sitePaths) {
FileBasedConfig cfg = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.DETECTED);
try {
@ -54,8 +55,6 @@ public class SwitchSecureStore extends SiteProgram {
return cfg.getString("gerrit", null, "secureStoreClass");
}
private static final Logger log = LoggerFactory.getLogger(SwitchSecureStore.class);
@Option(
name = "--new-secure-store-lib",
usage = "Path to new SecureStore implementation",
@ -68,7 +67,7 @@ public class SwitchSecureStore extends SiteProgram {
SitePaths sitePaths = new SitePaths(getSitePath());
Path newSecureStorePath = Paths.get(newSecureStoreLib);
if (!Files.exists(newSecureStorePath)) {
log.error("File {} doesn't exist", newSecureStorePath.toAbsolutePath());
logger.atSevere().log("File %s doesn't exist", newSecureStorePath.toAbsolutePath());
return -1;
}
@ -76,18 +75,22 @@ public class SwitchSecureStore extends SiteProgram {
String currentSecureStoreName = getCurrentSecureStoreClassName(sitePaths);
if (currentSecureStoreName.equals(newSecureStore)) {
log.error(
"Old and new SecureStore implementation names are the same. Migration will not work");
logger
.atSevere()
.log(
"Old and new SecureStore implementation names "
+ "are the same. Migration will not work");
return -1;
}
IoUtil.loadJARs(newSecureStorePath);
SiteLibraryLoaderUtil.loadSiteLib(sitePaths.lib_dir);
log.info(
"Current secureStoreClass property ({}) will be replaced with {}",
currentSecureStoreName,
newSecureStore);
logger
.atInfo()
.log(
"Current secureStoreClass property (%s) will be replaced with %s",
currentSecureStoreName, newSecureStore);
Injector dbInjector = createDbInjector(SINGLE_USER);
SecureStore currentStore = getSecureStore(currentSecureStoreName, dbInjector);
SecureStore newStore = getSecureStore(newSecureStore, dbInjector);
@ -103,7 +106,7 @@ public class SwitchSecureStore extends SiteProgram {
}
private void migrateProperties(SecureStore currentStore, SecureStore newStore) {
log.info("Migrate entries");
logger.atInfo().log("Migrate entries");
for (EntryKey key : currentStore.list()) {
String[] value = currentStore.getList(key.section, key.subsection, key.name);
if (value != null) {
@ -122,26 +125,35 @@ public class SwitchSecureStore extends SiteProgram {
private void removeOldLib(SitePaths sitePaths, String currentSecureStoreName) throws IOException {
Path oldSecureStore = findJarWithSecureStore(sitePaths, currentSecureStoreName);
if (oldSecureStore != null) {
log.info("Removing old SecureStore ({}) from lib/ directory", oldSecureStore.getFileName());
logger
.atInfo()
.log("Removing old SecureStore (%s) from lib/ directory", oldSecureStore.getFileName());
try {
Files.delete(oldSecureStore);
} catch (IOException e) {
log.error("Cannot remove {}", oldSecureStore.toAbsolutePath(), e);
logger.atSevere().withCause(e).log("Cannot remove %s", oldSecureStore.toAbsolutePath());
}
} else {
log.info(
"Cannot find jar with old SecureStore ({}) in lib/ directory", currentSecureStoreName);
logger
.atInfo()
.log(
"Cannot find jar with old SecureStore (%s) in lib/ directory",
currentSecureStoreName);
}
}
private void copyNewLib(SitePaths sitePaths, Path newSecureStorePath) throws IOException {
log.info("Copy new SecureStore ({}) into lib/ directory", newSecureStorePath.getFileName());
logger
.atInfo()
.log("Copy new SecureStore (%s) into lib/ directory", newSecureStorePath.getFileName());
Files.copy(newSecureStorePath, sitePaths.lib_dir.resolve(newSecureStorePath.getFileName()));
}
private void updateGerritConfig(SitePaths sitePaths, String newSecureStore)
throws IOException, ConfigInvalidException {
log.info("Set gerrit.secureStoreClass property of gerrit.config to {}", newSecureStore);
logger
.atInfo()
.log("Set gerrit.secureStoreClass property of gerrit.config to %s", newSecureStore);
FileBasedConfig config = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.DETECTED);
config.load();
config.setString("gerrit", null, "secureStoreClass", newSecureStore);
@ -197,7 +209,7 @@ public class SwitchSecureStore extends SiteProgram {
return jar;
}
} catch (IOException e) {
log.error(e.getMessage(), e);
logger.atSevere().withCause(e).log(e.getMessage());
}
}
return null;

View File

@ -17,6 +17,7 @@ package com.google.gerrit.pgm;
import static com.google.gerrit.pgm.init.InitPlugins.JAR;
import static com.google.gerrit.pgm.init.InitPlugins.PLUGIN_DIR;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.launcher.GerritLauncher;
import com.google.gerrit.pgm.init.PluginsDistribution;
import com.google.inject.Singleton;
@ -28,12 +29,10 @@ import java.util.Enumeration;
import java.util.List;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class WarDistribution implements PluginsDistribution {
private static final Logger log = LoggerFactory.getLogger(WarDistribution.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Override
public void foreach(Processor processor) throws IOException {
@ -53,8 +52,7 @@ public class WarDistribution implements PluginsDistribution {
try (InputStream in = zf.getInputStream(ze)) {
processor.process(pluginName, in);
} catch (IOException ioe) {
log.error(
String.format("Error opening plugin %s: %s", ze.getName(), ioe.getMessage()));
logger.atSevere().log("Error opening plugin %s: %s", ze.getName(), ioe.getMessage());
}
}
}

View File

@ -13,6 +13,7 @@ java_library(
"//java/com/google/gwtexpui/server",
"//lib:guava",
"//lib:servlet-api-3_1",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
"//lib/guice:guice-servlet",
@ -20,7 +21,6 @@ java_library(
"//lib/jetty:server",
"//lib/jetty:servlet",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/log:api",
"//lib/log:log4j",
],
)

View File

@ -17,6 +17,7 @@ package com.google.gerrit.pgm.http.jetty;
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gwtexpui.server.CacheHeaders;
import java.io.IOException;
import javax.servlet.ServletOutputStream;
@ -27,11 +28,9 @@ import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.HttpConnection;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
class HiddenErrorHandler extends ErrorHandler {
private static final Logger log = LoggerFactory.getLogger(HiddenErrorHandler.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Override
public void handle(
@ -79,7 +78,7 @@ class HiddenErrorHandler extends ErrorHandler {
if (!Strings.isNullOrEmpty(req.getQueryString())) {
uri += "?" + req.getQueryString();
}
log.error("Error in {} {}", req.getMethod(), uri, err);
logger.atSevere().withCause(err).log("Error in %s %s", req.getMethod(), uri);
}
}
}

View File

@ -23,9 +23,9 @@ java_library(
"//lib:gwtorm",
"//lib:h2",
"//lib/commons:validator",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/log:api",
],
)

View File

@ -21,6 +21,7 @@ import static com.google.inject.Stage.PRODUCTION;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Die;
import com.google.gerrit.common.IoUtil;
import com.google.gerrit.metrics.DisabledMetricMaker;
@ -75,12 +76,10 @@ import java.util.Collections;
import java.util.List;
import java.util.Set;
import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Initialize a new Gerrit installation. */
public class BaseInit extends SiteProgram {
private static final Logger log = LoggerFactory.getLogger(BaseInit.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final boolean standalone;
private final boolean initDb;
@ -205,7 +204,9 @@ public class BaseInit extends SiteProgram {
}
return names;
} catch (FileNotFoundException e) {
log.warn("Couldn't find distribution archive location. No plugin will be installed");
logger
.atWarning()
.log("Couldn't find distribution archive location. No plugin will be installed");
return null;
}
}

View File

@ -14,6 +14,7 @@
package com.google.gerrit.pgm.init.api;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.SitePaths;
@ -26,11 +27,9 @@ import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RepositoryCache;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class AllProjectsConfig extends VersionedMetaDataOnInit {
private static final Logger log = LoggerFactory.getLogger(AllProjectsConfig.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private Config cfg;
private GroupList groupList;
@ -64,7 +63,10 @@ public class AllProjectsConfig extends VersionedMetaDataOnInit {
return GroupList.parse(
new Project.NameKey(project),
readUTF8(GroupList.FILE_NAME),
error -> log.error("Error parsing file {}: {}", GroupList.FILE_NAME, error.getMessage()));
error ->
logger
.atSevere()
.log("Error parsing file %s: %s", GroupList.FILE_NAME, error.getMessage()));
}
public void save(String pluginName, String message) throws IOException, ConfigInvalidException {

View File

@ -9,9 +9,9 @@ java_library(
"//java/com/google/gerrit/server",
"//lib:guava",
"//lib:gwtorm",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/log:api",
],
)

View File

@ -24,7 +24,6 @@ java_library(
"//lib/flogger:api",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/log:api",
"//lib/log:jsonevent-layout",
"//lib/log:log4j",
],

View File

@ -17,6 +17,7 @@ package com.google.gerrit.pgm.util;
import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.common.flogger.FluentLogger;
import com.google.common.io.ByteStreams;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
@ -36,12 +37,10 @@ import java.time.temporal.ChronoUnit;
import java.util.concurrent.Future;
import java.util.zip.GZIPOutputStream;
import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Compresses the old error logs. */
public class LogFileCompressor implements Runnable {
private static final Logger log = LoggerFactory.getLogger(LogFileCompressor.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public static class Module extends LifecycleModule {
@Override
@ -113,10 +112,10 @@ public class LogFileCompressor implements Runnable {
}
}
} catch (IOException e) {
log.error("Error listing logs to compress in " + logs_dir, e);
logger.atSevere().withCause(e).log("Error listing logs to compress in %s", logs_dir);
}
} catch (Exception e) {
log.error("Failed to compress log files: " + e.getMessage(), e);
logger.atSevere().withCause(e).log("Failed to compress log files: %s", e.getMessage());
}
}
@ -156,11 +155,11 @@ public class LogFileCompressor implements Runnable {
}
Files.delete(src);
} catch (IOException e) {
log.error("Cannot compress " + src, e);
logger.atSevere().withCause(e).log("Cannot compress %s", src);
try {
Files.deleteIfExists(tmp);
} catch (IOException e2) {
log.warn("Failed to delete temporary log file " + tmp, e2);
logger.atWarning().withCause(e2).log("Failed to delete temporary log file %s", tmp);
}
}
}

View File

@ -14,10 +14,9 @@
package com.google.gerrit.pgm.util;
import com.google.common.flogger.FluentLogger;
import java.util.ArrayList;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class RuntimeShutdown {
private static final ShutdownCallback cb = new ShutdownCallback();
@ -45,7 +44,7 @@ public class RuntimeShutdown {
private RuntimeShutdown() {}
private static class ShutdownCallback extends Thread {
private static final Logger log = LoggerFactory.getLogger(ShutdownCallback.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final List<Runnable> tasks = new ArrayList<>();
private boolean shutdownStarted;
@ -72,7 +71,7 @@ public class RuntimeShutdown {
@Override
public void run() {
log.debug("Graceful shutdown requested");
logger.atFine().log("Graceful shutdown requested");
List<Runnable> taskList;
synchronized (this) {
@ -84,11 +83,11 @@ public class RuntimeShutdown {
try {
task.run();
} catch (Exception err) {
log.error("Cleanup task failed", err);
logger.atSevere().withCause(err).log("Cleanup task failed");
}
}
log.debug("Shutdown complete");
logger.atFine().log("Shutdown complete");
synchronized (this) {
shutdownComplete = true;

View File

@ -14,19 +14,18 @@
package com.google.gerrit.pgm.util;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.ThreadSettingsConfig;
import com.google.gerrit.server.schema.DataSourceType;
import com.google.inject.Injector;
import com.google.inject.Key;
import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
// TODO(dborowitz): Not necessary once we switch to NoteDb.
/** Utility to limit threads used by a batch program. */
public class ThreadLimiter {
private static final Logger log = LoggerFactory.getLogger(ThreadLimiter.class);
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public static int limitThreads(Injector dbInjector, int threads) {
return limitThreads(
@ -41,7 +40,7 @@ public class ThreadLimiter {
boolean usePool = cfg.getBoolean("database", "connectionpool", dst.usePool());
int poolLimit = threadSettingsConfig.getDatabasePoolLimit();
if (usePool && threads > poolLimit) {
log.warn("Limiting program to " + poolLimit + " threads due to database.poolLimit");
logger.atWarning().log("Limiting program to %d threads due to database.poolLimit", poolLimit);
return poolLimit;
}
return threads;