Support tracing requests that match a header

Gerrit supports tracing requests that match a configured trace
configuration [1]. E.g. it's possible to switch on tracing for all
REST requests that match a given request URI pattern.

With this change we add a new parameter to the tracing configuration
that allows matching requests by a header. E.g. this allows matching
requests that are sent with a certain user agent (e.g. headerPattern =
User-Agent=foo-.*).

[1] https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#tracing.traceid

Release-Notes: skip
Change-Id: I25bba9409a1ef66d4808efaa4b27656836fd76b5
Signed-off-by: Edwin Kempin <ekempin@google.com>
(cherry picked from commit 3ad0ffed9e8828643df2fedcaf33ce792e55f8f9)

5 files changed, 131 insertions, 11 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index f795defe15..2bfaa40a40 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -5770,6 +5770,21 @@ Example:
 +
 By default, unset (all request query strings are matched).
 
+[[tracing.traceid.headerPattern]]tracing.<trace-id>.headerPattern::
++
+Regular expression to match headers for which request tracing should be
+enabled. The regular expression is matched against the headers in the
+format '<header-name>=<header-value>'.
++
+May be specified multiple times.
++
+Example:
+----
+  requestQueryStringPattern = User-Agent=foo-.*
+----
++
+By default, unset (all headers are matched).
+
 [[tracing.traceid.account]]tracing.<trace-id>.account::
 +
 Account ID of an account for which request tracing should be always
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index f3806ebe17..9b53c1792e 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -164,6 +164,7 @@ import java.lang.reflect.Type;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -328,8 +329,7 @@ public class RestApiServlet extends HttpServlet {
 
       try (PerThreadCache ignored = PerThreadCache.create()) {
         List<IdString> path = splitPath(req);
-        RequestInfo requestInfo =
-            createRequestInfo(traceContext, requestUri, req.getQueryString(), path);
+        RequestInfo requestInfo = createRequestInfo(traceContext, req, requestUri, path);
         globals.requestListeners.runEach(l -> l.onRequest(requestInfo));
 
         // It's important that the PerformanceLogContext is closed before the response is sent to
@@ -1670,16 +1670,23 @@ public class RestApiServlet extends HttpServlet {
   }
 
   private RequestInfo createRequestInfo(
-      TraceContext traceContext,
-      String requestUri,
-      @Nullable String queryString,
-      List<IdString> path) {
+      TraceContext traceContext, HttpServletRequest req, String requestUri, List<IdString> path) {
     RequestInfo.Builder requestInfo =
         RequestInfo.builder(RequestInfo.RequestType.REST, globals.currentUser.get(), traceContext)
             .requestUri(requestUri);
 
-    if (queryString != null) {
-      requestInfo.requestQueryString(queryString);
+    if (req.getQueryString() != null) {
+      requestInfo.requestQueryString(req.getQueryString());
+    }
+
+    Enumeration<String> headerNames = req.getHeaderNames();
+    while (headerNames.hasMoreElements()) {
+      String headerName = headerNames.nextElement();
+      Enumeration<String> headerValues = req.getHeaders(headerName);
+      while (headerValues.hasMoreElements()) {
+        String headerValue = headerValues.nextElement();
+        requestInfo.addHeader(headerName, headerValue);
+      }
     }
 
     if (path.size() < 1) {
diff --git a/java/com/google/gerrit/server/RequestConfig.java b/java/com/google/gerrit/server/RequestConfig.java
index a157c090d1..f942c5e899 100644
--- a/java/com/google/gerrit/server/RequestConfig.java
+++ b/java/com/google/gerrit/server/RequestConfig.java
@@ -43,6 +43,7 @@ public abstract class RequestConfig {
         requestConfig.requestUriPatterns(parseRequestUriPatterns(cfg, section, id));
         requestConfig.excludedRequestUriPatterns(parseExcludedRequestUriPatterns(cfg, section, id));
         requestConfig.requestQueryStringPatterns(parseRequestQueryStringPatterns(cfg, section, id));
+        requestConfig.headerPatterns(parseHeaderPatterns(cfg, section, id));
         requestConfig.accountIds(parseAccounts(cfg, section, id));
         requestConfig.projectPatterns(parseProjectPatterns(cfg, section, id));
         requestConfigs.add(requestConfig.build());
@@ -73,6 +74,11 @@ public abstract class RequestConfig {
     return parsePatterns(cfg, section, id, "requestQueryStringPattern");
   }
 
+  private static ImmutableSet<Pattern> parseHeaderPatterns(Config cfg, String section, String id)
+      throws ConfigInvalidException {
+    return parsePatterns(cfg, section, id, "headerPattern");
+  }
+
   private static ImmutableSet<Account.Id> parseAccounts(Config cfg, String section, String id)
       throws ConfigInvalidException {
     ImmutableSet.Builder<Account.Id> accountIds = ImmutableSet.builder();
@@ -133,6 +139,9 @@ public abstract class RequestConfig {
   /** pattern matching request query strings */
   abstract ImmutableSet<Pattern> requestQueryStringPatterns();
 
+  /** pattern matching headers */
+  abstract ImmutableSet<Pattern> headerPatterns();
+
   /** accounts IDs matching calling user */
   abstract ImmutableSet<Account.Id> accountIds();
 
@@ -194,6 +203,22 @@ public abstract class RequestConfig {
       }
     }
 
+    // If in the request config header patterns are set and none of them matches, then the request
+    // is not matched.
+    if (!headerPatterns().isEmpty()) {
+      if (requestInfo.headers().isEmpty()) {
+        // The request has no headers, hence it cannot match a header pattern.
+        return false;
+      }
+
+      if (headerPatterns().stream()
+          .noneMatch(
+              p ->
+                  requestInfo.headers().stream().anyMatch(header -> p.matcher(header).matches()))) {
+        return false;
+      }
+    }
+
     // If in the request config accounts are set and none of them matches, then the request is not
     // matched.
     if (!accountIds().isEmpty()) {
@@ -223,8 +248,8 @@ public abstract class RequestConfig {
     }
 
     // For any match criteria (request type, request URI pattern, request query string pattern,
-    // account, project pattern) that was specified in the request config, at least one of the
-    // configured value matched the request.
+    // header, account, project pattern) that was specified in the request config, at least one of
+    // the configured value matched the request.
     return true;
   }
 
@@ -244,6 +269,8 @@ public abstract class RequestConfig {
 
     abstract Builder requestQueryStringPatterns(ImmutableSet<Pattern> requestQueryStringPatterns);
 
+    abstract Builder headerPatterns(ImmutableSet<Pattern> headerPatterns);
+
     abstract Builder accountIds(ImmutableSet<Account.Id> accountIds);
 
     abstract Builder projectPatterns(ImmutableSet<Pattern> projectPatterns);
diff --git a/java/com/google/gerrit/server/RequestInfo.java b/java/com/google/gerrit/server/RequestInfo.java
index bdbac70447..e1a5148ca0 100644
--- a/java/com/google/gerrit/server/RequestInfo.java
+++ b/java/com/google/gerrit/server/RequestInfo.java
@@ -20,6 +20,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.auto.value.AutoValue;
 import com.google.auto.value.extension.memoized.Memoized;
 import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.UsedAt;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.server.logging.TraceContext;
@@ -70,6 +71,9 @@ public abstract class RequestInfo {
    */
   public abstract Optional<String> requestQueryString();
 
+  /** Request headers in the form '{@code <header-name>:<header-value>}'. */
+  public abstract ImmutableList<String> headers();
+
   /**
    * Redacted request URI.
    *
@@ -176,6 +180,15 @@ public abstract class RequestInfo {
 
     public abstract Builder requestQueryString(String requestQueryString);
 
+    /** Gets a builder for adding reasons for this status. */
+    abstract ImmutableList.Builder<String> headersBuilder();
+
+    /** Adds a header. */
+    public Builder addHeader(String headerName, String headerValue) {
+      headersBuilder().add(headerName + "=" + headerValue);
+      return this;
+    }
+
     public abstract Builder callingUser(CurrentUser callingUser);
 
     public abstract Builder traceContext(TraceContext traceContext);
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 892ee9e985..2d73e97c48 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -18,9 +18,9 @@ import static com.google.common.truth.Truth.assertThat;
 import static org.apache.http.HttpStatus.SC_CREATED;
 import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
 import static org.apache.http.HttpStatus.SC_OK;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
@@ -853,6 +853,64 @@ public class TraceIT extends AbstractDaemonTest {
   }
 
   @Test
+  @GerritConfig(name = "tracing.issue123.headerPattern", value = "User-Agent=foo.*")
+  public void traceHeader() throws Exception {
+    String changeId = createChange().getChangeId();
+    TraceReviewerSuggestion reviewerSuggestion = new TraceReviewerSuggestion();
+    try (Registration registration =
+        extensionRegistry.newRegistration().add(reviewerSuggestion, /* exportName= */ "foo")) {
+
+      RestResponse response =
+          adminRestSession.getWithHeaders(
+              String.format("/changes/%s/suggest_reviewers?limit=10", changeId),
+              new BasicHeader("User-Agent", "foo-bar"),
+              new BasicHeader("Other-Header", "baz"));
+      assertThat(response.getStatusCode()).isEqualTo(SC_OK);
+      assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
+      assertThat(reviewerSuggestion.traceId).isEqualTo("issue123");
+      assertThat(reviewerSuggestion.isLoggingForced).isTrue();
+    }
+  }
+
+  @Test
+  @GerritConfig(name = "tracing.issue123.headerPattern", value = "User-Agent=bar.*")
+  public void traceHeaderNoMatch() throws Exception {
+    String changeId = createChange().getChangeId();
+    TraceReviewerSuggestion reviewerSuggestion = new TraceReviewerSuggestion();
+    try (Registration registration =
+        extensionRegistry.newRegistration().add(reviewerSuggestion, /* exportName= */ "foo")) {
+      RestResponse response =
+          adminRestSession.getWithHeaders(
+              String.format("/changes/%s/suggest_reviewers?limit=10", changeId),
+              new BasicHeader("User-Agent", "foo-bar"),
+              new BasicHeader("Other-Header", "baz"));
+      assertThat(response.getStatusCode()).isEqualTo(SC_OK);
+      assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
+      assertThat(reviewerSuggestion.traceId).isNull();
+      assertThat(reviewerSuggestion.isLoggingForced).isFalse();
+    }
+  }
+
+  @Test
+  @GerritConfig(name = "tracing.issue123.headerPattern", value = "][")
+  public void traceHeaderInvalidRegEx() throws Exception {
+    String changeId = createChange().getChangeId();
+    TraceReviewerSuggestion reviewerSuggestion = new TraceReviewerSuggestion();
+    try (Registration registration =
+        extensionRegistry.newRegistration().add(reviewerSuggestion, /* exportName= */ "foo")) {
+      RestResponse response =
+          adminRestSession.getWithHeaders(
+              String.format("/changes/%s/suggest_reviewers?limit=10", changeId),
+              new BasicHeader("User-Agent", "foo-bar"),
+              new BasicHeader("Other-Header", "baz"));
+      assertThat(response.getStatusCode()).isEqualTo(SC_OK);
+      assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
+      assertThat(reviewerSuggestion.traceId).isNull();
+      assertThat(reviewerSuggestion.isLoggingForced).isFalse();
+    }
+  }
+
+  @Test
   @GerritConfig(name = "retry.retryWithTraceOnFailure", value = "true")
   public void autoRetryWithTrace() throws Exception {
     String changeId = createChange().getChangeId();