commit 4a8ba76fdd8e1703f7f7ffe895d6b6d688305740
author Youssef Elghareeb <ghareeb@google.com> Wed Oct 11 10:33:21 2023 +0200
committer Youssef Elghareeb <ghareeb@google.com> Wed Oct 11 10:47:54 2023 +0200
tree 26f1ac891decccf0ca526a0f8c513499efdd885f
parent a9b5c3509c467839946018cc96c9abc62d636b0a

Add metric for tracking latency of parent data computation

This feature was added with change I9d572e61 adding a new field
(ParentInfo) that can be returned with RevisionInfo if a new list change
option is supplied. After rolling out this feature with the UI (see
change I271d8cdc) we saw an increase in send-reply latency.

Adding a metric in this change to track the latency of this computation.

Release-Notes: skip
Change-Id: Ic1e50fc470ee67bc9f2e9045715cff41a9ff98a5

Documentation/metrics.txt

diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 0d72447..a560b84 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -279,6 +279,9 @@
    view implementation class
 * `http/server/rest_api/change_json/to_change_info_latency`: Latency for
   toChangeInfo invocations in ChangeJson.
+* `http/server/rest_api/change_json/to_change_info_latency/parent_data_computation`:
+   Latency for computing parent data information in toRevisionInfo invocations
+   in RevisionJson. See link:rest-api-changes.html#parent-info[ParentInfo].
 * `http/server/rest_api/change_json/to_change_infos_latency`: Latency for
   toChangeInfos invocations in ChangeJson.
 * `http/server/rest_api/change_json/format_query_results_latency`: Latency for

java/com/google/gerrit/server/change/RevisionJson.java

diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
index 7dcec60..2ab7e15 100644
--- a/java/com/google/gerrit/server/change/RevisionJson.java
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -52,6 +52,10 @@
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.registration.Extension;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.GpgException;
@@ -70,6 +74,7 @@
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
+import com.google.inject.Singleton;
 import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -91,6 +96,23 @@
     RevisionJson create(Iterable<ListChangesOption> options);
   }
 
+  @Singleton
+  private static class Metrics {
+    private final Timer0 parentDataLatency;
+
+    @Inject
+    Metrics(MetricMaker metricMaker) {
+      parentDataLatency =
+          metricMaker.newTimer(
+              "http/server/rest_api/change_json/to_change_info_latency/parent_data_computation",
+              new Description(
+                      "Latency for computing parent data information in toRevisionInfo"
+                          + " invocations in RevisionJson")
+                  .setCumulative()
+                  .setUnit(Units.MILLISECONDS));
+    }
+  }
+
   private final MergeUtilFactory mergeUtilFactory;
   private final IdentifiedUser.GenericFactory userFactory;
   private final FileInfoJson fileInfoJson;
@@ -109,6 +131,7 @@
   private final GitRepositoryManager repoManager;
   private final PermissionBackend permissionBackend;
   private final ParentDataProvider parentDataProvider;
+  private final Metrics metrics;
 
   @Inject
   RevisionJson(
@@ -129,6 +152,7 @@
       GitRepositoryManager repoManager,
       PermissionBackend permissionBackend,
       ParentDataProvider parentDataProvider,
+      Metrics metrics,
       @Assisted Iterable<ListChangesOption> options) {
     this.userProvider = userProvider;
     this.anonymous = anonymous;
@@ -147,6 +171,7 @@
     this.permissionBackend = permissionBackend;
     this.repoManager = repoManager;
     this.parentDataProvider = parentDataProvider;
+    this.metrics = metrics;
     this.options = ImmutableSet.copyOf(options);
   }
 
@@ -339,14 +364,17 @@
                 c.getId().get());
       }
       if (has(PARENTS)) {
-        String targetBranch =
-            in.branch().isPresent() ? in.branch().get() : cd.change().getDest().branch();
-        List<ParentCommitData> parentData = new ArrayList<>();
-        for (RevCommit parent : commit.getParents()) {
-          ParentCommitData p = parentDataProvider.get(project, repo, parent.getId(), targetBranch);
-          parentData.add(p);
+        try (Timer0.Context ignored = metrics.parentDataLatency.start()) {
+          String targetBranch =
+              in.branch().isPresent() ? in.branch().get() : cd.change().getDest().branch();
+          List<ParentCommitData> parentData = new ArrayList<>();
+          for (RevCommit parent : commit.getParents()) {
+            ParentCommitData p =
+                parentDataProvider.get(project, repo, parent.getId(), targetBranch);
+            parentData.add(p);
+          }
+          out.parentsData = getParentInfo(parentData);
         }
-        out.parentsData = getParentInfo(parentData);
       }
       if (addFooters) {
         Ref ref = repo.exactRef(branchName);