Merge branch 'stable-3.0' into stable-3.1
* stable-3.0:
Add missing Singleton to ActiveWokersCheck and DeadlockCheck
Add active http workers check
Change-Id: Ie60eb05bac2b22f7bb1156d2609b4a38467aecd7
diff --git a/README.md b/README.md
index 0fe07ba..8be0b33 100644
--- a/README.md
+++ b/README.md
@@ -25,9 +25,9 @@
## How to install
Copy the healthcheck.jar into the Gerrit's /plugins directory and wait for the plugin to be automatically loaded.
-The healthcheck plugin is compatible with both Gerrit master and slave setups. The only difference to bear in mind
-is that some checks will be automatically disabled on slaves (e.g. query changes) because the associated subsystem
-is switched off.
+The healthcheck plugin is compatible with both primary Gerrit setups and Gerrit replicas. The only difference to bear
+in mind is that some checks will be automatically disabled on replicas (e.g. query changes) because the associated
+subsystem is switched off.
## How to use
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
index 126c26f..651dbe9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
@@ -15,16 +15,17 @@
package com.googlesource.gerrit.plugins.healthcheck;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.BLOCKEDTHREADS;
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.QUERYCHANGES;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.GerritIsReplica;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -62,11 +63,11 @@
@PluginName String pluginName,
AllProjectsName allProjectsName,
AllUsersName allUsersName,
- @GerritServerConfig Config gerritConfig) {
+ @GerritIsReplica boolean isReplica) {
config = configFactory.getGlobalPluginConfig(pluginName);
this.allProjectsName = allProjectsName;
this.allUsersName = allUsersName;
- isReplica = gerritConfig.getBoolean("container", "slave", false);
+ this.isReplica = isReplica;
}
@VisibleForTesting
@@ -119,7 +120,7 @@
public Set<Project.NameKey> getJGITRepositories(String healthCheckName) {
Set<Project.NameKey> repos =
Stream.of(config.getStringList(HEALTHCHECK, healthCheckName, "project"))
- .map(Project.NameKey::new)
+ .map(Project::nameKey)
.collect(Collectors.toSet());
repos.add(allProjectsName);
repos.add(allUsersName);
@@ -142,6 +143,10 @@
HEALTHCHECK, checkNotNull(healthCheckName), "enabled", HEALTH_CHECK_ENABLED_DEFAULT);
}
+ public String[] getListOfBlockedThreadsThresholds() {
+ return config.getStringList(HEALTHCHECK, BLOCKEDTHREADS, "threshold");
+ }
+
private String getStringWithFallback(
String parameter, String healthCheckName, String defaultValue) {
String fallbackDefault =
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..1f88020 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetrics.java
@@ -14,83 +14,42 @@
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.gerrit.metrics.Timer0;
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;
+import com.google.inject.assistedinject.Assisted;
-@Singleton
-public class HealthCheckMetrics implements LifecycleListener {
+public class HealthCheckMetrics {
- private final DynamicSet<HealthCheck> healthChecks;
+ public interface Factory {
+ HealthCheckMetrics create(String name);
+ }
+
private final MetricMaker metricMaker;
- private final Set<RegistrationHandle> registeredMetrics;
- private final Set<Runnable> triggers;
+ private final String name;
@Inject
- public HealthCheckMetrics(DynamicSet<HealthCheck> healthChecks, MetricMaker metricMaker) {
- this.healthChecks = healthChecks;
+ public HealthCheckMetrics(MetricMaker metricMaker, @Assisted String name) {
this.metricMaker = metricMaker;
- this.registeredMetrics = Collections.synchronizedSet(new HashSet<>());
- this.triggers = Collections.synchronizedSet(new HashSet<>());
+ this.name = name;
}
- @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);
- }
+ public Counter0 getFailureCounterMetric() {
+ return metricMaker.newCounter(
+ String.format("%s/failure", name),
+ new Description(String.format("%s healthcheck failures count", name))
+ .setCumulative()
+ .setRate()
+ .setUnit("failures"));
}
- @Override
- public void stop() {
- for (RegistrationHandle handle : registeredMetrics) {
- handle.remove();
- }
- }
-
- @VisibleForTesting
- public void triggerAll() {
- triggers.forEach(Runnable::run);
+ public Timer0 getLatencyMetric() {
+ return metricMaker.newTimer(
+ String.format("%s/latest_latency", name),
+ new Description(String.format("%s health check latency execution (ms)", name))
+ .setCumulative()
+ .setUnit(Description.Units.MILLISECONDS));
}
}
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 62cfeb4..bb70839 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckSubsystemsModule.java
@@ -14,11 +14,11 @@
package com.googlesource.gerrit.plugins.healthcheck;
-import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.inject.AbstractModule;
import com.googlesource.gerrit.plugins.healthcheck.check.ActiveWorkersCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.AuthHealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.DeadlockCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HttpActiveWorkersCheck;
@@ -26,7 +26,7 @@
import com.googlesource.gerrit.plugins.healthcheck.check.ProjectsListHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.QueryChangesHealthCheck;
-public class HealthCheckSubsystemsModule extends AbstractModule {
+public class HealthCheckSubsystemsModule extends FactoryModule {
@Override
protected void configure() {
@@ -37,7 +37,10 @@
bindChecker(ActiveWorkersCheck.class);
bindChecker(HttpActiveWorkersCheck.class);
bindChecker(DeadlockCheck.class);
- bind(LifecycleListener.class).to(HealthCheckMetrics.class);
+ bindChecker(BlockedThreadsCheck.class);
+
+ factory(HealthCheckMetrics.Factory.class);
+ install(BlockedThreadsCheck.SUB_CHECKS);
}
private void bindChecker(Class<? extends HealthCheck> healthCheckClass) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HttpModule.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HttpModule.java
index c62a261..5217dff 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HttpModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HttpModule.java
@@ -16,24 +16,23 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.httpd.AllRequestFilter;
-import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.GerritIsReplica;
import com.google.inject.Inject;
import com.google.inject.Scopes;
import com.google.inject.servlet.ServletModule;
import com.googlesource.gerrit.plugins.healthcheck.filter.HealthCheckStatusFilter;
-import org.eclipse.jgit.lib.Config;
public class HttpModule extends ServletModule {
- private boolean isSlave;
+ private boolean isReplica;
@Inject
- public HttpModule(@GerritServerConfig Config gerritConfig) {
- isSlave = gerritConfig.getBoolean("container", "slave", false);
+ public HttpModule(@GerritIsReplica boolean isReplica) {
+ this.isReplica = isReplica;
}
@Override
protected void configureServlets() {
- if (isSlave) {
+ if (isReplica) {
DynamicSet.bind(binder(), AllRequestFilter.class)
.to(HealthCheckStatusFilter.class)
.in(Scopes.SINGLETON);
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 b032c02..2d5f7de 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
@@ -20,6 +20,7 @@
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.check.GlobalHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
import java.util.Map;
import javax.servlet.http.HttpServletResponse;
@@ -34,26 +35,19 @@
}
@Override
- public Object apply(ConfigResource resource)
+ 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);
+ HealthCheck.StatusSummary globalHealthCheckStatus = healthChecks.run();
+
+ Map<String, Object> result = globalHealthCheckStatus.subChecks;
+ result.put("ts", globalHealthCheckStatus.ts);
+ result.put("elapsed", globalHealthCheckStatus.elapsed);
+ return Response.withStatusCode(getHTTPResultCode(globalHealthCheckStatus), 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(HealthCheck.StatusSummary checkStatus) {
+ return checkStatus.result == Result.FAILED
+ ? HttpServletResponse.SC_INTERNAL_SERVER_ERROR
+ : HttpServletResponse.SC_OK;
}
}
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 a6eae79..9e4544a 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
@@ -16,7 +16,11 @@
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.Timer0;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
+import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics;
+import java.util.Collections;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
@@ -28,16 +32,26 @@
private final long timeout;
private final String name;
private final ListeningExecutorService executor;
- protected StatusSummary latestStatus;
+ protected volatile StatusSummary latestStatus;
protected HealthCheckConfig config;
+ protected final Counter0 failureCounterMetric;
+ protected final Timer0 latencyMetric;
+
protected AbstractHealthCheck(
- ListeningExecutorService executor, HealthCheckConfig config, String name) {
+ ListeningExecutorService executor,
+ HealthCheckConfig config,
+ String name,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
this.executor = executor;
this.name = name;
this.timeout = config.getTimeout(name);
this.config = config;
this.latestStatus = StatusSummary.INITIAL_STATUS;
+
+ HealthCheckMetrics healthCheckMetrics = healthCheckMetricsFactory.create(name);
+ this.failureCounterMetric = healthCheckMetrics.getFailureCounterMetric();
+ this.latencyMetric = healthCheckMetrics.getLatencyMetric();
}
@Override
@@ -47,6 +61,7 @@
@Override
public StatusSummary run() {
+ StatusSummary checkStatusSummary;
boolean enabled = config.healthCheckEnabled(name);
final long ts = System.currentTimeMillis();
ListenableFuture<StatusSummary> resultFuture =
@@ -59,24 +74,32 @@
log.warn("Check {} failed", name, e);
healthy = Result.FAILED;
}
- return new StatusSummary(healthy, ts, System.currentTimeMillis() - ts);
+ Long elapsed = System.currentTimeMillis() - ts;
+ StatusSummary statusSummary =
+ new StatusSummary(healthy, ts, elapsed, Collections.emptyMap());
+ if (statusSummary.isFailure()) {
+ failureCounterMetric.increment();
+ }
+ latencyMetric.record(elapsed, TimeUnit.MILLISECONDS);
+ return statusSummary;
});
try {
- latestStatus = resultFuture.get(timeout, TimeUnit.MILLISECONDS);
+ checkStatusSummary = resultFuture.get(timeout, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
log.warn("Check {} timed out", name, e);
- latestStatus = new StatusSummary(Result.TIMEOUT, ts, System.currentTimeMillis() - ts);
+ Long elapsed = System.currentTimeMillis() - ts;
+ checkStatusSummary = new StatusSummary(Result.TIMEOUT, ts, elapsed, Collections.emptyMap());
+ failureCounterMetric.increment();
+ latencyMetric.record(elapsed, TimeUnit.MILLISECONDS);
} catch (InterruptedException | ExecutionException e) {
log.warn("Check {} failed while waiting for its future result", name, e);
- latestStatus = new StatusSummary(Result.FAILED, ts, System.currentTimeMillis() - ts);
+ Long elapsed = System.currentTimeMillis() - ts;
+ checkStatusSummary = new StatusSummary(Result.FAILED, ts, elapsed, Collections.emptyMap());
+ failureCounterMetric.increment();
+ latencyMetric.record(elapsed, TimeUnit.MILLISECONDS);
}
- return latestStatus;
+ return checkStatusSummary;
}
protected abstract Result doCheck() throws Exception;
-
- @Override
- public StatusSummary getLatestStatus() {
- return latestStatus;
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractWorkersHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractWorkersHealthCheck.java
index 8e5f8fb..a01eba1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractWorkersHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractWorkersHealthCheck.java
@@ -18,6 +18,7 @@
import com.codahale.metrics.MetricRegistry;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
+import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics;
import java.util.Optional;
public abstract class AbstractWorkersHealthCheck extends AbstractHealthCheck {
@@ -33,8 +34,9 @@
MetricRegistry metricRegistry,
String name,
String metricName,
- Integer maxPoolSize) {
- super(executor, config, name);
+ Integer maxPoolSize,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(executor, config, name, healthCheckMetricsFactory);
this.metricRegistry = metricRegistry;
this.metricName = metricName;
this.maxPoolSize = maxPoolSize;
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 8770a62..a23567c 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
@@ -23,6 +23,7 @@
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 org.eclipse.jgit.lib.Config;
@Singleton
@@ -37,7 +38,8 @@
ListeningExecutorService executor,
HealthCheckConfig healthCheckConfig,
ThreadSettingsConfig threadSettingsConfig,
- MetricRegistry metricRegistry) {
+ MetricRegistry metricRegistry,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
super(
executor,
@@ -45,7 +47,8 @@
metricRegistry,
ACTIVEWORKERS,
ACTIVE_WORKERS_METRIC_NAME,
- getInteractiveThreadsMaxPoolSize(threadSettingsConfig, gerritConfig));
+ getInteractiveThreadsMaxPoolSize(threadSettingsConfig, gerritConfig),
+ healthCheckMetricsFactory);
}
/**
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 0dcb7a5..ce9f8d9 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
@@ -24,6 +24,7 @@
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;
@@ -41,8 +42,9 @@
ListeningExecutorService executor,
HealthCheckConfig config,
Realm realm,
- AccountCache byIdCache) {
- super(executor, config, AUTH);
+ AccountCache byIdCache,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(executor, config, AUTH, healthCheckMetricsFactory);
this.realm = realm;
this.byIdCache = byIdCache;
@@ -61,7 +63,7 @@
log.error("Cannot load account state for username " + username);
return Result.FAILED;
}
- if (!accountState.get().getAccount().isActive()) {
+ if (!accountState.get().account().isActive()) {
log.error("Authentication error, account " + username + " is inactive");
return Result.FAILED;
}
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
new file mode 100644
index 0000000..78b8b01
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsCheck.java
@@ -0,0 +1,121 @@
+// 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.check;
+
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.BLOCKEDTHREADS;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.gerrit.extensions.config.FactoryModule;
+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;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Stream;
+
+@Singleton
+public class BlockedThreadsCheck extends AbstractHealthCheck {
+ public static Module SUB_CHECKS =
+ new FactoryModule() {
+ @Override
+ protected void configure() {
+ factory(BlockedThreadsSubCheck.Factory.class);
+ }
+ };
+
+ private final ThreadMXBean threads;
+ private final BlockedThreadsConfigurator collectorsSupplier;
+
+ @Inject
+ public BlockedThreadsCheck(
+ ListeningExecutorService executor,
+ HealthCheckConfig healthCheckConfig,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory,
+ ThreadBeanProvider threadBeanProvider,
+ BlockedThreadsConfigurator checksConfig) {
+ super(executor, healthCheckConfig, BLOCKEDTHREADS, healthCheckMetricsFactory);
+ this.threads = threadBeanProvider.get();
+ this.collectorsSupplier = checksConfig;
+ }
+
+ @Override
+ protected Result doCheck() throws Exception {
+ List<Collector> collectors = collectorsSupplier.createCollectors();
+ dumpAllThreads().forEach(info -> collectors.forEach(c -> c.collect(info)));
+
+ // call check on all sub-checks so that metrics are populated
+ collectors.forEach(Collector::check);
+
+ // report unhealthy instance if any of sub-checks failed
+ return collectors.stream()
+ .map(Collector::result)
+ .filter(r -> Result.FAILED == r)
+ .findAny()
+ .orElse(Result.PASSED);
+ }
+
+ private Stream<ThreadInfo> dumpAllThreads() {
+ // getting all thread ids and translating it into thread infos is noticeably faster then call to
+ // ThreadMXBean.dumpAllThreads as it doesn't calculate StackTrace. Note that some threads could
+ // be already finished (between call to get all ids and translate them to ThreadInfo objects
+ // hence they have to be filtered out).
+ return Arrays.stream(threads.getThreadInfo(threads.getAllThreadIds(), 0))
+ .filter(Objects::nonNull);
+ }
+
+ @VisibleForTesting
+ public static class ThreadBeanProvider {
+ public ThreadMXBean get() {
+ return ManagementFactory.getThreadMXBean();
+ }
+ }
+
+ static class Collector {
+ protected final Integer threshold;
+
+ protected int blocked;
+ protected int total;
+ protected Result result;
+
+ Collector(Integer threshold) {
+ this.threshold = threshold;
+ }
+
+ void collect(ThreadInfo info) {
+ total += 1;
+ if (Thread.State.BLOCKED == info.getThreadState()) {
+ blocked += 1;
+ }
+ }
+
+ void check() {
+ result = blocked * 100 <= threshold * total ? Result.PASSED : Result.FAILED;
+ }
+
+ Result result() {
+ return result;
+ }
+ }
+
+ interface CollectorProvider<T extends Collector> extends Provider<T> {}
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsConfigurator.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsConfigurator.java
new file mode 100644
index 0000000..40030e1
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsConfigurator.java
@@ -0,0 +1,205 @@
+// 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.check;
+
+import static java.util.stream.Collectors.groupingBy;
+import static java.util.stream.Collectors.toList;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
+import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck.Collector;
+import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck.CollectorProvider;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@VisibleForTesting
+public class BlockedThreadsConfigurator {
+ private static final Logger log = LoggerFactory.getLogger(BlockedThreadsConfigurator.class);
+ private static final Pattern THRESHOLD_PATTERN = Pattern.compile("^(\\d\\d?)$");
+
+ static final int DEFAULT_BLOCKED_THREADS_THRESHOLD = 50;
+
+ private final List<CollectorProvider<?>> providers;
+
+ @Inject
+ BlockedThreadsConfigurator(
+ BlockedThreadsSubCheck.Factory subchecks, HealthCheckConfig healthCheckConfig) {
+ this.providers = getProviders(subchecks, healthCheckConfig);
+ }
+
+ List<Collector> createCollectors() {
+ return providers.stream().map(CollectorProvider::get).collect(toList());
+ }
+
+ private static List<CollectorProvider<?>> getProviders(
+ BlockedThreadsSubCheck.Factory subchecksFactory, HealthCheckConfig healthCheckConfig) {
+ return getConfig(healthCheckConfig.getListOfBlockedThreadsThresholds()).stream()
+ .map(spec -> collectorProvider(subchecksFactory, spec))
+ .collect(toList());
+ }
+
+ private static CollectorProvider<?> collectorProvider(
+ BlockedThreadsSubCheck.Factory subchecksFactory, Threshold spec) {
+ return spec.prefix.isPresent()
+ ? subchecksFactory.create(spec.prefix.get(), spec.value)
+ : () -> new BlockedThreadsCheck.Collector(spec.value);
+ }
+
+ @VisibleForTesting
+ static Collection<Threshold> getConfig(String[] thresholds) {
+ // Threshold can be defined as a sole value e.g
+ // threshold = 80
+ // and would become a default one for all blocked threads check or as a set of specific thread
+ // groups checks defined like
+ // threshold = foo=30
+ // threshold = bar=40
+ // ...
+ // they are mutually exclusive which means that one either checks all threads or groups
+ Map<Boolean, List<Threshold>> specsClassified =
+ Arrays.stream(thresholds)
+ .filter(spec -> !spec.isEmpty())
+ .map(BlockedThreadsConfigurator::getSpec)
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .collect(groupingBy(Threshold::hasPrefix));
+
+ // check configuration consistency
+ if (specsClassified.size() > 1) {
+ Collection<Threshold> specs = deduplicatePrefixes(specsClassified.get(true));
+ log.warn(
+ "Global and specific thresholds were configured for blocked threads check. Specific"
+ + " configuration is used {}.",
+ specs);
+ return specs;
+ }
+
+ if (specsClassified.size() == 1) {
+ Map.Entry<Boolean, List<Threshold>> entry = specsClassified.entrySet().iterator().next();
+ return Boolean.TRUE == entry.getKey()
+ ? deduplicatePrefixes(entry.getValue())
+ : deduplicateGlobal(entry.getValue());
+ }
+
+ log.info(
+ "Default blocked threads check is configured with {}% threshold",
+ DEFAULT_BLOCKED_THREADS_THRESHOLD);
+ return ImmutableSet.of(new Threshold(DEFAULT_BLOCKED_THREADS_THRESHOLD));
+ }
+
+ private static Collection<Threshold> deduplicateGlobal(List<Threshold> input) {
+ if (input.size() > 1) {
+ Threshold spec = input.get(input.size() - 1);
+ log.warn("Multiple threshold values were configured. Using {}", spec);
+ return ImmutableSet.of(spec);
+ }
+ return input;
+ }
+
+ private static Collection<Threshold> deduplicatePrefixes(Collection<Threshold> input) {
+ Map<String, Threshold> deduplicated = new HashMap<>();
+ input.forEach(t -> deduplicated.put(t.prefix.get(), t));
+ if (deduplicated.size() != input.size()) {
+ log.warn(
+ "The same prefixes were configured multiple times. The following configuration is used"
+ + " {}",
+ deduplicated.values());
+ }
+ return deduplicated.values();
+ }
+
+ private static Optional<Threshold> getSpec(String spec) {
+ int equals = spec.lastIndexOf('=');
+ if (equals != -1) {
+ Optional<Integer> maybeThreshold = isThresholdDefined(spec.substring(equals + 1));
+ if (maybeThreshold.isPresent()) {
+ return Optional.of(new Threshold(spec.substring(0, equals).trim(), maybeThreshold.get()));
+ }
+ } else {
+ Optional<Integer> maybeThreshold = isThresholdDefined(spec);
+ if (maybeThreshold.isPresent()) {
+ return Optional.of(new Threshold(maybeThreshold.get()));
+ }
+ }
+
+ log.warn("Invalid configuration of blocked threads threshold [{}]", spec);
+ return Optional.empty();
+ }
+
+ private static Optional<Integer> isThresholdDefined(String input) {
+ Matcher value = THRESHOLD_PATTERN.matcher(input.trim());
+ if (value.matches()) {
+ return Optional.of(Integer.valueOf(value.group(1)));
+ }
+ return Optional.empty();
+ }
+
+ @VisibleForTesting
+ static class Threshold {
+ final Optional<String> prefix;
+ final Integer value;
+
+ Threshold(int value) {
+ this(null, value);
+ }
+
+ Threshold(String prefix, int value) {
+ this.prefix = Optional.ofNullable(prefix);
+ this.value = value;
+ }
+
+ boolean hasPrefix() {
+ return prefix.isPresent();
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(prefix, value);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || getClass() != obj.getClass()) {
+ return false;
+ }
+ Threshold other = (Threshold) obj;
+ return Objects.equals(prefix, other.prefix) && Objects.equals(value, other.value);
+ }
+
+ @Override
+ public String toString() {
+ return new StringBuilder()
+ .append("Threshold [prefix=")
+ .append(prefix)
+ .append(", value=")
+ .append(value)
+ .append("]")
+ .toString();
+ }
+ }
+}
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
new file mode 100644
index 0000000..d4f0fe8
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsSubCheck.java
@@ -0,0 +1,83 @@
+// 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.check;
+
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.BLOCKEDTHREADS;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.metrics.Counter0;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
+import java.lang.management.ThreadInfo;
+import java.util.function.Supplier;
+
+class BlockedThreadsSubCheck
+ implements BlockedThreadsCheck.CollectorProvider<BlockedThreadsSubCheck.SubCheckCollector> {
+ interface Factory {
+ BlockedThreadsSubCheck create(String prefix, Integer threshold);
+ }
+
+ static class SubCheckCollector extends BlockedThreadsCheck.Collector {
+ private final String prefix;
+ private final Counter0 failureCounterMetric;
+
+ SubCheckCollector(String prefix, Integer threshold, Counter0 failureCounterMetric) {
+ super(threshold);
+ this.prefix = prefix;
+ this.failureCounterMetric = failureCounterMetric;
+ }
+
+ @Override
+ void collect(ThreadInfo info) {
+ String threadName = info.getThreadName();
+ if (!Strings.isNullOrEmpty(threadName) && threadName.startsWith(prefix)) {
+ total += 1;
+ if (Thread.State.BLOCKED == info.getThreadState()) {
+ blocked += 1;
+ }
+ }
+ }
+
+ @Override
+ void check() {
+ super.check();
+ if (Result.FAILED == result) {
+ failureCounterMetric.increment();
+ }
+ }
+ }
+
+ private final Supplier<SubCheckCollector> collector;
+
+ @Inject
+ BlockedThreadsSubCheck(
+ HealthCheckMetrics.Factory healthCheckMetricsFactory,
+ @Assisted String prefix,
+ @Assisted Integer threshold) {
+ HealthCheckMetrics healthCheckMetrics =
+ healthCheckMetricsFactory.create(
+ String.format(
+ "%s-%s", BLOCKEDTHREADS, prefix.toLowerCase().replaceAll("[^\\w-/]", "_")));
+ Counter0 failureCounterMetric = healthCheckMetrics.getFailureCounterMetric();
+ this.collector = () -> new SubCheckCollector(prefix, threshold, failureCounterMetric);
+ }
+
+ @Override
+ public SubCheckCollector get() {
+ return collector.get();
+ }
+}
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 47ad976..85e721d 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
@@ -21,6 +21,7 @@
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;
@Singleton
@@ -35,8 +36,9 @@
public DeadlockCheck(
ListeningExecutorService executor,
HealthCheckConfig healthCheckConfig,
- MetricRegistry metricRegistry) {
- super(executor, healthCheckConfig, DEADLOCK);
+ MetricRegistry metricRegistry,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(executor, healthCheckConfig, DEADLOCK, healthCheckMetricsFactory);
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 76e8706..b004eaa 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
@@ -14,26 +14,69 @@
package com.googlesource.gerrit.plugins.healthcheck.check;
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.GLOBAL;
+
+import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.extensions.registration.DynamicSet;
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;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
-public class GlobalHealthCheck {
+@Singleton
+public class GlobalHealthCheck extends AbstractHealthCheck {
private final DynamicSet<HealthCheck> healthChecks;
@Inject
- public GlobalHealthCheck(DynamicSet<HealthCheck> healthChecks) {
+ public GlobalHealthCheck(
+ DynamicSet<HealthCheck> healthChecks,
+ ListeningExecutorService executor,
+ HealthCheckConfig healthCheckConfig,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(executor, healthCheckConfig, GLOBAL, healthCheckMetricsFactory);
this.healthChecks = healthChecks;
}
- public Map<String, Object> run() {
+ @Override
+ public HealthCheck.StatusSummary 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;
+ StatusSummary globalStatus =
+ new HealthCheck.StatusSummary(
+ hasAnyFailureOnResults(checkToResults) ? Result.FAILED : Result.PASSED,
+ ts,
+ elapsed,
+ checkToResults);
+ if (globalStatus.isFailure()) {
+ failureCounterMetric.increment();
+ }
+ latencyMetric.record(elapsed, TimeUnit.MILLISECONDS);
+ return globalStatus;
+ }
+
+ @Override
+ protected Result doCheck() {
+ return run().result;
+ }
+
+ public static boolean hasAnyFailureOnResults(Map<String, Object> results) {
+ return results.values().stream()
+ .filter(
+ res ->
+ res instanceof HealthCheck.StatusSummary
+ && ((HealthCheck.StatusSummary) res).isFailure())
+ .findAny()
+ .isPresent();
}
}
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 de571d1..7c4f8a1 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
@@ -16,7 +16,9 @@
import com.google.gson.annotations.SerializedName;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
public interface HealthCheck {
@@ -34,17 +36,20 @@
public class StatusSummary {
public static final StatusSummary INITIAL_STATUS =
- new StatusSummary(Result.PASSED, System.currentTimeMillis(), 0L);
+ new StatusSummary(Result.PASSED, System.currentTimeMillis(), 0L, Collections.emptyMap());
public final Result result;
public final long ts;
public final long elapsed;
+ public final transient Map<String, Object> subChecks;
+
public static final Set<Result> failingResults =
new HashSet<>(Arrays.asList(Result.FAILED, Result.TIMEOUT));
- public StatusSummary(Result result, long ts, long elapsed) {
+ public StatusSummary(Result result, long ts, long elapsed, Map<String, Object> subChecks) {
this.result = result;
this.ts = ts;
this.elapsed = elapsed;
+ this.subChecks = subChecks;
}
public Boolean isFailure() {
@@ -55,6 +60,4 @@
StatusSummary run();
String name();
-
- StatusSummary getLatestStatus();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java
index a2f6ddb..4604af0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheckNames.java
@@ -22,4 +22,6 @@
String ACTIVEWORKERS = "activeworkers";
String HTTPACTIVEWORKERS = "httpactiveworkers";
String DEADLOCK = "deadlock";
+ String BLOCKEDTHREADS = "blockedthreads";
+ String GLOBAL = "global";
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HttpActiveWorkersCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HttpActiveWorkersCheck.java
index 0690596..19c2815 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HttpActiveWorkersCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HttpActiveWorkersCheck.java
@@ -21,6 +21,7 @@
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;
public class HttpActiveWorkersCheck extends AbstractWorkersHealthCheck {
public static final String HTTP_WORKERS_METRIC_NAME =
@@ -31,13 +32,15 @@
ListeningExecutorService executor,
HealthCheckConfig healthCheckConfig,
ThreadSettingsConfig threadSettingsConfig,
- MetricRegistry metricRegistry) {
+ MetricRegistry metricRegistry,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
super(
executor,
healthCheckConfig,
metricRegistry,
HTTPACTIVEWORKERS,
HTTP_WORKERS_METRIC_NAME,
- threadSettingsConfig.getHttpdMaxThreads());
+ threadSettingsConfig.getHttpdMaxThreads(),
+ healthCheckMetricsFactory);
}
}
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 fe59bbb..9196cdb 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
@@ -17,11 +17,12 @@
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.JGIT;
import com.google.common.util.concurrent.ListeningExecutorService;
-import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.entities.Project;
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;
@@ -34,9 +35,9 @@
public JGitHealthCheck(
ListeningExecutorService executor,
HealthCheckConfig config,
- GitRepositoryManager repositoryManager) {
- super(executor, config, JGIT);
-
+ GitRepositoryManager repositoryManager,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(executor, config, JGIT, healthCheckMetricsFactory);
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 4da142d..78cff2a 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
@@ -19,9 +19,13 @@
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.server.restapi.project.ListProjects;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
+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;
@@ -30,31 +34,40 @@
public class ProjectsListHealthCheck extends AbstractHealthCheck {
private static final Logger log = LoggerFactory.getLogger(ProjectsListHealthCheck.class);
private static final int PROJECTS_LIST_LIMIT = 100;
- private final ListProjects listProjects;
+ private final Provider<ListProjects> listProjectsProvider;
+ private final OneOffRequestContext oneOffCtx;
@Inject
public ProjectsListHealthCheck(
- ListeningExecutorService executor, HealthCheckConfig config, ListProjects listProjects) {
- super(executor, config, PROJECTSLIST);
+ ListeningExecutorService executor,
+ HealthCheckConfig config,
+ Provider<ListProjects> listProjectsProvider,
+ OneOffRequestContext oneOffCtx,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(executor, config, PROJECTSLIST, healthCheckMetricsFactory);
- this.listProjects = listProjects;
+ this.listProjectsProvider = listProjectsProvider;
+ this.oneOffCtx = oneOffCtx;
}
@Override
protected Result doCheck() {
- listProjects.setStart(0);
- listProjects.setLimit(PROJECTS_LIST_LIMIT);
- listProjects.setShowDescription(true);
- listProjects.setMatchPrefix("All-");
- try {
- SortedMap<String, ProjectInfo> projects = listProjects.apply();
- if (projects != null && projects.size() > 0) {
- return Result.PASSED;
+ try (ManualRequestContext ctx = oneOffCtx.open()) {
+ ListProjects listProjects = listProjectsProvider.get();
+ listProjects.setStart(0);
+ listProjects.setLimit(PROJECTS_LIST_LIMIT);
+ listProjects.setShowDescription(true);
+ listProjects.setMatchPrefix("All-");
+ try {
+ SortedMap<String, ProjectInfo> projects = listProjects.apply();
+ if (projects != null && projects.size() > 0) {
+ return Result.PASSED;
+ }
+ log.warn("Empty or null projects list: Gerrit should always have at least 1 project");
+ } catch (Exception e) {
+ log.warn("Unable to list projects", e);
}
- log.warn("Empty or null projects list: Gerrit should always have at least 1 project");
- } catch (Exception e) {
- log.warn("Unable to list projects", e);
+ return Result.FAILED;
}
- return Result.FAILED;
}
}
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 331ef21..a786606 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
@@ -24,6 +24,7 @@
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;
@@ -32,7 +33,6 @@
public class QueryChangesHealthCheck extends AbstractHealthCheck {
private static final Logger log = LoggerFactory.getLogger(QueryChangesHealthCheck.class);
private final Provider<QueryChanges> queryChangesProvider;
- private final HealthCheckConfig config;
private final int limit;
private final OneOffRequestContext oneOffCtx;
@@ -41,8 +41,9 @@
ListeningExecutorService executor,
HealthCheckConfig config,
Provider<QueryChanges> queryChangesProvider,
- OneOffRequestContext oneOffCtx) {
- super(executor, config, QUERYCHANGES);
+ OneOffRequestContext oneOffCtx,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(executor, config, QUERYCHANGES, healthCheckMetricsFactory);
this.queryChangesProvider = queryChangesProvider;
this.config = config;
this.limit = config.getLimit(QUERYCHANGES);
@@ -59,7 +60,7 @@
queryChanges.addQuery(config.getQuery(QUERYCHANGES));
queryChanges.setStart(0);
- List<?> changes = queryChanges.apply(null);
+ List<?> changes = queryChanges.apply(null).value();
if (changes == null) {
log.warn("Cannot query changes: received a null list of results");
return Result.FAILED;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java
index b2e91b1..75e48fd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/filter/HealthCheckStatusFilter.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.healthcheck.filter;
+import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.httpd.AllRequestFilter;
import com.google.gerrit.httpd.restapi.RestApiServlet;
@@ -35,11 +36,14 @@
public class HealthCheckStatusFilter extends AllRequestFilter {
private final HealthCheckStatusEndpoint statusEndpoint;
private final Gson gson;
+ private final String pluginName;
@Inject
- public HealthCheckStatusFilter(HealthCheckStatusEndpoint statusEndpoint) {
+ public HealthCheckStatusFilter(
+ HealthCheckStatusEndpoint statusEndpoint, @PluginName String pluginName) {
this.statusEndpoint = statusEndpoint;
this.gson = OutputFormat.JSON.newGsonBuilder().create();
+ this.pluginName = pluginName;
}
@Override
@@ -61,7 +65,9 @@
}
private boolean isStatusCheck(HttpServletRequest httpServletRequest) {
- return httpServletRequest.getRequestURI().matches("(?:/a)?/config/server/healthcheck~status");
+ return httpServletRequest
+ .getRequestURI()
+ .matches("(?:/a)?/config/server/" + pluginName + "~status");
}
private void doStatusCheck(HttpServletResponse httpResponse) throws ServletException {
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 0eda18f..be418b8 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -13,7 +13,12 @@
)]}'
{
"ts": 139402910202,
- "elapsed": 100,
+ "elapsed": 120,
+ "blockedthreads": {
+ "ts": 139402910202,
+ "elapsed": 30,
+ "result": "passed"
+ },
"querychanges": {
"ts": 139402910202,
"elapsed": 20,
@@ -21,12 +26,12 @@
},
"projectslist": {
"ts": 139402910202,
- "elapsed": 100,
+ "elapsed": 40,
"result": "passed"
},
"auth": {
"ts": 139402910202,
- "elapsed": 80,
+ "elapsed": 30,
"result": "passed"
}
}
@@ -52,6 +57,7 @@
- `httpactiveworkers`: check the number of active HTTP worker threads and the ability
to create a new one
- `deadlock` : check if Java deadlocks are reported by the JVM
+- `blockedthreads` : check the number of blocked threads
Each check name can be disabled by setting the `enabled` parameter to **false**,
by default this parameter is set to **true**
@@ -87,12 +93,49 @@
Default: no password
- - `healthcheck.activeworkers.threshold` : Percent of queue occupancy above which queue is consider
- as full.
+ - `healthcheck.activeworkers.threshold` : Percent of queue occupancy above which queue is
+ considered as full.
Default: 80
- - `healthcheck.httpactiveworkers.threshold` : Percent of queue occupancy above which queue is
- considered as full.
+ - `healthcheck.blockedthreads.threshold` : Percent of all threads that are blocked above which instance
+ is considered as unhealthy.
- Default: 80
\ No newline at end of file
+ Default: 50
+
+By default `healthcheck.blockedthreads` is calculated as ratio of BLOCKED threads against the all
+Gerrit threads. It might be not sufficient for instance in case of `SSH-Interactive-Worker` threads
+that could be all blocked making effectively a Gerrit instance unhealthy (neither fetch nor push
+would succeed) but the threshold could be still not reached. Therefore one can fine tune the check
+by putting detailed configuration for one or more thread groups (all threads that have the name
+starting with a given prefix) to be checked according to the following template:
+
+```
+[healthcheck "blockedthreads"]
+ threshold = [prefix]=[XX]
+```
+
+Note that in case when specific thread groups are configured all threads are no longer checked.
+
+* **Example 1:** _check if BLOCKED threads are above the limit of 70_
+
+ ```
+ [healthcheck "blockedthreads"]
+ threshold = 70
+ ```
+
+* **Example 2:** _check if BLOCKED `foo` threads are above the 33 limit_
+
+ ```
+ [healthcheck "blockedthreads"]
+ threshold = foo=45
+ ```
+
+* **Example 3:** _check if BLOCKED `foo` threads are above the 33 limit and if BLOCKED `bar`_
+ _threads are above the the 60 limit_
+
+ ```
+ [healthcheck "blockedthreads"]
+ threshold = foo=33
+ threshold = bar=60
+ ```
diff --git a/src/resources/Documentation/config.md b/src/resources/Documentation/config.md
index aa59c29..76e6eb5 100644
--- a/src/resources/Documentation/config.md
+++ b/src/resources/Documentation/config.md
@@ -48,6 +48,9 @@
# TYPE plugins_healthcheck_projectslist_latest_measured_latency gauge
plugins_healthcheck_projectslist_latest_measured_latency 5.0
+# HELP plugins_healthcheck_blockedthreads_latest_measured_latency Generated from Dropwizard metric import (metric=plugins/healthcheck/blockedthreads/latency, type=com.google.gerrit.metrics.dropwizard.CallbackMetricImpl0$1)
+# TYPE plugins_healthcheck_blockedthreads_latest_measured_latency gauge
+plugins_healthcheck_blockedthreads_latest_measured_latency 6.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
@@ -56,6 +59,29 @@
# 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
plugins_healthcheck_projectslist_failure_total 0.0
+
+# HELP plugins_healthcheck_blockedthreads_failure_total Generated from Dropwizard metric import (metric=plugins/healthcheck/blockedthreads/failure, type=com.codahale.metrics.Meter)
+# TYPE plugins_healthcheck_blockedthreads_failure_total counter
+plugins_healthcheck_blockedthreads_failure_total 1.0
```
+Note that additionally to the default `blockedthreads` metrics pair failures counter will reported for
+each configured prefix. For given config:
+
+```
+[healthcheck "blockedthreads"]
+ threshold = Foo=33
+```
+
+the following additional metric will be exposed and populated:
+
+```
+# HELP plugins_healthcheck_blockedthreads_foo_failure_total Generated from Dropwizard metric import (metric=plugins/healthcheck/blockedthreads-foo/failure, type=com.codahale.metrics.Meter)
+# TYPE plugins_healthcheck_blockedthreads_foo_failure_total counter
+plugins_healthcheck_blockedthreads_foo_failure_total 2.0
+```
+
+Note that prefix is used as postfix for a metric name but it is lower-cased and sanitized as only
+`a-zA-Z0-9_-/` chars are allowed to be a metric name (chars outside this set are turned to `_`).
+
Metrics will be exposed to prometheus by the [metrics-reporter-prometheus](https://gerrit.googlesource.com/plugins/metrics-reporter-prometheus/) plugin.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java
new file mode 100644
index 0000000..fada3bb
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java
@@ -0,0 +1,188 @@
+// 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.common.util.concurrent.MoreExecutors;
+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.gerrit.metrics.Timer0;
+import com.googlesource.gerrit.plugins.healthcheck.check.AbstractHealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import org.junit.Before;
+import org.junit.Test;
+
+public class AbstractHealthCheckTest {
+
+ MetricMaker testMetricMaker;
+ DummyHealthCheckMetricsFactory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory();
+
+ @Before
+ public void setUp() throws Exception {
+ testMetricMaker = new TestMetricMaker();
+ }
+
+ @Test
+ public void shouldPublishAllMetricsWhenFailing() {
+ TestCheck testCheck = createFailingTestCheck();
+
+ testCheck.run();
+
+ TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker();
+ assertThat(testMetricMaker.getFailureCount()).isEqualTo(TestMetricMaker.expectedFailureCount);
+ assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency);
+ }
+
+ @Test
+ public void shouldPublishAllMetricsWhenTimingOut() {
+ TestCheck testCheck = createTimingOutTestCheck();
+
+ testCheck.run();
+
+ TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker();
+ assertThat(testMetricMaker.getFailureCount()).isEqualTo(TestMetricMaker.expectedFailureCount);
+ assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency);
+ }
+
+ @Test
+ public void shouldPublishOnlyLatencyMetricsWhenPassing() {
+ TestCheck testCheck = createPassingTestCheck();
+
+ testCheck.run();
+
+ TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker();
+ assertThat(testMetricMaker.getFailureCount()).isEqualTo(0L);
+ assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency);
+ }
+
+ @Test
+ public void shouldPublishOnlyLatencyMetricsWhenDisabled() {
+ TestCheck testCheck = createDisabledTestCheck();
+
+ testCheck.run();
+
+ TestMetricMaker testMetricMaker = healthCheckMetricsFactory.getMetricMaker();
+ assertThat(testMetricMaker.getFailureCount()).isEqualTo(0L);
+ assertThat(testMetricMaker.getLatency()).isEqualTo(TestMetricMaker.expectedLatency);
+ }
+
+ public TestCheck createPassingTestCheck() {
+ return createTestCheckWithStatus(HealthCheck.Result.PASSED);
+ }
+
+ public TestCheck createFailingTestCheck() {
+ return createTestCheckWithStatus(HealthCheck.Result.FAILED);
+ }
+
+ public TestCheck createDisabledTestCheck() {
+ return createTestCheckWithStatus(HealthCheck.Result.DISABLED);
+ }
+
+ public TestCheck createTimingOutTestCheck() {
+ return createTestCheckWithStatus(HealthCheck.Result.TIMEOUT);
+ }
+
+ private TestCheck createTestCheckWithStatus(HealthCheck.Result result) {
+ return new TestCheck(
+ HealthCheckConfig.DEFAULT_CONFIG, "testCheck", healthCheckMetricsFactory, result);
+ }
+
+ private static class TestCheck extends AbstractHealthCheck {
+ private final Result finalResult;
+
+ public TestCheck(
+ HealthCheckConfig config,
+ String name,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory,
+ Result finalResult) {
+ super(
+ MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)),
+ config,
+ name,
+ healthCheckMetricsFactory);
+ this.finalResult = finalResult;
+ }
+
+ @Override
+ protected Result doCheck() throws Exception {
+ return finalResult;
+ }
+ }
+
+ private static class TestMetricMaker extends DisabledMetricMaker {
+ private Long failureCount = 0L;
+ private Long latency = 0L;
+ public static Long expectedFailureCount = 1L;
+ public static Long expectedLatency = 100L;
+
+ @Override
+ public Counter0 newCounter(String name, Description desc) {
+ return new Counter0() {
+
+ @Override
+ public void incrementBy(long value) {
+ failureCount += value;
+ }
+
+ @Override
+ public void increment() {
+ failureCount = expectedFailureCount;
+ }
+
+ @Override
+ public void remove() {}
+ };
+ }
+
+ @Override
+ public Timer0 newTimer(String name, Description desc) {
+ return new Timer0(name) {
+ @Override
+ protected void doRecord(long value, TimeUnit unit) {
+ latency = expectedLatency;
+ }
+
+ @Override
+ public void remove() {}
+ };
+ }
+
+ public Long getLatency() {
+ return latency;
+ }
+
+ public Long getFailureCount() {
+ 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 df5a754..2d2ebd4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ActiveWorkersCheckTest.java
@@ -32,15 +32,7 @@
import org.junit.Test;
public class ActiveWorkersCheckTest {
-
- @Test
- public void shouldPassCheckWhenNoMetric() {
-
- Injector injector = testInjector(new TestModule(new Config(), new MetricRegistry()));
-
- ActiveWorkersCheck check = createCheck(injector);
- assertThat(check.run().result).isEqualTo(Result.PASSED);
- }
+ HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory();
@Test
public void shouldPassCheckWhenNoActiveWorkers() {
@@ -160,7 +152,8 @@
injector.getInstance(ListeningExecutorService.class),
healtchCheckConfig,
injector.getInstance(ThreadSettingsConfig.class),
- injector.getInstance(MetricRegistry.class));
+ injector.getInstance(MetricRegistry.class),
+ healthCheckMetricsFactory);
}
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
new file mode 100644
index 0000000..c52d646
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/BlockedThreadsCheckTest.java
@@ -0,0 +1,228 @@
+// 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 static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.BLOCKEDTHREADS;
+import static java.util.Collections.nCopies;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsCheck.ThreadBeanProvider;
+import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class BlockedThreadsCheckTest {
+ private static final String HEALTHCHECK_CONFIG_BODY_THRESHOLD =
+ "[healthcheck \"" + BLOCKEDTHREADS + "\"]\n" + " threshold = ";
+ private static final HealthCheckConfig CONFIG_THRESHOLD_25 =
+ new HealthCheckConfig(HEALTHCHECK_CONFIG_BODY_THRESHOLD + "25");
+ private static final HealthCheckConfig CONFIG_THRESHOLD_33 =
+ new HealthCheckConfig(HEALTHCHECK_CONFIG_BODY_THRESHOLD + "33");
+
+ @Mock BlockedThreadsCheck.ThreadBeanProvider threadBeanProviderMock;
+
+ @Mock ThreadMXBean beanMock;
+
+ private Injector testInjector;
+
+ @Before
+ public void setUp() {
+ when(threadBeanProviderMock.get()).thenReturn(beanMock);
+ testInjector = createTestInjector(HealthCheckConfig.DEFAULT_CONFIG);
+ }
+
+ @Test
+ public void shouldPassCheckWhenNoThreadsAreReturned() {
+ BlockedThreadsCheck objectUnderTest = createCheck();
+ when(beanMock.getThreadInfo(null, 0)).thenReturn(new ThreadInfo[0]);
+ assertThat(objectUnderTest.run().result).isEqualTo(Result.PASSED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenNoThreadsAreBlocked() {
+ int running = 1;
+ int blocked = 0;
+ mockThreadsAndCheckResult(running, blocked, Result.PASSED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenBlockedThreadsAreLessThanDefaultThreshold() {
+ int running = 2;
+ int blocked = 1;
+ mockThreadsAndCheckResult(running, blocked, Result.PASSED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenBlockedThreadsAreEqualToDefaultThreshold() {
+ int running = 1;
+ int blocked = 1;
+ mockThreadsAndCheckResult(running, blocked, Result.PASSED);
+ }
+
+ @Test
+ public void shouldFailCheckWhenBlockedThreadsAreAboveTheDefaultThreshold() {
+ int running = 1;
+ int blocked = 2;
+ mockThreadsAndCheckResult(running, blocked, Result.FAILED);
+ }
+
+ @Test
+ public void shouldFailThenPassCheck() {
+ String prefix = "some-prefix";
+
+ List<ThreadInfo> failingThreadInfo = new ArrayList<>();
+ failingThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.RUNNABLE, prefix)));
+ failingThreadInfo.addAll(nCopies(3, mockInfo(Thread.State.BLOCKED, prefix)));
+
+ List<ThreadInfo> successThreadInfo = new ArrayList<>();
+ successThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.RUNNABLE, prefix)));
+ successThreadInfo.addAll(nCopies(1, mockInfo(Thread.State.BLOCKED, prefix)));
+
+ when(beanMock.getThreadInfo(null, 0))
+ .thenReturn(
+ failingThreadInfo.toArray(new ThreadInfo[0]),
+ successThreadInfo.toArray(new ThreadInfo[0]));
+
+ assertThat(createCheck().run().result).isEqualTo(Result.FAILED);
+ assertThat(createCheck().run().result).isEqualTo(Result.PASSED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenBlockedThreadsAreLessThenThreshold() {
+ int running = 3;
+ int blocked = 1;
+ testInjector = createTestInjector(CONFIG_THRESHOLD_25);
+
+ mockThreadsAndCheckResult(running, blocked, Result.PASSED);
+ }
+
+ @Test
+ public void shouldFailCheckWhenBlockedThreadsAreAboveTheThreshold() {
+ int running = 1;
+ int blocked = 1;
+ testInjector = createTestInjector(CONFIG_THRESHOLD_33);
+
+ mockThreadsAndCheckResult(running, blocked, Result.FAILED);
+ }
+
+ @Test
+ public void shouldPassCheckWhenBlockedThreadsWithPrefixAreLessThenThreshold() {
+ int running = 3;
+ int blocked = 1;
+ String prefix = "blocked-threads-prefix";
+ testInjector = createTestInjector(CONFIG_THRESHOLD_25);
+
+ mockThreadsAndCheckResult(running, blocked, Result.PASSED, prefix);
+ }
+
+ @Test
+ public void shouldFailCheckWhenBlockedThreadsWithPrefixAreAboveTheThreshold() {
+ int running = 1;
+ int blocked = 1;
+ String prefix = "blocked-threads-prefix";
+ testInjector = createTestInjector(CONFIG_THRESHOLD_33);
+
+ mockThreadsAndCheckResult(running, blocked, Result.FAILED, prefix);
+ }
+
+ @Test
+ public void shouldFailCheckWhenAnyOfTheBlockedThreadsWithPrefixAreAboveTheThreshold() {
+ int running = 1;
+ int blocked = 1;
+ String blockedPrefix = "blocked-threads-prefix";
+ String notBlockedPrefix = "running-threads";
+ HealthCheckConfig config =
+ new HealthCheckConfig(
+ "[healthcheck \""
+ + BLOCKEDTHREADS
+ + "\"]\n"
+ + " threshold = "
+ + blockedPrefix
+ + " = 33"
+ + "\nthreshold = "
+ + notBlockedPrefix
+ + "=33");
+ testInjector = createTestInjector(config);
+
+ List<ThreadInfo> infos = new ArrayList<>(running + blocked + running);
+ infos.addAll(nCopies(running, mockInfo(Thread.State.RUNNABLE, blockedPrefix)));
+ infos.addAll(nCopies(blocked, mockInfo(Thread.State.BLOCKED, blockedPrefix)));
+ infos.addAll(nCopies(running, mockInfo(Thread.State.RUNNABLE, notBlockedPrefix)));
+ when(beanMock.getThreadInfo(null, 0)).thenReturn(infos.toArray(new ThreadInfo[infos.size()]));
+ checkResult(Result.FAILED);
+ }
+
+ private void mockThreadsAndCheckResult(int running, int blocked, Result expected) {
+ mockThreadsAndCheckResult(running, blocked, expected, "some-prefix");
+ }
+
+ private void mockThreadsAndCheckResult(int running, int blocked, Result expected, String prefix) {
+ mockThreads(running, blocked, prefix);
+ checkResult(expected);
+ }
+
+ private void checkResult(Result expected) {
+ BlockedThreadsCheck objectUnderTest = createCheck();
+ assertThat(objectUnderTest.run().result).isEqualTo(expected);
+ }
+
+ private void mockThreads(int running, int blocked, String prefix) {
+ List<ThreadInfo> infos = new ArrayList<>(running + blocked);
+ infos.addAll(nCopies(running, mockInfo(Thread.State.RUNNABLE, prefix)));
+ infos.addAll(nCopies(blocked, mockInfo(Thread.State.BLOCKED, prefix)));
+ when(beanMock.getThreadInfo(null, 0)).thenReturn(infos.toArray(new ThreadInfo[infos.size()]));
+ }
+
+ private ThreadInfo mockInfo(Thread.State state, String prefix) {
+ ThreadInfo infoMock = mock(ThreadInfo.class);
+ when(infoMock.getThreadState()).thenReturn(state);
+ when(infoMock.getThreadName()).thenReturn(prefix);
+ return infoMock;
+ }
+
+ private BlockedThreadsCheck createCheck() {
+ return testInjector.getInstance(BlockedThreadsCheck.class);
+ }
+
+ private Injector createTestInjector(HealthCheckConfig config) {
+ Injector injector =
+ Guice.createInjector(
+ new HealthCheckModule(),
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(HealthCheckConfig.class).toInstance(config);
+ bind(HealthCheckMetrics.Factory.class).to(DummyHealthCheckMetricsFactory.class);
+ bind(ThreadBeanProvider.class).toInstance(threadBeanProviderMock);
+ }
+ },
+ BlockedThreadsCheck.SUB_CHECKS);
+ return injector;
+ }
+}
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 01a1147..aa880a3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig.DEFAULT_CONFIG;
-import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.DEADLOCK;
import com.codahale.metrics.Gauge;
import com.codahale.metrics.MetricRegistry;
@@ -33,6 +32,8 @@
public class DeadlockCheckTest {
+ HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory();
+
@Test
public void shouldPassCheckWhenNoMetric() {
@@ -44,9 +45,6 @@
@Test
public void shouldPassCheckWhenNoDeadlock() {
-
- MetricRegistry metricRegistry = createMetricRegistry(0);
-
Injector injector = testInjector(new TestModule(createMetricRegistry(0)));
DeadlockCheck check = createCheck(injector);
@@ -71,7 +69,6 @@
assertThat(check.run().result).isEqualTo(Result.FAILED);
}
-
private Injector testInjector(AbstractModule testModule) {
return Guice.createInjector(new HealthCheckModule(), testModule);
}
@@ -93,7 +90,8 @@
return new DeadlockCheck(
injector.getInstance(ListeningExecutorService.class),
DEFAULT_CONFIG,
- injector.getInstance(MetricRegistry.class));
+ injector.getInstance(MetricRegistry.class),
+ healthCheckMetricsFactory);
}
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
new file mode 100644
index 0000000..7523e95
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DummyHealthCheckMetricsFactory.java
@@ -0,0 +1,28 @@
+// 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 ef5dc8e..ae3f51d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -26,8 +26,10 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gson.Gson;
import com.google.gson.JsonObject;
+import com.google.inject.Key;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -35,13 +37,14 @@
import org.junit.Test;
@TestPlugin(
- name = "healthcheck",
+ name = "healthcheck-test",
sysModule = "com.googlesource.gerrit.plugins.healthcheck.Module",
httpModule = "com.googlesource.gerrit.plugins.healthcheck.HttpModule")
@Sandboxed
public class HealthCheckIT extends LightweightPluginDaemonTest {
Gson gson = new Gson();
HealthCheckConfig config;
+ String healthCheckUriPath;
@Override
@Before
@@ -54,6 +57,11 @@
createChange("refs/for/master");
}
accountCreator.create(config.getUsername(HealthCheckNames.AUTH));
+
+ healthCheckUriPath =
+ String.format(
+ "/config/server/%s~status",
+ plugin.getSysInjector().getInstance(Key.get(String.class, PluginName.class)));
}
@Test
@@ -62,6 +70,12 @@
}
@Test
+ public void shouldReturnOkWhenHealthyAndAnonymousReadIsBlocked() throws Exception {
+ blockAnonymousRead();
+ getHealthCheckStatus().assertOK();
+ }
+
+ @Test
public void shouldReturnAJsonPayload() throws Exception {
assertThat(getHealthCheckStatus().getHeader(CONTENT_TYPE)).contains("application/json");
}
@@ -84,7 +98,7 @@
}
@Test
- @GerritConfig(name = "container.slave", value = "true")
+ @GerritConfig(name = "container.replica", value = "true")
public void shouldReturnJGitCheckForReplicaWhenAuthenticated() throws Exception {
RestResponse resp = getHealthCheckStatus();
resp.assertOK();
@@ -92,7 +106,7 @@
}
@Test
- @GerritConfig(name = "container.slave", value = "true")
+ @GerritConfig(name = "container.replica", value = "true")
public void shouldReturnJGitCheckForReplicaAnonymously() throws Exception {
RestResponse resp = getHealthCheckStatusAnonymously();
resp.assertOK();
@@ -135,8 +149,8 @@
}
@Test
- @GerritConfig(name = "container.slave", value = "true")
- public void shouldReturnQueryChangesAsDisabledForSlave() throws Exception {
+ @GerritConfig(name = "container.replica", value = "true")
+ public void shouldReturnQueryChangesAsDisabledForReplica() throws Exception {
RestResponse resp = getHealthCheckStatus();
resp.assertOK();
@@ -182,11 +196,11 @@
}
private RestResponse getHealthCheckStatus() throws IOException {
- return adminRestSession.get("/config/server/healthcheck~status");
+ return adminRestSession.get(healthCheckUriPath);
}
private RestResponse getHealthCheckStatusAnonymously() throws IOException {
- return anonymousRestSession.get("/config/server/healthcheck~status");
+ return anonymousRestSession.get(healthCheckUriPath);
}
private void assertCheckResult(JsonObject respPayload, String checkName, String result) {
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..b186658 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckMetricsTest.java
@@ -16,112 +16,58 @@
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 com.google.gerrit.metrics.Timer0;
+import java.util.concurrent.TimeUnit;
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 shouldReturnACounterWithCorrectValues() {
+ String testMetricName = "testMetric";
+ TestMetrics testMetrics = new TestMetrics();
+ HealthCheckMetrics healthCheckMetrics = new HealthCheckMetrics(testMetrics, testMetricName);
+ healthCheckMetrics.getFailureCounterMetric();
+ assertThat(testMetrics.metricName).isEqualTo(testMetricName + "/failure");
+ assertThat(testMetrics.metricDescription.toString())
+ .isEqualTo(
+ new Description(String.format("%s healthcheck failures count", testMetricName))
+ .setCumulative()
+ .setRate()
+ .setUnit("failures")
+ .toString());
}
@Test
- public void shouldSendCounterWhenStatusSummaryFailed() {
- Long elapsed = 100L;
- setWithStatusSummary(new StatusSummary(Result.FAILED, 1L, elapsed));
-
- assertThat(testMetrics.getFailures()).isEqualTo(1);
- assertThat(testMetrics.getLatency()).isEqualTo(elapsed);
+ public void shouldReturnATimerWithCorrectValues() {
+ String testMetricName = "testMetric";
+ TestMetrics testMetrics = new TestMetrics();
+ HealthCheckMetrics healthCheckMetrics = new HealthCheckMetrics(testMetrics, testMetricName);
+ healthCheckMetrics.getLatencyMetric();
+ assertThat(testMetrics.metricName).isEqualTo(testMetricName + "/latest_latency");
+ assertThat(testMetrics.metricDescription.toString())
+ .isEqualTo(
+ new Description(String.format("%s health check latency execution (ms)", testMetricName))
+ .setCumulative()
+ .setUnit(Description.Units.MILLISECONDS)
+ .toString());
}
- @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;
- }
+ String metricName;
+ Description metricDescription;
@Override
public Counter0 newCounter(String name, Description desc) {
+ metricName = name;
+ metricDescription = desc;
return new Counter0() {
+
@Override
- public void incrementBy(long value) {
- failures += value;
- }
+ public void incrementBy(long value) {}
@Override
public void remove() {}
@@ -129,36 +75,16 @@
}
@Override
- public <V> CallbackMetric0<V> newCallbackMetric(
- String name, Class<V> valueClass, Description desc) {
- return new CallbackMetric0<V>() {
+ public Timer0 newTimer(String name, Description desc) {
+ metricName = name;
+ metricDescription = desc;
+ return new Timer0(name) {
@Override
- public void set(V value) {
- latency = (Long) value;
- }
+ protected void doRecord(long value, TimeUnit unit) {}
@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 78b89e4..b971054 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
@@ -17,16 +17,17 @@
import static com.google.common.truth.Truth.assertThat;
import static com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig.DEFAULT_CONFIG;
+import com.google.common.util.concurrent.ListeningExecutorService;
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;
import com.google.inject.Injector;
import com.googlesource.gerrit.plugins.healthcheck.api.HealthCheckStatusEndpoint;
import com.googlesource.gerrit.plugins.healthcheck.check.AbstractHealthCheck;
+import com.googlesource.gerrit.plugins.healthcheck.check.GlobalHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import java.util.concurrent.Executors;
import javax.servlet.http.HttpServletResponse;
@@ -34,13 +35,24 @@
public class HealthCheckStatusEndpointTest {
+ @Inject private ListeningExecutorService executor;
+ HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory();
+
public static class TestHealthCheck extends AbstractHealthCheck {
private final HealthCheck.Result checkResult;
private final long sleep;
public TestHealthCheck(
- HealthCheckConfig config, String checkName, HealthCheck.Result result, long sleep) {
- super(MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)), config, checkName);
+ HealthCheckConfig config,
+ String checkName,
+ HealthCheck.Result result,
+ long sleep,
+ HealthCheckMetrics.Factory healthCheckMetricsFactory) {
+ super(
+ MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)),
+ config,
+ checkName,
+ healthCheckMetricsFactory);
this.checkResult = result;
this.sleep = sleep;
}
@@ -53,11 +65,6 @@
}
return checkResult;
}
-
- @Override
- public StatusSummary getLatestStatus() {
- return this.latestStatus;
- }
}
@Test
@@ -67,18 +74,28 @@
new AbstractModule() {
@Override
protected void configure() {
- DynamicSet.bind(binder(), HealthCheck.class)
- .toInstance(
- new TestHealthCheck(
- DEFAULT_CONFIG, "checkOk", HealthCheck.Result.PASSED, 0));
- bind(HealthCheckConfig.class).toInstance(DEFAULT_CONFIG);
- DynamicSet.bind(binder(), MetricMaker.class).toInstance(new DisabledMetricMaker());
+ HealthCheckConfig config =
+ new HealthCheckConfig("[healthcheck]\n" + "timeout = 20");
+ TestHealthCheck passingHealthCheck =
+ new TestHealthCheck(
+ DEFAULT_CONFIG,
+ "checkOk",
+ HealthCheck.Result.PASSED,
+ 0,
+ healthCheckMetricsFactory);
+ DynamicSet<HealthCheck> healthChecks = new DynamicSet<>();
+ healthChecks.add("passingHealthCheck", passingHealthCheck);
+ GlobalHealthCheck globalHealthCheck =
+ new GlobalHealthCheck(
+ healthChecks, executor, config, healthCheckMetricsFactory);
+ bind(GlobalHealthCheck.class).toInstance(globalHealthCheck);
+ bind(HealthCheckConfig.class).toInstance(config);
}
});
HealthCheckStatusEndpoint healthCheckApi =
injector.getInstance(HealthCheckStatusEndpoint.class);
- Response<?> resp = (Response<?>) healthCheckApi.apply(null);
+ Response<?> resp = healthCheckApi.apply(null);
assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
}
@@ -92,16 +109,26 @@
protected void configure() {
HealthCheckConfig config =
new HealthCheckConfig("[healthcheck]\n" + "timeout = 20");
- DynamicSet.bind(binder(), HealthCheck.class)
- .toInstance(
- new TestHealthCheck(config, "checkOk", HealthCheck.Result.PASSED, 30));
+ TestHealthCheck testHealthCheck =
+ new TestHealthCheck(
+ config,
+ "checkOk",
+ HealthCheck.Result.PASSED,
+ 30,
+ healthCheckMetricsFactory);
+ DynamicSet<HealthCheck> healthChecks = new DynamicSet<>();
+ healthChecks.add("testHealthCheck", testHealthCheck);
+ GlobalHealthCheck globalHealthCheck =
+ new GlobalHealthCheck(
+ healthChecks, executor, config, healthCheckMetricsFactory);
+ bind(GlobalHealthCheck.class).toInstance(globalHealthCheck);
bind(HealthCheckConfig.class).toInstance(config);
}
});
HealthCheckStatusEndpoint healthCheckApi =
injector.getInstance(HealthCheckStatusEndpoint.class);
- Response<?> resp = (Response<?>) healthCheckApi.apply(null);
+ Response<?> resp = healthCheckApi.apply(null);
assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
@@ -113,20 +140,38 @@
new AbstractModule() {
@Override
protected void configure() {
- DynamicSet.bind(binder(), HealthCheck.class)
- .toInstance(
- new TestHealthCheck(
- DEFAULT_CONFIG, "checkOk", HealthCheck.Result.PASSED, 0));
- DynamicSet.bind(binder(), HealthCheck.class)
- .toInstance(
- new TestHealthCheck(
- DEFAULT_CONFIG, "checkKo", HealthCheck.Result.FAILED, 0));
+ HealthCheckConfig config =
+ new HealthCheckConfig("[healthcheck]\n" + "timeout = 20");
+ TestHealthCheck passingHealthCheck =
+ new TestHealthCheck(
+ DEFAULT_CONFIG,
+ "checkOk",
+ HealthCheck.Result.PASSED,
+ 0,
+ healthCheckMetricsFactory);
+
+ TestHealthCheck failingHealthCheck =
+ new TestHealthCheck(
+ DEFAULT_CONFIG,
+ "checkKo",
+ HealthCheck.Result.FAILED,
+ 0,
+ healthCheckMetricsFactory);
+
+ DynamicSet<HealthCheck> healthChecks = new DynamicSet<>();
+ healthChecks.add("passingHealthCheck", passingHealthCheck);
+ healthChecks.add("failingHealthCheck", failingHealthCheck);
+ GlobalHealthCheck globalHealthCheck =
+ new GlobalHealthCheck(
+ healthChecks, executor, config, healthCheckMetricsFactory);
+ bind(GlobalHealthCheck.class).toInstance(globalHealthCheck);
+ bind(HealthCheckConfig.class).toInstance(config);
}
});
HealthCheckStatusEndpoint healthCheckApi =
injector.getInstance(HealthCheckStatusEndpoint.class);
- Response<?> resp = (Response<?>) healthCheckApi.apply(null);
+ Response<?> resp = healthCheckApi.apply(null);
assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HttpActiveWorkersCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HttpActiveWorkersCheckTest.java
index b1557e2..515a8e6 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HttpActiveWorkersCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HttpActiveWorkersCheckTest.java
@@ -116,7 +116,8 @@
injector.getInstance(ListeningExecutorService.class),
healtchCheckConfig,
injector.getInstance(ThreadSettingsConfig.class),
- injector.getInstance(MetricRegistry.class));
+ injector.getInstance(MetricRegistry.class),
+ new DummyHealthCheckMetricsFactory());
}
private Injector testInjector(AbstractModule 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 59dfd71..3c0adae 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/JGitHealthCheckTest.java
@@ -20,7 +20,7 @@
import static org.eclipse.jgit.lib.RefUpdate.Result.NEW;
import com.google.common.util.concurrent.ListeningExecutorService;
-import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -52,6 +52,8 @@
@Inject private ListeningExecutorService executor;
+ HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory();
+
@Before
public void setupAllProjects() throws Exception {
Guice.createInjector(new HealthCheckModule()).injectMembers(this);
@@ -68,14 +70,16 @@
@Test
public void shouldBeHealthyWhenJGitIsWorking() {
JGitHealthCheck check =
- new JGitHealthCheck(executor, DEFAULT_CONFIG, getWorkingRepositoryManager());
+ new JGitHealthCheck(
+ executor, DEFAULT_CONFIG, getWorkingRepositoryManager(), healthCheckMetricsFactory);
assertThat(check.run().result).isEqualTo(Result.PASSED);
}
@Test
public void shouldBeUnhealthyWhenJGitIsFailingForAllRepos() {
JGitHealthCheck jGitHealthCheck =
- new JGitHealthCheck(executor, DEFAULT_CONFIG, getFailingGitRepositoryManager());
+ new JGitHealthCheck(
+ executor, DEFAULT_CONFIG, getFailingGitRepositoryManager(), healthCheckMetricsFactory);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED);
}
@@ -89,7 +93,8 @@
+ " project = All-Users\n"
+ " project = Not-Existing-Repo");
JGitHealthCheck jGitHealthCheck =
- new JGitHealthCheck(executor, config, getWorkingRepositoryManager());
+ new JGitHealthCheck(
+ executor, config, getWorkingRepositoryManager(), healthCheckMetricsFactory);
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 2ab0d83..58009ea 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
@@ -21,8 +21,11 @@
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.server.restapi.project.ListProjects;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Guice;
import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.util.Providers;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
import com.googlesource.gerrit.plugins.healthcheck.check.ProjectsListHealthCheck;
import java.util.SortedMap;
@@ -30,12 +33,20 @@
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+@RunWith(MockitoJUnitRunner.class)
public class ProjectsListHealthCheckTest {
@Inject private ListeningExecutorService executor;
+ HealthCheckMetrics.Factory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory();
+
private Config gerritConfig = new Config();
+ @Mock private OneOffRequestContext mockOneOffCtx;
+
@Before
public void setUp() throws Exception {
Guice.createInjector(new HealthCheckModule()).injectMembers(this);
@@ -44,14 +55,24 @@
@Test
public void shouldBeHealthyWhenListProjectsWorks() {
ProjectsListHealthCheck jGitHealthCheck =
- new ProjectsListHealthCheck(executor, DEFAULT_CONFIG, getWorkingProjectList(0));
+ new ProjectsListHealthCheck(
+ executor,
+ DEFAULT_CONFIG,
+ getWorkingProjectList(0),
+ mockOneOffCtx,
+ healthCheckMetricsFactory);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.PASSED);
}
@Test
public void shouldBeUnhealthyWhenListProjectsIsFailing() {
ProjectsListHealthCheck jGitHealthCheck =
- new ProjectsListHealthCheck(executor, DEFAULT_CONFIG, getFailingProjectList());
+ new ProjectsListHealthCheck(
+ executor,
+ DEFAULT_CONFIG,
+ getFailingProjectList(),
+ mockOneOffCtx,
+ healthCheckMetricsFactory);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED);
}
@@ -59,34 +80,40 @@
public void shouldBeUnhealthyWhenListProjectsIsTimingOut() {
ProjectsListHealthCheck jGitHealthCheck =
new ProjectsListHealthCheck(
- executor, DEFAULT_CONFIG, getWorkingProjectList(DEFAULT_CONFIG.getTimeout() * 2));
+ executor,
+ DEFAULT_CONFIG,
+ getWorkingProjectList(DEFAULT_CONFIG.getTimeout() * 2),
+ mockOneOffCtx,
+ healthCheckMetricsFactory);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.TIMEOUT);
}
- private ListProjects getFailingProjectList() {
- return new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
+ private Provider<ListProjects> getFailingProjectList() {
+ return Providers.of(
+ new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
- @Override
- public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
- throw new IllegalArgumentException("Unable to return project list");
- }
- };
+ @Override
+ public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
+ throw new IllegalArgumentException("Unable to return project list");
+ }
+ });
}
- private ListProjects getWorkingProjectList(long execTime) {
- return new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
+ private Provider<ListProjects> getWorkingProjectList(long execTime) {
+ return Providers.of(
+ new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
- @Override
- public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
- SortedMap<String, ProjectInfo> projects = new TreeMap<>();
- projects.put("testproject", new ProjectInfo());
- try {
- Thread.sleep(execTime);
- } catch (InterruptedException e) {
- throw new IllegalStateException(e);
- }
- return projects;
- }
- };
+ @Override
+ public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
+ SortedMap<String, ProjectInfo> projects = new TreeMap<>();
+ projects.put("testproject", new ProjectInfo());
+ try {
+ Thread.sleep(execTime);
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(e);
+ }
+ return projects;
+ }
+ });
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsConfiguratorConfigsTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsConfiguratorConfigsTest.java
new file mode 100644
index 0000000..d22ebbd
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/check/BlockedThreadsConfiguratorConfigsTest.java
@@ -0,0 +1,77 @@
+// 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.check;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.healthcheck.check.BlockedThreadsConfigurator.DEFAULT_BLOCKED_THREADS_THRESHOLD;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class BlockedThreadsConfiguratorConfigsTest {
+ private final String[] input;
+ private final Collection<BlockedThreadsConfigurator.Threshold> expected;
+
+ public BlockedThreadsConfiguratorConfigsTest(
+ String[] input, Collection<BlockedThreadsConfigurator.Threshold> expected) {
+ this.input = input;
+ this.expected = expected;
+ }
+
+ @Test
+ public void shouldReturnExpectedConfig() {
+ Collection<BlockedThreadsConfigurator.Threshold> result =
+ BlockedThreadsConfigurator.getConfig(input);
+ assertThat(result).containsExactlyElementsIn(expected);
+ }
+
+ @Parameterized.Parameters
+ public static Collection<Object[]> configs() {
+ return Arrays.asList(
+ new Object[][] {
+ {new String[] {}, specs(threshold(DEFAULT_BLOCKED_THREADS_THRESHOLD))},
+ {new String[] {"30"}, specs(threshold(30))},
+ {
+ new String[] {"prefix1=40", "prefix2=70", "prefix3 = 80"},
+ specs(threshold("prefix1", 40), threshold("prefix2", 70), threshold("prefix3", 80))
+ },
+ // the latter configuration is selected
+ {new String[] {"30", "40"}, specs(threshold(40))},
+ // the latter configuration is selected
+ {new String[] {"prefix1=40", "prefix1=70"}, specs(threshold("prefix1", 70))},
+ // specific prefix configuration is favored over the global one
+ {new String[] {"30", "prefix1=40"}, specs(threshold("prefix1", 40))},
+ // specific prefix configuration is favored over the global one and it is deduplicated
+ {new String[] {"30", "prefix1=40", "prefix1=70"}, specs(threshold("prefix1", 70))},
+ });
+ }
+
+ private static BlockedThreadsConfigurator.Threshold threshold(int value) {
+ return threshold(null, value);
+ }
+
+ private static BlockedThreadsConfigurator.Threshold threshold(String prefix, int value) {
+ return new BlockedThreadsConfigurator.Threshold(prefix, value);
+ }
+
+ private static Collection<BlockedThreadsConfigurator.Threshold> specs(
+ BlockedThreadsConfigurator.Threshold... thresholds) {
+ return Arrays.asList(thresholds);
+ }
+}