Use Flogger for writing logs
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ie3a64e3feb23b5b229a25df6956ada2818b31fe2
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java
index 9af6912..c8ac730 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/AutomergeChangeAction.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.automerger;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -29,14 +30,12 @@
import java.io.IOException;
import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/** Implementation behind the "Recreate Automerges" button. */
class AutomergeChangeAction
implements UiAction<RevisionResource>,
RestModifyView<RevisionResource, AutomergeChangeAction.Input> {
- private static final Logger log = LoggerFactory.getLogger(AutomergeChangeAction.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private Provider<CurrentUser> user;
private ConfigLoader config;
@@ -68,7 +67,7 @@
Change change = rev.getChange();
if (branchMap == null) {
- log.debug("Branch map is empty for change {}", change.getKey().get());
+ logger.atFine().log("Branch map is empty for change %s", change.getKey().get());
return Response.none();
}
String revision = rev.getPatchSet().commitId().name();
@@ -83,7 +82,7 @@
mdsMergeInput.obsoleteRevision = revision;
mdsMergeInput.currentRevision = revision;
- log.debug("Multiple downstream merge input: {}", mdsMergeInput.dsBranchMap);
+ logger.atFine().log("Multiple downstream merge input: %s", mdsMergeInput.dsBranchMap);
try {
dsCreator.createMergesAndHandleConflicts(mdsMergeInput);
@@ -116,7 +115,7 @@
desc = desc.setVisible(user.get() instanceof IdentifiedUser);
}
} catch (RestApiException | IOException | ConfigInvalidException e) {
- log.error("Failed to recreate automerges for {} on {}", project, branch);
+ logger.atSevere().log("Failed to recreate automerges for %s on %s", project, branch);
desc = desc.setVisible(false);
}
return desc;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
index e2abb1f..4d6dd8b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
@@ -17,6 +17,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.api.GerritApi;
@@ -39,13 +40,11 @@
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/** Class to read the config. */
@Singleton
public class ConfigLoader {
- private static final Logger log = LoggerFactory.getLogger(ConfigLoader.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String BRANCH_DELIMITER = ":";
private static final String DEFAULT_CONFLICT_MESSAGE = "Merge conflict found on ${branch}";
@@ -194,10 +193,10 @@
Set<String> projectSet = getManifestProjects(fromBranch, toBranch);
projectSet = applyConfig(fromBranch, toBranch, projectSet);
- log.debug("Project set for {} to {} is {}", fromBranch, toBranch, projectSet);
+ logger.atFine().log("Project set for %s to %s is %s", fromBranch, toBranch, projectSet);
return projectSet;
} catch (RestApiException | IOException e) {
- log.error("Error reading manifest for {}!", fromBranch, e);
+ logger.atSevere().withCause(e).log("Error reading manifest for %s!", fromBranch);
throw e;
}
}
@@ -362,7 +361,7 @@
ManifestReader manifestReader = new ManifestReader(branch, manifestConfig.asString());
return manifestReader.getProjects();
} catch (ResourceNotFoundException e) {
- log.debug("Manifest for {} not found", branch);
+ logger.atFine().log("Manifest for %s not found", branch);
return new HashSet<>();
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
index 63416a4..5b06a9d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Strings.isNullOrEmpty;
import com.google.common.base.Joiner;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AbandonInput;
@@ -55,8 +56,6 @@
import java.util.TreeMap;
import java.util.UUID;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* DownstreamCreator will receive an event on an uploaded, published, or restored patchset, and
@@ -70,7 +69,7 @@
CommentAddedListener,
RevisionCreatedListener,
TopicEditedListener {
- private static final Logger log = LoggerFactory.getLogger(DownstreamCreator.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String AUTOMERGER_TAG = "autogenerated:Automerger";
private static final String MERGE_CONFLICT_TAG = "autogenerated:MergeConflict";
private static final String SUBJECT_PREFIX = "automerger";
@@ -106,10 +105,11 @@
.revision(event.getRevision()._number)
.commit(false)
.commit;
- log.debug("Detected revision {} abandoned on {}.", revision, change.project);
+ logger.atFine().log("Detected revision %s abandoned on %s.", revision, change.project);
abandonDownstream(change, revision);
} catch (ConfigInvalidException | StorageException | RestApiException e) {
- log.error("Automerger plugin failed onChangeAbandoned for {}", event.getChange().id, e);
+ logger.atSevere().withCause(e).log(
+ "Automerger plugin failed onChangeAbandoned for %s", event.getChange().id);
}
}
@@ -130,7 +130,8 @@
.id(eventChange._number)
.get(EnumSet.of(ListChangesOption.CURRENT_REVISION));
} catch (RestApiException e) {
- log.error("Automerger could not get change with current revision for onTopicEdited: ", e);
+ logger.atSevere().withCause(e).log(
+ "Automerger could not get change with current revision for onTopicEdited.");
return;
}
String oldTopic = event.getOldTopic();
@@ -139,12 +140,13 @@
try {
downstreamBranches = config.getDownstreamBranches(change.branch, change.project);
} catch (RestApiException | IOException | ConfigInvalidException e) {
- log.error("Failed to edit downstream topics of {}", change.id, e);
+ logger.atSevere().withCause(e).log("Failed to edit downstream topics of %s", change.id);
return;
}
if (downstreamBranches.isEmpty()) {
- log.debug("Downstream branches of {} on {} are empty", change.branch, change.project);
+ logger.atFine().log(
+ "Downstream branches of %s on %s are empty", change.branch, change.project);
return;
}
@@ -159,7 +161,8 @@
reviewInput.notify = NotifyHandling.NONE;
gApi.changes().id(change._number).revision(CURRENT).review(reviewInput);
} catch (RestApiException e) {
- log.error("Failed to prevent setting empty topic for automerger plugin.", e);
+ logger.atSevere().withCause(e).log(
+ "Failed to prevent setting empty topic for automerger plugin.");
}
} else {
for (String downstreamBranch : downstreamBranches) {
@@ -167,16 +170,17 @@
List<Integer> existingDownstream =
getExistingMergesOnBranch(revision, oldTopic, downstreamBranch);
for (Integer changeNumber : existingDownstream) {
- log.debug("Setting topic {} on {}", change.topic, changeNumber);
+ logger.atFine().log("Setting topic %s on %s", change.topic, changeNumber);
gApi.changes().id(changeNumber).topic(change.topic);
}
} catch (RestApiException | InvalidQueryParameterException e) {
- log.error("Failed to edit downstream topics of {}", change.id, e);
+ logger.atSevere().withCause(e).log("Failed to edit downstream topics of %s", change.id);
}
}
}
} catch (StorageException | ConfigInvalidException e) {
- log.error("Automerger plugin failed onTopicEdited for {}", event.getChange().id, e);
+ logger.atSevere().withCause(e).log(
+ "Automerger plugin failed onTopicEdited for %s", event.getChange().id);
}
}
@@ -190,8 +194,8 @@
try (ManualRequestContext ctx = oneOffRequestContext.openAs(config.getContextUserId())) {
RevisionInfo eventRevision = event.getRevision();
if (!eventRevision.isCurrent) {
- log.info(
- "Not updating downstream votes since revision {} is not current.",
+ logger.atInfo().log(
+ "Not updating downstream votes since revision %s is not current.",
eventRevision._number);
return;
}
@@ -201,7 +205,8 @@
downstreamBranches = config.getDownstreamBranches(change.branch, change.project);
if (downstreamBranches.isEmpty()) {
- log.debug("Downstream branches of {} on {} are empty", change.branch, change.project);
+ logger.atFine().log(
+ "Downstream branches of %s on %s are empty", change.branch, change.project);
return;
}
@@ -233,11 +238,13 @@
}
}
} catch (RestApiException | InvalidQueryParameterException e) {
- log.error("Exception when updating downstream votes of {}", change.id, e);
+ logger.atSevere().withCause(e).log(
+ "Exception when updating downstream votes of %s", change.id);
}
}
} catch (StorageException | ConfigInvalidException | RestApiException | IOException e) {
- log.error("Automerger plugin failed onCommentAdded for {}", event.getChange().id, e);
+ logger.atSevere().withCause(e).log(
+ "Automerger plugin failed onCommentAdded for %s", event.getChange().id);
}
}
@@ -256,7 +263,8 @@
| ConfigInvalidException
| InvalidQueryParameterException
| StorageException e) {
- log.error("Automerger plugin failed onChangeRestored for {}", event.getChange().id, e);
+ logger.atSevere().withCause(e).log(
+ "Automerger plugin failed onChangeRestored for %s", event.getChange().id);
}
}
@@ -275,7 +283,8 @@
| ConfigInvalidException
| InvalidQueryParameterException
| StorageException e) {
- log.error("Automerger plugin failed onRevisionCreated for {}", event.getChange().id, e);
+ logger.atSevere().withCause(e).log(
+ "Automerger plugin failed onRevisionCreated for %s", event.getChange().id);
}
}
@@ -284,7 +293,7 @@
try (ManualRequestContext ctx = oneOffRequestContext.openAs(config.getContextUserId())) {
if (isNullOrEmpty(topic)) {
topic = "am-" + UUID.randomUUID();
- log.debug("Setting original change {} topic to {}", sourceId, topic);
+ logger.atFine().log("Setting original change %s topic to %s", sourceId, topic);
gApi.changes().id(sourceId).topic(topic);
}
return topic;
@@ -367,10 +376,9 @@
getExistingMergesOnBranch(
mdsMergeInput.obsoleteRevision, mdsMergeInput.topic, downstreamBranch);
if (!existingDownstream.isEmpty()) {
- log.debug(
- "Attempting to update downstream merge of {} on branch {}",
- mdsMergeInput.currentRevision,
- downstreamBranch);
+ logger.atFine().log(
+ "Attempting to update downstream merge of %s on branch %s",
+ mdsMergeInput.currentRevision, downstreamBranch);
// existingDownstream should almost always be of length one, but
// it's possible to construct it so that it's not
for (Integer dsChangeNumber : existingDownstream) {
@@ -385,18 +393,17 @@
createDownstreams = false;
} catch (MergeConflictException e) {
failedMergeBranchMap.put(downstreamBranch, e.getMessage());
- log.debug(
- "Abandoning existing, obsolete {} due to merge conflict.", dsChangeNumber);
+ logger.atFine().log(
+ "Abandoning existing, obsolete %s due to merge conflict.", dsChangeNumber);
abandonChange(dsChangeNumber);
}
}
}
}
if (createDownstreams) {
- log.debug(
- "Attempting to create downstream merge of {} on branch {}",
- mdsMergeInput.currentRevision,
- downstreamBranch);
+ logger.atFine().log(
+ "Attempting to create downstream merge of %s on branch %s",
+ mdsMergeInput.currentRevision, downstreamBranch);
SingleDownstreamMergeInput sdsMergeInput = new SingleDownstreamMergeInput();
sdsMergeInput.currentRevision = mdsMergeInput.currentRevision;
sdsMergeInput.changeNumber = mdsMergeInput.changeNumber;
@@ -482,10 +489,9 @@
String currentTopic = getOrSetTopic(sdsMergeInput.changeNumber, sdsMergeInput.topic);
if (isAlreadyMerged(sdsMergeInput, currentTopic)) {
- log.info(
- "Commit {} already merged into {}, not automerging again.",
- sdsMergeInput.currentRevision,
- sdsMergeInput.downstreamBranch);
+ logger.atInfo().log(
+ "Commit %s already merged into %s, not automerging again.",
+ sdsMergeInput.currentRevision, sdsMergeInput.downstreamBranch);
return;
}
@@ -493,7 +499,7 @@
mergeInput.source = sdsMergeInput.currentRevision;
mergeInput.strategy = "recursive";
- log.debug("Creating downstream merge for {}", sdsMergeInput.currentRevision);
+ logger.atFine().log("Creating downstream merge for %s", sdsMergeInput.currentRevision);
ChangeInput downstreamChangeInput = new ChangeInput();
downstreamChangeInput.project = sdsMergeInput.project;
downstreamChangeInput.branch = sdsMergeInput.downstreamBranch;
@@ -513,10 +519,9 @@
downstreamChangeInput.subject =
getSubjectForDownstreamMerge(
sdsMergeInput.subject, sdsMergeInput.currentRevision, true);
- log.debug(
- "Skipping merge for {} to {}",
- sdsMergeInput.currentRevision,
- sdsMergeInput.downstreamBranch);
+ logger.atFine().log(
+ "Skipping merge for %s to %s",
+ sdsMergeInput.currentRevision, sdsMergeInput.downstreamBranch);
}
ChangeApi downstreamChange = gApi.changes().create(downstreamChangeInput);
@@ -539,7 +544,7 @@
private String getBaseChangeId(List<String> parents, String branch)
throws InvalidQueryParameterException, RestApiException {
if (parents.isEmpty()) {
- log.info("No base change id for change with no parents.");
+ logger.atInfo().log("No base change id for change with no parents.");
return null;
}
// 1) Get topic of first parent
@@ -565,13 +570,14 @@
StorageException {
String currentRevision =
gApi.changes().id(change._number).revision(revisionInfo._number).commit(false).commit;
- log.debug(
- "Handling patchsetevent with change id {} and revision {}", change.id, currentRevision);
+ logger.atFine().log(
+ "Handling patchsetevent with change id %s and revision %s", change.id, currentRevision);
Set<String> downstreamBranches = config.getDownstreamBranches(change.branch, change.project);
if (downstreamBranches.isEmpty()) {
- log.debug("Downstream branches of {} on {} are empty", change.branch, change.project);
+ logger.atFine().log(
+ "Downstream branches of %s on %s are empty", change.branch, change.project);
return;
}
@@ -581,7 +587,7 @@
boolean isSkipMerge = config.isSkipMerge(change.branch, downstreamBranch, change.subject);
dsBranchMap.put(downstreamBranch, !isSkipMerge);
}
- log.debug("Automerging change {} from branch {}", change.id, change.branch);
+ logger.atFine().log("Automerging change %s from branch %s", change.id, change.branch);
ChangeApi currentChange = gApi.changes().id(change._number);
String previousRevision = getPreviousRevision(currentChange, revisionInfo._number);
@@ -604,25 +610,26 @@
try {
Set<String> downstreamBranches = config.getDownstreamBranches(change.branch, change.project);
if (downstreamBranches.isEmpty()) {
- log.debug("Downstream branches of {} on {} are empty", change.branch, change.project);
+ logger.atFine().log(
+ "Downstream branches of %s on %s are empty", change.branch, change.project);
return;
}
for (String downstreamBranch : downstreamBranches) {
List<Integer> existingDownstream =
getExistingMergesOnBranch(revision, change.topic, downstreamBranch);
- log.debug("Abandoning existing downstreams: {}", existingDownstream);
+ logger.atFine().log("Abandoning existing downstreams: %s", existingDownstream);
for (Integer changeNumber : existingDownstream) {
abandonChange(changeNumber);
}
}
} catch (RestApiException | IOException | InvalidQueryParameterException e) {
- log.error("Failed to abandon downstreams of {}", change.id, e);
+ logger.atSevere().withCause(e).log("Failed to abandon downstreams of %s", change.id);
}
}
private void updateVote(ChangeInfo change, String label, short vote) throws RestApiException {
- log.debug("Giving {} for label {} to {}", vote, label, change.id);
+ logger.atFine().log("Giving %s for label %s to %s", vote, label, change.id);
// Vote on all downstream branches unless merge conflict.
ReviewInput reviewInput = new ReviewInput();
Map<String, Short> labels = new HashMap<>();
@@ -633,7 +640,7 @@
try {
gApi.changes().id(change.id).revision(CURRENT).review(reviewInput);
} catch (AuthException e) {
- log.error("Automerger could not set label, but still continuing.", e);
+ logger.atSevere().withCause(e).log("Automerger could not set label, but still continuing.");
}
}
@@ -645,7 +652,7 @@
try {
gApi.changes().id(change.id).revision(CURRENT).review(reviewInput);
} catch (AuthException e) {
- log.error("Automerger could not set label, but still continuing.", e);
+ logger.atSevere().withCause(e).log("Automerger could not set label, but still continuing.");
}
}
@@ -668,7 +675,7 @@
mergeInput.strategy = "ours";
mergePatchSetInput.subject =
getSubjectForDownstreamMerge(upstreamSubject, newParentRevision, true);
- log.debug("Skipping merge for {} on {}", newParentRevision, sourceNum);
+ logger.atFine().log("Skipping merge for %s on %s", newParentRevision, sourceNum);
}
mergePatchSetInput.merge = mergeInput;
@@ -737,7 +744,7 @@
}
private void abandonChange(Integer changeNumber) throws RestApiException {
- log.debug("Abandoning change: {}", changeNumber);
+ logger.atFine().log("Abandoning change: %s", changeNumber);
AbandonInput abandonInput = new AbandonInput();
abandonInput.notify = NotifyHandling.NONE;
abandonInput.message = "Merge parent updated; abandoning due to upstream conflict.";
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ManifestReader.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ManifestReader.java
index a71e1c0..37b6f92 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/ManifestReader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ManifestReader.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.automerger;
+import com.google.common.flogger.FluentLogger;
import java.io.IOException;
import java.io.StringReader;
import java.util.HashSet;
@@ -21,8 +22,6 @@
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
@@ -32,7 +31,7 @@
/** Class to read a repo manifest. */
public class ManifestReader {
- private static final Logger log = LoggerFactory.getLogger(ManifestReader.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final String manifestString;
private final String branch;
@@ -79,7 +78,7 @@
}
} catch (SAXException | ParserConfigurationException | IOException e) {
- log.error("Exception on manifest for branch {}", branch, e);
+ logger.atSevere().withCause(e).log("Exception on manifest for branch %s", branch);
}
return projectSet;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java
index 6a27eeb..16459a6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java
@@ -16,6 +16,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.PatchSet;
@@ -41,15 +42,13 @@
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Repository;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* MergeValidator will validate that all downstream changes are uploaded for review before
* submission.
*/
public class MergeValidator implements MergeValidationListener {
- private static final Logger log = LoggerFactory.getLogger(MergeValidator.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
protected GerritApi gApi;
protected ConfigLoader config;
@@ -82,7 +81,7 @@
| IOException
| ConfigInvalidException
| InvalidQueryParameterException e) {
- log.error("Automerger plugin failed onPreMerge for {}", changeId, e);
+ logger.atSevere().withCause(e).log("Automerger plugin failed onPreMerge for %s", changeId);
e.printStackTrace();
throw new MergeValidationException("Error when validating merge for: " + changeId);
}
