Fix latency metrics for computing file statuses
The metrics intended to capture the time that it takes to compute the
file statuses. Instead they only captured the time that is needed to
create the stream, but most of the time is spent afterwards when the
stream is processed.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ica5b5b7ec8ee6881b32226f9abeef3e301cc0309
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index ae354de..9bf9eb2 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -128,7 +128,13 @@
public ImmutableList<Path> getOwnedPaths(
ChangeNotes changeNotes, PatchSet patchSet, Account.Id accountId)
throws ResourceConflictException {
- try {
+ try (Timer0.Context ctx = codeOwnerMetrics.computeOwnedPaths.start()) {
+ logger.atFine().log(
+ "compute owned paths for account %d (project = %s, change = %d, patch set = %d)",
+ accountId.get(),
+ changeNotes.getProjectName(),
+ changeNotes.getChangeId().get(),
+ patchSet.id().get());
return getFileStatusesForAccount(changeNotes, patchSet, accountId)
.flatMap(
fileCodeOwnerStatus ->
@@ -181,6 +187,22 @@
/**
* Gets the code owner statuses for all files/paths that were changed in the current revision of
+ * the given change as a set.
+ *
+ * @see #getFileStatuses(ChangeNotes)
+ */
+ public ImmutableSet<FileCodeOwnerStatus> getFileStatusesAsSet(ChangeNotes changeNotes)
+ throws ResourceConflictException, IOException, PatchListNotAvailableException {
+ try (Timer0.Context ctx = codeOwnerMetrics.computeFileStatuses.start()) {
+ logger.atFine().log(
+ "compute file statuses (project = %s, change = %d)",
+ changeNotes.getProjectName(), changeNotes.getChangeId().get());
+ return getFileStatuses(changeNotes).collect(toImmutableSet());
+ }
+ }
+
+ /**
+ * Gets the code owner statuses for all files/paths that were changed in the current revision of
* the given change.
*
* <p>The code owner statuses tell the user which code owner approvals are still missing in order
@@ -208,9 +230,9 @@
public Stream<FileCodeOwnerStatus> getFileStatuses(ChangeNotes changeNotes)
throws ResourceConflictException, IOException, PatchListNotAvailableException {
requireNonNull(changeNotes, "changeNotes");
- try (Timer0.Context ctx = codeOwnerMetrics.computeFileStatuses.start()) {
+ try (Timer0.Context ctx = codeOwnerMetrics.prepareFileStatusComputation.start()) {
logger.atFine().log(
- "compute file statuses (project = %s, change = %d)",
+ "prepare stream to compute file statuses (project = %s, change = %d)",
changeNotes.getProjectName(), changeNotes.getChangeId().get());
if (codeOwnersPluginConfiguration.arePureRevertsExempted(changeNotes.getProjectName())
@@ -298,9 +320,10 @@
requireNonNull(changeNotes, "changeNotes");
requireNonNull(patchSet, "patchSet");
requireNonNull(accountId, "accountId");
- try (Timer0.Context ctx = codeOwnerMetrics.computeFileStatusesForAccount.start()) {
+ try (Timer0.Context ctx = codeOwnerMetrics.prepareFileStatusComputationForAccount.start()) {
logger.atFine().log(
- "compute file statuses for account %d (project = %s, change = %d, patch set = %d)",
+ "prepare stream to compute file statuses for account %d (project = %s, change = %d,"
+ + " patch set = %d)",
accountId.get(),
changeNotes.getProjectName(),
changeNotes.getChangeId().get(),
diff --git a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
index e1ce3bf..2dc30a8 100644
--- a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
@@ -26,7 +26,9 @@
public class CodeOwnerMetrics {
public final Timer0 computeChangedFiles;
public final Timer0 computeFileStatuses;
- public final Timer0 computeFileStatusesForAccount;
+ public final Timer0 computeOwnedPaths;
+ public final Timer0 prepareFileStatusComputation;
+ public final Timer0 prepareFileStatusComputationForAccount;
public final Timer0 resolveCodeOwnerConfig;
public final Timer0 resolveCodeOwnerConfigImport;
public final Timer0 resolveCodeOwnerConfigImports;
@@ -43,7 +45,14 @@
createLatencyTimer("compute_changed_files", "Latency for computing changed files");
this.computeFileStatuses =
createLatencyTimer("compute_file_statuses", "Latency for computing file statuses");
- this.computeFileStatusesForAccount =
+ this.computeOwnedPaths =
+ createLatencyTimer(
+ "compute_owned_paths",
+ "Latency for computing the files in a change that are owned by a user");
+ this.prepareFileStatusComputation =
+ createLatencyTimer(
+ "prepare_file_status_computation", "Latency for preparing the file status computation");
+ this.prepareFileStatusComputationForAccount =
createLatencyTimer(
"compute_file_statuses_for_account",
"Latency for computing file statuses for an account");
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java
index 005f3e7..fdf7bd7 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java
@@ -14,8 +14,6 @@
package com.google.gerrit.plugins.codeowners.restapi;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
-
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -64,7 +62,7 @@
throws RestApiException, IOException, PermissionBackendException,
PatchListNotAvailableException {
ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(changeResource.getNotes()).collect(toImmutableSet());
+ codeOwnerApprovalCheck.getFileStatusesAsSet(changeResource.getNotes());
return Response.ok(
CodeOwnerStatusInfoJson.format(
changeResource.getNotes().getCurrentPatchSet().id(), fileCodeOwnerStatuses));
diff --git a/resources/Documentation/metrics.md b/resources/Documentation/metrics.md
index 2e63c23..f7247da 100644
--- a/resources/Documentation/metrics.md
+++ b/resources/Documentation/metrics.md
@@ -9,8 +9,12 @@
Latency for computing changed files.
* `compute_file_statuses`:
Latency for computing file statuses.
-* `compute_file_statuses_for_account`:
- Latency for computing file statuses for an account.
+* `compute_owned_paths`:
+ Latency for computing the files in a change that are owned by a user.
+* `prepare_file_status_computation`:
+ Latency for preparing the file status computation.
+* `prepare_file_status_computation_for_account`:
+ Latency for preparing the file status computation for an account.
* `resolve_code_owner_config`:
Latency for resolving a code owner config file.
* `resolve_code_owner_config_import`:
