Use Flogger for writing logs
Using Flogger for logging is consistent with Gerrit core. Also Flogger
has a fluent API that is less error-prone and easier to read.
Signed-off-by: Edwin Kempin <[email protected]>
Change-Id: I891db9d59a907b786533162abb119ef16ca3a3c8
Reviewed-on: https://chromium-review.googlesource.com/c/infra/gerrit-plugins/git-numberer/+/3525115
Reviewed-by: Youssef Elghareeb <[email protected]>
diff --git a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/CommitValidator.java b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/CommitValidator.java
index 14a8ae7..8d785f8 100644
--- a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/CommitValidator.java
+++ b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/CommitValidator.java
@@ -7,6 +7,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.config.PluginConfig;
import java.io.IOException;
import java.util.ArrayList;
@@ -17,12 +18,10 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/** Validates new commit objects from single ref's point of view. */
public class CommitValidator {
- private static final Logger log = LoggerFactory.getLogger(CommitValidator.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
static boolean isEnabled(String ref, PluginConfig pluginConfig) {
return GitNumbererModule.isRefEnabled(
@@ -44,7 +43,7 @@
validateWhileSameOriginatingBranch(walk, ref, currentTip, newTip);
if (fromOtherBranch == null) {
- log.info("verified {} to be a valid orphan branch", ref);
+ logger.atInfo().log("verified %s to be a valid orphan branch", ref);
return;
}
if (!currentTip.equals(ObjectId.zeroId())) {
@@ -178,7 +177,7 @@
case 1:
RevCommit parent = commit.getParent(0);
walk.parseBody(parent);
- log.debug("validating {} with 1 parent {}", commit, parent);
+ logger.atFine().log("validating %s with 1 parent %s", commit, parent);
PositionFooters pFooters = extractAndValidateParentFooters(parent);
checkState(pFooters.position != null);
if (destBranch.equals(pFooters.position.branch)) {
diff --git a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGenerator.java b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGenerator.java
index 25b4952..de0047a 100644
--- a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGenerator.java
+++ b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGenerator.java
@@ -4,6 +4,7 @@
package com.googlesource.chromium.plugins.gitnumberer;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
@@ -16,11 +17,9 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class GitNumberFooterGenerator implements ChangeMessageModifier {
- private static final Logger log = LoggerFactory.getLogger(GitNumberFooterGenerator.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Inject private com.google.gerrit.server.config.PluginConfigFactory cfg;
@Inject private @PluginName String pluginName;
@@ -47,7 +46,7 @@
pluginConfig = cfg.getFromProjectConfig(project, pluginName);
} catch (NoSuchProjectException e) {
String msg = "Can't fetch config for project " + project;
- log.error(msg);
+ logger.atSevere().log(msg);
throw new IllegalStateException(msg, e);
}
if (!Generator.isEnabled(destination.branch(), pluginConfig)) {
@@ -55,24 +54,25 @@
}
if (baseCommit == null) {
- log.warn("baseCommit is null");
+ logger.atWarning().log("baseCommit is null");
return newCommitMessage;
}
if (baseCommit.getName() == null || original.getName() == null) {
- log.warn("baseCommit {} or original {} is null", baseCommit, original);
+ logger.atWarning().log("baseCommit %s or original %s is null", baseCommit, original);
return newCommitMessage;
}
if (baseCommit.getName().equals(original.getName())) {
- log.debug("same original {} and baseCommit {}", original, baseCommit);
+ logger.atFine().log("same original %s and baseCommit %s", original, baseCommit);
return newCommitMessage;
}
try {
- log.debug("generating footers for {} on top of {}", destination.branch(), baseCommit);
+ logger.atFine().log(
+ "generating footers for %s on top of %s", destination.branch(), baseCommit);
return Generator.childMessage(baseCommit, newCommitMessage, destination.branch());
} catch (GitNumberValidationError e) {
- log.info("{}, no generation", e.error);
+ logger.atInfo().log("%s, no generation", e.error);
failedAttempts.increment();
/*
* This means that parent commit has invalid footers, which implies
diff --git a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerifier.java b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerifier.java
index 830aad8..d656a79 100644
--- a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerifier.java
+++ b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerifier.java
@@ -4,6 +4,7 @@
package com.googlesource.chromium.plugins.gitnumberer;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.events.RefReceivedEvent;
@@ -26,14 +27,11 @@
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.ReceiveCommand.Type;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
class GitNumberFooterVerifier
implements RefOperationValidationListener, OnSubmitValidationListener {
-
- private static final Logger log = LoggerFactory.getLogger(GitNumberFooterVerifier.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Inject private com.google.gerrit.server.config.PluginConfigFactory cfg;
@@ -53,7 +51,8 @@
if (!CommitValidator.isEnabled(event.getRefName(), pluginConfig)) {
return Collections.emptyList();
}
- log.debug("validating refOp {}: proposed tip {}", event.getRefName(), event.command.getNewId());
+ logger.atFine().log(
+ "validating refOp %s: proposed tip %s", event.getRefName(), event.command.getNewId());
ObjectId currentTip = ObjectId.zeroId();
switch (event.command.getType()) {
@@ -107,7 +106,8 @@
}
ReceiveCommand cmd = args.getCommands().get(refName);
try {
- log.info("validating submit into {}: {} .. {}", refName, cmd.getOldId(), cmd.getNewId());
+ logger.atInfo().log(
+ "validating submit into %s: %s .. %s", refName, cmd.getOldId(), cmd.getNewId());
CommitValidator.validateMany(
args::getRef, args.getRevWalk(), config, refName, cmd.getOldId(), cmd.getNewId());
} catch (IOException e) {
diff --git a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumbererModule.java b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumbererModule.java
index dd1f830..e6766c6 100644
--- a/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumbererModule.java
+++ b/src/main/java/com/googlesource/chromium/plugins/gitnumberer/GitNumbererModule.java
@@ -4,6 +4,7 @@
package com.googlesource.chromium.plugins.gitnumberer;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType;
@@ -14,11 +15,9 @@
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
import com.google.gerrit.server.util.MagicBranch;
import com.google.inject.AbstractModule;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class GitNumbererModule extends AbstractModule {
- private static final Logger log = LoggerFactory.getLogger(CommitValidator.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
static boolean isRefEnabled(
String ref, String op, String[] enabledRefGlobs, String[] disabledRefGlobs) {
@@ -32,24 +31,24 @@
if (RefNames.REFS_CONFIG.equals(ref)
|| MagicBranch.isMagicBranch(ref)
|| ref.startsWith(RefNames.REFS_CHANGES)) {
- log.debug(
- "skipping {} on {} because it is magic or {} or a new change patchset",
- op,
- ref,
- RefNames.REFS_CONFIG);
+ logger.atFine().log(
+ "skipping %s on %s because it is magic or %s or a new change patchset",
+ op, ref, RefNames.REFS_CONFIG);
return false;
}
if (RefGlobMatcher.matches(ref, disabledRefGlobs)) {
- log.debug("skipping {} on {} because it's in disabled refs {}", op, ref, disabledRefGlobs);
+ logger.atFine().log(
+ "skipping %s on %s because it's in disabled refs %s", op, ref, disabledRefGlobs);
return false;
}
if (RefGlobMatcher.matches(ref, enabledRefGlobs)) {
- log.debug("{} enabled on {}", op, ref);
+ logger.atFine().log("%s enabled on %s", op, ref);
return true;
}
- log.debug("skipping {} on {} because it is not in enabled refs {}", op, ref, enabledRefGlobs);
+ logger.atFine().log(
+ "skipping %s on %s because it is not in enabled refs %s", op, ref, enabledRefGlobs);
return false;
}
diff --git a/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGeneratorIT.java b/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGeneratorIT.java
index a700afb..7afab2c 100644
--- a/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGeneratorIT.java
+++ b/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterGeneratorIT.java
@@ -3,6 +3,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.common.FooterConstants;
@@ -15,11 +16,9 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class GitNumberFooterGeneratorIT extends LightweightWithConfigIT {
- private static final Logger log = LoggerFactory.getLogger(GitNumberFooterGeneratorIT.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Override
protected void updateProjectInput(ProjectInput in) {
@@ -99,10 +98,9 @@
configurePlugin("refs/heads/main");
assertEnabledPlugin("refs/heads/main");
// Now we can push child and its message should be auto-generated.
- log.info(
- "ready to create a good change on top of good {} with message\n{}\n",
- r.getName(),
- r.getFullMessage());
+ logger.atInfo().log(
+ "ready to create a good change on top of good %s with message\n%s\n",
+ r.getName(), r.getFullMessage());
// Create Change
PushOneCommit.Result change = createChange("refs/for/main");
diff --git a/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerfierIT.java b/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerfierIT.java
index 1563d68..ecc6ce5 100644
--- a/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerfierIT.java
+++ b/src/test/java/com/googlesource/chromium/plugins/gitnumberer/GitNumberFooterVerfierIT.java
@@ -9,6 +9,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -22,11 +23,9 @@
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class GitNumberFooterVerfierIT extends LightweightWithConfigIT {
- private static final Logger log = LoggerFactory.getLogger(GitNumberFooterVerfierIT.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Inject private ProjectOperations projectOperations;
@@ -303,7 +302,7 @@
* @return PushOneCommit that has be pushed for commit to be created.
*/
PushOneCommit prepareOneCommit(String message) {
- log.info("creatingCommit: \n{}", message);
+ logger.atInfo().log("creatingCommit: \n%s", message);
return pushFactory.create(admin.newIdent(), testRepo, message, "file", message);
}
@@ -326,10 +325,9 @@
prepareOneCommit("Initial commit")
.to("refs/heads/main");
first.assertOkStatus();
- log.info(
- "Created initial commit {} with no footers (remote: {})",
- first.getCommit(),
- this.projectOperations.project(project).getHead("refs/heads/main"));
+ logger.atInfo().log(
+ "Created initial commit %s with no footers (remote: %s)",
+ first.getCommit(), this.projectOperations.project(project).getHead("refs/heads/main"));
// testRepo = cloneProject(project);
assertThat(this.projectOperations.project(project).getHead("main"))
.isEqualTo(first.getCommit());
@@ -345,10 +343,9 @@
prepareOneCommit("Start of main branch\n\nCr-Commit-Position: refs/heads/main@{#1}")
.to("refs/heads/main");
first.assertOkStatus();
- log.info(
- "Created initial commit {} with valid footers (remote: {})",
- first.getCommit(),
- this.projectOperations.project(project).getHead("refs/heads/main"));
+ logger.atInfo().log(
+ "Created initial commit %s with valid footers (remote: %s)",
+ first.getCommit(), this.projectOperations.project(project).getHead("refs/heads/main"));
// testRepo = cloneProject(project);
assertThat(this.projectOperations.project(project).getHead("main"))
.isEqualTo(first.getCommit());
diff --git a/src/test/java/com/googlesource/chromium/plugins/gitnumberer/LightweightWithConfigIT.java b/src/test/java/com/googlesource/chromium/plugins/gitnumberer/LightweightWithConfigIT.java
index 8342e82..b9d97a5 100644
--- a/src/test/java/com/googlesource/chromium/plugins/gitnumberer/LightweightWithConfigIT.java
+++ b/src/test/java/com/googlesource/chromium/plugins/gitnumberer/LightweightWithConfigIT.java
@@ -7,6 +7,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.server.config.PluginConfig;
@@ -19,15 +20,13 @@
import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@TestPlugin(
name = "git-numberer",
sysModule = "com.googlesource.chromium.plugins.gitnumberer.GitNumbererModule")
public class LightweightWithConfigIT extends LightweightPluginDaemonTest {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
protected static String pluginName = "git-numberer";
- private static final Logger log = LoggerFactory.getLogger(LightweightWithConfigIT.class);
@Inject private com.google.gerrit.server.config.PluginConfigFactory pluginCfgFactory;
@@ -115,8 +114,8 @@
projectConfig.commit(md);
projectCache.evict(project);
}
- log.info(
- "configured plugin {} on {} excluding {} for {}", pluginName, enabled, disabled, configFor);
+ logger.atInfo().log(
+ "configured plugin %s on %s excluding %s for %s", pluginName, enabled, disabled, configFor);
}
protected void assertEnabledPlugin(String ref) throws NoSuchProjectException {
