Collect metrics upon ref replication event
In a multi-site/HA setup metrics would only be collected
on the node receiving the receive-pack.
In case of an active/passive configuration, metrics won't
be collected at all on the passive nodes.
Collect metrics on *-replication-done events.
Bug: Issue 16278
Change-Id: Ie838afd9b9cf340c5551f3930b07cbcdc4f1622a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java
index e9e4d98..3ac4903 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java
@@ -15,34 +15,56 @@
package com.googlesource.gerrit.plugins.gitrepometrics;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.server.config.GerritInstanceId;
+import com.google.gerrit.server.events.Event;
+import com.google.gerrit.server.events.EventListener;
+import com.google.gerrit.server.events.ProjectEvent;
+import com.google.gerrit.server.events.RefUpdatedEvent;
import com.google.inject.Inject;
+import java.util.Objects;
import java.util.concurrent.ExecutorService;
-public class GitRepoUpdateListener implements GitReferenceUpdatedListener {
+class GitRepoUpdateListener implements EventListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ExecutorService executor;
private final UpdateGitMetricsTask.Factory updateGitMetricsTaskFactory;
private final GitRepoMetricsCache gitRepoMetricsCache;
+ private final String instanceId;
@Inject
- GitRepoUpdateListener(
+ protected GitRepoUpdateListener(
+ @GerritInstanceId String instanceId,
@UpdateGitMetricsExecutor ExecutorService executor,
UpdateGitMetricsTask.Factory updateGitMetricsTaskFactory,
GitRepoMetricsCache gitRepoMetricsCache) {
+ this.instanceId = instanceId;
this.executor = executor;
this.updateGitMetricsTaskFactory = updateGitMetricsTaskFactory;
this.gitRepoMetricsCache = gitRepoMetricsCache;
}
@Override
- public void onGitReferenceUpdated(Event event) {
- String projectName = event.getProjectName();
- logger.atFine().log("Got an update for project %s", projectName);
+ public void onEvent(Event event) {
+ if (event instanceof RefUpdatedEvent || isReplicationDoneEvent(event)) {
+ String projectName = ((ProjectEvent) event).getProjectNameKey().get();
+ logger.atFine().log(
+ String.format(
+ "Got %s event from %s. Might need to collect metrics for project %s",
+ event.type, event.instanceId, projectName));
- if (gitRepoMetricsCache.shouldCollectStats(projectName)) {
- UpdateGitMetricsTask updateGitMetricsTask = updateGitMetricsTaskFactory.create(projectName);
- executor.execute(updateGitMetricsTask);
+ if (gitRepoMetricsCache.shouldCollectStats(projectName)) {
+ UpdateGitMetricsTask updateGitMetricsTask = updateGitMetricsTaskFactory.create(projectName);
+ executor.execute(updateGitMetricsTask);
+ }
}
}
+
+ private boolean isReplicationDoneEvent(Event event) {
+ // Check the name of the event instead of checking the class type
+ // to avoid importing pull and push replication plugin dependencies
+ // only for this check.
+ return event.type != null
+ && !Objects.equals(event.instanceId, instanceId)
+ && event.type.endsWith("-replication-done");
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/Module.java b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/Module.java
index 8428ad8..10c116a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/Module.java
@@ -14,9 +14,9 @@
package com.googlesource.gerrit.plugins.gitrepometrics;
-import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.events.EventListener;
import com.google.inject.Scopes;
import com.googlesource.gerrit.plugins.gitrepometrics.collectors.FSMetricsCollector;
import com.googlesource.gerrit.plugins.gitrepometrics.collectors.GitStatsMetricsCollector;
@@ -32,7 +32,8 @@
.annotatedWith(UpdateGitMetricsExecutor.class)
.toProvider(UpdateGitMetricsExecutorProvider.class);
bind(GitRepoUpdateListener.class);
- DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(GitRepoUpdateListener.class);
+ DynamicSet.bind(binder(), EventListener.class).to(GitRepoUpdateListener.class);
+
DynamicSet.setOf(binder(), MetricsCollector.class);
DynamicSet.bind(binder(), MetricsCollector.class).to(GitStatsMetricsCollector.class);
DynamicSet.bind(binder(), MetricsCollector.class).to(FSMetricsCollector.class);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java
index a97adc2..095d8a6 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java
@@ -18,15 +18,16 @@
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verifyNoInteractions;
import com.codahale.metrics.MetricRegistry;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.extensions.api.changes.NotifyHandling;
-import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.server.data.RefUpdateAttribute;
+import com.google.gerrit.server.events.RefEvent;
+import com.google.gerrit.server.events.RefUpdatedEvent;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.testing.InMemoryModule;
import com.google.gerrit.testing.InMemoryRepositoryManager;
@@ -38,7 +39,6 @@
import java.io.IOException;
import java.util.Collections;
import java.util.concurrent.ExecutorService;
-import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
@@ -47,23 +47,24 @@
private final GitRepositoryManager repoManager = new InMemoryRepositoryManager();
private final ExecutorService mockedExecutorService = mock(ExecutorService.class);
private GitRepoUpdateListener gitRepoUpdateListener;
- private final String testProject = "testProject";
- private final Project.NameKey testProjectNameKey = Project.nameKey("testProject");
- private Repository repository;
- ArgumentCaptor<UpdateGitMetricsTask> valueCapture =
+ private final String enabledProject = "enabledProject";
+ private final Project.NameKey enabledProjectNameKey = Project.nameKey(enabledProject);
+
+ ArgumentCaptor<UpdateGitMetricsTask> updateGitMetricsTaskCaptor =
ArgumentCaptor.forClass(UpdateGitMetricsTask.class);
private GitRepoMetricsCache gitRepoMetricsCache;
-
private final String disabledProject = "disabledProject";
private final Project.NameKey disabledProjectNameKey = Project.nameKey(disabledProject);
- private Repository disabledRepository;
+ private final String producerInstanceId = "producerInstanceId";
+ private final String consumerInstanceId = "consumerInstanceId";
+ private final String refReplicationDoneType = "ref-replication-done";
@Inject private UpdateGitMetricsTask.Factory updateGitMetricsTaskFactory;
@Before
public void setupRepo() throws IOException {
ConfigSetupUtils configSetupUtils =
- new ConfigSetupUtils(Collections.singletonList(testProject));
+ new ConfigSetupUtils(Collections.singletonList(enabledProject));
gitRepoMetricsCache =
new GitRepoMetricsCache(
new DynamicSet<>(),
@@ -87,73 +88,105 @@
injector.injectMembers(this);
reset(mockedExecutorService);
+ doNothing().when(mockedExecutorService).execute(updateGitMetricsTaskCaptor.capture());
+ repoManager.createRepository(enabledProjectNameKey);
+ repoManager.createRepository(disabledProjectNameKey);
+
gitRepoUpdateListener =
new GitRepoUpdateListener(
- mockedExecutorService, updateGitMetricsTaskFactory, gitRepoMetricsCache);
- repository = repoManager.createRepository(testProjectNameKey);
- disabledRepository = repoManager.createRepository(disabledProjectNameKey);
+ producerInstanceId,
+ mockedExecutorService,
+ updateGitMetricsTaskFactory,
+ gitRepoMetricsCache);
}
@Test
- public void shouldUpdateMetricsIfProjectIsEnabled() {
- doNothing().when(mockedExecutorService).execute(valueCapture.capture());
- gitRepoUpdateListener.onGitReferenceUpdated(new TestEvent(testProject));
- UpdateGitMetricsTask expectedUpdateGitMetricsTask =
- updateGitMetricsTaskFactory.create(testProject);
- assertThat(valueCapture.getValue().toString())
- .isEqualTo(expectedUpdateGitMetricsTask.toString());
+ public void shouldUpdateMetricsIfProjectIsEnabledOnRefUpdated() {
+ gitRepoUpdateListener.onEvent(getRefUpdatedEvent(enabledProject));
+ assertMetricsAreUpdated();
}
- public static class TestEvent implements GitReferenceUpdatedListener.Event {
+ @Test
+ public void shouldNotUpdateMetricsIfProjectIsDisabledOnRefUpdated() {
+ gitRepoUpdateListener.onEvent(getRefUpdatedEvent(disabledProject));
+ assertMetricsAreNotUpdated();
+ }
+
+ @Test
+ public void shouldUpdateMetricsIfProjectIsEnabledOnRefReplicationDone() {
+ gitRepoUpdateListener.onEvent(
+ getRefReplicationEvent(refReplicationDoneType, enabledProject, consumerInstanceId));
+ assertMetricsAreUpdated();
+ }
+
+ @Test
+ public void shouldNotUpdateMetricsIfProjectIsDisabledOnReplicationDone() {
+ gitRepoUpdateListener.onEvent(
+ getRefReplicationEvent(refReplicationDoneType, disabledProject, consumerInstanceId));
+ assertMetricsAreNotUpdated();
+ }
+
+ @Test
+ public void shouldNotUpdateMetricsOnUnknownEvent() {
+ gitRepoUpdateListener.onEvent(
+ getRefReplicationEvent("any-event", enabledProject, consumerInstanceId));
+ assertMetricsAreNotUpdated();
+ }
+
+ @Test
+ public void shouldNotUpdateMetricsOnRefReplicationDoneFromSameNode() {
+ gitRepoUpdateListener.onEvent(
+ getRefReplicationEvent(refReplicationDoneType, enabledProject, producerInstanceId));
+ assertMetricsAreNotUpdated();
+ }
+
+ private RefUpdatedEvent getRefUpdatedEvent(String projectName) {
+ RefUpdatedEvent refUpdatedEvent = new RefUpdatedEvent();
+ refUpdatedEvent.refUpdate =
+ () -> {
+ RefUpdateAttribute attributes = new RefUpdateAttribute();
+ attributes.project = projectName;
+ attributes.refName = "refs/for/master";
+ return attributes;
+ };
+ return refUpdatedEvent;
+ }
+
+ private ReplicationTestEvent getRefReplicationEvent(
+ String type, String projectName, String instanceId) {
+ ReplicationTestEvent event = new ReplicationTestEvent(type, projectName);
+ event.instanceId = instanceId;
+ return event;
+ }
+
+ private static class ReplicationTestEvent extends RefEvent {
private final String projectName;
- protected TestEvent(String projectName) {
+ private ReplicationTestEvent(String type, String projectName) {
+ super(type);
this.projectName = projectName;
}
@Override
- public String getProjectName() {
- return projectName;
- }
-
- @Override
- public NotifyHandling getNotify() {
- return null;
+ public Project.NameKey getProjectNameKey() {
+ return Project.NameKey.parse(projectName);
}
@Override
public String getRefName() {
- return null;
+ return "refs/for/test";
}
+ }
- @Override
- public String getOldObjectId() {
- return null;
- }
+ private void assertMetricsAreUpdated() {
+ UpdateGitMetricsTask expectedUpdateGitMetricsTask =
+ updateGitMetricsTaskFactory.create(enabledProject);
+ assertThat(updateGitMetricsTaskCaptor.getValue().toString())
+ .isEqualTo(expectedUpdateGitMetricsTask.toString());
+ }
- @Override
- public String getNewObjectId() {
- return null;
- }
-
- @Override
- public boolean isCreate() {
- return false;
- }
-
- @Override
- public boolean isDelete() {
- return false;
- }
-
- @Override
- public boolean isNonFastForward() {
- return false;
- }
-
- @Override
- public AccountInfo getUpdater() {
- return null;
- }
+ private void assertMetricsAreNotUpdated() {
+ updateGitMetricsTaskFactory.create(enabledProject);
+ verifyNoInteractions(mockedExecutorService);
}
}
