Add metric that counts how often invalid OWNERS files cause failures
Fixing invalid OWNERS files is the responsibility of the project team
since they are maintaining the OWNERS files, however we are interested
to know how often invalid OWNERS files cause requests to fail (they
return '409 Conflict' so it's not a server failure that counts against
SLOs).
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Id9e5bec319cffdd91c683ad7db58cee9b04793ef
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java
index 4f0d371..90fd878 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java
@@ -213,8 +213,9 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
if (revision != null) {
- Optional<String> codeOwnerConfigFileContent =
- getFileIfItExists(JgitPath.of(codeOwnerConfigKey.filePath(defaultFileName)).get());
+ String codeOwnerConfigFilePath =
+ JgitPath.of(codeOwnerConfigKey.filePath(defaultFileName)).get();
+ Optional<String> codeOwnerConfigFileContent = getFileIfItExists(codeOwnerConfigFilePath);
if (codeOwnerConfigFileContent.isPresent()) {
try (Timer1.Context<String> ctx =
codeOwnerMetrics.parseCodeOwnerConfig.start(
@@ -225,7 +226,11 @@
revision, codeOwnerConfigKey, codeOwnerConfigFileContent.get()));
} catch (CodeOwnerConfigParseException e) {
throw new InvalidCodeOwnerConfigException(
- e.getFullMessage(defaultFileName), projectName, e);
+ e.getFullMessage(defaultFileName),
+ projectName,
+ getRefName(),
+ codeOwnerConfigFilePath,
+ e);
}
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
index 142b273..468f5bb 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
@@ -19,6 +19,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.backend.config.InvalidPluginConfigurationException;
+import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.server.ExceptionHook;
import com.google.inject.Inject;
import java.nio.file.InvalidPathException;
@@ -39,10 +40,14 @@
*/
public class CodeOwnersExceptionHook implements ExceptionHook {
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
+ private final CodeOwnerMetrics codeOwnerMetrics;
@Inject
- CodeOwnersExceptionHook(CodeOwnersPluginConfiguration codeOwnersPluginConfiguration) {
+ CodeOwnersExceptionHook(
+ CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
+ CodeOwnerMetrics codeOwnerMetric) {
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
+ this.codeOwnerMetrics = codeOwnerMetric;
}
@Override
@@ -63,6 +68,11 @@
Optional<InvalidCodeOwnerConfigException> invalidCodeOwnerConfigException =
CodeOwners.getInvalidCodeOwnerConfigCause(throwable);
if (invalidCodeOwnerConfigException.isPresent()) {
+ codeOwnerMetrics.countInvalidCodeOwnerConfigFiles.increment(
+ invalidCodeOwnerConfigException.get().getProjectName().get(),
+ invalidCodeOwnerConfigException.get().getRef(),
+ invalidCodeOwnerConfigException.get().getCodeOwnerConfigFilePath());
+
ImmutableList.Builder<String> messages = ImmutableList.builder();
messages.add(invalidCodeOwnerConfigException.get().getMessage());
codeOwnersPluginConfiguration
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/InvalidCodeOwnerConfigException.java b/java/com/google/gerrit/plugins/codeowners/backend/InvalidCodeOwnerConfigException.java
index d2df76d..7806c59 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/InvalidCodeOwnerConfigException.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/InvalidCodeOwnerConfigException.java
@@ -16,6 +16,7 @@
import static java.util.Objects.requireNonNull;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -24,21 +25,37 @@
private static final long serialVersionUID = 1L;
private final Project.NameKey projectName;
+ private final String ref;
+ private final String codeOwnerConfigFilePath;
- public InvalidCodeOwnerConfigException(String message, Project.NameKey projectName) {
- super(message);
-
- this.projectName = requireNonNull(projectName, "projectName");
+ public InvalidCodeOwnerConfigException(
+ String message, Project.NameKey projectName, String ref, String codeOwnerConfigFilePath) {
+ this(message, projectName, ref, codeOwnerConfigFilePath, /* cause= */ null);
}
public InvalidCodeOwnerConfigException(
- String message, Project.NameKey projectName, Throwable cause) {
+ String message,
+ Project.NameKey projectName,
+ String ref,
+ String codeOwnerConfigFilePath,
+ @Nullable Throwable cause) {
super(message, cause);
this.projectName = requireNonNull(projectName, "projectName");
+ this.ref = requireNonNull(ref, "ref");
+ this.codeOwnerConfigFilePath =
+ requireNonNull(codeOwnerConfigFilePath, "codeOwnerConfigFilePath");
}
public Project.NameKey getProjectName() {
return projectName;
}
+
+ public String getRef() {
+ return ref;
+ }
+
+ public String getCodeOwnerConfigFilePath() {
+ return codeOwnerConfigFilePath;
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
index f3cc516..26b81c8 100644
--- a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
@@ -15,6 +15,7 @@
package com.google.gerrit.plugins.codeowners.metrics;
import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Counter3;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
import com.google.gerrit.metrics.Field;
@@ -22,6 +23,7 @@
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.metrics.Timer1;
+import com.google.gerrit.server.logging.Metadata;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -56,6 +58,7 @@
public final Counter0 countCodeOwnerConfigReads;
public final Counter0 countCodeOwnerConfigCacheReads;
public final Counter0 countCodeOwnerSubmitRuleRuns;
+ public final Counter3<String, String, String> countInvalidCodeOwnerConfigFiles;
private final MetricMaker metricMaker;
@@ -152,6 +155,22 @@
this.countCodeOwnerSubmitRuleRuns =
createCounter(
"count_code_owner_submit_rule_runs", "Total number of code owner submit rule runs");
+ this.countInvalidCodeOwnerConfigFiles =
+ createCounter3(
+ "count_invalid_code_owner_config_files",
+ "Total number of failed requests caused by an invalid / non-parsable code owner config"
+ + " file",
+ Field.ofString("project", Metadata.Builder::projectName)
+ .description(
+ "The name of the project that contains the invalid code owner config file.")
+ .build(),
+ Field.ofString("branch", Metadata.Builder::branchName)
+ .description(
+ "The name of the branch that contains the invalid code owner config file.")
+ .build(),
+ Field.ofString("path", Metadata.Builder::filePath)
+ .description("The path of the invalid code owner config file.")
+ .build());
}
private Timer0 createLatencyTimer(String name, String description) {
@@ -177,6 +196,12 @@
return metricMaker.newCounter("code_owners/" + name, new Description(description).setRate());
}
+ private <F1, F2, F3> Counter3<F1, F2, F3> createCounter3(
+ String name, String description, Field<F1> field1, Field<F2> field2, Field<F3> field3) {
+ return metricMaker.newCounter(
+ "code_owners/" + name, new Description(description).setRate(), field1, field2, field3);
+ }
+
private Histogram0 createHistogram(String name, String description) {
return metricMaker.newHistogram(
"code_owners/" + name, new Description(description).setCumulative());
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
index 5d81963..1c0f9c1 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
@@ -156,7 +156,7 @@
}
private InvalidCodeOwnerConfigException newInvalidCodeOwnerConfigException() {
- return new InvalidCodeOwnerConfigException("message", project);
+ return new InvalidCodeOwnerConfigException("message", project, "refs/heads/master", "/OWNERS");
}
private InvalidPathException newInvalidPathException() {
diff --git a/resources/Documentation/metrics.md b/resources/Documentation/metrics.md
index 565ca67..7459fb3 100644
--- a/resources/Documentation/metrics.md
+++ b/resources/Documentation/metrics.md
@@ -59,6 +59,15 @@
Total number of code owner config reads from cache.
* `count_code_owner_submit_rule_runs`:
Total number of code owner submit rule runs.
+* `count_invalid_code_owner_config_files`:
+ Total number of failed requests caused by an invalid / non-parsable code owner
+ config file.
+ * `project`:
+ The name of the project that contains the invalid code owner config file.
+ * `branch`:
+ The name of the branch that contains the invalid code owner config file.
+ * `path`:
+ The path of the invalid code owner config file.
---
