Skip to content

Commit

Permalink
feat: simplify HealthCheckService (#4218)
Browse files Browse the repository at this point in the history 
* make component name mandatory

* add tests

* removed health check config
  • Loading branch information
@paullatzelsperger
paullatzelsperger authored May 28, 2024
1 parent 6eb2a5b commit 7aa6b80
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 362 deletions.
40 changes: 1 addition & 39 deletions core/common/boot/src/main/java/org/eclipse/edc/boot/BootServicesExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@
package org.eclipse.edc.boot;

import org.eclipse.edc.boot.apiversion.ApiVersionServiceImpl;
import org.eclipse.edc.boot.health.HealthCheckServiceConfiguration;
import org.eclipse.edc.boot.health.HealthCheckServiceImpl;
import org.eclipse.edc.boot.system.ExtensionLoader;
import org.eclipse.edc.boot.vault.InMemoryVault;
import org.eclipse.edc.runtime.metamodel.annotation.BaseExtension;
import org.eclipse.edc.runtime.metamodel.annotation.Extension;
import org.eclipse.edc.runtime.metamodel.annotation.Inject;
import org.eclipse.edc.runtime.metamodel.annotation.Provider;
import org.eclipse.edc.runtime.metamodel.annotation.Setting;
import org.eclipse.edc.spi.security.Vault;
Expand All @@ -33,7 +31,6 @@
import org.eclipse.edc.spi.telemetry.Telemetry;

import java.time.Clock;
import java.time.Duration;


@BaseExtension
Expand All @@ -42,26 +39,12 @@ public class BootServicesExtension implements ServiceExtension {

public static final String NAME = "Boot Services";

@Setting
public static final String LIVENESS_PERIOD_SECONDS_SETTING = "edc.core.system.health.check.liveness-period";
@Setting
public static final String STARTUP_PERIOD_SECONDS_SETTING = "edc.core.system.health.check.startup-period";
@Setting
public static final String READINESS_PERIOD_SECONDS_SETTING = "edc.core.system.health.check.readiness-period";
@Setting
public static final String THREADPOOL_SIZE_SETTING = "edc.core.system.health.check.threadpool-size";
@Setting(value = "Configures the participant id this runtime is operating on behalf of")
public static final String PARTICIPANT_ID = "edc.participant.id";

@Setting(value = "Configures the runtime id", defaultValue = "<random UUID>")
public static final String RUNTIME_ID = "edc.runtime.id";

private static final long DEFAULT_DURATION = 60;
private static final int DEFAULT_TP_SIZE = 3;

@Inject
private ExecutorInstrumentation instrumentation;

private HealthCheckServiceImpl healthCheckService;

@Override
Expand All @@ -71,20 +54,9 @@ public String name() {

@Override
public void initialize(ServiceExtensionContext context) {
var config = getHealthCheckConfig(context);
healthCheckService = new HealthCheckServiceImpl(config, instrumentation);
healthCheckService = new HealthCheckServiceImpl();
}

@Override
public void start() {
healthCheckService.start();
}

@Override
public void shutdown() {
healthCheckService.stop();
ServiceExtension.super.shutdown();
}

@Provider
public Clock clock() {
Expand Down Expand Up @@ -122,15 +94,5 @@ public ApiVersionService apiVersionService() {
return new ApiVersionServiceImpl();
}

private HealthCheckServiceConfiguration getHealthCheckConfig(ServiceExtensionContext context) {
return HealthCheckServiceConfiguration.Builder.newInstance()
.livenessPeriod(Duration.ofSeconds(context.getSetting(LIVENESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
.startupStatusPeriod(Duration.ofSeconds(context.getSetting(STARTUP_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
.readinessPeriod(Duration.ofSeconds(context.getSetting(READINESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
.readinessPeriod(Duration.ofSeconds(context.getSetting(READINESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
.threadPoolSize(context.getSetting(THREADPOOL_SIZE_SETTING, DEFAULT_TP_SIZE))
.build();
}


}
9 changes: 4 additions & 5 deletions core/common/boot/src/main/java/org/eclipse/edc/boot/system/runtime/BaseRuntime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ protected List<InjectionContainer<ServiceExtension>> createExtensions(ServiceExt
* this would likely need to be overridden.
*
* @param monitor a Monitor
* @param config the cofiguratiohn
* @param config the cofiguratiohn
* @return a {@code ServiceExtensionContext}
*/
@NotNull
Expand Down Expand Up @@ -184,13 +184,12 @@ private void boot(boolean addShutdownHook) {
}

if (context.hasService(HealthCheckService.class)) {
var startupStatus = new AtomicReference<>(HealthCheckResult.failed("Startup not complete"));
var statusbuilder = HealthCheckResult.Builder.newInstance().component("BaseRuntime");
var startupStatus = new AtomicReference<>(statusbuilder.failure("Startup not complete").build());
var healthCheckService = context.getService(HealthCheckService.class);
healthCheckService.addStartupStatusProvider(startupStatus::get);

startupStatus.set(HealthCheckResult.success());

healthCheckService.refresh();
startupStatus.set(statusbuilder.success().build());
}

} catch (Exception e) {
Expand Down
21 changes: 10 additions & 11 deletions core/common/boot/src/test/java/org/eclipse/edc/boot/system/runtime/BaseRuntimeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ public class BaseRuntimeTest {
private final ServiceLocator serviceLocator = mock();
private final BaseRuntime runtime = new BaseRuntimeFixture(monitor, serviceLocator);

@NotNull
private static ServiceExtension registerService(Class<HealthCheckService> serviceClass, HealthCheckService healthCheckService) {
return new ServiceExtension() {
@Override
public void initialize(ServiceExtensionContext context) {
context.registerService(serviceClass, healthCheckService);
}
};
}

@Test
void baseRuntime_shouldBoot() {
when(serviceLocator.loadImplementors(eq(ServiceExtension.class), anyBoolean())).thenReturn(List.of(new BaseExtension()));
Expand Down Expand Up @@ -76,7 +86,6 @@ void shouldSetStartupCheckProvider_whenHealthCheckServiceIsRegistered() {
runtime.boot();

verify(healthCheckService).addStartupStatusProvider(any());
verify(healthCheckService).refresh();
}

@Test
Expand All @@ -88,16 +97,6 @@ void shouldLoadConfiguration() {
verify(serviceLocator).loadImplementors(ConfigurationExtension.class, false);
}

@NotNull
private static ServiceExtension registerService(Class<HealthCheckService> serviceClass, HealthCheckService healthCheckService) {
return new ServiceExtension() {
@Override
public void initialize(ServiceExtensionContext context) {
context.registerService(serviceClass, healthCheckService);
}
};
}

private static class BaseRuntimeFixture extends BaseRuntime {

private final Monitor monitor;
Expand Down
97 changes: 0 additions & 97 deletions ...b/boot-lib/src/main/java/org/eclipse/edc/boot/health/HealthCheckServiceConfiguration.java
View file

This file was deleted.

79 changes: 14 additions & 65 deletions ...common/lib/boot-lib/src/main/java/org/eclipse/edc/boot/health/HealthCheckServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,49 +14,29 @@

package org.eclipse.edc.boot.health;

import org.eclipse.edc.spi.system.ExecutorInstrumentation;
import org.eclipse.edc.spi.system.health.HealthCheckResult;
import org.eclipse.edc.spi.system.health.HealthCheckService;
import org.eclipse.edc.spi.system.health.HealthStatus;
import org.eclipse.edc.spi.system.health.LivenessProvider;
import org.eclipse.edc.spi.system.health.ReadinessProvider;
import org.eclipse.edc.spi.system.health.StartupStatusProvider;
import org.jetbrains.annotations.NotNull;

import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.Supplier;

public class HealthCheckServiceImpl implements HealthCheckService {
private final List<LivenessProvider> livenessProviders;
private final List<ReadinessProvider> readinessProviders;
private final List<StartupStatusProvider> startupStatusProviders;

private final Map<LivenessProvider, HealthCheckResult> cachedLivenessResults;
private final Map<ReadinessProvider, HealthCheckResult> cachedReadinessResults;
private final Map<StartupStatusProvider, HealthCheckResult> cachedStartupStatus;

private final ScheduledExecutorService executor;
private final HealthCheckServiceConfiguration configuration;

public HealthCheckServiceImpl(HealthCheckServiceConfiguration configuration,
ExecutorInstrumentation executorInstrumentation) {
this.configuration = configuration;
public HealthCheckServiceImpl() {
readinessProviders = new CopyOnWriteArrayList<>();
livenessProviders = new CopyOnWriteArrayList<>();
startupStatusProviders = new CopyOnWriteArrayList<>();

cachedLivenessResults = new ConcurrentHashMap<>();
cachedReadinessResults = new ConcurrentHashMap<>();
cachedStartupStatus = new ConcurrentHashMap<>();

executor = executorInstrumentation.instrument(
Executors.newScheduledThreadPool(configuration.getThreadPoolSize()),
HealthCheckService.class.getSimpleName());
}

@Override
Expand All @@ -76,57 +56,26 @@ public void addStartupStatusProvider(StartupStatusProvider provider) {

@Override
public HealthStatus isLive() {
return new HealthStatus(cachedLivenessResults.values());
return new HealthStatus(livenessProviders.stream().map(getSilent()).toList());
}

@Override
public HealthStatus isReady() {
return new HealthStatus(cachedReadinessResults.values());
return new HealthStatus(readinessProviders.stream().map(getSilent()).toList());
}

@Override
public HealthStatus getStartupStatus() {
return new HealthStatus(cachedStartupStatus.values());
return new HealthStatus(startupStatusProviders.stream().map(getSilent()).toList());
}

@Override
public void refresh() {
executor.execute(this::queryReadiness);
executor.execute(this::queryLiveness);
executor.execute(this::queryStartupStatus);
private @NotNull Function<Supplier<HealthCheckResult>, HealthCheckResult> getSilent() {
return supplier -> {
try {
return supplier.get();
} catch (Exception e) {
return HealthCheckResult.Builder.newInstance().component(supplier.getClass().getName()).failure(e.getMessage()).build();
}
};
}

public void stop() {
if (!executor.isShutdown()) {
executor.shutdownNow();
}
}

public void start() {
//todo: maybe providers should provide their desired timeout instead of a global config?
executor.scheduleAtFixedRate(this::queryReadiness, 0, configuration.getReadinessPeriod().toMillis(), TimeUnit.MILLISECONDS);
executor.scheduleAtFixedRate(this::queryLiveness, 0, configuration.getLivenessPeriod().toMillis(), TimeUnit.MILLISECONDS);
executor.scheduleAtFixedRate(this::queryStartupStatus, 0, configuration.getStartupStatusPeriod().toMillis(), TimeUnit.MILLISECONDS);
}

private void queryReadiness() {
readinessProviders.parallelStream().forEach(provider -> updateCache(provider, cachedReadinessResults));
}

private void queryLiveness() {
livenessProviders.parallelStream().forEach(provider -> updateCache(provider, cachedLivenessResults));
}

private void queryStartupStatus() {
startupStatusProviders.parallelStream().forEach(provider -> updateCache(provider, cachedStartupStatus));
}

private <T extends Supplier<HealthCheckResult>> void updateCache(T provider, Map<T, HealthCheckResult> cache) {
try {
cache.put(provider, provider.get());
} catch (Exception ex) {
cache.put(provider, HealthCheckResult.failed(ex.getMessage()));
}
}

}
Loading

0 comments on commit 7aa6b80

Please sign in to comment.