Add field to change/count_rebases to record allow_conflicts flag
This new field is useful to understand whether rebases that are not done
on behalf of the uploader are due to this flag or triggered from bots
calling the REST API directly.
Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I2e8897c1d8da85658df3fc4fac62866c03190df6
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index e6494e6..cf5da07 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -204,6 +204,8 @@
Whether the rebase was done on behalf of the uploader.
** `rebase_chain`:
Whether a chain was rebased.
+** `allow_conflicts`:
+ Whether the rebase was done with allowing conflicts.
* `change/submitted_with_rebaser_approval`: Number of rebased changes that were
submitted with a Code-Review approval of the rebaser that would not have been
submittable if the rebase was not done on behalf of the uploader.
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 3cb1870..dc3cbdc 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -127,7 +127,7 @@
bu.addOp(change.getId(), rebaseOp);
bu.execute();
- rebaseMetrics.countRebase(input.onBehalfOfUploader);
+ rebaseMetrics.countRebase(input.onBehalfOfUploader, input.allowConflicts);
ChangeInfo changeInfo = json.create(OPTIONS).format(change.getProject(), change.getId());
changeInfo.containsGitConflicts =
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChain.java b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
index 32b5b11..bbac318 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChain.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
@@ -208,7 +208,7 @@
}
}
- rebaseMetrics.countRebaseChain(input.onBehalfOfUploader);
+ rebaseMetrics.countRebaseChain(input.onBehalfOfUploader, input.allowConflicts);
RebaseChainInfo res = new RebaseChainInfo();
res.rebasedChanges = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseMetrics.java b/java/com/google/gerrit/server/restapi/change/RebaseMetrics.java
index 114a112..d6577ea 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseMetrics.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseMetrics.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.restapi.change;
-import com.google.gerrit.metrics.Counter2;
+import com.google.gerrit.metrics.Counter3;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
@@ -24,7 +24,7 @@
/** Metrics for the rebase REST endpoints ({@link Rebase} and {@link RebaseChain}). */
@Singleton
public class RebaseMetrics {
- private final Counter2<Boolean, Boolean> countRebases;
+ private final Counter3<Boolean, Boolean, Boolean> countRebases;
@Inject
public RebaseMetrics(MetricMaker metricMaker) {
@@ -37,18 +37,25 @@
.build(),
Field.ofBoolean("rebase_chain", (metadataBuilder, isRebaseChain) -> {})
.description("Whether a chain was rebased.")
+ .build(),
+ Field.ofBoolean("allow_conflicts", (metadataBuilder, allow_conflicts) -> {})
+ .description("Whether the rebase was done with allowing conflicts.")
.build());
}
- public void countRebase(boolean isOnBehalfOfUploader) {
- countRebase(isOnBehalfOfUploader, /* isRebaseChain= */ false);
+ public void countRebase(boolean isOnBehalfOfUploader, boolean allowConflicts) {
+ countRebase(isOnBehalfOfUploader, /* isRebaseChain= */ false, allowConflicts);
}
- public void countRebaseChain(boolean isOnBehalfOfUploader) {
- countRebase(isOnBehalfOfUploader, /* isRebaseChain= */ true);
+ public void countRebaseChain(boolean isOnBehalfOfUploader, boolean allowConflicts) {
+ countRebase(isOnBehalfOfUploader, /* isRebaseChain= */ true, allowConflicts);
}
- private void countRebase(boolean isOnBehalfOfUploader, boolean isRebaseChain) {
- countRebases.increment(/* field1= */ isOnBehalfOfUploader, /* field2= */ isRebaseChain);
+ private void countRebase(
+ boolean isOnBehalfOfUploader, boolean isRebaseChain, boolean allowConflicts) {
+ countRebases.increment(
+ /* field1= */ isOnBehalfOfUploader,
+ /* field2= */ isRebaseChain,
+ /* field3= */ allowConflicts);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
index 13f9904..e4a6247 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
@@ -1234,11 +1234,11 @@
RebaseInput rebaseInput = new RebaseInput();
rebaseInput.onBehalfOfUploader = true;
gApi.changes().id(changeToBeRebased2.get()).rebaseChain(rebaseInput);
- // field1 is on_behalf_of_uploader, field2 is rebase_chain
- assertThat(testMetricMaker.getCount("change/count_rebases", true, true)).isEqualTo(1);
- assertThat(testMetricMaker.getCount("change/count_rebases", true, false)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", false, false)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", false, true)).isEqualTo(0);
+ // field1 is on_behalf_of_uploader, field2 is rebase_chain, field3 is allow_conflicts
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, true, false)).isEqualTo(1);
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, false, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, false, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, true, false)).isEqualTo(0);
}
private void assertRebase(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
index dee8d1f..4e95032 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
@@ -665,10 +665,15 @@
extensionRegistry.newRegistration().add(wipStateChangedListener)) {
RebaseInput rebaseInput = new RebaseInput();
rebaseInput.allowConflicts = true;
+ testMetricMaker.reset();
ChangeInfo changeInfo =
gApi.changes().id(changeId).revision(patchSet.name()).rebaseAsInfo(rebaseInput);
assertThat(changeInfo.containsGitConflicts).isTrue();
assertThat(changeInfo.workInProgress).isTrue();
+
+ // field1 is on_behalf_of_uploader, field2 is rebase_chain, field3 is allow_conflicts
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, false, true))
+ .isEqualTo(1);
}
assertThat(wipStateChangedListener.invoked).isTrue();
assertThat(wipStateChangedListener.wip).isTrue();
@@ -790,11 +795,12 @@
// Rebase the second change
testMetricMaker.reset();
rebaseCallWithInput.call(r2.getChangeId(), new RebaseInput());
- // field1 is on_behalf_of_uploader, field2 is rebase_chain
- assertThat(testMetricMaker.getCount("change/count_rebases", false, false)).isEqualTo(1);
- assertThat(testMetricMaker.getCount("change/count_rebases", true, false)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", true, true)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", false, true)).isEqualTo(0);
+ // field1 is on_behalf_of_uploader, field2 is rebase_chain, field3 is allow_conflicts
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, false, false))
+ .isEqualTo(1);
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, false, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, true, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, true, false)).isEqualTo(0);
}
@Test
@@ -1249,11 +1255,12 @@
testMetricMaker.reset();
verifyRebaseChainResponse(
gApi.changes().id(r4.getChangeId()).rebaseChain(), false, r2, r3, r4);
- // field1 is on_behalf_of_uploader, field2 is rebase_chain
- assertThat(testMetricMaker.getCount("change/count_rebases", false, true)).isEqualTo(1);
- assertThat(testMetricMaker.getCount("change/count_rebases", false, false)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", true, true)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", true, false)).isEqualTo(0);
+ // field1 is on_behalf_of_uploader, field2 is rebase_chain, field3 is allow_conflicts
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, true, false)).isEqualTo(1);
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, false, false))
+ .isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, true, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, false, false)).isEqualTo(0);
}
private void verifyRebaseChainResponse(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
index 8565ced..4d194f01 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
@@ -1105,11 +1105,11 @@
RebaseInput rebaseInput = new RebaseInput();
rebaseInput.onBehalfOfUploader = true;
gApi.changes().id(changeToBeRebased.get()).rebase(rebaseInput);
- // field1 is on_behalf_of_uploader, field2 is rebase_chain
- assertThat(testMetricMaker.getCount("change/count_rebases", true, false)).isEqualTo(1);
- assertThat(testMetricMaker.getCount("change/count_rebases", false, false)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", true, true)).isEqualTo(0);
- assertThat(testMetricMaker.getCount("change/count_rebases", false, true)).isEqualTo(0);
+ // field1 is on_behalf_of_uploader, field2 is rebase_chain, field3 is allow_conflicts
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, false, false)).isEqualTo(1);
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, false, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", true, true, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount("change/count_rebases", false, true, false)).isEqualTo(0);
}
private void allowPermissionToAllUsers(String permission) {
