Allow plugin to be reloaded The plugin tries to register metrics every time is reloaded. This raises an exception since metrics might already be registered in the MetricRegistry. Avoid re-registering a metric if already present. Bug: Issue 16082 Change-Id: I616ec0f01cdf87efef15927401944e3473d21620

diff --git a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCache.java b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCache.java
index f1d46b2..ced3ab2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCache.java

@@ -14,6 +14,7 @@
 
 package com.googlesource.gerrit.plugins.gitrepometrics;
 
+import com.codahale.metrics.MetricRegistry;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Supplier;
 import com.google.common.collect.Maps;
@@ -33,6 +34,7 @@
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
   private Map<String, Long> metrics;
   private final MetricMaker metricMaker;
+  private final MetricRegistry metricRegistry;
   private final List<String> projects;
   private Map<String, Long> collectedAt;
   private final long gracePeriodMs;
@@ -42,8 +44,13 @@
   public static List<GitRepoMetric> metricsNames = new ArrayList<>(GitStats.availableMetrics());
 
   @VisibleForTesting
-  GitRepoMetricsCache(MetricMaker metricMaker, GitRepoMetricsConfig config, Clock clock) {
+  GitRepoMetricsCache(
+      MetricMaker metricMaker,
+      MetricRegistry metricRegistry,
+      GitRepoMetricsConfig config,
+      Clock clock) {
     this.metricMaker = metricMaker;
+    this.metricRegistry = metricRegistry;
     this.projects = config.getRepositoryNames();
     this.metrics = Maps.newHashMap();
     this.collectedAt = Maps.newHashMap();
@@ -52,8 +59,9 @@
   }
 
   @Inject
-  GitRepoMetricsCache(MetricMaker metricMaker, GitRepoMetricsConfig config) {
-    this(metricMaker, config, Clock.systemDefaultZone());
+  GitRepoMetricsCache(
+      MetricMaker metricMaker, MetricRegistry metricRegistry, GitRepoMetricsConfig config) {
+    this(metricMaker, metricRegistry, config, Clock.systemDefaultZone());
   }
 
   public Map<String, Long> getMetrics() {
@@ -88,13 +96,19 @@
                       }
                     };
 
-                metricMaker.newCallbackMetric(
-                    name,
-                    Long.class,
-                    new Description(gitRepoMetric.getDescription())
-                        .setRate()
-                        .setUnit(gitRepoMetric.getUnit()),
-                    supplier);
+                if (!metricRegistry
+                    .getMetrics()
+                    .containsKey(
+                        GitRepoMetricsCache.getFullyQualifiedMetricName(
+                            gitRepoMetric.getName(), projectName))) {
+                  metricMaker.newCallbackMetric(
+                      name,
+                      Long.class,
+                      new Description(gitRepoMetric.getDescription())
+                          .setRate()
+                          .setUnit(gitRepoMetric.getUnit()),
+                      supplier);
+                }
               });
         });
   }
@@ -103,6 +117,12 @@
     return String.format("%s_%s", metricName, projectName).toLowerCase(Locale.ROOT);
   }
 
