Split latency metric for computing changed files
The logic to compute the changed files against auto merge and first
parent is quite different, hence latency should be recorded separately.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ice2857a1510f129030f8d6553358122df539f2ec
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/metrics/CodeOwnerMetrics.java b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
index 8cfc74d..2f3ce40 100644
--- a/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
+++ b/java/com/google/gerrit/plugins/codeowners/metrics/CodeOwnerMetrics.java
@@ -28,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;
@@ -56,8 +57,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");
diff --git a/resources/Documentation/metrics.md b/resources/Documentation/metrics.md
index bc80415..481c61a 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`:
