Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: simplify HealthCheckService #4218

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}


}
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
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

This file was deleted.

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
Loading