+  @VisibleForTesting
+  static String getFullyQualifiedMetricName(String metricName, String projectName) {
+    return String.format(
+        "%s/%s/%s", "plugins", "git-repo-metrics", getMetricName(metricName, projectName));
+  }
+
   public boolean shouldCollectStats(String projectName) {
     long lastCollectionTime = collectedAt.getOrDefault(projectName, 0L);
     long currentTimeMs = System.currentTimeMillis();

diff --git a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCacheTest.java b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCacheTest.java
index 70c2dd9..dcd9a88 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCacheTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoMetricsCacheTest.java

@@ -16,6 +16,8 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricRegistry;
 import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.metrics.CallbackMetric0;
 import com.google.gerrit.metrics.Description;
@@ -45,7 +47,30 @@
   public void shouldRegisterMetrics() throws IOException {
     gitRepoMetricsConfig = configSetupUtils.getGitRepoMetricsConfig();
     fakeMetricMaker = new FakeMetricMaker();
-    gitRepoMetricsCache = new GitRepoMetricsCache(fakeMetricMaker, gitRepoMetricsConfig);
+    gitRepoMetricsCache =
+        new GitRepoMetricsCache(fakeMetricMaker, new MetricRegistry(), gitRepoMetricsConfig);
+    gitRepoMetricsCache.initCache();
+    assertThat(fakeMetricMaker.callsCounter).isEqualTo(GitStats.availableMetrics().size());
+  }
+
+  @Test
+  public void shouldRegisterMetricsOnlyOnce() throws IOException {
+    gitRepoMetricsConfig = configSetupUtils.getGitRepoMetricsConfig();
+    MetricRegistry metricRegistry = new MetricRegistry();
+    fakeMetricMaker = new FakeMetricMaker();
+    gitRepoMetricsCache =
+        new GitRepoMetricsCache(fakeMetricMaker, metricRegistry, gitRepoMetricsConfig);
+
+    gitRepoMetricsCache.initCache();
+    assertThat(fakeMetricMaker.callsCounter).isEqualTo(GitStats.availableMetrics().size());
+
+    GitRepoMetricsCache.metricsNames.forEach(
+        gitRepoMetric ->
+            metricRegistry.register(
+                GitRepoMetricsCache.getFullyQualifiedMetricName(
+                    gitRepoMetric.getName(), enabledRepo),
+                new FakeMetric()));
+
     gitRepoMetricsCache.initCache();
     assertThat(fakeMetricMaker.callsCounter).isEqualTo(GitStats.availableMetrics().size());
   }
@@ -53,7 +78,8 @@
   @Test
   public void shouldCollectStatsForEnabledRepo() throws IOException {
     gitRepoMetricsConfig = configSetupUtils.getGitRepoMetricsConfig();
-    gitRepoMetricsCache = new GitRepoMetricsCache(new FakeMetricMaker(), gitRepoMetricsConfig);
+    gitRepoMetricsCache =
+        new GitRepoMetricsCache(new FakeMetricMaker(), new MetricRegistry(), gitRepoMetricsConfig);
 
     assertThat(gitRepoMetricsCache.shouldCollectStats(enabledRepo)).isTrue();
   }
@@ -62,7 +88,8 @@
   public void shouldNotCollectStatsForDisabledRepo() throws IOException {
     String disabledRepo = "disabledRepo";
     gitRepoMetricsConfig = configSetupUtils.getGitRepoMetricsConfig();
-    gitRepoMetricsCache = new GitRepoMetricsCache(new FakeMetricMaker(), gitRepoMetricsConfig);
+    gitRepoMetricsCache =
+        new GitRepoMetricsCache(new FakeMetricMaker(), new MetricRegistry(), gitRepoMetricsConfig);
 
     assertThat(gitRepoMetricsCache.shouldCollectStats(disabledRepo)).isFalse();
   }
@@ -72,7 +99,8 @@
     ConfigSetupUtils configSetupUtils =
         new ConfigSetupUtils(Collections.singletonList(enabledRepo));
     gitRepoMetricsConfig = configSetupUtils.getGitRepoMetricsConfig();
-    gitRepoMetricsCache = new GitRepoMetricsCache(new FakeMetricMaker(), gitRepoMetricsConfig);
+    gitRepoMetricsCache =
+        new GitRepoMetricsCache(new FakeMetricMaker(), new MetricRegistry(), gitRepoMetricsConfig);
 
     gitRepoMetricsCache.setMetrics(ImmutableMap.of("anyMetric", 0L), enabledRepo);
 
@@ -84,7 +112,8 @@
     ConfigSetupUtils configSetupUtils =
         new ConfigSetupUtils(Collections.singletonList(enabledRepo), "5 m");
     gitRepoMetricsConfig = configSetupUtils.getGitRepoMetricsConfig();
-    gitRepoMetricsCache = new GitRepoMetricsCache(new FakeMetricMaker(), gitRepoMetricsConfig);
+    gitRepoMetricsCache =
+        new GitRepoMetricsCache(new FakeMetricMaker(), new MetricRegistry(), gitRepoMetricsConfig);
 
     gitRepoMetricsCache.setMetrics(ImmutableMap.of("anyMetric", 0L), enabledRepo);
 
@@ -99,6 +128,7 @@
     gitRepoMetricsCache =
         new GitRepoMetricsCache(
             new FakeMetricMaker(),
+            new MetricRegistry(),
             gitRepoMetricsConfig,
             Clock.fixed(
                 Instant.now().minus(2, ChronoUnit.SECONDS), Clock.systemDefaultZone().getZone()));
@@ -113,7 +143,8 @@
     ConfigSetupUtils configSetupUtils =
         new ConfigSetupUtils(Collections.singletonList(enabledRepo));
     gitRepoMetricsConfig = configSetupUtils.getGitRepoMetricsConfig();
-    gitRepoMetricsCache = new GitRepoMetricsCache(new FakeMetricMaker(), gitRepoMetricsConfig);
+    gitRepoMetricsCache =
+        new GitRepoMetricsCache(new FakeMetricMaker(), new MetricRegistry(), gitRepoMetricsConfig);
 
     long currentTimeStamp = System.currentTimeMillis();
 
@@ -144,4 +175,8 @@
       };
     }
   }
+
+  private class FakeMetric implements Metric {
+    FakeMetric() {}
+  }
 }

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 0747b23..f6cdb11 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java

@@ -19,6 +19,7 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 
+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;
@@ -64,7 +65,9 @@
         new ConfigSetupUtils(Collections.singletonList(testProject));
     gitRepoMetricsCache =
         new GitRepoMetricsCache(
-            new DisabledMetricMaker(), configSetupUtils.getGitRepoMetricsConfig());
+            new DisabledMetricMaker(),
+            new MetricRegistry(),
+            configSetupUtils.getGitRepoMetricsConfig());
 
     AbstractModule m =
         new AbstractModule() {

diff --git a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java
index d27c22f..45fb988 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java

@@ -17,6 +17,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static java.nio.file.Files.delete;
 
+import com.codahale.metrics.MetricRegistry;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.metrics.DisabledMetricMaker;
 import com.google.gerrit.server.config.GerritServerConfig;
@@ -49,7 +50,9 @@
     ConfigSetupUtils configSetupUtils = new ConfigSetupUtils(Collections.singletonList("repo1"));
     gitRepoMetricsCache =
         new GitRepoMetricsCache(
-            new DisabledMetricMaker(), configSetupUtils.getGitRepoMetricsConfig());
+            new DisabledMetricMaker(),
+            new MetricRegistry(),
+            configSetupUtils.getGitRepoMetricsConfig());
 
     AbstractModule m =
         new AbstractModule() {