Model healthcheck latency as gauge rather than timer
When running an healthcheck we care about the last result status, rather
than the distributions of the previous runs.
This change re-models the healthcheck metrics by registering them at
plugin startup and allowing the latest status result to be accessible from
the healthcheck component itself.
Change-Id: I2b2d5f22b9e85eaba427a4b5b264274773a6c4a3
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java
new file mode 100644
index 0000000..edc3223
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java
@@ -0,0 +1,96 @@
+// Copyright (C) 2019 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.common.annotations.VisibleForTesting;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.metrics.CallbackMetric0;
+import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+@Singleton
+public class HealthCheckMetrics implements LifecycleListener {
+
+ private final DynamicSet<HealthCheck> healthChecks;
+ private final MetricMaker metricMaker;
+ private final Set<RegistrationHandle> registeredMetrics;
+ private final Set<Runnable> triggers;
+
+ @Inject
+ public HealthCheckMetrics(DynamicSet<HealthCheck> healthChecks, MetricMaker metricMaker) {
+ this.healthChecks = healthChecks;
+ this.metricMaker = metricMaker;
+ this.registeredMetrics = Collections.synchronizedSet(new HashSet<>());
+ this.triggers = Collections.synchronizedSet(new HashSet<>());
+ }
+
+ @Override
+ public void start() {
+
+ for (HealthCheck healthCheck : healthChecks) {
+ String name = healthCheck.name();
+
+ Counter0 failureMetric =
+ metricMaker.newCounter(
+ String.format("%s/failure", name),
+ new Description(String.format("%s healthcheck failures count", name))
+ .setCumulative()
+ .setRate()
+ .setUnit("failures"));
+
+ CallbackMetric0<Long> latencyMetric =
+ metricMaker.newCallbackMetric(
+ String.format("%s/latest_latency", name),
+ Long.class,
+ new Description(String.format("%s health check latency execution (ms)", name))
+ .setGauge()
+ .setUnit(Description.Units.MILLISECONDS));
+
+ Runnable metricCallBack =
+ () -> {
+ HealthCheck.StatusSummary status = healthCheck.getLatestStatus();
+ latencyMetric.set(healthCheck.getLatestStatus().elapsed);
+ if (status.isFailure()) {
+ failureMetric.increment();
+ }
+ };
+
+ registeredMetrics.add(failureMetric);
+ registeredMetrics.add(metricMaker.newTrigger(latencyMetric, metricCallBack));
+ triggers.add(metricCallBack);
+ }
+ }
+
+ @Override
+ public void stop() {
+ for (RegistrationHandle handle : registeredMetrics) {
+ handle.remove();
+ }
+ }
+
+ @VisibleForTesting
+ public void triggerAll() {
+ triggers.forEach(Runnable::run);
+ }
+}
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 9c6b17d..ab2ad91 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
@@ -14,12 +14,11 @@
package com.googlesource.gerrit.plugins.healthcheck;
+import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.inject.AbstractModule;
-import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.JGitHealthCheck;
-import com.googlesource.gerrit.plugins.healthcheck.check.MetricsHandler;
import com.googlesource.gerrit.plugins.healthcheck.check.ProjectsListHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.QueryChangesHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.ReviewDbHealthCheck;
@@ -28,15 +27,11 @@
@Override
protected void configure() {
- install(
- new FactoryModuleBuilder()
- .implement(MetricsHandler.class, MetricsHandler.class)
- .build(MetricsHandler.Factory.class));
-
bindChecker(ReviewDbHealthCheck.class);
bindChecker(JGitHealthCheck.class);
bindChecker(ProjectsListHealthCheck.class);
bindChecker(QueryChangesHealthCheck.class);
+ bind(LifecycleListener.class).to(HealthCheckMetrics.class);
}
private void bindChecker(Class<? extends HealthCheck> healthCheckClass) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java
index 87a1fc8..26a2042 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java
@@ -50,8 +50,8 @@
.stream()
.filter(
res ->
- res instanceof HealthCheck.Status
- && ((HealthCheck.Status) res).result != HealthCheck.Result.PASSED)
+ res instanceof HealthCheck.StatusSummary
+ && ((HealthCheck.StatusSummary) res).result != HealthCheck.Result.PASSED)
.findFirst()
.isPresent()) {
return HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
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 8aa6432..26ef1ee 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
@@ -28,17 +28,14 @@
private final long timeout;
private final String name;
private final ListeningExecutorService executor;
- private final MetricsHandler metricsHandler;
+ protected StatusSummary latestStatus;
protected AbstractHealthCheck(
- ListeningExecutorService executor,
- HealthCheckConfig config,
- String name,
- MetricsHandler.Factory metricsHandler) {
+ ListeningExecutorService executor, HealthCheckConfig config, String name) {
this.executor = executor;
this.name = name;
- this.metricsHandler = metricsHandler.create(name);
this.timeout = config.getTimeout(name);
+ this.latestStatus = StatusSummary.INITIAL_STATUS;
}
@Override
@@ -47,10 +44,9 @@
}
@Override
- public Status run() {
+ public StatusSummary run() {
final long ts = System.currentTimeMillis();
- Status status = null;
- ListenableFuture<Status> resultFuture =
+ ListenableFuture<StatusSummary> resultFuture =
executor.submit(
() -> {
Result healthy;
@@ -60,21 +56,24 @@
log.warn("Check {} failed", name, e);
healthy = Result.FAILED;
}
- return new Status(healthy, ts, System.currentTimeMillis() - ts);
+ return new StatusSummary(healthy, ts, System.currentTimeMillis() - ts);
});
try {
- status = resultFuture.get(timeout, TimeUnit.MILLISECONDS);
+ latestStatus = resultFuture.get(timeout, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
log.warn("Check {} timed out", name, e);
- status = new Status(Result.TIMEOUT, ts, System.currentTimeMillis() - ts);
+ latestStatus = new StatusSummary(Result.TIMEOUT, ts, System.currentTimeMillis() - ts);
} catch (InterruptedException | ExecutionException e) {
log.warn("Check {} failed while waiting for its future result", name, e);
- status = new Status(Result.FAILED, ts, System.currentTimeMillis() - ts);
- } finally {
- metricsHandler.sendMetrics(status);
+ latestStatus = new StatusSummary(Result.FAILED, ts, System.currentTimeMillis() - ts);
}
- return status;
+ return latestStatus;
}
protected abstract Result doCheck() throws Exception;
+
+ @Override
+ public StatusSummary getLatestStatus() {
+ return latestStatus;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
index 6f44ca4..4f6a532 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
@@ -27,23 +27,27 @@
TIMEOUT;
}
- public class Status {
+ public class StatusSummary {
+ public static final StatusSummary INITIAL_STATUS =
+ new StatusSummary(Result.PASSED, System.currentTimeMillis(), 0L);
public final Result result;
public final long ts;
public final long elapsed;
- public Status(Result result, long ts, long elapsed) {
+ public StatusSummary(Result result, long ts, long elapsed) {
this.result = result;
this.ts = ts;
this.elapsed = elapsed;
}
- protected Boolean isFailure() {
+ public Boolean isFailure() {
return this.result != Result.PASSED;
}
}
- Status run();
+ StatusSummary run();
String name();
+
+ StatusSummary getLatestStatus();
}
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 90c9fd9..73fa88d 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
@@ -20,10 +20,12 @@
import com.google.gerrit.server.config.AllProjectsName;
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 org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+@Singleton
public class JGitHealthCheck extends AbstractHealthCheck {
private final GitRepositoryManager repositoryManager;
private final AllProjectsName allProjectsName;
@@ -33,9 +35,9 @@
ListeningExecutorService executor,
HealthCheckConfig config,
GitRepositoryManager repositoryManager,
- AllProjectsName allProjectsName,
- MetricsHandler.Factory metricsHandlerFactory) {
- super(executor, config, JGIT, metricsHandlerFactory);
+ AllProjectsName allProjectsName) {
+ super(executor, config, JGIT);
+
this.repositoryManager = repositoryManager;
this.allProjectsName = allProjectsName;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/MetricsHandler.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/MetricsHandler.java
deleted file mode 100644
index c28f52a..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/MetricsHandler.java
+++ /dev/null
@@ -1,43 +0,0 @@
-package com.googlesource.gerrit.plugins.healthcheck.check;
-
-import com.google.gerrit.metrics.Counter0;
-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;
-import java.util.concurrent.TimeUnit;
-
-public class MetricsHandler {
- private final Timer0 latencyMetric;
- private final Counter0 failureMetric;
-
- public interface Factory {
- public MetricsHandler create(String name);
- }
-
- @Inject
- public MetricsHandler(@Assisted String name, MetricMaker metricMaker) {
- this.latencyMetric =
- metricMaker.newTimer(
- name + "/latency",
- new Description(String.format("%s health check latency execution (ms)", name))
- .setCumulative()
- .setUnit(Description.Units.MILLISECONDS));
-
- this.failureMetric =
- metricMaker.newCounter(
- name + "/failure",
- new Description(String.format("%s healthcheck failures count", name))
- .setCumulative()
- .setRate()
- .setUnit("failures"));
- }
-
- public void sendMetrics(HealthCheck.Status status) {
- latencyMetric.record(status.elapsed, TimeUnit.MILLISECONDS);
- if (status.isFailure()) {
- failureMetric.increment();
- }
- }
-}
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 0c9a120..1dbe422 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
@@ -20,11 +20,13 @@
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.server.project.ListProjects;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
import java.util.SortedMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+@Singleton
public class ProjectsListHealthCheck extends AbstractHealthCheck {
private static final Logger log = LoggerFactory.getLogger(ProjectsListHealthCheck.class);
private static final int PROJECTS_LIST_LIMIT = 100;
@@ -32,11 +34,9 @@
@Inject
public ProjectsListHealthCheck(
- ListeningExecutorService executor,
- HealthCheckConfig config,
- ListProjects listProjects,
- MetricsHandler.Factory metricHandlerFactory) {
- super(executor, config, PROJECTSLIST, metricHandlerFactory);
+ ListeningExecutorService executor, HealthCheckConfig config, ListProjects listProjects) {
+ super(executor, config, PROJECTSLIST);
+
this.listProjects = listProjects;
}
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 67bb733..ce6b168 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
@@ -21,11 +21,13 @@
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+@Singleton
public class QueryChangesHealthCheck extends AbstractHealthCheck {
private static final Logger log = LoggerFactory.getLogger(QueryChangesHealthCheck.class);
private final QueryChanges queryChanges;
@@ -37,9 +39,8 @@
ListeningExecutorService executor,
HealthCheckConfig config,
QueryChanges queryChanges,
- OneOffRequestContext oneOffCtx,
- MetricsHandler.Factory metricsHandlerFactory) {
- super(executor, config, QUERYCHANGES, metricsHandlerFactory);
+ OneOffRequestContext oneOffCtx) {
+ super(executor, config, QUERYCHANGES);
this.queryChanges = queryChanges;
this.limit = config.getLimit(QUERYCHANGES);
queryChanges.setLimit(limit);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java
index 805eaa1..2a184c8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ReviewDbHealthCheck.java
@@ -21,8 +21,10 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
+@Singleton
public class ReviewDbHealthCheck extends AbstractHealthCheck {
private final SchemaFactory<ReviewDb> reviewDb;
@@ -30,9 +32,8 @@
public ReviewDbHealthCheck(
ListeningExecutorService executor,
HealthCheckConfig config,
- SchemaFactory<ReviewDb> reviewDb,
- MetricsHandler.Factory metricHandlerFactory) {
- super(executor, config, REVIEWDB, metricHandlerFactory);
+ SchemaFactory<ReviewDb> reviewDb) {
+ super(executor, config, REVIEWDB);
this.reviewDb = reviewDb;
}
diff --git a/src/resources/Documentation/config.md b/src/resources/Documentation/config.md
index 72a3ad5..ea51c10 100644
--- a/src/resources/Documentation/config.md
+++ b/src/resources/Documentation/config.md
@@ -36,43 +36,25 @@
However some extra metrics are also emitted to expose more details about the healthcheck result.
Specifically two metrics are emitted for each component contributing to the overall healthcheck result (JGIT, PROJECTLIST, REVIEWDB).
-* plugins_healthcheck_<healthcheck_component>_latency: the cumulative latency (in ms) of performing the healthcheck for this specific component
+* plugins_healthcheck_<healthcheck_component>_latest_measured_latency: the latency of this component in the latest executed healthcheck run
* plugins_healthcheck_jgit_failure_total: the cumulative number of failures for this specific component
```
-# HELP plugins_healthcheck_jgit_latency Generated from Dropwizard metric import (metric=plugins/healthcheck/jgit/latency, type=com.codahale.metrics.Timer)
-# TYPE plugins_healthcheck_jgit_latency summary
-plugins_healthcheck_jgit_latency{quantile="0.5",} 0.002
-plugins_healthcheck_jgit_latency{quantile="0.75",} 0.002
-plugins_healthcheck_jgit_latency{quantile="0.95",} 0.002
-plugins_healthcheck_jgit_latency{quantile="0.98",} 0.002
-plugins_healthcheck_jgit_latency{quantile="0.99",} 0.002
-plugins_healthcheck_jgit_latency{quantile="0.999",} 0.002
-plugins_healthcheck_jgit_latency_count 1.0
+# HELP plugins_healthcheck_jgit_latest_measured_latency Generated from Dropwizard metric import (metric=plugins/healthcheck/jgit/latency, type=com.google.gerrit.metrics.dropwizard.CallbackMetricImpl0$1)
+# TYPE plugins_healthcheck_jgit_latest_measured_latency gauge
+plugins_healthcheck_jgit_latest_measured_latency 4.0
-# HELP plugins_healthcheck_projectslist_latency Generated from Dropwizard metric import (metric=plugins/healthcheck/projectslist/latency, type=com.codahale.metrics.Timer)
-# TYPE plugins_healthcheck_projectslist_latency summary
-plugins_healthcheck_projectslist_latency{quantile="0.5",} 0.001
-plugins_healthcheck_projectslist_latency{quantile="0.75",} 0.001
-plugins_healthcheck_projectslist_latency{quantile="0.95",} 0.001
-plugins_healthcheck_projectslist_latency{quantile="0.98",} 0.001
-plugins_healthcheck_projectslist_latency{quantile="0.99",} 0.001
-plugins_healthcheck_projectslist_latency{quantile="0.999",} 0.001
-plugins_healthcheck_projectslist_latency_count 1.0
+# HELP plugins_healthcheck_projectslist_latest_measured_latency Generated from Dropwizard metric import (metric=plugins/healthcheck/projectslist/latency, type=com.google.gerrit.metrics.dropwizard.CallbackMetricImpl0$1)
+# TYPE plugins_healthcheck_projectslist_latest_measured_latency gauge
+plugins_healthcheck_projectslist_latest_measured_latency 5.0
-# HELP plugins_healthcheck_reviewdb_latency Generated from Dropwizard metric import (metric=plugins/healthcheck/reviewdb/latency, type=com.codahale.metrics.Timer)
-# TYPE plugins_healthcheck_reviewdb_latency summary
-plugins_healthcheck_reviewdb_latency{quantile="0.5",} 0.001
-plugins_healthcheck_reviewdb_latency{quantile="0.75",} 0.001
-plugins_healthcheck_reviewdb_latency{quantile="0.95",} 0.001
-plugins_healthcheck_reviewdb_latency{quantile="0.98",} 0.001
-plugins_healthcheck_reviewdb_latency{quantile="0.99",} 0.001
-plugins_healthcheck_reviewdb_latency{quantile="0.999",} 0.001
-plugins_healthcheck_reviewdb_latency_count 1.0
+# HELP plugins_healthcheck_reviewdb_latest_measured_latency Generated from Dropwizard metric import (metric=plugins/healthcheck/reviewdb/latency, type=com.google.gerrit.metrics.dropwizard.CallbackMetricImpl0$1)
+# TYPE plugins_healthcheck_reviewdb_latest_measured_latency gauge
+plugins_healthcheck_reviewdb_latest_measured_latency 3.0
# HELP plugins_healthcheck_jgit_failure_total Generated from Dropwizard metric import (metric=plugins/healthcheck/jgit/failure, type=com.codahale.metrics.Meter)
# TYPE plugins_healthcheck_jgit_failure_total counter
-plugins_healthcheck_jgit_failure_total 0.0
+plugins_healthcheck_jgit_failure_total 3.0
# HELP plugins_healthcheck_projectslist_failure_total Generated from Dropwizard metric import (metric=plugins/healthcheck/projectslist/failure, type=com.codahale.metrics.Meter)
# TYPE plugins_healthcheck_projectslist_failure_total counter
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java
new file mode 100644
index 0000000..9955a33
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java
@@ -0,0 +1,164 @@
+// Copyright (C) 2019 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 static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.metrics.CallbackMetric0;
+import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Description;
+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;
+import com.google.inject.Injector;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.healthcheck.check.AbstractHealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.StatusSummary;
+import org.junit.Before;
+import org.junit.Test;
+
+public class HealthCheckMetricsTest {
+
+ @Inject private ListeningExecutorService executor;
+ private TestMetrics testMetrics = new TestMetrics();
+
+ @Before
+ public void setUp() throws Exception {
+ testMetrics.reset();
+ }
+
+ private void setWithStatusSummary(StatusSummary StatusSummary) {
+ TestHealthCheck testHealthCheck = new TestHealthCheck(executor, "test", StatusSummary);
+
+ Injector injector =
+ testInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ DynamicSet.bind(binder(), HealthCheck.class).toInstance(testHealthCheck);
+ bind(MetricMaker.class).toInstance(testMetrics);
+ bind(LifecycleListener.class).to(HealthCheckMetrics.class);
+ }
+ });
+ HealthCheckMetrics healthCheckMetrics = injector.getInstance(HealthCheckMetrics.class);
+
+ healthCheckMetrics.start();
+ testHealthCheck.run();
+ healthCheckMetrics.triggerAll();
+ }
+
+ @Test
+ public void shouldSendCounterWhenStatusSummaryFailed() {
+ Long elapsed = 100L;
+ setWithStatusSummary(new StatusSummary(Result.FAILED, 1L, elapsed));
+
+ assertThat(testMetrics.getFailures()).isEqualTo(1);
+ assertThat(testMetrics.getLatency()).isEqualTo(elapsed);
+ }
+
+ @Test
+ public void shouldSendCounterWhenStatusSummaryTimeout() {
+ Long elapsed = 100L;
+ setWithStatusSummary(new StatusSummary(Result.TIMEOUT, 1L, elapsed));
+
+ assertThat(testMetrics.getFailures()).isEqualTo(1);
+ assertThat(testMetrics.getLatency()).isEqualTo(elapsed);
+ }
+
+ @Test
+ public void shouldNOTSendCounterWhenStatusSummarySuccess() {
+ Long elapsed = 100L;
+ setWithStatusSummary(new StatusSummary(Result.PASSED, 1L, elapsed));
+
+ assertThat(testMetrics.failures).isEqualTo(0L);
+ assertThat(testMetrics.getLatency()).isEqualTo(elapsed);
+ }
+
+ private Injector testInjector(AbstractModule testModule) {
+ return Guice.createInjector(new HealthCheckModule(), testModule);
+ }
+
+ @Singleton
+ private static class TestMetrics extends DisabledMetricMaker {
+ private Long failures = 0L;
+ private Long latency = 0L;
+
+ public Long getFailures() {
+ return failures;
+ }
+
+ public Long getLatency() {
+ return latency;
+ }
+
+ public void reset() {
+ failures = 0L;
+ latency = 0L;
+ }
+
+ @Override
+ public Counter0 newCounter(String name, Description desc) {
+ return new Counter0() {
+ @Override
+ public void incrementBy(long value) {
+ failures += value;
+ }
+
+ @Override
+ public void remove() {}
+ };
+ }
+
+ @Override
+ public <V> CallbackMetric0<V> newCallbackMetric(
+ String name, Class<V> valueClass, Description desc) {
+ return new CallbackMetric0<V>() {
+ @Override
+ public void set(V value) {
+ latency = (Long) value;
+ }
+
+ @Override
+ public void remove() {}
+ };
+ }
+ }
+
+ private static class TestHealthCheck extends AbstractHealthCheck {
+
+ protected TestHealthCheck(
+ ListeningExecutorService executor, String name, StatusSummary returnStatusSummary) {
+ super(executor, HealthCheckConfig.DEFAULT_CONFIG, name);
+ this.latestStatus = returnStatusSummary;
+ }
+
+ @Override
+ protected Result doCheck() {
+ return latestStatus.result;
+ }
+
+ @Override
+ public StatusSummary run() {
+ return latestStatus;
+ }
+ }
+}
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 4bc63ba..78b89e4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
@@ -28,7 +28,6 @@
import com.googlesource.gerrit.plugins.healthcheck.api.HealthCheckStatusEndpoint;
import com.googlesource.gerrit.plugins.healthcheck.check.AbstractHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
-import com.googlesource.gerrit.plugins.healthcheck.check.MetricsHandler;
import java.util.concurrent.Executors;
import javax.servlet.http.HttpServletResponse;
import org.junit.Test;
@@ -41,17 +40,7 @@
public TestHealthCheck(
HealthCheckConfig config, String checkName, HealthCheck.Result result, long sleep) {
- super(
- MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)),
- config,
- checkName,
- new MetricsHandler.Factory() {
-
- @Override
- public MetricsHandler create(String name) {
- return new MetricsHandler(checkName, new DisabledMetricMaker());
- }
- });
+ super(MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)), config, checkName);
this.checkResult = result;
this.sleep = sleep;
}
@@ -64,6 +53,11 @@
}
return checkResult;
}
+
+ @Override
+ public StatusSummary getLatestStatus() {
+ return this.latestStatus;
+ }
}
@Test
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 6e78dfe..b44ca9a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
@@ -19,7 +19,6 @@
import static org.eclipse.jgit.lib.RefUpdate.Result.NEW;
import com.google.common.util.concurrent.ListeningExecutorService;
-import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -29,7 +28,6 @@
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
import com.googlesource.gerrit.plugins.healthcheck.check.JGitHealthCheck;
-import com.googlesource.gerrit.plugins.healthcheck.check.MetricsHandler;
import java.io.IOException;
import java.util.Collections;
import java.util.SortedSet;
@@ -48,13 +46,6 @@
private AllProjectsName allProjectsName = new AllProjectsName("All-Projects");
private InMemoryRepositoryManager inMemoryRepositoryManager = new InMemoryRepositoryManager();
private PersonIdent personIdent = new PersonIdent("Gerrit Rietveld", "gerrit@rietveld.nl");
- private final MetricsHandler.Factory metricsHandlerFactory =
- new MetricsHandler.Factory() {
- @Override
- public MetricsHandler create(String name) {
- return new MetricsHandler("foo", new DisabledMetricMaker());
- }
- };
@Inject private ListeningExecutorService executor;
@@ -71,11 +62,7 @@
public void shouldBeHealthyWhenJGitIsWorking() {
JGitHealthCheck reviewDbCheck =
new JGitHealthCheck(
- executor,
- DEFAULT_CONFIG,
- getWorkingRepositoryManager(),
- allProjectsName,
- metricsHandlerFactory);
+ executor, DEFAULT_CONFIG, getWorkingRepositoryManager(), allProjectsName);
assertThat(reviewDbCheck.run().result).isEqualTo(Result.PASSED);
}
@@ -83,11 +70,7 @@
public void shouldBeUnhealthyWhenJGitIsFailing() {
JGitHealthCheck jGitHealthCheck =
new JGitHealthCheck(
- executor,
- DEFAULT_CONFIG,
- getFailingGitRepositoryManager(),
- allProjectsName,
- metricsHandlerFactory);
+ executor, DEFAULT_CONFIG, getFailingGitRepositoryManager(), allProjectsName);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/MetricsHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/MetricsHandlerTest.java
deleted file mode 100644
index aa66b1f..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/MetricsHandlerTest.java
+++ /dev/null
@@ -1,99 +0,0 @@
-package com.googlesource.gerrit.plugins.healthcheck;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.gerrit.metrics.Counter0;
-import com.google.gerrit.metrics.Description;
-import com.google.gerrit.metrics.DisabledMetricMaker;
-import com.google.gerrit.metrics.Timer0;
-import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
-import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Status;
-import com.googlesource.gerrit.plugins.healthcheck.check.MetricsHandler;
-import java.util.concurrent.TimeUnit;
-import org.junit.Test;
-
-public class MetricsHandlerTest {
-
- private class TestMetricsMaker extends DisabledMetricMaker {
- private Long failures = 0L;
- private Long latency = 0L;
-
- @Override
- public Counter0 newCounter(String name, Description desc) {
- return new Counter0() {
- @Override
- public void incrementBy(long value) {
- failures += value;
- }
-
- @Override
- public void remove() {}
- };
- }
-
- @Override
- public Timer0 newTimer(String name, Description desc) {
- return new Timer0() {
- @Override
- public void remove() {}
-
- @Override
- public void record(long value, TimeUnit unit) {
- latency = value;
- }
- };
- }
- }
-
- @Test
- public void shouldSendCounterWhenStatusFailed() {
- TestMetricsMaker metricMaker = new TestMetricsMaker();
- MetricsHandler handler = new MetricsHandler("test", metricMaker);
-
- handler.sendMetrics(new Status(Result.FAILED, 1L, 1L));
-
- assertThat(metricMaker.failures).isEqualTo(1L);
- }
-
- @Test
- public void shouldSendCounterWhenStatusTimeout() {
- TestMetricsMaker metricMaker = new TestMetricsMaker();
- MetricsHandler handler = new MetricsHandler("test", metricMaker);
-
- handler.sendMetrics(new Status(Result.TIMEOUT, 1L, 1L));
-
- assertThat(metricMaker.failures).isEqualTo(1L);
- }
-
- @Test
- public void shouldNOTSendCounterWhenStatusSuccess() {
- TestMetricsMaker metricMaker = new TestMetricsMaker();
- MetricsHandler handler = new MetricsHandler("test", metricMaker);
-
- handler.sendMetrics(new Status(Result.PASSED, 1L, 1L));
-
- assertThat(metricMaker.failures).isEqualTo(0L);
- }
-
- @Test
- public void shouldRecordLatencyWhenSuccess() {
- TestMetricsMaker metricMaker = new TestMetricsMaker();
- MetricsHandler handler = new MetricsHandler("test", metricMaker);
- Long elapsed = System.currentTimeMillis();
-
- handler.sendMetrics(new Status(Result.PASSED, 1L, elapsed));
-
- assertThat(metricMaker.latency).isEqualTo(elapsed);
- }
-
- @Test
- public void shouldRecordLatencyWhenFailure() {
- TestMetricsMaker metricMaker = new TestMetricsMaker();
- MetricsHandler handler = new MetricsHandler("test", metricMaker);
- Long elapsed = System.currentTimeMillis();
-
- handler.sendMetrics(new Status(Result.FAILED, 1L, elapsed));
-
- assertThat(metricMaker.latency).isEqualTo(elapsed);
- }
-}
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 e07c12c..0d29109 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
@@ -20,12 +20,10 @@
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.server.project.ListProjects;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
-import com.googlesource.gerrit.plugins.healthcheck.check.MetricsHandler;
import com.googlesource.gerrit.plugins.healthcheck.check.ProjectsListHealthCheck;
import java.util.SortedMap;
import java.util.TreeMap;
@@ -33,13 +31,6 @@
import org.junit.Test;
public class ProjectsListHealthCheckTest {
- private final MetricsHandler.Factory metricsHandlerFactory =
- new MetricsHandler.Factory() {
- @Override
- public MetricsHandler create(String name) {
- return new MetricsHandler("foo", new DisabledMetricMaker());
- }
- };
@Inject private ListeningExecutorService executor;
@Before
@@ -50,16 +41,14 @@
@Test
public void shouldBeHealthyWhenListProjectsWorks() {
ProjectsListHealthCheck jGitHealthCheck =
- new ProjectsListHealthCheck(
- executor, DEFAULT_CONFIG, getWorkingProjectList(0), metricsHandlerFactory);
+ new ProjectsListHealthCheck(executor, DEFAULT_CONFIG, getWorkingProjectList(0));
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.PASSED);
}
@Test
public void shouldBeUnhealthyWhenListProjectsIsFailing() {
ProjectsListHealthCheck jGitHealthCheck =
- new ProjectsListHealthCheck(
- executor, DEFAULT_CONFIG, getFailingProjectList(), metricsHandlerFactory);
+ new ProjectsListHealthCheck(executor, DEFAULT_CONFIG, getFailingProjectList());
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED);
}
@@ -67,10 +56,7 @@
public void shouldBeUnhealthyWhenListProjectsIsTimingOut() {
ProjectsListHealthCheck jGitHealthCheck =
new ProjectsListHealthCheck(
- executor,
- DEFAULT_CONFIG,
- getWorkingProjectList(DEFAULT_CONFIG.getTimeout() * 2),
- metricsHandlerFactory);
+ executor, DEFAULT_CONFIG, getWorkingProjectList(DEFAULT_CONFIG.getTimeout() * 2));
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.TIMEOUT);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java
index fc818b7..71b1684 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ReviewDbHealthCheckTest.java
@@ -19,7 +19,6 @@
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.lifecycle.LifecycleManager;
-import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.testutil.DisabledReviewDb;
import com.google.gerrit.testutil.InMemoryDatabase;
@@ -28,20 +27,12 @@
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
-import com.googlesource.gerrit.plugins.healthcheck.check.MetricsHandler;
import com.googlesource.gerrit.plugins.healthcheck.check.ReviewDbHealthCheck;
import org.junit.Before;
import org.junit.Test;
public class ReviewDbHealthCheckTest {
private SchemaFactory<ReviewDb> workingReviewDbFactory;
- private final MetricsHandler.Factory metricsHandlerFactory =
- new MetricsHandler.Factory() {
- @Override
- public MetricsHandler create(String name) {
- return new MetricsHandler("foo", new DisabledMetricMaker());
- }
- };
@Inject private ListeningExecutorService executor;
@Before
@@ -55,16 +46,14 @@
@Test
public void shouldBeHealthyWhenReviewDbIsWorking() {
ReviewDbHealthCheck reviewDbCheck =
- new ReviewDbHealthCheck(
- executor, DEFAULT_CONFIG, workingReviewDbFactory, metricsHandlerFactory);
+ new ReviewDbHealthCheck(executor, DEFAULT_CONFIG, workingReviewDbFactory);
assertThat(reviewDbCheck.run().result).isEqualTo(HealthCheck.Result.PASSED);
}
@Test
public void shouldBeUnhealthyWhenReviewDbIsFailing() {
ReviewDbHealthCheck reviewDbCheck =
- new ReviewDbHealthCheck(
- executor, DEFAULT_CONFIG, getFailingReviewDbProvider(), metricsHandlerFactory);
+ new ReviewDbHealthCheck(executor, DEFAULT_CONFIG, getFailingReviewDbProvider());
assertThat(reviewDbCheck.run().result).isEqualTo(HealthCheck.Result.FAILED);
}
