Merge changes I7401c1bc,If420ca31,Ic328c4d6
* changes:
Add a request listener that sets a logging tag with the project
TraceIT: Add JavaDoc that explains how the tests are working
Add extension points that allows to listen to incoming requests
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 72b92a7..f0aedbd 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2810,6 +2810,12 @@
end of a request (REST call, SSH call, git push). Implementors can write the
execution times into a performance log for further analysis.
+[[request-listener]]
+== Request Listener
+
+`com.google.gerrit.server.RequestListener` is an extension point that is
+invoked each time the server executes a request from a user.
+
[[plugins_hosting]]
== Plugins source code hosting
diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD
index 5009211..f86b35d5 100644
--- a/java/com/google/gerrit/httpd/BUILD
+++ b/java/com/google/gerrit/httpd/BUILD
@@ -13,6 +13,7 @@
"//java/com/google/gerrit/exceptions",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/git",
+ "//java/com/google/gerrit/index:query_exception",
"//java/com/google/gerrit/json",
"//java/com/google/gerrit/launcher",
"//java/com/google/gerrit/lifecycle",
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index 23afbd3..d66e8ac 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -26,6 +26,8 @@
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.RequestInfo;
+import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.audit.HttpAuditEvent;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
@@ -35,6 +37,7 @@
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
import com.google.gerrit.server.git.validators.UploadValidators;
import com.google.gerrit.server.group.GroupAuditService;
+import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -339,6 +342,7 @@
private final Provider<CurrentUser> userProvider;
private final GroupAuditService groupAuditService;
private final Metrics metrics;
+ private final PluginSetContext<RequestListener> requestListeners;
@Inject
UploadFilter(
@@ -346,12 +350,14 @@
PermissionBackend permissionBackend,
Provider<CurrentUser> userProvider,
GroupAuditService groupAuditService,
- Metrics metrics) {
+ Metrics metrics,
+ PluginSetContext<RequestListener> requestListeners) {
this.uploadValidatorsFactory = uploadValidatorsFactory;
this.permissionBackend = permissionBackend;
this.userProvider = userProvider;
this.groupAuditService = groupAuditService;
this.metrics = metrics;
+ this.requestListeners = requestListeners;
}
@Override
@@ -369,7 +375,14 @@
HttpServletRequest httpRequest = (HttpServletRequest) request;
String sessionId = httpRequest.getSession().getId();
- try {
+ try (TraceContext traceContext = TraceContext.open()) {
+ RequestInfo requestInfo =
+ RequestInfo.builder(
+ RequestInfo.RequestType.GIT_UPLOAD, userProvider.get(), traceContext)
+ .project(state.getNameKey())
+ .build();
+ requestListeners.runEach(l -> l.onRequest(requestInfo));
+
try {
perm.check(ProjectPermission.RUN_UPLOAD_PACK);
} catch (AuthException e) {
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index a4d1534..736018a 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -102,22 +102,30 @@
import com.google.gerrit.httpd.WebSession;
import com.google.gerrit.httpd.restapi.ParameterParser.QueryParams;
import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.OptionUtil;
+import com.google.gerrit.server.RequestInfo;
+import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.audit.ExtendedHttpAuditEvent;
import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.GroupAuditService;
import com.google.gerrit.server.logging.PerformanceLogContext;
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.quota.QuotaException;
+import com.google.gerrit.server.restapi.change.ChangesCollection;
+import com.google.gerrit.server.restapi.project.ProjectsCollection;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.util.http.CacheHeaders;
@@ -227,6 +235,7 @@
final Provider<CurrentUser> currentUser;
final DynamicItem<WebSession> webSession;
final Provider<ParameterParser> paramParser;
+ final PluginSetContext<RequestListener> requestListeners;
final PermissionBackend permissionBackend;
final GroupAuditService auditService;
final RestApiMetrics metrics;
@@ -234,27 +243,32 @@
final RestApiQuotaEnforcer quotaChecker;
final Config config;
final DynamicSet<PerformanceLogger> performanceLoggers;
+ final ChangeFinder changeFinder;
@Inject
Globals(
Provider<CurrentUser> currentUser,
DynamicItem<WebSession> webSession,
Provider<ParameterParser> paramParser,
+ PluginSetContext<RequestListener> requestListeners,
PermissionBackend permissionBackend,
GroupAuditService auditService,
RestApiMetrics metrics,
RestApiQuotaEnforcer quotaChecker,
@GerritServerConfig Config config,
- DynamicSet<PerformanceLogger> performanceLoggers) {
+ DynamicSet<PerformanceLogger> performanceLoggers,
+ ChangeFinder changeFinder) {
this.currentUser = currentUser;
this.webSession = webSession;
this.paramParser = paramParser;
+ this.requestListeners = requestListeners;
this.permissionBackend = permissionBackend;
this.auditService = auditService;
this.metrics = metrics;
this.quotaChecker = quotaChecker;
this.config = config;
this.performanceLoggers = performanceLoggers;
+ this.changeFinder = changeFinder;
allowOrigin = makeAllowOrigin(config);
}
@@ -301,6 +315,11 @@
ViewData viewData = null;
try (TraceContext traceContext = enableTracing(req, res)) {
+ List<IdString> path = splitPath(req);
+
+ RequestInfo requestInfo = createRequestInfo(traceContext, path);
+ globals.requestListeners.runEach(l -> l.onRequest(requestInfo));
+
try (PerThreadCache ignored = PerThreadCache.create()) {
// It's important that the PerformanceLogContext is closed before the response is sent to
// the client. Only this way it is ensured that the invocation of the PerformanceLogger
@@ -329,7 +348,6 @@
}
checkUserSession(req);
- List<IdString> path = splitPath(req);
RestCollection<RestResource, RestResource> rc = members.get();
globals
.permissionBackend
@@ -1406,6 +1424,27 @@
return traceContext;
}
+ private RequestInfo createRequestInfo(TraceContext traceContext, List<IdString> path) {
+ RequestInfo.Builder requestInfo =
+ RequestInfo.builder(RequestInfo.RequestType.REST, globals.currentUser.get(), traceContext);
+
+ if (path.size() < 1) {
+ return requestInfo.build();
+ }
+
+ RestCollection<?, ?> rootCollection = members.get();
+ String resourceId = path.get(0).get();
+ if (rootCollection instanceof ProjectsCollection) {
+ requestInfo.project(Project.nameKey(resourceId));
+ } else if (rootCollection instanceof ChangesCollection) {
+ ChangeNotes changeNotes = globals.changeFinder.findOne(resourceId);
+ if (changeNotes != null) {
+ requestInfo.project(changeNotes.getProjectName());
+ }
+ }
+ return requestInfo.build();
+ }
+
private boolean isDelete(HttpServletRequest req) {
return "DELETE".equals(req.getMethod());
}
diff --git a/java/com/google/gerrit/server/RequestInfo.java b/java/com/google/gerrit/server/RequestInfo.java
new file mode 100644
index 0000000..c86d34d
--- /dev/null
+++ b/java/com/google/gerrit/server/RequestInfo.java
@@ -0,0 +1,85 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.logging.TraceContext;
+import java.util.Optional;
+
+/** Information about a request that was received from a user. */
+@AutoValue
+public abstract class RequestInfo {
+ /** Channel through which a user request was received. */
+ public enum RequestType {
+ /** request type for git push */
+ GIT_RECEIVE,
+
+ /** request type for git fetch */
+ GIT_UPLOAD,
+
+ /** request type for call to REST API */
+ REST,
+
+ /** request type for call to SSH API */
+ SSH
+ }
+
+ /**
+ * Type of the request, telling through which channel the request was coming in.
+ *
+ * <p>See {@link RequestType} for the types that are used by Gerrit core. Other request types are
+ * possible, e.g. if a plugin supports receiving requests through another channel.
+ */
+ public abstract String requestType();
+
+ /** The user that has sent the request. */
+ public abstract CurrentUser callingUser();
+
+ /** The trace context of the request. */
+ public abstract TraceContext traceContext();
+
+ /**
+ * The name of the project for which the request is being done. Only available if the request is
+ * tied to a project or change. If a project is available it's not guaranteed that it actually
+ * exists (e.g. if a user made a request for a project that doesn't exist).
+ */
+ public abstract Optional<Project.NameKey> project();
+
+ public static RequestInfo.Builder builder(
+ RequestType requestType, CurrentUser callingUser, TraceContext traceContext) {
+ return new AutoValue_RequestInfo.Builder()
+ .requestType(requestType)
+ .callingUser(callingUser)
+ .traceContext(traceContext);
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder requestType(String requestType);
+
+ public Builder requestType(RequestType requestType) {
+ return requestType(requestType.name());
+ }
+
+ public abstract Builder callingUser(CurrentUser callingUser);
+
+ public abstract Builder traceContext(TraceContext traceContext);
+
+ public abstract Builder project(Project.NameKey projectName);
+
+ public abstract RequestInfo build();
+ }
+}
diff --git a/java/com/google/gerrit/server/RequestListener.java b/java/com/google/gerrit/server/RequestListener.java
new file mode 100644
index 0000000..461b91a
--- /dev/null
+++ b/java/com/google/gerrit/server/RequestListener.java
@@ -0,0 +1,22 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+
+@ExtensionPoint
+public interface RequestListener {
+ void onRequest(RequestInfo requestInfo);
+}
diff --git a/java/com/google/gerrit/server/TraceRequestListener.java b/java/com/google/gerrit/server/TraceRequestListener.java
new file mode 100644
index 0000000..1ae39e1
--- /dev/null
+++ b/java/com/google/gerrit/server/TraceRequestListener.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server;
+
+import com.google.inject.Singleton;
+
+@Singleton
+public class TraceRequestListener implements RequestListener {
+ @Override
+ public void onRequest(RequestInfo requestInfo) {
+ requestInfo.project().ifPresent(p -> requestInfo.traceContext().addTag("project", p));
+ }
+}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index afe4e52..bbc5748 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -79,6 +79,8 @@
import com.google.gerrit.server.CreateGroupPermissionSyncer;
import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.RequestListener;
+import com.google.gerrit.server.TraceRequestListener;
import com.google.gerrit.server.account.AccountCacheImpl;
import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountDeactivator;
@@ -384,6 +386,8 @@
DynamicSet.setOf(binder(), SubmitRule.class);
DynamicSet.setOf(binder(), QuotaEnforcer.class);
DynamicSet.setOf(binder(), PerformanceLogger.class);
+ DynamicSet.setOf(binder(), RequestListener.class);
+ DynamicSet.bind(binder(), RequestListener.class).to(TraceRequestListener.class);
DynamicMap.mapOf(binder(), MailFilter.class);
bind(MailFilter.class).annotatedWith(Exports.named("ListMailFilter")).to(ListMailFilter.class);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index a474c1a..09c1b40 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -101,6 +101,8 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.PublishCommentUtil;
+import com.google.gerrit.server.RequestInfo;
+import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.NotifyResolver;
@@ -329,6 +331,7 @@
private final ReceiveConfig receiveConfig;
private final RefOperationValidators.Factory refValidatorsFactory;
private final ReplaceOp.Factory replaceOpFactory;
+ private final PluginSetContext<RequestListener> requestListeners;
private final RetryHelper retryHelper;
private final RequestScopePropagator requestScopePropagator;
private final Sequences seq;
@@ -407,6 +410,7 @@
ReceiveConfig receiveConfig,
RefOperationValidators.Factory refValidatorsFactory,
ReplaceOp.Factory replaceOpFactory,
+ PluginSetContext<RequestListener> requestListeners,
RetryHelper retryHelper,
RequestScopePropagator requestScopePropagator,
Sequences seq,
@@ -452,6 +456,7 @@
this.receiveConfig = receiveConfig;
this.refValidatorsFactory = refValidatorsFactory;
this.replaceOpFactory = replaceOpFactory;
+ this.requestListeners = requestListeners;
this.retryHelper = retryHelper;
this.requestScopePropagator = requestScopePropagator;
this.seq = seq;
@@ -537,6 +542,11 @@
newTimer("processCommands", Metadata.builder().resourceCount(commandCount));
PerformanceLogContext performanceLogContext =
new PerformanceLogContext(config, performanceLoggers)) {
+ RequestInfo requestInfo =
+ RequestInfo.builder(RequestInfo.RequestType.GIT_RECEIVE, user, traceContext)
+ .project(project.getNameKey())
+ .build();
+ requestListeners.runEach(l -> l.onRequest(requestInfo));
traceContext.addTag(RequestId.Type.RECEIVE_ID, new RequestId(project.getNameKey().get()));
// Log the push options here, rather than in parsePushOptions(), so that they are included
diff --git a/java/com/google/gerrit/server/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index 441d104..06db7b4 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -243,6 +243,15 @@
return this;
}
+ public boolean isTracing() {
+ return LoggingContext.getInstance().isLoggingForced();
+ }
+
+ public Optional<String> getTraceId() {
+ return LoggingContext.getInstance().getTagsAsMap().get(RequestId.Type.TRACE_ID.name()).stream()
+ .findFirst();
+ }
+
@Override
public void close() {
for (Table.Cell<String, String, Boolean> cell : tags.cellSet()) {
diff --git a/java/com/google/gerrit/sshd/SshCommand.java b/java/com/google/gerrit/sshd/SshCommand.java
index 2bbdc49..2590188 100644
--- a/java/com/google/gerrit/sshd/SshCommand.java
+++ b/java/com/google/gerrit/sshd/SshCommand.java
@@ -16,10 +16,13 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.AccessPath;
+import com.google.gerrit.server.RequestInfo;
+import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.logging.PerformanceLogContext;
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import java.io.IOException;
import java.io.PrintWriter;
@@ -29,6 +32,7 @@
public abstract class SshCommand extends BaseCommand {
@Inject private DynamicSet<PerformanceLogger> performanceLoggers;
+ @Inject private PluginSetContext<RequestListener> requestListeners;
@Inject @GerritServerConfig private Config config;
@Option(name = "--trace", usage = "enable request tracing")
@@ -50,6 +54,9 @@
try (TraceContext traceContext = enableTracing();
PerformanceLogContext performanceLogContext =
new PerformanceLogContext(config, performanceLoggers)) {
+ RequestInfo requestInfo =
+ RequestInfo.builder(RequestInfo.RequestType.SSH, user, traceContext).build();
+ requestListeners.runEach(l -> l.onRequest(requestInfo));
SshCommand.this.run();
} finally {
stdout.flush();
diff --git a/java/com/google/gerrit/sshd/commands/Upload.java b/java/com/google/gerrit/sshd/commands/Upload.java
index 24a6975..a22cdaf 100644
--- a/java/com/google/gerrit/sshd/commands/Upload.java
+++ b/java/com/google/gerrit/sshd/commands/Upload.java
@@ -17,15 +17,19 @@
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.RequestInfo;
+import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.validators.UploadValidationException;
import com.google.gerrit.server.git.validators.UploadValidators;
+import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.sshd.AbstractGitCommand;
import com.google.gerrit.sshd.SshSession;
import com.google.inject.Inject;
@@ -43,6 +47,7 @@
@Inject private DynamicSet<PreUploadHook> preUploadHooks;
@Inject private DynamicSet<PostUploadHook> postUploadHooks;
@Inject private DynamicSet<UploadPackInitializer> uploadPackInitializers;
+ @Inject private PluginSetContext<RequestListener> requestListeners;
@Inject private UploadValidators.Factory uploadValidatorsFactory;
@Inject private SshSession session;
@Inject private PermissionBackend permissionBackend;
@@ -73,7 +78,13 @@
for (UploadPackInitializer initializer : uploadPackInitializers) {
initializer.init(projectState.getNameKey(), up);
}
- try {
+ try (TraceContext traceContext = TraceContext.open()) {
+ RequestInfo requestInfo =
+ RequestInfo.builder(RequestInfo.RequestType.GIT_UPLOAD, user, traceContext)
+ .project(projectState.getNameKey())
+ .build();
+ requestListeners.runEach(l -> l.onRequest(requestInfo));
+
up.upload(in, out, err);
session.setPeerAgent(up.getPeerUserAgent());
} catch (UploadValidationException e) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 95f3015..50d6dff 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -16,16 +16,20 @@
import static com.google.common.truth.Truth.assertThat;
import static org.apache.http.HttpStatus.SC_CREATED;
+import static org.apache.http.HttpStatus.SC_OK;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
import com.google.common.truth.Expect;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.httpd.restapi.ParameterParser;
@@ -53,11 +57,27 @@
import org.junit.Rule;
import org.junit.Test;
+/**
+ * This test tests the tracing of requests.
+ *
+ * <p>To verify that tracing is working we do:
+ *
+ * <ul>
+ * <li>Register a plugin extension that we know is invoked when the request is done. Within the
+ * implementation of this plugin extension we access the status of the thread local state in
+ * the {@link LoggingContext} and store it locally in the plugin extension class.
+ * <li>Do a request (e.g. REST) that triggers the plugin extension.
+ * <li>When the plugin extension is invoked it records the current logging context.
+ * <li>After the request is done the test verifies that logging context that was recorded by the
+ * plugin extension has the expected state.
+ * </ul>
+ */
public class TraceIT extends AbstractDaemonTest {
@Rule public final Expect expect = Expect.create();
@Inject private DynamicSet<ProjectCreationValidationListener> projectCreationValidationListeners;
@Inject private DynamicSet<CommitValidationListener> commitValidationListeners;
+ @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
@Inject private DynamicSet<PerformanceLogger> performanceLoggers;
@Inject private WorkQueue workQueue;
@@ -94,6 +114,29 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
assertThat(projectCreationListener.traceId).isNull();
assertThat(projectCreationListener.isLoggingForced).isFalse();
+
+ // The logging tag with the project name is also set if tracing is off.
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new1");
+ }
+
+ @Test
+ public void restCallForChangeSetsProjectTag() throws Exception {
+ String changeId = createChange().getChangeId();
+
+ TraceChangeIndexedListener changeIndexedListener = new TraceChangeIndexedListener();
+ RegistrationHandle registrationHandle =
+ changeIndexedListeners.add("gerrit", changeIndexedListener);
+ try {
+ RestResponse response =
+ adminRestSession.post(
+ "/changes/" + changeId + "/revisions/current/review", ReviewInput.approve());
+ assertThat(response.getStatusCode()).isEqualTo(SC_OK);
+
+ // The logging tag with the project name is also set if tracing is off.
+ assertThat(changeIndexedListener.tags.get("project")).containsExactly(project.get());
+ } finally {
+ registrationHandle.remove();
+ }
}
@Test
@@ -104,6 +147,7 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull();
assertThat(projectCreationListener.traceId).isNotNull();
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new2");
}
@Test
@@ -114,6 +158,7 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123");
assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new3");
}
@Test
@@ -125,6 +170,7 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull();
assertThat(projectCreationListener.traceId).isNotNull();
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new4");
}
@Test
@@ -136,6 +182,7 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123");
assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new5");
}
@Test
@@ -148,6 +195,7 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123");
assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new6");
// trace ID only specified by trace request parameter
response =
@@ -157,6 +205,7 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123");
assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new7");
// same trace ID specified by trace header and trace request parameter
response =
@@ -167,6 +216,7 @@
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123");
assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new8");
// different trace IDs specified by trace header and trace request parameter
response =
@@ -178,6 +228,7 @@
.containsExactly("issue/123", "issue/456");
assertThat(projectCreationListener.traceIds).containsExactly("issue/123", "issue/456");
assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.tags.get("project")).containsExactly("new9");
}
@Test
@@ -187,6 +238,9 @@
r.assertOkStatus();
assertThat(commitValidationListener.traceId).isNull();
assertThat(commitValidationListener.isLoggingForced).isFalse();
+
+ // The logging tag with the project name is also set if tracing is off.
+ assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get());
}
@Test
@@ -197,6 +251,7 @@
r.assertOkStatus();
assertThat(commitValidationListener.traceId).isNotNull();
assertThat(commitValidationListener.isLoggingForced).isTrue();
+ assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get());
}
@Test
@@ -207,6 +262,7 @@
r.assertOkStatus();
assertThat(commitValidationListener.traceId).isEqualTo("issue/123");
assertThat(commitValidationListener.isLoggingForced).isTrue();
+ assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get());
}
@Test
@@ -216,6 +272,9 @@
r.assertOkStatus();
assertThat(commitValidationListener.traceId).isNull();
assertThat(commitValidationListener.isLoggingForced).isFalse();
+
+ // The logging tag with the project name is also set if tracing is off.
+ assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get());
}
@Test
@@ -226,6 +285,7 @@
r.assertOkStatus();
assertThat(commitValidationListener.traceId).isNotNull();
assertThat(commitValidationListener.isLoggingForced).isTrue();
+ assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get());
}
@Test
@@ -236,6 +296,7 @@
r.assertOkStatus();
assertThat(commitValidationListener.traceId).isEqualTo("issue/123");
assertThat(commitValidationListener.isLoggingForced).isTrue();
+ assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get());
}
@Test
@@ -316,6 +377,7 @@
String traceId;
ImmutableSet<String> traceIds;
Boolean isLoggingForced;
+ ImmutableSetMultimap<String, String> tags;
@Override
public void validateNewProject(CreateProjectArgs args) throws ValidationException {
@@ -323,12 +385,14 @@
Iterables.getFirst(LoggingContext.getInstance().getTagsAsMap().get("TRACE_ID"), null);
this.traceIds = LoggingContext.getInstance().getTagsAsMap().get("TRACE_ID");
this.isLoggingForced = LoggingContext.getInstance().shouldForceLogging(null, null, false);
+ this.tags = LoggingContext.getInstance().getTagsAsMap();
}
}
private static class TraceValidatingCommitValidationListener implements CommitValidationListener {
String traceId;
Boolean isLoggingForced;
+ ImmutableSetMultimap<String, String> tags;
@Override
public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
@@ -336,10 +400,23 @@
this.traceId =
Iterables.getFirst(LoggingContext.getInstance().getTagsAsMap().get("TRACE_ID"), null);
this.isLoggingForced = LoggingContext.getInstance().shouldForceLogging(null, null, false);
+ this.tags = LoggingContext.getInstance().getTagsAsMap();
return ImmutableList.of();
}
}
+ private static class TraceChangeIndexedListener implements ChangeIndexedListener {
+ ImmutableSetMultimap<String, String> tags;
+
+ @Override
+ public void onChangeIndexed(String projectName, int id) {
+ this.tags = LoggingContext.getInstance().getTagsAsMap();
+ }
+
+ @Override
+ public void onChangeDeleted(int id) {}
+ }
+
private static class TestPerformanceLogger implements PerformanceLogger {
private List<PerformanceLogEntry> logEntries = new ArrayList<>();
diff --git a/javatests/com/google/gerrit/httpd/BUILD b/javatests/com/google/gerrit/httpd/BUILD
index 0fbd922..6849d66 100644
--- a/javatests/com/google/gerrit/httpd/BUILD
+++ b/javatests/com/google/gerrit/httpd/BUILD
@@ -25,5 +25,6 @@
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
"//lib/truth",
+ "//lib/truth:truth-java8-extension",
],
)
