Merge "Add metrics to count the number of OWNERS cache/backend loads per change"
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index c25fe52..897131a 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -289,23 +289,30 @@
FallbackCodeOwners fallbackCodeOwners = codeOwnersConfig.getFallbackCodeOwners();
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy = codeOwnerConfigHierarchyProvider.get();
- return changedFiles
- .compute(changeNotes.getProjectName(), changeNotes.getCurrentPatchSet().commitId())
- .stream()
- .map(
- changedFile ->
- getFileStatus(
- codeOwnerConfigHierarchy,
- branch,
- revision,
- globalCodeOwners,
- enableImplicitApprovalFromUploader,
- patchSetUploader,
- reviewerAccountIds,
- approverAccountIds,
- fallbackCodeOwners,
- hasOverride,
- changedFile));
+ try {
+ return changedFiles
+ .compute(changeNotes.getProjectName(), changeNotes.getCurrentPatchSet().commitId())
+ .stream()
+ .map(
+ changedFile ->
+ getFileStatus(
+ codeOwnerConfigHierarchy,
+ branch,
+ revision,
+ globalCodeOwners,
+ enableImplicitApprovalFromUploader,
+ patchSetUploader,
+ reviewerAccountIds,
+ approverAccountIds,
+ fallbackCodeOwners,
+ hasOverride,
+ changedFile));
+ } finally {
+ codeOwnerMetrics.codeOwnerConfigBackendReadsPerChange.record(
+ codeOwnerConfigHierarchy.getCodeOwnerConfigCounters().getBackendReadCount());
+ codeOwnerMetrics.codeOwnerConfigCacheReadsPerChange.record(
+ codeOwnerConfigHierarchy.getCodeOwnerConfigCounters().getCacheReadCount());
+ }
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java
index 6b4da27..6ce1a87 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigHierarchy.java
@@ -234,4 +234,9 @@
String.format("failed to read %s", metaCodeOwnerConfigKey), e);
}
}
+
+ /** Returns the counters for cache and backend reads of code owner config files. */
+ public TransientCodeOwnerConfigCache.Counters getCodeOwnerConfigCounters() {
+ return transientCodeOwnerConfigCache.getCounters();
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java b/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
index 96e0a31..61f014b 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
@@ -38,7 +38,7 @@
public class TransientCodeOwnerConfigCache implements CodeOwnerConfigLoader {
private final GitRepositoryManager repoManager;
private final CodeOwners codeOwners;
- private final CodeOwnerMetrics codeOwnerMetrics;
+ private final Counters counters;
private final HashMap<CacheKey, Optional<CodeOwnerConfig>> cache = new HashMap<>();
@Inject
@@ -46,7 +46,7 @@
GitRepositoryManager repoManager, CodeOwners codeOwners, CodeOwnerMetrics codeOwnerMetrics) {
this.repoManager = repoManager;
this.codeOwners = codeOwners;
- this.codeOwnerMetrics = codeOwnerMetrics;
+ this.counters = new Counters(codeOwnerMetrics);
}
/**
@@ -59,7 +59,7 @@
CacheKey cacheKey = CacheKey.create(codeOwnerConfigKey, revision);
Optional<CodeOwnerConfig> cachedCodeOwnerConfig = cache.get(cacheKey);
if (cachedCodeOwnerConfig != null) {
- codeOwnerMetrics.countCodeOwnerConfigCacheReads.increment();
+ counters.incrementCacheReads();
return cachedCodeOwnerConfig;
}
return loadAndCache(cacheKey);
@@ -76,6 +76,7 @@
/** Load a code owner config and puts it into the cache. */
private Optional<CodeOwnerConfig> loadAndCache(CacheKey cacheKey) {
+ counters.incrementBackendReads();
Optional<CodeOwnerConfig> codeOwnerConfig;
if (cacheKey.revision().isPresent()) {
codeOwnerConfig = codeOwners.get(cacheKey.codeOwnerConfigKey(), cacheKey.revision().get());
@@ -128,4 +129,38 @@
codeOwnerConfigKey, Optional.ofNullable(revision));
}
}
+
+ public Counters getCounters() {
+ return counters;
+ }
+
+ public static class Counters {
+ private final CodeOwnerMetrics codeOwnerMetrics;
+
+ private int cacheReadCount;
+ private int backendReadCount;
+
+ private Counters(CodeOwnerMetrics codeOwnerMetrics) {
+ this.codeOwnerMetrics = codeOwnerMetrics;
+ }
+
+ private void incrementCacheReads() {
+ codeOwnerMetrics.countCodeOwnerConfigCacheReads.increment();
+ cacheReadCount++;
+ }
+
+ private void incrementBackendReads() {
+ // we do not increase the countCodeOwnerConfigReads metric here, since this is already done in
+ // CodeOwners
+ backendReadCount++;
+ }
+
+ public int getBackendReadCount() {
+ return backendReadCount;
+ }
+
+ public int getCacheReadCount() {
+ return cacheReadCount;
+ }
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
index 56aec3c..5d523a6 100644
--- a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
@@ -18,6 +18,7 @@
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.Histogram0;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.metrics.Timer1;
@@ -41,7 +42,9 @@
public final Timer0 resolvePathCodeOwners;
public final Timer0 runCodeOwnerSubmitRule;
- // code owner config latency metrics
+ // code owner config metrics
+ public final Histogram0 codeOwnerConfigBackendReadsPerChange;
+ public final Histogram0 codeOwnerConfigCacheReadsPerChange;
public final Timer1<String> loadCodeOwnerConfig;
public final Timer0 readCodeOwnerConfig;
public final Timer1<String> parseCodeOwnerConfig;
@@ -102,7 +105,15 @@
createLatencyTimer(
"run_code_owner_submit_rule", "Latency for running the code owner submit rule");
- // code owner config latency metrics
+ // code owner config metrics
+ this.codeOwnerConfigBackendReadsPerChange =
+ createHistogram(
+ "code_owner_config_backend_reads_per_change",
+ "Number of code owner config backend reads per change");
+ this.codeOwnerConfigCacheReadsPerChange =
+ createHistogram(
+ "code_owner_config_cache_reads_per_change",
+ "Number of code owner config cache reads per change");
this.loadCodeOwnerConfig =
createTimerWithClassField(
"load_code_owner_config",
@@ -151,4 +162,9 @@
private Counter0 createCounter(String name, String description) {
return metricMaker.newCounter("code_owners/" + name, new Description(description).setRate());
}
+
+ private Histogram0 createHistogram(String name, String description) {
+ return metricMaker.newHistogram(
+ "code_owners/" + name, new Description(description).setCumulative());
+ }
}
diff --git a/resources/Documentation/metrics.md b/resources/Documentation/metrics.md
index 3abb6f8..8e6ee6d 100644
--- a/resources/Documentation/metrics.md
+++ b/resources/Documentation/metrics.md
@@ -30,8 +30,12 @@
* `run_code_owner_submit_rule`:
Latency for running the code owner submit rule.
-### <a id="codeOwnerConfigLatencyMetrics"> Code Owner Config Latency Metrics
+## <a id="codeOwnerConfigMetrics"> Code Owner Config Metrics
+* `code_owner_config_cache_reads_per_change`:
+ Number of code owner config cache reads per change.
+* `code_owner_config_backend_reads_per_change`:
+ Number of code owner config backend reads per change.
* `load_code_owner_config`:
Latency for loading a code owner config file (read + parse).
* `parse_code_owner_config`:
