Merge changes Iea433d14,Ice2857a1,Ib6927fa6
* changes:
Add metric to count the number of code owner submit rule runs
Split latency metric for computing changed files
Add metrics to record latency for loading/reading/parsing OWNERS files
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java b/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java
index 2824246..20a44c2 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackend.java
@@ -52,6 +52,7 @@
private final MetaDataUpdate.InternalFactory metaDataUpdateInternalFactory;
private final RetryHelper retryHelper;
private final String defaultFileName;
+ private final CodeOwnerConfigFile.Factory codeOwnerConfigFileFactory;
private final CodeOwnerConfigParser codeOwnerConfigParser;
protected AbstractFileBasedCodeOwnerBackend(
@@ -61,6 +62,7 @@
MetaDataUpdate.InternalFactory metaDataUpdateInternalFactory,
RetryHelper retryHelper,
String defaultFileName,
+ CodeOwnerConfigFile.Factory codeOwnerConfigFileFactory,
CodeOwnerConfigParser codeOwnerConfigParser) {
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.repoManager = repoManager;
@@ -68,6 +70,7 @@
this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
this.retryHelper = retryHelper;
this.defaultFileName = defaultFileName;
+ this.codeOwnerConfigFileFactory = codeOwnerConfigFileFactory;
this.codeOwnerConfigParser = codeOwnerConfigParser;
}
@@ -103,7 +106,7 @@
@Nullable ObjectId revision) {
try (Repository repository = repoManager.openRepository(codeOwnerConfigKey.project())) {
if (revision == null) {
- return CodeOwnerConfigFile.loadCurrent(
+ return codeOwnerConfigFileFactory.loadCurrent(
fileName, codeOwnerConfigParser, repository, codeOwnerConfigKey);
}
@@ -113,7 +116,7 @@
revWalk = new RevWalk(repository);
}
try {
- return CodeOwnerConfigFile.load(
+ return codeOwnerConfigFileFactory.load(
fileName, codeOwnerConfigParser, revWalk, revision, codeOwnerConfigKey);
} finally {
if (closeRevWalk) {
@@ -222,7 +225,8 @@
CodeOwnerConfigUpdate codeOwnerConfigUpdate) {
try (Repository repository = repoManager.openRepository(codeOwnerConfigKey.project())) {
CodeOwnerConfigFile codeOwnerConfigFile =
- CodeOwnerConfigFile.loadCurrent(
+ codeOwnerConfigFileFactory
+ .loadCurrent(
getFileName(codeOwnerConfigKey.project()),
codeOwnerConfigParser,
repository,
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
index cf88e62..4fdab65 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
@@ -146,14 +146,12 @@
logger.atFine().log(
"computing changed files for revision %s in project %s", revCommit.name(), project);
- try (Timer0.Context ctx = codeOwnerMetrics.computeChangedFiles.start()) {
- if (revCommit.getParentCount() > 1
- && MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION.equals(mergeCommitStrategy)) {
- return computeByComparingAgainstAutoMerge(project, revCommit);
- }
-
- return computeByComparingAgainstFirstParent(repoConfig, revWalk, revCommit);
+ if (revCommit.getParentCount() > 1
+ && MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION.equals(mergeCommitStrategy)) {
+ return computeByComparingAgainstAutoMerge(project, revCommit);
}
+
+ return computeByComparingAgainstFirstParent(repoConfig, revWalk, revCommit);
}
/**
@@ -171,16 +169,19 @@
checkState(
mergeCommit.getParentCount() > 1, "expected %s to be a merge commit", mergeCommit.name());
- // for merge commits the default base is the auto merge commit
- PatchListKey patchListKey =
- PatchListKey.againstDefaultBase(mergeCommit, Whitespace.IGNORE_NONE);
+ try (Timer0.Context ctx = codeOwnerMetrics.computeChangedFilesAgainstAutoMerge.start()) {
+ // for merge commits the default base is the auto merge commit
+ PatchListKey patchListKey =
+ PatchListKey.againstDefaultBase(mergeCommit, Whitespace.IGNORE_NONE);
- return patchListCache.get(patchListKey, project).getPatches().stream()
- .filter(
- patchListEntry ->
- patchListEntry.getNewName() == null || !Patch.isMagic(patchListEntry.getNewName()))
- .map(ChangedFile::create)
- .collect(toImmutableSet());
+ return patchListCache.get(patchListKey, project).getPatches().stream()
+ .filter(
+ patchListEntry ->
+ patchListEntry.getNewName() == null
+ || !Patch.isMagic(patchListEntry.getNewName()))
+ .map(ChangedFile::create)
+ .collect(toImmutableSet());
+ }
}
/**
@@ -195,25 +196,27 @@
*/
private ImmutableSet<ChangedFile> computeByComparingAgainstFirstParent(
Config repoConfig, RevWalk revWalk, RevCommit revCommit) throws IOException {
- RevCommit baseCommit = revCommit.getParentCount() > 0 ? revCommit.getParent(0) : null;
- logger.atFine().log("baseCommit = %s", baseCommit != null ? baseCommit.name() : "n/a");
+ try (Timer0.Context ctx = codeOwnerMetrics.computeChangedFilesAgainstFirstParent.start()) {
+ RevCommit baseCommit = revCommit.getParentCount() > 0 ? revCommit.getParent(0) : null;
+ logger.atFine().log("baseCommit = %s", baseCommit != null ? baseCommit.name() : "n/a");
- try (DiffFormatter diffFormatter = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
- diffFormatter.setReader(revWalk.getObjectReader(), repoConfig);
- diffFormatter.setDiffComparator(RawTextComparator.DEFAULT);
- diffFormatter.setDetectRenames(true);
- List<DiffEntry> diffEntries = diffFormatter.scan(baseCommit, revCommit);
- ImmutableSet<ChangedFile> changedFiles =
- diffEntries.stream().map(ChangedFile::create).collect(toImmutableSet());
- if (changedFiles.size() <= MAX_CHANGED_FILES_TO_LOG) {
- logger.atFine().log("changed files = %s", changedFiles);
- } else {
- logger.atFine().log(
- "changed files = %s (and %d more)",
- changedFiles.asList().subList(0, MAX_CHANGED_FILES_TO_LOG),
- changedFiles.size() - MAX_CHANGED_FILES_TO_LOG);
+ try (DiffFormatter diffFormatter = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
+ diffFormatter.setReader(revWalk.getObjectReader(), repoConfig);
+ diffFormatter.setDiffComparator(RawTextComparator.DEFAULT);
+ diffFormatter.setDetectRenames(true);
+ List<DiffEntry> diffEntries = diffFormatter.scan(baseCommit, revCommit);
+ ImmutableSet<ChangedFile> changedFiles =
+ diffEntries.stream().map(ChangedFile::create).collect(toImmutableSet());
+ if (changedFiles.size() <= MAX_CHANGED_FILES_TO_LOG) {
+ logger.atFine().log("changed files = %s", changedFiles);
+ } else {
+ logger.atFine().log(
+ "changed files = %s (and %d more)",
+ changedFiles.asList().subList(0, MAX_CHANGED_FILES_TO_LOG),
+ changedFiles.size() - MAX_CHANGED_FILES_TO_LOG);
+ }
+ return changedFiles;
}
- return changedFiles;
}
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java
index 4cceb13..b704f2b 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFile.java
@@ -19,9 +19,13 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
+import com.google.gerrit.metrics.Timer0;
+import com.google.gerrit.metrics.Timer1;
+import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.git.meta.VersionedMetaData;
+import com.google.inject.Inject;
import java.io.IOException;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -36,9 +40,9 @@
* A representation of a code owner config that is stored as an {@code OWNERS} file in a source
* branch.
*
- * <p>For reading code owner configs or creating/updating them, refer to {@link #load(String,
- * CodeOwnerConfigParser, RevWalk, ObjectId, CodeOwnerConfig.Key)} and {@link #loadCurrent(String,
- * CodeOwnerConfigParser, Repository, CodeOwnerConfig.Key)}.
+ * <p>For reading code owner configs or creating/updating them, refer to {@link Factory#load(String,
+ * CodeOwnerConfigParser, RevWalk, ObjectId, CodeOwnerConfig.Key)} and {@link
+ * Factory#loadCurrent(String, CodeOwnerConfigParser, Repository, CodeOwnerConfig.Key)}.
*
* <p><strong>Note:</strong> Any modification (code owner config creation or update) only becomes
* permanent (and hence written to repository) if {@link
@@ -46,83 +50,95 @@
*/
@VisibleForTesting
public class CodeOwnerConfigFile extends VersionedMetaData {
- /**
- * Creates a {@link CodeOwnerConfigFile} for a code owner config.
- *
- * <p>The code owner config is automatically loaded within this method and can be accessed via
- * {@link #getLoadedCodeOwnerConfig()}.
- *
- * <p>It's safe to call this method for non-existing code owner configs. In that case, {@link
- * #getLoadedCodeOwnerConfig()} won't return any code owner config. Thus, the existence of a code
- * owner config can be easily tested.
- *
- * <p>The code owner config represented by the returned {@link CodeOwnerConfigFile} can be
- * created/updated by setting an {@link CodeOwnerConfigUpdate} via {@link
- * #setCodeOwnerConfigUpdate(CodeOwnerConfigUpdate)} and committing the {@link
- * CodeOwnerConfigUpdate} via {@link #commit(com.google.gerrit.server.git.meta.MetaDataUpdate)}.
- *
- * @param defaultFileName the name of the code owner configuration files that should be used if
- * none is specified in the code owner config key
- * @param codeOwnerConfigParser the parser that should be used to parse code owner config files
- * @param revWalk the revWalk that should be used to load the revision
- * @param revision the branch revision from which the code owner config file should be loaded
- * @param codeOwnerConfigKey the key of the code owner config
- * @return a {@link CodeOwnerConfigFile} for the code owner config with the specified key
- * @throws IOException if the repository can't be accessed for some reason
- * @throws ConfigInvalidException if the code owner config exists but can't be read due to an
- * invalid format
- */
- public static CodeOwnerConfigFile load(
- String defaultFileName,
- CodeOwnerConfigParser codeOwnerConfigParser,
- RevWalk revWalk,
- ObjectId revision,
- CodeOwnerConfig.Key codeOwnerConfigKey)
- throws IOException, ConfigInvalidException {
- requireNonNull(defaultFileName, "defaultFileName");
- requireNonNull(codeOwnerConfigParser, "codeOwnerConfigParser");
- requireNonNull(revWalk, "revWalk");
- requireNonNull(revision, "revision");
- requireNonNull(codeOwnerConfigKey, "codeOwnerConfigKey");
+ public static class Factory {
+ private final CodeOwnerMetrics codeOwnerMetrics;
- CodeOwnerConfigFile codeOwnerConfigFile =
- new CodeOwnerConfigFile(defaultFileName, codeOwnerConfigParser, codeOwnerConfigKey);
- codeOwnerConfigFile.load(codeOwnerConfigKey.project(), revWalk, revision);
- return codeOwnerConfigFile;
+ @Inject
+ Factory(CodeOwnerMetrics codeOwnerMetrics) {
+ this.codeOwnerMetrics = codeOwnerMetrics;
+ }
+
+ /**
+ * Creates a {@link CodeOwnerConfigFile} for a code owner config.
+ *
+ * <p>The code owner config is automatically loaded within this method and can be accessed via
+ * {@link #getLoadedCodeOwnerConfig()}.
+ *
+ * <p>It's safe to call this method for non-existing code owner configs. In that case, {@link
+ * #getLoadedCodeOwnerConfig()} won't return any code owner config. Thus, the existence of a
+ * code owner config can be easily tested.
+ *
+ * <p>The code owner config represented by the returned {@link CodeOwnerConfigFile} can be
+ * created/updated by setting an {@link CodeOwnerConfigUpdate} via {@link
+ * #setCodeOwnerConfigUpdate(CodeOwnerConfigUpdate)} and committing the {@link
+ * CodeOwnerConfigUpdate} via {@link #commit(com.google.gerrit.server.git.meta.MetaDataUpdate)}.
+ *
+ * @param defaultFileName the name of the code owner configuration files that should be used if
+ * none is specified in the code owner config key
+ * @param codeOwnerConfigParser the parser that should be used to parse code owner config files
+ * @param revWalk the revWalk that should be used to load the revision
+ * @param revision the branch revision from which the code owner config file should be loaded
+ * @param codeOwnerConfigKey the key of the code owner config
+ * @return a {@link CodeOwnerConfigFile} for the code owner config with the specified key
+ * @throws IOException if the repository can't be accessed for some reason
+ * @throws ConfigInvalidException if the code owner config exists but can't be read due to an
+ * invalid format
+ */
+ public CodeOwnerConfigFile load(
+ String defaultFileName,
+ CodeOwnerConfigParser codeOwnerConfigParser,
+ RevWalk revWalk,
+ ObjectId revision,
+ CodeOwnerConfig.Key codeOwnerConfigKey)
+ throws IOException, ConfigInvalidException {
+ requireNonNull(defaultFileName, "defaultFileName");
+ requireNonNull(codeOwnerConfigParser, "codeOwnerConfigParser");
+ requireNonNull(revWalk, "revWalk");
+ requireNonNull(revision, "revision");
+ requireNonNull(codeOwnerConfigKey, "codeOwnerConfigKey");
+
+ CodeOwnerConfigFile codeOwnerConfigFile =
+ new CodeOwnerConfigFile(
+ codeOwnerMetrics, defaultFileName, codeOwnerConfigParser, codeOwnerConfigKey);
+ codeOwnerConfigFile.load(codeOwnerConfigKey.project(), revWalk, revision);
+ return codeOwnerConfigFile;
+ }
+
+ /**
+ * Creates a {@link CodeOwnerConfigFile} for a code owner config from the current revision in
+ * the branch.
+ *
+ * @param defaultFileName the name of the code owner configuration files that should be used if
+ * none is specified in the code owner config key
+ * @param codeOwnerConfigParser the parser that should be used to parse code owner config files
+ * @param repository the repository in which the code owner config is stored
+ * @param codeOwnerConfigKey the key of the code owner config
+ * @return a {@link CodeOwnerConfigFile} for the code owner config with the specified key
+ * @throws IOException if the repository can't be accessed for some reason
+ * @throws ConfigInvalidException if the code owner config exists but can't be read due to an
+ * invalid format
+ * @see #load(String, CodeOwnerConfigParser, RevWalk, ObjectId, CodeOwnerConfig.Key)
+ */
+ public CodeOwnerConfigFile loadCurrent(
+ String defaultFileName,
+ CodeOwnerConfigParser codeOwnerConfigParser,
+ Repository repository,
+ CodeOwnerConfig.Key codeOwnerConfigKey)
+ throws IOException, ConfigInvalidException {
+ requireNonNull(defaultFileName, "defaultFileName");
+ requireNonNull(codeOwnerConfigParser, "codeOwnerConfigParser");
+ requireNonNull(repository, "repository");
+ requireNonNull(codeOwnerConfigKey, "codeOwnerConfigKey");
+
+ CodeOwnerConfigFile codeOwnerConfigFile =
+ new CodeOwnerConfigFile(
+ codeOwnerMetrics, defaultFileName, codeOwnerConfigParser, codeOwnerConfigKey);
+ codeOwnerConfigFile.load(codeOwnerConfigKey.project(), repository);
+ return codeOwnerConfigFile;
+ }
}
- /**
- * Creates a {@link CodeOwnerConfigFile} for a code owner config from the current revision in the
- * branch.
- *
- * @param defaultFileName the name of the code owner configuration files that should be used if
- * none is specified in the code owner config key
- * @param codeOwnerConfigParser the parser that should be used to parse code owner config files
- * @param repository the repository in which the code owner config is stored
- * @param codeOwnerConfigKey the key of the code owner config
- * @return a {@link CodeOwnerConfigFile} for the code owner config with the specified key
- * @throws IOException if the repository can't be accessed for some reason
- * @throws ConfigInvalidException if the code owner config exists but can't be read due to an
- * invalid format
- * @see #load(String, CodeOwnerConfigParser, RevWalk, ObjectId, CodeOwnerConfig.Key)
- */
- public static CodeOwnerConfigFile loadCurrent(
- String defaultFileName,
- CodeOwnerConfigParser codeOwnerConfigParser,
- Repository repository,
- CodeOwnerConfig.Key codeOwnerConfigKey)
- throws IOException, ConfigInvalidException {
- requireNonNull(defaultFileName, "defaultFileName");
- requireNonNull(codeOwnerConfigParser, "codeOwnerConfigParser");
- requireNonNull(repository, "repository");
- requireNonNull(codeOwnerConfigKey, "codeOwnerConfigKey");
-
- CodeOwnerConfigFile codeOwnerConfigFile =
- new CodeOwnerConfigFile(defaultFileName, codeOwnerConfigParser, codeOwnerConfigKey);
- codeOwnerConfigFile.load(codeOwnerConfigKey.project(), repository);
- return codeOwnerConfigFile;
- }
-
+ private final CodeOwnerMetrics codeOwnerMetrics;
private final String defaultFileName;
private final CodeOwnerConfigParser codeOwnerConfigParser;
private final CodeOwnerConfig.Key codeOwnerConfigKey;
@@ -132,9 +148,11 @@
private Optional<CodeOwnerConfigUpdate> codeOwnerConfigUpdate = Optional.empty();
private CodeOwnerConfigFile(
+ CodeOwnerMetrics codeOwnerMetrics,
String defaultFileName,
CodeOwnerConfigParser codeOwnerConfigParser,
CodeOwnerConfig.Key codeOwnerConfigKey) {
+ this.codeOwnerMetrics = codeOwnerMetrics;
this.defaultFileName = defaultFileName;
this.codeOwnerConfigParser = codeOwnerConfigParser;
this.codeOwnerConfigKey = codeOwnerConfigKey;
@@ -186,12 +204,21 @@
}
@Override
+ protected byte[] readFile(String fileName) throws IOException {
+ try (Timer0.Context ctx = codeOwnerMetrics.readCodeOwnerConfig.start()) {
+ return super.readFile(fileName);
+ }
+ }
+
+ @Override
protected void onLoad() throws IOException, ConfigInvalidException {
if (revision != null) {
Optional<String> codeOwnerConfigFileContent =
getFileIfItExists(JgitPath.of(codeOwnerConfigKey.filePath(defaultFileName)).get());
if (codeOwnerConfigFileContent.isPresent()) {
- try {
+ try (Timer1.Context<String> ctx =
+ codeOwnerMetrics.parseCodeOwnerConfig.start(
+ codeOwnerConfigParser.getClass().getSimpleName())) {
loadedCodeOwnersConfig =
Optional.of(
codeOwnerConfigParser.parse(
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
index 0683b25..1785a1d 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
@@ -78,6 +78,7 @@
}
try (Timer0.Context ctx = codeOwnerMetrics.runCodeOwnerSubmitRule.start()) {
+ codeOwnerMetrics.countCodeOwnerConfigReads.increment();
logger.atFine().log(
"run code owner submit rule (project = %s, change = %d)",
changeData.project().get(), changeData.getId().get());
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java
index 4247849..32ad7b5 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java
@@ -17,6 +17,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.base.Throwables;
+import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.inject.Inject;
@@ -57,7 +58,10 @@
codeOwnersPluginConfiguration
.getProjectConfig(codeOwnerConfigKey.project())
.getBackend(codeOwnerConfigKey.branchNameKey().branch());
- return codeOwnerBackend.getCodeOwnerConfig(codeOwnerConfigKey, revision);
+ try (Timer1.Context<String> ctx =
+ codeOwnerMetrics.loadCodeOwnerConfig.start(codeOwnerBackend.getClass().getSimpleName())) {
+ return codeOwnerBackend.getCodeOwnerConfig(codeOwnerConfigKey, revision);
+ }
}
@Override
@@ -68,7 +72,10 @@
codeOwnersPluginConfiguration
.getProjectConfig(codeOwnerConfigKey.project())
.getBackend(codeOwnerConfigKey.branchNameKey().branch());
- return codeOwnerBackend.getCodeOwnerConfig(codeOwnerConfigKey, /* revision= */ null);
+ try (Timer1.Context<String> ctx =
+ codeOwnerMetrics.loadCodeOwnerConfig.start(codeOwnerBackend.getClass().getSimpleName())) {
+ return codeOwnerBackend.getCodeOwnerConfig(codeOwnerConfigKey, /* revision= */ null);
+ }
}
/**
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersBackend.java b/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersBackend.java
index 257c1d4..0e923d7 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersBackend.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersBackend.java
@@ -17,6 +17,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.plugins.codeowners.backend.AbstractFileBasedCodeOwnerBackend;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigFile;
import com.google.gerrit.plugins.codeowners.backend.FindOwnersGlobMatcher;
import com.google.gerrit.plugins.codeowners.backend.PathExpressionMatcher;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
@@ -44,6 +45,7 @@
@Inject
FindOwnersBackend(
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
+ CodeOwnerConfigFile.Factory codeOwnerConfigFileFactory,
FindOwnersCodeOwnerConfigParser codeOwnerConfigParser,
GitRepositoryManager repoManager,
@GerritPersonIdent PersonIdent serverIdent,
@@ -56,6 +58,7 @@
metaDataUpdateInternalFactory,
retryHelper,
CODE_OWNER_CONFIG_FILE_NAME,
+ codeOwnerConfigFileFactory,
codeOwnerConfigParser);
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/proto/ProtoBackend.java b/java/com/google/gerrit/plugins/codeowners/backend/proto/ProtoBackend.java
index e8e0694..b273ab2 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/proto/ProtoBackend.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/proto/ProtoBackend.java
@@ -17,6 +17,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.plugins.codeowners.backend.AbstractFileBasedCodeOwnerBackend;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigFile;
import com.google.gerrit.plugins.codeowners.backend.PathExpressionMatcher;
import com.google.gerrit.plugins.codeowners.backend.SimplePathExpressionMatcher;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
@@ -52,6 +53,7 @@
@GerritPersonIdent PersonIdent serverIdent,
MetaDataUpdate.InternalFactory metaDataUpdateInternalFactory,
RetryHelper retryHelper,
+ CodeOwnerConfigFile.Factory codeOwnerConfigFileFactory,
ProtoCodeOwnerConfigParser codeOwnerConfigParser) {
super(
codeOwnersPluginConfiguration,
@@ -60,6 +62,7 @@
metaDataUpdateInternalFactory,
retryHelper,
CODE_OWNER_CONFIG_FILE_NAME,
+ codeOwnerConfigFileFactory,
codeOwnerConfigParser);
}
diff --git a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
index e5a3c58..56aec3c 100644
--- a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
@@ -17,8 +17,10 @@
import com.google.gerrit.metrics.Counter0;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
+import com.google.gerrit.metrics.Timer1;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -26,7 +28,8 @@
@Singleton
public class CodeOwnerMetrics {
// latency metrics
- public final Timer0 computeChangedFiles;
+ public final Timer0 computeChangedFilesAgainstAutoMerge;
+ public final Timer0 computeChangedFilesAgainstFirstParent;
public final Timer0 computeFileStatus;
public final Timer0 computeFileStatuses;
public final Timer0 computeOwnedPaths;
@@ -38,9 +41,15 @@
public final Timer0 resolvePathCodeOwners;
public final Timer0 runCodeOwnerSubmitRule;
+ // code owner config latency metrics
+ public final Timer1<String> loadCodeOwnerConfig;
+ public final Timer0 readCodeOwnerConfig;
+ public final Timer1<String> parseCodeOwnerConfig;
+
// counter metrics
public final Counter0 countCodeOwnerConfigReads;
public final Counter0 countCodeOwnerConfigCacheReads;
+ public final Counter0 countCodeOwnerSubmitRuleRuns;
private final MetricMaker metricMaker;
@@ -49,8 +58,14 @@
this.metricMaker = metricMaker;
// latency metrics
- this.computeChangedFiles =
- createLatencyTimer("compute_changed_files", "Latency for computing changed files");
+ this.computeChangedFilesAgainstAutoMerge =
+ createLatencyTimer(
+ "compute_changed_files_against_auto_merge",
+ "Latency for computing changed files against auto merge");
+ this.computeChangedFilesAgainstFirstParent =
+ createLatencyTimer(
+ "compute_changed_files_against_first_parent",
+ "Latency for computing changed files against first parent");
this.computeFileStatus =
createLatencyTimer(
"compute_file_status", "Latency for computing the file status of one file");
@@ -87,6 +102,19 @@
createLatencyTimer(
"run_code_owner_submit_rule", "Latency for running the code owner submit rule");
+ // code owner config latency metrics
+ this.loadCodeOwnerConfig =
+ createTimerWithClassField(
+ "load_code_owner_config",
+ "Latency for loading a code owner config file (read + parse)",
+ "backend");
+ this.parseCodeOwnerConfig =
+ createTimerWithClassField(
+ "parse_code_owner_config", "Latency for parsing a code owner config file", "parser");
+ this.readCodeOwnerConfig =
+ createLatencyTimer(
+ "read_code_owner_config", "Latency for reading a code owner config file");
+
// counter metrics
this.countCodeOwnerConfigReads =
createCounter(
@@ -96,6 +124,9 @@
createCounter(
"count_code_owner_config_cache_reads",
"Total number of code owner config reads from cache");
+ this.countCodeOwnerSubmitRuleRuns =
+ createCounter(
+ "count_code_owner_submit_rule_runs", "Total number of code owner submit rule runs");
}
private Timer0 createLatencyTimer(String name, String description) {
@@ -104,6 +135,19 @@
new Description(description).setCumulative().setUnit(Units.MILLISECONDS));
}
+ private Timer1<String> createTimerWithClassField(
+ String name, String description, String fieldName) {
+ Field<String> CODE_OWNER_BACKEND_FIELD =
+ Field.ofString(
+ fieldName, (metadataBuilder, fieldValue) -> metadataBuilder.className(fieldValue))
+ .build();
+
+ return metricMaker.newTimer(
+ "code_owners/" + name,
+ new Description(description).setCumulative().setUnit(Description.Units.MILLISECONDS),
+ CODE_OWNER_BACKEND_FIELD);
+ }
+
private Counter0 createCounter(String name, String description) {
return metricMaker.newCounter("code_owners/" + name, new Description(description).setRate());
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileTest.java
index 61bbdb7..e6b776d 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigFileTest.java
@@ -47,10 +47,13 @@
@Inject private ProjectOperations projectOperations;
@Inject private MetaDataUpdate.Server metaDataUpdateServer;
+ private CodeOwnerConfigFile.Factory codeOwnerConfigFileFactory;
private TestCodeOwnerConfigStorage testCodeOwnerConfigStorage;
@Before
public void setUpCodeOwnersPlugin() throws Exception {
+ codeOwnerConfigFileFactory =
+ plugin.getSysInjector().getInstance(CodeOwnerConfigFile.Factory.class);
testCodeOwnerConfigStorage =
plugin
.getSysInjector()
@@ -534,7 +537,7 @@
private CodeOwnerConfigFile loadCodeOwnerConfig(CodeOwnerConfig.Key codeOwnerConfigKey)
throws IOException, ConfigInvalidException {
try (Repository repository = repoManager.openRepository(codeOwnerConfigKey.project())) {
- return CodeOwnerConfigFile.loadCurrent(
+ return codeOwnerConfigFileFactory.loadCurrent(
CODE_OWNER_CONFIG_FILE_NAME, CODE_OWNER_CONFIG_PARSER, repository, codeOwnerConfigKey);
}
}
@@ -544,7 +547,7 @@
throws IOException, ConfigInvalidException {
try (Repository repository = repoManager.openRepository(codeOwnerConfigKey.project());
RevWalk revWalk = new RevWalk(repository)) {
- return CodeOwnerConfigFile.load(
+ return codeOwnerConfigFileFactory.load(
CODE_OWNER_CONFIG_FILE_NAME,
CODE_OWNER_CONFIG_PARSER,
revWalk,
diff --git a/resources/Documentation/metrics.md b/resources/Documentation/metrics.md
index d236b3e..3abb6f8 100644
--- a/resources/Documentation/metrics.md
+++ b/resources/Documentation/metrics.md
@@ -5,8 +5,10 @@
## <a id="latencyMetrics"> Latency Metrics
-* `compute_changed_files`:
- Latency for computing changed files.
+* `compute_changed_files_against_auto_merge`:
+ Latency for computing changed files against auto merge.
+* `compute_changed_files_against_first_parent`:
+ Latency for computing changed files against first parent.
* `compute_file_status`:
Latency for computing the file status for one file.
* `compute_file_statuses`:
@@ -28,12 +30,23 @@
* `run_code_owner_submit_rule`:
Latency for running the code owner submit rule.
+### <a id="codeOwnerConfigLatencyMetrics"> Code Owner Config Latency Metrics
+
+* `load_code_owner_config`:
+ Latency for loading a code owner config file (read + parse).
+* `parse_code_owner_config`:
+ Latency for parsing a code owner config file.
+* `read_code_owner_config`:
+ Latency for reading a code owner config file.
+
## <a id="counterMetrics"> Counter Metrics
* `count_code_owner_config_reads`:
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_submit_rule_runs`:
+ Total number of code owner submit rule runs.
---
