Merge branch 'stable-3.2' into stable-3.3 * stable-3.2: Missing global metric Change-Id: Ib05c3f40b41eee490b748b8c929f7192d1b5adfb
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