Add metric to record code owner config validations
Android would like to know how many pushes would be rejected if they
would turn on the code owner config validation. Collect this information
as a metric. Since the metric also covers the dry run mode, Android can
collect the stats with the dry run mode before turning on the validation
for real.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I257a9d2c3a13a603026f4da31120962700188c6e
diff --git a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
index 06664ba..3bfcc54 100644
--- a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
@@ -59,6 +59,8 @@
// counter metrics
public final Counter0 countCodeOwnerConfigReads;
public final Counter0 countCodeOwnerConfigCacheReads;
+ public final Counter3<ValidationTrigger, ValidationResult, Boolean>
+ countCodeOwnerConfigValidations;
public final Counter1<String> countCodeOwnerSubmitRuleErrors;
public final Counter0 countCodeOwnerSubmitRuleRuns;
public final Counter1<Boolean> countCodeOwnerSuggestions;
@@ -159,6 +161,20 @@
createCounter(
"count_code_owner_config_cache_reads",
"Total number of code owner config reads from cache");
+ this.countCodeOwnerConfigValidations =
+ createCounter3(
+ "count_code_owner_config_validations",
+ "Total number of code owner config file validations",
+ Field.ofEnum(
+ ValidationTrigger.class, "trigger", (metadataBuilder, resolveAllUsers) -> {})
+ .description("The trigger of the validation.")
+ .build(),
+ Field.ofEnum(ValidationResult.class, "result", (metadataBuilder, resolveAllUsers) -> {})
+ .description("The result of the validation.")
+ .build(),
+ Field.ofBoolean("dry_run", (metadataBuilder, resolveAllUsers) -> {})
+ .description("Whether the validation was a dry run.")
+ .build());
this.countCodeOwnerSubmitRuleErrors =
createCounter1(
"count_code_owner_submit_rule_errors",
diff --git a/java/com/google/gerrit/plugins/codeowners/metrics/ValidationResult.java b/java/com/google/gerrit/plugins/codeowners/metrics/ValidationResult.java
new file mode 100644
index 0000000..5267736
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/ValidationResult.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2020 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.plugins.codeowners.metrics;
+
+/** Enum that represents the result of a validation. */
+public enum ValidationResult {
+ /** The validation found issues that caused a rejection. */
+ REJECTED,
+
+ /** The validation passed without finding rejection reasons. */
+ PASSED,
+
+ /** The validation couldn't be performed due to a server error. */
+ FAILED;
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/metrics/ValidationTrigger.java b/java/com/google/gerrit/plugins/codeowners/metrics/ValidationTrigger.java
new file mode 100644
index 0000000..4d28b70
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/ValidationTrigger.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2020 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.plugins.codeowners.metrics;
+
+/** Enum to express which event triggered the validation. */
+public enum ValidationTrigger {
+ /** A new commit was received that should be validated. */
+ COMMIT_RECEIVED,
+
+ /** A commit is about to be merged and should be validated. */
+ PRE_MERGE;
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/BUILD b/java/com/google/gerrit/plugins/codeowners/validation/BUILD
index ba32dc0..f66a8a5 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/BUILD
+++ b/java/com/google/gerrit/plugins/codeowners/validation/BUILD
@@ -8,6 +8,7 @@
deps = PLUGIN_DEPS_NEVERLINK + [
"//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/backend",
"//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/common",
+ "//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/metrics",
"//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/util",
],
)
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 24be1ce..6292194 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -47,6 +47,8 @@
import com.google.gerrit.plugins.codeowners.common.ChangedFile;
import com.google.gerrit.plugins.codeowners.common.CodeOwnerConfigValidationPolicy;
import com.google.gerrit.plugins.codeowners.common.MergeCommitStrategy;
+import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
+import com.google.gerrit.plugins.codeowners.metrics.ValidationTrigger;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
@@ -140,6 +142,7 @@
private final PatchSetUtil patchSetUtil;
private final IdentifiedUser.GenericFactory userFactory;
private final SkipCodeOwnerConfigValidationPushOption skipCodeOwnerConfigValidationPushOption;
+ private final CodeOwnerMetrics codeOwnerMetrics;
@Inject
CodeOwnerConfigValidator(
@@ -153,7 +156,8 @@
ChangeNotes.Factory changeNotesFactory,
PatchSetUtil patchSetUtil,
IdentifiedUser.GenericFactory userFactory,
- SkipCodeOwnerConfigValidationPushOption skipCodeOwnerConfigValidationPushOption) {
+ SkipCodeOwnerConfigValidationPushOption skipCodeOwnerConfigValidationPushOption,
+ CodeOwnerMetrics codeOwnerMetrics) {
this.pluginName = pluginName;
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.repoManager = repoManager;
@@ -165,6 +169,7 @@
this.patchSetUtil = patchSetUtil;
this.userFactory = userFactory;
this.skipCodeOwnerConfigValidationPushOption = skipCodeOwnerConfigValidationPushOption;
+ this.codeOwnerMetrics = codeOwnerMetrics;
}
@Override
@@ -184,6 +189,7 @@
.getProjectConfig(receiveEvent.getProjectNameKey())
.getCodeOwnerConfigValidationPolicyForCommitReceived(receiveEvent.refName);
logger.atFine().log("codeOwnerConfigValidationPolicy = %s", codeOwnerConfigValidationPolicy);
+ boolean metricRecordingDone = false;
Optional<ValidationResult> validationResult;
if (!codeOwnerConfigValidationPolicy.runValidation()) {
validationResult =
@@ -205,6 +211,12 @@
codeOwnerConfigValidationPolicy.isForced(),
receiveEvent.pushOptions);
} catch (RuntimeException e) {
+ codeOwnerMetrics.countCodeOwnerConfigValidations.increment(
+ ValidationTrigger.COMMIT_RECEIVED,
+ com.google.gerrit.plugins.codeowners.metrics.ValidationResult.FAILED,
+ codeOwnerConfigValidationPolicy.isDryRun());
+ metricRecordingDone = true;
+
if (!codeOwnerConfigValidationPolicy.isDryRun()) {
throw e;
}
@@ -225,6 +237,14 @@
}
logger.atFine().log("validation result = %s", validationResult.get());
+ if (!metricRecordingDone) {
+ codeOwnerMetrics.countCodeOwnerConfigValidations.increment(
+ ValidationTrigger.COMMIT_RECEIVED,
+ validationResult.get().hasError()
+ ? com.google.gerrit.plugins.codeowners.metrics.ValidationResult.REJECTED
+ : com.google.gerrit.plugins.codeowners.metrics.ValidationResult.PASSED,
+ codeOwnerConfigValidationPolicy.isDryRun());
+ }
return validationResult
.get()
.processForOnCommitReceived(codeOwnerConfigValidationPolicy.isDryRun());
@@ -281,6 +301,11 @@
codeOwnerConfigValidationPolicy.isForced(),
/* pushOptions= */ ImmutableListMultimap.of());
} catch (RuntimeException e) {
+ codeOwnerMetrics.countCodeOwnerConfigValidations.increment(
+ ValidationTrigger.PRE_MERGE,
+ com.google.gerrit.plugins.codeowners.metrics.ValidationResult.FAILED,
+ codeOwnerConfigValidationPolicy.isDryRun());
+
if (!codeOwnerConfigValidationPolicy.isDryRun()) {
throw e;
}
@@ -296,6 +321,12 @@
}
if (validationResult.isPresent()) {
logger.atFine().log("validation result = %s", validationResult.get());
+ codeOwnerMetrics.countCodeOwnerConfigValidations.increment(
+ ValidationTrigger.PRE_MERGE,
+ validationResult.get().hasError()
+ ? com.google.gerrit.plugins.codeowners.metrics.ValidationResult.REJECTED
+ : com.google.gerrit.plugins.codeowners.metrics.ValidationResult.PASSED,
+ codeOwnerConfigValidationPolicy.isDryRun());
validationResult.get().processForOnPreMerge(codeOwnerConfigValidationPolicy.isDryRun());
}
}
@@ -1260,7 +1291,7 @@
}
/** Checks whether any of the validation messages is an error. */
- private boolean hasError() {
+ public boolean hasError() {
return validationMessages().stream()
.anyMatch(
validationMessage ->
diff --git a/resources/Documentation/metrics.md b/resources/Documentation/metrics.md
index d06a9f3..93c6a3b 100644
--- a/resources/Documentation/metrics.md
+++ b/resources/Documentation/metrics.md
@@ -63,6 +63,14 @@
Total number of code owner config reads from backend.
* `count_code_owner_config_cache_reads`:
Total number of code owner config reads from cache.
+* `count_code_owner_config_validations`:
+ Total number of code owner config validations.
+ * `trigger`:
+ The trigger of the validation.
+ * `result`:
+ The result of the validation.
+ * `dry_run`:
+ Whether the validation was a dry run.
* `count_code_owner_submit_rule_errors`:
Total number of code owner submit rule errors.
* `cause`:
