Missing global metric
Global metrics for success/failure of the checks rely on
the API ones exposed by default by Gerrit.
Since replicas don't have a REST API those metrics are
not exposed.
As part if this change the calculation of response code code
for the HealthCheckStatusEndpoint has been moved to the
GlobalHealthCheck.
Apart for fixing the responsability of this method, it also helps
in the global metric exposure.
Bug: Issue 14381
Change-Id: I586576d0b49da284004da7dbc4fa69ea4db469ef
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 edc3223..fb298fd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java
@@ -24,6 +24,7 @@
import com.google.gerrit.metrics.MetricMaker;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.healthcheck.check.GlobalHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import java.util.Collections;
import java.util.HashSet;
@@ -36,13 +37,18 @@
private final MetricMaker metricMaker;
private final Set<RegistrationHandle> registeredMetrics;
private final Set<Runnable> triggers;
+ private final GlobalHealthCheck globalHealthCheck;
@Inject
- public HealthCheckMetrics(DynamicSet<HealthCheck> healthChecks, MetricMaker metricMaker) {
+ public HealthCheckMetrics(
+ DynamicSet<HealthCheck> healthChecks,
+ GlobalHealthCheck globalHealthCheck,
+ MetricMaker metricMaker) {
this.healthChecks = healthChecks;
this.metricMaker = metricMaker;
this.registeredMetrics = Collections.synchronizedSet(new HashSet<>());
this.triggers = Collections.synchronizedSet(new HashSet<>());
+ this.globalHealthCheck = globalHealthCheck;
}
@Override
@@ -51,35 +57,58 @@
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();
- }
- };
+ Counter0 failureMetric = getFailureMetric(name);
+ CallbackMetric0<Long> latencyMetric = getLatencyMetric(name);
+ Runnable metricCallBack = getCallbackMetric(healthCheck, failureMetric, latencyMetric);
registeredMetrics.add(failureMetric);
registeredMetrics.add(metricMaker.newTrigger(latencyMetric, metricCallBack));
triggers.add(metricCallBack);
}
+
+ Counter0 globalFailureMetric = getFailureMetric("global");
+ CallbackMetric0<Long> globalLatencyMetric = getLatencyMetric("global");
+ Runnable globalMetricCallBack =
+ () -> {
+ HealthCheck.StatusSummary status = globalHealthCheck.getGlobalStatusSummary();
+ globalLatencyMetric.set(status.elapsed);
+ if (status.isFailure()) {
+ globalFailureMetric.increment();
+ }
+ };
+
+ registeredMetrics.add(globalFailureMetric);
+ registeredMetrics.add(metricMaker.newTrigger(globalLatencyMetric, globalMetricCallBack));
+ triggers.add(globalMetricCallBack);
+ }
+
+ private Runnable getCallbackMetric(
+ HealthCheck healthCheck, Counter0 failureMetric, CallbackMetric0<Long> latencyMetric) {
+ return () -> {
+ HealthCheck.StatusSummary status = healthCheck.getLatestStatus();
+ latencyMetric.set(healthCheck.getLatestStatus().elapsed);
+ if (status.isFailure()) {
+ failureMetric.increment();
+ }
+ };
+ }
+
+ private CallbackMetric0<Long> getLatencyMetric(String name) {
+ return 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));
+ }
+
+ private Counter0 getFailureMetric(String name) {
+ return metricMaker.newCounter(
+ String.format("%s/failure", name),
+ new Description(String.format("%s healthcheck failures count", name))
+ .setCumulative()
+ .setRate()
+ .setUnit("failures"));
}
@Override
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 ecae202..9e0a607 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
@@ -36,24 +36,16 @@
@Override
public Response<Map<String, Object>> apply(ConfigResource resource)
throws AuthException, BadRequestException, ResourceConflictException, Exception {
- long ts = System.currentTimeMillis();
Map<String, Object> result = healthChecks.run();
- long elapsed = System.currentTimeMillis() - ts;
- result.put("ts", new Long(ts));
- result.put("elapsed", new Long(elapsed));
- return Response.withStatusCode(getResultStatus(result), result);
+
+ result.put("ts", healthChecks.getGlobalStatusSummary().ts);
+ result.put("elapsed", healthChecks.getGlobalStatusSummary().elapsed);
+ return Response.withStatusCode(getHTTPResultCode(result), result);
}
- private int getResultStatus(Map<String, Object> result) {
- if (result.values().stream()
- .filter(
- res ->
- res instanceof HealthCheck.StatusSummary
- && ((HealthCheck.StatusSummary) res).isFailure())
- .findFirst()
- .isPresent()) {
- return HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
- }
- return HttpServletResponse.SC_OK;
+ private int getHTTPResultCode(Map<String, Object> result) {
+ return healthChecks.getResultStatus(result) == HealthCheck.Result.FAILED
+ ? HttpServletResponse.SC_INTERNAL_SERVER_ERROR
+ : HttpServletResponse.SC_OK;
}
}
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 76e8706..c9066c5 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
@@ -16,24 +16,51 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import java.util.Arrays;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
+@Singleton
public class GlobalHealthCheck {
private final DynamicSet<HealthCheck> healthChecks;
+ private HealthCheck.StatusSummary globalStatusSummary;
@Inject
public GlobalHealthCheck(DynamicSet<HealthCheck> healthChecks) {
this.healthChecks = healthChecks;
+ this.globalStatusSummary = HealthCheck.StatusSummary.INITIAL_STATUS;
}
public Map<String, Object> run() {
Iterable<HealthCheck> iterable = () -> healthChecks.iterator();
- return StreamSupport.stream(iterable.spliterator(), false)
- .map(check -> Arrays.asList(check.name(), check.run()))
- .collect(Collectors.toMap(k -> (String) k.get(0), v -> v.get(1)));
+ long ts = System.currentTimeMillis();
+ Map<String, Object> checkToResults =
+ StreamSupport.stream(iterable.spliterator(), false)
+ .map(check -> Arrays.asList(check.name(), check.run()))
+ .collect(Collectors.toMap(k -> (String) k.get(0), v -> v.get(1)));
+ long elapsed = System.currentTimeMillis() - ts;
+ globalStatusSummary =
+ new HealthCheck.StatusSummary(getResultStatus(checkToResults), ts, elapsed);
+ return checkToResults;
+ }
+
+ public HealthCheck.StatusSummary getGlobalStatusSummary() {
+ return this.globalStatusSummary;
+ }
+
+ public HealthCheck.Result getResultStatus(Map<String, Object> result) {
+ if (result.values().stream()
+ .filter(
+ res ->
+ res instanceof HealthCheck.StatusSummary
+ && ((HealthCheck.StatusSummary) res).isFailure())
+ .findFirst()
+ .isPresent()) {
+ return HealthCheck.Result.FAILED;
+ }
+ return HealthCheck.Result.PASSED;
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/GlobalHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/GlobalHealthCheckTest.java
new file mode 100644
index 0000000..a1c792e
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/GlobalHealthCheckTest.java
@@ -0,0 +1,55 @@
+// 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 static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.healthcheck.check.GlobalHealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
+import java.util.HashMap;
+import org.junit.Test;
+
+public class GlobalHealthCheckTest {
+
+ @Inject private DynamicSet<HealthCheck> healthChecks;
+
+ GlobalHealthCheck globalHealthCheck = new GlobalHealthCheck(healthChecks);
+
+ @Test
+ public void shouldFailIfOneCheckFails() {
+
+ HashMap<String, Object> checks =
+ new HashMap<String, Object>() {
+ {
+ put("failingCheck", new HealthCheck.StatusSummary(HealthCheck.Result.FAILED, 0L, 0L));
+ put("passingCheck", new HealthCheck.StatusSummary(HealthCheck.Result.PASSED, 0L, 0L));
+ }
+ };
+ assertThat(globalHealthCheck.getResultStatus(checks)).isEqualTo(HealthCheck.Result.FAILED);
+ }
+
+ @Test
+ public void shouldPassIfAllChecksPass() {
+ HashMap<String, Object> checks =
+ new HashMap<String, Object>() {
+ {
+ put("passingCheck", new HealthCheck.StatusSummary(HealthCheck.Result.PASSED, 0L, 0L));
+ }
+ };
+ assertThat(globalHealthCheck.getResultStatus(checks)).isEqualTo(HealthCheck.Result.PASSED);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java
index 9955a33..fb64048 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java
@@ -120,7 +120,9 @@
return new Counter0() {
@Override
public void incrementBy(long value) {
- failures += value;
+ if (!name.startsWith("global/")) {
+ failures += value;
+ }
}
@Override
@@ -134,7 +136,9 @@
return new CallbackMetric0<V>() {
@Override
public void set(V value) {
- latency = (Long) value;
+ if (!name.startsWith("global/")) {
+ latency = (Long) value;
+ }
}
@Override
