opendev/gerrit
Code Issues Proposed changes

Set logging tag with plugin name on invocation of extension point

Browse Source 
With this logging tag one can easily see in the log file which logs have
been triggered by the plugin. For traces this is interesting because
then for actions such as loading files, doing index queries etc. one can
see if they have been triggered by a plugin. E.g. if there is an
extensive number of index queries that are triggered by a plugin we want
to see this in the trace.

Such logging tags are mostly useful for extension points that are likely
to have complex or expensive implementations. We may not want to set
such a logging tag for extension points which only have trivial
implementations.

Setting the logging tag with the plugin name when an extension point is
invoked is done by opening a trace context. To invoke extension points
with such a trace context they should be invoked through a plugin
context. The plugin context can be directly injected. Instead of
injecting DynamicItem/DynamicSet/DynamicMap you can now inject
PluginItemContext/PluginSetContext/PluginMapContext.

The plugin context classes offer methods to invoke the extension
points that automatically set the trace context. In addition they
provide functionality for catching and logging exceptions from plugins.
This makes the logging and exception handling for plugin invocations
more consistent across the code base and removes the need to have code
for this in multiple places.

Since exception handling with plugin contexts is easy now more plugin
invocations handle exceptions now which makes Gerrit more resilient
against plugin failures.

This change adapts calls to most important extensions points, such as
validators and listeners.

Change-Id: Iab41d0059049f06ca41b697e93aa6a1f9668de5b
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-08-24 15:49:36 +02:00
parent 92225eb622
commit 5eb0219eb3
57 changed files with 1589 additions and 594 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
java/com/google/gerrit/server/change/SetAssigneeOp.java
View File

@@ -17,7 +17,6 @@ package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change;
@@ -27,6 +26,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.AssigneeChanged;
import com.google.gerrit.server.mail.send.SetAssigneeSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -45,7 +45,7 @@ public class SetAssigneeOp implements BatchUpdateOp {
}
private final ChangeMessagesUtil cmUtil;
private final DynamicSet<AssigneeValidationListener> validationListeners;
private final PluginSetContext<AssigneeValidationListener> validationListeners;
private final IdentifiedUser newAssignee;
private final AssigneeChanged assigneeChanged;
private final SetAssigneeSender.Factory setAssigneeSenderFactory;
@@ -58,7 +58,7 @@ public class SetAssigneeOp implements BatchUpdateOp {
@Inject
SetAssigneeOp(
ChangeMessagesUtil cmUtil,
DynamicSet<AssigneeValidationListener> validationListeners,
PluginSetContext<AssigneeValidationListener> validationListeners,
AssigneeChanged assigneeChanged,
SetAssigneeSender.Factory setAssigneeSenderFactory,
Provider<IdentifiedUser> user,
@@ -80,11 +80,10 @@ public class SetAssigneeOp implements BatchUpdateOp {
return false;
}
try {
for (AssigneeValidationListener validator : validationListeners) {
validator.validateAssignee(change, newAssignee.getAccount());
}
validationListeners.runEach(
l -> l.validateAssignee(change, newAssignee.getAccount()), ValidationException.class);
} catch (ValidationException e) {
throw new ResourceConflictException(e.getMessage());
throw new ResourceConflictException(e.getMessage(), e);
}
if (change.getAssignee() != null) {