Introduce healthcheck extension Allow plugins to provide their own health check that should participate in the definition of healthiness of the node. In order to achieve this, the signature of `AbstractHealthCheck` had to be changed, so that no implementation had to be injected. It now takes the `MetricMaker` interface, rather than an instance of `HealthCheckMetrics.Factory`, which would require an implementation of MetricMaker which is not already bound. This refactoring leverages on change I63c9b7f2a1, which allows gerrit plugins to expose a new `Gerrit-ApiModule` in the manifest enabling other plugins to register their extensions. Bug: Issue 40015586 Change-Id: Ibc822ec3981eb12a29439fba76706c387aede786
diff --git a/BUILD b/BUILD index e87843c..24a2d60 100644 --- a/BUILD +++ b/BUILD
@@ -13,6 +13,7 @@ "Gerrit-PluginName: healthcheck", "Gerrit-Module: com.googlesource.gerrit.plugins.healthcheck.Module", "Gerrit-HttpModule: com.googlesource.gerrit.plugins.healthcheck.HttpModule", + "Gerrit-ApiModule: com.googlesource.gerrit.plugins.healthcheck.HealthCheckExtensionApiModule", ], resources = glob(["src/main/resources/**/*"]), )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckModule.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckExtensionApiModule.java similarity index 94% rename from src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckModule.java rename to src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckExtensionApiModule.java index 0fa02f2..efa6be4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckExtensionApiModule.java
@@ -21,7 +21,7 @@ import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck; import java.util.concurrent.Executors; -public class HealthCheckModule extends AbstractModule { +public class HealthCheckExtensionApiModule extends AbstractModule { public static final int CHECK_THREADS_DEFAULT = 10; @Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java index 1f88020..9635938 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java
@@ -18,20 +18,13 @@ import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.Timer0; -import com.google.inject.Inject; -import com.google.inject.assistedinject.Assisted; public class HealthCheckMetrics { - public interface Factory { - HealthCheckMetrics create(String name); - } - private final MetricMaker metricMaker; private final String name; - @Inject - public HealthCheckMetrics(MetricMaker metricMaker, @Assisted String name) { + public HealthCheckMetrics(MetricMaker metricMaker, String name) { this.metricMaker = metricMaker; this.name = name; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java index 3cda7b6..aea007a 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
@@ -37,7 +37,6 @@ bindChecker(DeadlockCheck.class); bindChecker(BlockedThreadsCheck.class); - factory(HealthCheckMetrics.Factory.class); install(BlockedThreadsCheck.SUB_CHECKS); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/Module.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/Module.java index 9508084..54cd5a0 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/Module.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/Module.java
@@ -20,7 +20,6 @@ @Override protected void configure() { - install(new HealthCheckModule()); install(new HealthCheckSubsystemsModule()); install(new HealthCheckApiModule()); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java index 9e4544a..5a8246b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java
@@ -17,6 +17,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.metrics.Counter0; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.Timer0; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; @@ -42,14 +43,14 @@ ListeningExecutorService executor, HealthCheckConfig config, String name, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { + MetricMaker metricMaker) { this.executor = executor; this.name = name; this.timeout = config.getTimeout(name); this.config = config; this.latestStatus = StatusSummary.INITIAL_STATUS; - HealthCheckMetrics healthCheckMetrics = healthCheckMetricsFactory.create(name); + HealthCheckMetrics healthCheckMetrics = new HealthCheckMetrics(metricMaker, name); this.failureCounterMetric = healthCheckMetrics.getFailureCounterMetric(); this.latencyMetric = healthCheckMetrics.getLatencyMetric(); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java index 8e64593..8684ef8 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java
@@ -18,11 +18,11 @@ import com.codahale.metrics.MetricRegistry; import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.ThreadSettingsConfig; import com.google.inject.Inject; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.util.Optional; import org.eclipse.jgit.lib.Config; @@ -42,8 +42,8 @@ HealthCheckConfig healthCheckConfig, ThreadSettingsConfig threadSettingsConfig, MetricRegistry metricRegistry, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { - super(executor, healthCheckConfig, ACTIVEWORKERS, healthCheckMetricsFactory); + MetricMaker metricMaker) { + super(executor, healthCheckConfig, ACTIVEWORKERS, metricMaker); this.threshold = healthCheckConfig.getActiveWorkersThreshold(ACTIVEWORKERS); this.metricRegistry = metricRegistry; this.interactiveThreadsMaxPoolSize =
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java index c2b0f3e..2dc22dc 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java
@@ -17,6 +17,7 @@ import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.AUTH; import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AuthRequest; @@ -24,7 +25,6 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.util.Optional; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,8 +45,8 @@ Realm realm, AccountCache byIdCache, AuthRequest.Factory authRequestFactory, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { - super(executor, config, AUTH, healthCheckMetricsFactory); + MetricMaker metricMaker) { + super(executor, config, AUTH, metricMaker); this.realm = realm; this.byIdCache = byIdCache;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java index 78b8b01..1e282b3 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java
@@ -19,12 +19,12 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.extensions.config.FactoryModule; +import com.google.gerrit.metrics.MetricMaker; import com.google.inject.Inject; import com.google.inject.Module; import com.google.inject.Provider; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.lang.management.ManagementFactory; import java.lang.management.ThreadInfo; import java.lang.management.ThreadMXBean; @@ -50,10 +50,10 @@ public BlockedThreadsCheck( ListeningExecutorService executor, HealthCheckConfig healthCheckConfig, - HealthCheckMetrics.Factory healthCheckMetricsFactory, + MetricMaker metricMaker, ThreadBeanProvider threadBeanProvider, BlockedThreadsConfigurator checksConfig) { - super(executor, healthCheckConfig, BLOCKEDTHREADS, healthCheckMetricsFactory); + super(executor, healthCheckConfig, BLOCKEDTHREADS, metricMaker); this.threads = threadBeanProvider.get(); this.collectorsSupplier = checksConfig; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsSubCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsSubCheck.java index d4f0fe8..7dcca27 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsSubCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsSubCheck.java
@@ -18,6 +18,7 @@ import com.google.common.base.Strings; import com.google.gerrit.metrics.Counter0; +import com.google.gerrit.metrics.MetricMaker; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; @@ -65,11 +66,10 @@ @Inject BlockedThreadsSubCheck( - HealthCheckMetrics.Factory healthCheckMetricsFactory, - @Assisted String prefix, - @Assisted Integer threshold) { + MetricMaker metricMaker, @Assisted String prefix, @Assisted Integer threshold) { HealthCheckMetrics healthCheckMetrics = - healthCheckMetricsFactory.create( + new HealthCheckMetrics( + metricMaker, String.format( "%s-%s", BLOCKEDTHREADS, prefix.toLowerCase().replaceAll("[^\\w-/]", "_"))); Counter0 failureCounterMetric = healthCheckMetrics.getFailureCounterMetric();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java index 6a6ade6..3d4b672 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java
@@ -18,9 +18,9 @@ import com.codahale.metrics.MetricRegistry; import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.gerrit.metrics.MetricMaker; import com.google.inject.Inject; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.util.Optional; public class DeadlockCheck extends AbstractHealthCheck { @@ -35,8 +35,8 @@ ListeningExecutorService executor, HealthCheckConfig healthCheckConfig, MetricRegistry metricRegistry, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { - super(executor, healthCheckConfig, DEADLOCK, healthCheckMetricsFactory); + MetricMaker metricMaker) { + super(executor, healthCheckConfig, DEADLOCK, metricMaker); this.metricRegistry = metricRegistry; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java index b004eaa..fa5f14a 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/GlobalHealthCheck.java
@@ -18,10 +18,10 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.metrics.MetricMaker; import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.util.Arrays; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -38,8 +38,8 @@ DynamicSet<HealthCheck> healthChecks, ListeningExecutorService executor, HealthCheckConfig healthCheckConfig, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { - super(executor, healthCheckConfig, GLOBAL, healthCheckMetricsFactory); + MetricMaker metricMaker) { + super(executor, healthCheckConfig, GLOBAL, metricMaker); this.healthChecks = healthChecks; }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java index 9196cdb..8fd60e5 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java
@@ -18,11 +18,11 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.entities.Project; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.util.Set; import org.eclipse.jgit.lib.Repository; @@ -36,8 +36,8 @@ ListeningExecutorService executor, HealthCheckConfig config, GitRepositoryManager repositoryManager, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { - super(executor, config, JGIT, healthCheckMetricsFactory); + MetricMaker metricMaker) { + super(executor, config, JGIT, metricMaker); this.repositoryManager = repositoryManager; this.repositoryNameKeys = config.getJGITRepositories(JGIT); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java index 78cff2a..d448cf4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java
@@ -18,6 +18,7 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.extensions.common.ProjectInfo; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.restapi.project.ListProjects; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; @@ -25,7 +26,6 @@ import com.google.inject.Provider; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.util.SortedMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,8 +43,8 @@ HealthCheckConfig config, Provider<ListProjects> listProjectsProvider, OneOffRequestContext oneOffCtx, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { - super(executor, config, PROJECTSLIST, healthCheckMetricsFactory); + MetricMaker metricMaker) { + super(executor, config, PROJECTSLIST, metricMaker); this.listProjectsProvider = listProjectsProvider; this.oneOffCtx = oneOffCtx;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java index a786606..7165b9f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java
@@ -17,6 +17,7 @@ import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.QUERYCHANGES; import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.restapi.change.QueryChanges; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; @@ -24,7 +25,6 @@ import com.google.inject.Provider; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig; -import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics; import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,8 +42,8 @@ HealthCheckConfig config, Provider<QueryChanges> queryChangesProvider, OneOffRequestContext oneOffCtx, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { - super(executor, config, QUERYCHANGES, healthCheckMetricsFactory); + MetricMaker metricMaker) { + super(executor, config, QUERYCHANGES, metricMaker); this.queryChangesProvider = queryChangesProvider; this.config = config; this.limit = config.getLimit(QUERYCHANGES);
diff --git a/src/main/resources/Documentation/extensions.md b/src/main/resources/Documentation/extensions.md new file mode 100644 index 0000000..ccc07c7 --- /dev/null +++ b/src/main/resources/Documentation/extensions.md
@@ -0,0 +1,129 @@ +# Custom checks extensions + +The @PLUGIN@ plugin allows other plugins to provide their own custom check that +should participate in the definition of healthiness of the node. + +Plugins that are interested in doing so, will need to add a dependency on the +healthcheck plugin, for example, their `BUILD` file could be as such: + +``` +gerrit_plugin( + name = "plugin-foo", + srcs = glob(["src/main/java/**/*.java"]), + manifest_entries = [ + "Implementation-Title: Foo plugin", + "Implementation-URL: https://gerrit-review.googlesource.com/#/admin/projects/plugins/foo", + "Gerrit-PluginName: foo", + "Gerrit-Module: com.googlesource.gerrit.plugins.foo.Module", + ], + resources = glob(["src/main/resources/**/*"]), + deps = [ + "gerrit-healthcheck-neverlink", + ], +) + +java_library( + name = "gerrit-healthcheck-neverlink", + neverlink = True, + exports = ["//plugins/healthcheck"], +) +``` + +Then, the plugin can provide its own healthcheck logic via a concrete +implementation of an `AbstractHealthCheck`, for example: + +```java + +@Singleton +public class FooHealthCheck extends AbstractHealthCheck { + + @Inject + public FooHealthCheck( + ListeningExecutorService executor, + HealthCheckConfig config, + @PluginName String name, + MetricMaker metricMaker) { + super(executor, config, name, metricMaker); + } + + @Override + protected Result doCheck() throws Exception { + // health logic goes here + return Result.PASSED; + } +} +``` + +To build the plugin in the gerrit-CI, as +[documented](https://gerrit-review.googlesource.com/Documentation/dev-plugins.html#_cross_plugin_communication) +by gerrit, you should be configuring your build job as follows: + +```yaml +- project: + name: plugin-foo + jobs: + - 'plugin-{name}-bazel-{branch}': + extra-plugins: 'healthcheck' + branch: + - master +``` + +To get the CI to validate your plugin you should also add the dependency to +your `Jenkinsfile`: + +``` +pluginPipeline( + formatCheckId: 'gerritforge:foo-format-3852e64366bb37d13b8baf8af9b15cfd38eb9227', + buildCheckId: 'gerritforge:foo-3852e64366bb37d13b8baf8af9b15cfd38eb9227', + extraPlugins: ['healthcheck']) +``` + +Hitting the healthcheck status endpoint will now report the `foo` check, +where `foo` is the name of the plugin, as defined in its manifest: + +```shell +curl -v 'http://gerrit:8080/config/server/healthcheck~status' + +{ + "elapsed": 18, + "foo": { + "result": "passed", + "ts": 1683723806276, + "elapsed": 0 + }, + "auth": { + "result": "passed", + "ts": 1683723806269, + "elapsed": 4 + }, + "querychanges": { + "result": "passed", + "ts": 1683723806266, + "elapsed": 3 + }, + "jgit": { + "result": "passed", + "ts": 1683723806258, + "elapsed": 7 + }, + "projectslist": { + "result": "passed", + "ts": 1683723806265, + "elapsed": 1 + }, + "ts": 1683723806258 +} +``` + +and the metrics for such component will also be reported, out of the box. + +```shell +foo_latest_latency{quantile="0.5",} 0.008638662 +foo_latest_latency{quantile="0.75",} 0.008638662 +foo_latest_latency{quantile="0.95",} 0.010553707 +foo_latest_latency{quantile="0.98",} 0.147902061 +foo_latest_latency{quantile="0.99",} 0.147902061 +foo_latest_latency{quantile="0.999",} 0.147902061 +foo_latest_latency_count 4.0 +foo_failure_total 0.0 +```
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java index fada3bb..a874739 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java
@@ -31,8 +31,7 @@ public class AbstractHealthCheckTest { - MetricMaker testMetricMaker; - DummyHealthCheckMetricsFactory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory(); + TestMetricMaker testMetricMaker; @Before public void setUp() throws Exception { @@ -45,7 +44,6 @@ testCheck.run(); - TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker(); assertThat(testMetricMaker.getFailureCount()).isEqualTo(TestMetricMaker.expectedFailureCount); assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency); } @@ -56,7 +54,6 @@ testCheck.run(); - TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker(); assertThat(testMetricMaker.getFailureCount()).isEqualTo(TestMetricMaker.expectedFailureCount); assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency); } @@ -67,7 +64,6 @@ testCheck.run(); - TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker(); assertThat(testMetricMaker.getFailureCount()).isEqualTo(0L); assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency); } @@ -78,7 +74,6 @@ testCheck.run(); - TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker(); assertThat(testMetricMaker.getFailureCount()).isEqualTo(0L); assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency); } @@ -100,23 +95,19 @@ } private TestCheck createTestCheckWithStatus(HealthCheck.Result result) { - return new TestCheck( - HealthCheckConfig.DEFAULT_CONFIG, "testCheck", healthCheckMetricsFactory, result); + return new TestCheck(HealthCheckConfig.DEFAULT_CONFIG, "testCheck", testMetricMaker, result); } private static class TestCheck extends AbstractHealthCheck { private final Result finalResult; public TestCheck( - HealthCheckConfig config, - String name, - HealthCheckMetrics.Factory healthCheckMetricsFactory, - Result finalResult) { + HealthCheckConfig config, String name, MetricMaker metricMaker, Result finalResult) { super( MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)), config, name, - healthCheckMetricsFactory); + metricMaker); this.finalResult = finalResult; } @@ -172,17 +163,4 @@ return failureCount; } } - - private static class DummyHealthCheckMetricsFactory implements HealthCheckMetrics.Factory { - private final TestMetricMaker metricMaker = new TestMetricMaker(); - - @Override - public HealthCheckMetrics create(String name) { - return new HealthCheckMetrics(metricMaker, name); - } - - public TestMetricMaker getMetricMaker() { - return metricMaker; - } - } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java index df88c48..8bd3ddd 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java
@@ -21,6 +21,7 @@ import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.gerrit.metrics.DisabledMetricMaker; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.ThreadSettingsConfig; import com.google.inject.AbstractModule; @@ -32,7 +33,6 @@ import org.junit.Test; public class ActiveWorkersCheckTest { - HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory(); @Test public void shouldPassCheckWhenNoActiveWorkers() { @@ -105,7 +105,7 @@ } private Injector testInjector(AbstractModule testModule) { - return Guice.createInjector(new HealthCheckModule(), testModule); + return Guice.createInjector(new HealthCheckExtensionApiModule(), testModule); } private MetricRegistry createMetricRegistry(Long value) { @@ -132,7 +132,7 @@ healtchCheckConfig, injector.getInstance(ThreadSettingsConfig.class), injector.getInstance(MetricRegistry.class), - healthCheckMetricsFactory); + new DisabledMetricMaker()); } private class TestModule extends AbstractModule {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java index c52d646..737b4a7 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java
@@ -20,6 +20,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.google.gerrit.metrics.DisabledMetricMaker; +import com.google.gerrit.metrics.MetricMaker; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; @@ -213,12 +215,12 @@ private Injector createTestInjector(HealthCheckConfig config) { Injector injector = Guice.createInjector( - new HealthCheckModule(), + new HealthCheckExtensionApiModule(), new AbstractModule() { @Override protected void configure() { bind(HealthCheckConfig.class).toInstance(config); - bind(HealthCheckMetrics.Factory.class).to(DummyHealthCheckMetricsFactory.class); + bind(MetricMaker.class).toInstance(new DisabledMetricMaker()); bind(ThreadBeanProvider.class).toInstance(threadBeanProviderMock); } },
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java index aa880a3..a73ca15 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java
@@ -20,6 +20,7 @@ import com.codahale.metrics.Gauge; import com.codahale.metrics.MetricRegistry; import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.gerrit.metrics.DisabledMetricMaker; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.ThreadSettingsConfig; import com.google.inject.AbstractModule; @@ -32,8 +33,6 @@ public class DeadlockCheckTest { - HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory(); - @Test public void shouldPassCheckWhenNoMetric() { @@ -70,7 +69,7 @@ } private Injector testInjector(AbstractModule testModule) { - return Guice.createInjector(new HealthCheckModule(), testModule); + return Guice.createInjector(new HealthCheckExtensionApiModule(), testModule); } private MetricRegistry createMetricRegistry(Integer value) { @@ -91,7 +90,7 @@ injector.getInstance(ListeningExecutorService.class), DEFAULT_CONFIG, injector.getInstance(MetricRegistry.class), - healthCheckMetricsFactory); + new DisabledMetricMaker()); } private class TestModule extends AbstractModule {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DummyHealthCheckMetricsFactory.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DummyHealthCheckMetricsFactory.java deleted file mode 100644 index 7523e95..0000000 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DummyHealthCheckMetricsFactory.java +++ /dev/null
@@ -1,28 +0,0 @@ -// Copyright (C) 2021 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.healthcheck; - -import com.google.gerrit.metrics.DisabledMetricMaker; -import com.google.gerrit.metrics.MetricMaker; -import org.junit.Ignore; - -@Ignore -public class DummyHealthCheckMetricsFactory implements HealthCheckMetrics.Factory { - @Override - public HealthCheckMetrics create(String name) { - MetricMaker metricMaker = new DisabledMetricMaker(); - return new HealthCheckMetrics(metricMaker, name); - } -}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java index e9901a9..4c646e5 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -29,6 +29,7 @@ import com.google.gerrit.extensions.annotations.PluginName; import com.google.gson.Gson; import com.google.gson.JsonObject; +import com.google.inject.AbstractModule; import com.google.inject.Key; import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames; import java.io.IOException; @@ -38,7 +39,7 @@ @TestPlugin( name = "healthcheck-test", - sysModule = "com.googlesource.gerrit.plugins.healthcheck.Module", + sysModule = "com.googlesource.gerrit.plugins.healthcheck.HealthCheckIT$TestModule", httpModule = "com.googlesource.gerrit.plugins.healthcheck.HttpModule") @Sandboxed public class HealthCheckIT extends LightweightPluginDaemonTest { @@ -46,6 +47,15 @@ HealthCheckConfig config; String healthCheckUriPath; + public static class TestModule extends AbstractModule { + + @Override + protected void configure() { + install(new HealthCheckExtensionApiModule()); + install(new Module()); + } + } + @Override @Before public void setUpTestPlugin() throws Exception {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java index b971054..af09d07 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
@@ -21,6 +21,8 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.metrics.DisabledMetricMaker; +import com.google.gerrit.metrics.MetricMaker; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Inject; @@ -36,7 +38,7 @@ public class HealthCheckStatusEndpointTest { @Inject private ListeningExecutorService executor; - HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory(); + MetricMaker disabledMetricMaker = new DisabledMetricMaker(); public static class TestHealthCheck extends AbstractHealthCheck { private final HealthCheck.Result checkResult; @@ -47,12 +49,12 @@ String checkName, HealthCheck.Result result, long sleep, - HealthCheckMetrics.Factory healthCheckMetricsFactory) { + MetricMaker metricMaker) { super( MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)), config, checkName, - healthCheckMetricsFactory); + metricMaker); this.checkResult = result; this.sleep = sleep; } @@ -82,12 +84,11 @@ "checkOk", HealthCheck.Result.PASSED, 0, - healthCheckMetricsFactory); + disabledMetricMaker); DynamicSet<HealthCheck> healthChecks = new DynamicSet<>(); healthChecks.add("passingHealthCheck", passingHealthCheck); GlobalHealthCheck globalHealthCheck = - new GlobalHealthCheck( - healthChecks, executor, config, healthCheckMetricsFactory); + new GlobalHealthCheck(healthChecks, executor, config, disabledMetricMaker); bind(GlobalHealthCheck.class).toInstance(globalHealthCheck); bind(HealthCheckConfig.class).toInstance(config); } @@ -111,16 +112,11 @@ new HealthCheckConfig("[healthcheck]\n" + "timeout = 20"); TestHealthCheck testHealthCheck = new TestHealthCheck( - config, - "checkOk", - HealthCheck.Result.PASSED, - 30, - healthCheckMetricsFactory); + config, "checkOk", HealthCheck.Result.PASSED, 30, disabledMetricMaker); DynamicSet<HealthCheck> healthChecks = new DynamicSet<>(); healthChecks.add("testHealthCheck", testHealthCheck); GlobalHealthCheck globalHealthCheck = - new GlobalHealthCheck( - healthChecks, executor, config, healthCheckMetricsFactory); + new GlobalHealthCheck(healthChecks, executor, config, disabledMetricMaker); bind(GlobalHealthCheck.class).toInstance(globalHealthCheck); bind(HealthCheckConfig.class).toInstance(config); } @@ -148,7 +144,7 @@ "checkOk", HealthCheck.Result.PASSED, 0, - healthCheckMetricsFactory); + disabledMetricMaker); TestHealthCheck failingHealthCheck = new TestHealthCheck( @@ -156,14 +152,13 @@ "checkKo", HealthCheck.Result.FAILED, 0, - healthCheckMetricsFactory); + disabledMetricMaker); DynamicSet<HealthCheck> healthChecks = new DynamicSet<>(); healthChecks.add("passingHealthCheck", passingHealthCheck); healthChecks.add("failingHealthCheck", failingHealthCheck); GlobalHealthCheck globalHealthCheck = - new GlobalHealthCheck( - healthChecks, executor, config, healthCheckMetricsFactory); + new GlobalHealthCheck(healthChecks, executor, config, disabledMetricMaker); bind(GlobalHealthCheck.class).toInstance(globalHealthCheck); bind(HealthCheckConfig.class).toInstance(config); } @@ -177,6 +172,7 @@ } private Injector testInjector(AbstractModule testModule) { - return Guice.createInjector(new HealthCheckModule(), new HealthCheckApiModule(), testModule); + return Guice.createInjector( + new HealthCheckExtensionApiModule(), new HealthCheckApiModule(), testModule); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java index b589dcc..02b914c 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
@@ -22,6 +22,8 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.entities.Project; +import com.google.gerrit.metrics.DisabledMetricMaker; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; @@ -53,11 +55,11 @@ @Inject private ListeningExecutorService executor; - HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory(); + MetricMaker disabledMetricMaker = new DisabledMetricMaker(); @Before public void setupAllProjects() throws Exception { - Guice.createInjector(new HealthCheckModule()).injectMembers(this); + Guice.createInjector(new HealthCheckExtensionApiModule()).injectMembers(this); InMemoryRepositoryManager.Repo allProjects = inMemoryRepositoryManager.createRepository(allProjectsName); @@ -72,7 +74,7 @@ public void shouldBeHealthyWhenJGitIsWorking() { JGitHealthCheck check = new JGitHealthCheck( - executor, DEFAULT_CONFIG, getWorkingRepositoryManager(), healthCheckMetricsFactory); + executor, DEFAULT_CONFIG, getWorkingRepositoryManager(), disabledMetricMaker); assertThat(check.run().result).isEqualTo(Result.PASSED); } @@ -80,7 +82,7 @@ public void shouldBeUnhealthyWhenJGitIsFailingForAllRepos() { JGitHealthCheck jGitHealthCheck = new JGitHealthCheck( - executor, DEFAULT_CONFIG, getFailingGitRepositoryManager(), healthCheckMetricsFactory); + executor, DEFAULT_CONFIG, getFailingGitRepositoryManager(), disabledMetricMaker); assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED); } @@ -94,8 +96,7 @@ + " project = All-Users\n" + " project = Not-Existing-Repo"); JGitHealthCheck jGitHealthCheck = - new JGitHealthCheck( - executor, config, getWorkingRepositoryManager(), healthCheckMetricsFactory); + new JGitHealthCheck(executor, config, getWorkingRepositoryManager(), disabledMetricMaker); assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java index 58009ea..8b8e9ff 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
@@ -20,6 +20,8 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.extensions.common.ProjectInfo; import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.metrics.DisabledMetricMaker; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.restapi.project.ListProjects; import com.google.gerrit.server.util.OneOffRequestContext; import com.google.inject.Guice; @@ -41,7 +43,7 @@ public class ProjectsListHealthCheckTest { @Inject private ListeningExecutorService executor; - HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory(); + MetricMaker disabledMetricMaker = new DisabledMetricMaker(); private Config gerritConfig = new Config(); @@ -49,18 +51,14 @@ @Before public void setUp() throws Exception { - Guice.createInjector(new HealthCheckModule()).injectMembers(this); + Guice.createInjector(new HealthCheckExtensionApiModule()).injectMembers(this); } @Test public void shouldBeHealthyWhenListProjectsWorks() { ProjectsListHealthCheck jGitHealthCheck = new ProjectsListHealthCheck( - executor, - DEFAULT_CONFIG, - getWorkingProjectList(0), - mockOneOffCtx, - healthCheckMetricsFactory); + executor, DEFAULT_CONFIG, getWorkingProjectList(0), mockOneOffCtx, disabledMetricMaker); assertThat(jGitHealthCheck.run().result).isEqualTo(Result.PASSED); } @@ -68,11 +66,7 @@ public void shouldBeUnhealthyWhenListProjectsIsFailing() { ProjectsListHealthCheck jGitHealthCheck = new ProjectsListHealthCheck( - executor, - DEFAULT_CONFIG, - getFailingProjectList(), - mockOneOffCtx, - healthCheckMetricsFactory); + executor, DEFAULT_CONFIG, getFailingProjectList(), mockOneOffCtx, disabledMetricMaker); assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED); } @@ -84,7 +78,7 @@ DEFAULT_CONFIG, getWorkingProjectList(DEFAULT_CONFIG.getTimeout() * 2), mockOneOffCtx, - healthCheckMetricsFactory); + disabledMetricMaker); assertThat(jGitHealthCheck.run().result).isEqualTo(Result.TIMEOUT); }