Merge changes I4eb349fb,Ib51147ea,I14ae3f05
* changes:
Record in reject count metric whether the push was done by a service user
ReceiveCommits: Call ExceptionHook to get status for exceptions
ReceiveCommits: Log RuntimeException on severe level
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 1ca750b..a9b82d6 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -124,8 +124,10 @@
DELETE).
* `receivecommits/reject_count`: number of rejected pushes
** `kind`:
- The push kind ('magic' if it was a push for code review, 'direct' if it was
- a direct push or 'n/a' if the push kind couldn't be detected).
+ The push kind ('magic push'/'magic push by service user' if it was a push for
+ code review, 'direct push'/'direct push by service user' if it was a direct
+ push, 'magic push by service, 'magic or direct push'/'magic or direct push by
+ service user' if the push kind couldn't be detected).
** `reason`:
The rejection reason.
** `status`:
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 9f7b763..536e1fb 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -124,6 +124,7 @@
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AccountResolver.UnresolvableAccountException;
+import com.google.gerrit.server.account.ServiceUserClassifier;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.cancellation.RequestCancelledException;
import com.google.gerrit.server.cancellation.RequestStateContext;
@@ -234,6 +235,7 @@
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
+import java.util.logging.Level;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
@@ -426,6 +428,7 @@
private final Sequences seq;
private final SetHashtagsOp.Factory hashtagsFactory;
private final SetTopicOp.Factory setTopicFactory;
+ private final ServiceUserClassifier serviceUserClassifier;
private final ImmutableList<SubmissionListener> superprojectUpdateSubmissionListeners;
private final TagCache tagCache;
private final ProjectConfig.Factory projectConfigFactory;
@@ -516,6 +519,7 @@
Sequences seq,
SetHashtagsOp.Factory hashtagsFactory,
SetTopicOp.Factory setTopicFactory,
+ ServiceUserClassifier serviceUserClassifier,
@SuperprojectUpdateOnSubmission
ImmutableList<SubmissionListener> superprojectUpdateSubmissionListeners,
TagCache tagCache,
@@ -550,6 +554,7 @@
this.exceptionHooks = exceptionHooks;
this.hashtagsFactory = hashtagsFactory;
this.setTopicFactory = setTopicFactory;
+ this.serviceUserClassifier = serviceUserClassifier;
this.indexer = indexer;
this.initializers = initializers;
this.mergeOpProvider = mergeOpProvider;
@@ -762,8 +767,15 @@
logger.atFine().log("Processing commands done.");
} catch (RuntimeException e) {
String formattedCause = getFormattedCause(e).orElse(e.getClass().getSimpleName());
- logger.atFine().withCause(e).log("ReceiveCommits failed due to %s", formattedCause);
- metrics.rejectCount.increment("n/a", formattedCause, SC_INTERNAL_SERVER_ERROR);
+ int statusCode =
+ getStatus(e).map(ExceptionHook.Status::statusCode).orElse(SC_INTERNAL_SERVER_ERROR);
+ logger.at(statusCode < SC_INTERNAL_SERVER_ERROR ? Level.INFO : Level.SEVERE).withCause(e).log(
+ "ReceiveCommits failed due to %s", formattedCause);
+ String pushKind = "magic or direct push";
+ if (serviceUserClassifier.isServiceUser(user.getAccountId())) {
+ pushKind += " by service user";
+ }
+ metrics.rejectCount.increment(pushKind, formattedCause, statusCode);
throw e;
}
progress.end();
@@ -778,6 +790,14 @@
.findFirst();
}
+ private Optional<ExceptionHook.Status> getStatus(Throwable err) {
+ return exceptionHooks.stream()
+ .map(h -> h.getStatus(err))
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .findFirst();
+ }
+
// Process as many commands as possible, but may leave some commands in state NOT_ATTEMPTED.
private void processCommandsUnsafe(
Collection<ReceiveCommand> commands, MultiProgressMonitor progress) {
@@ -3934,10 +3954,13 @@
private void reject(ReceiveCommand cmd, RejectionReason reason) {
logger.atFine().log("Rejecting command '%s': %s", cmd, reason.why());
- metrics.rejectCount.increment(
- MagicBranch.isMagicBranch(cmd.getRefName()) ? "magic" : "direct",
- reason.metricBucket(),
- reason.statusCode());
+
+ String pushKind = (MagicBranch.isMagicBranch(cmd.getRefName()) ? "magic push" : "direct push");
+ if (serviceUserClassifier.isServiceUser(user.getAccountId())) {
+ pushKind += " by service user";
+ }
+ metrics.rejectCount.increment(pushKind, reason.metricBucket(), reason.statusCode());
+
cmd.setResult(REJECTED_OTHER_REASON, reason.why());
}
