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

Improve context based healthchecks #508

Merged
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 @@ -24,7 +24,6 @@
import jakarta.enterprise.inject.Instance;
import jakarta.enterprise.inject.spi.Bean;
import jakarta.enterprise.inject.spi.BeanManager;
import jakarta.enterprise.inject.spi.CDI;
import jakarta.inject.Inject;
import jakarta.json.JsonArray;
import jakarta.json.JsonArrayBuilder;
Expand Down Expand Up @@ -409,14 +408,12 @@ private <T> List<T> getHealthChecksWithoutHealthGroup(Class<T> checkClass) {
}

private Uni<SmallRyeHealth> getHealthAsync(Uni<SmallRyeHealth> cachedHealth, HealthType... types) {
if (!checksInitialized) {
initChecks();
}

if (contextPropagated) {
recreateCheckUnis();
return computeHealth(types);
return computeHealthWithContext(types);
} else {
if (!checksInitialized) {
initChecks();
}
if (additionalListsChanged(types) || additionalListsChanged || cachedHealth == null) {
additionalListsChanged = false;
cachedHealth = computeHealth(types);
Expand All @@ -426,14 +423,29 @@ private Uni<SmallRyeHealth> getHealthAsync(Uni<SmallRyeHealth> cachedHealth, Hea
}
}

private void recreateCheckUnis() {
livenessUnis.clear();
readinessUnis.clear();
wellnessUnis.clear();
startupUnis.clear();
checksInitialized = false;

initChecks();
private Uni<SmallRyeHealth> computeHealthWithContext(HealthType... types) {
List<Uni<HealthCheckResponse>> checks = new ArrayList<>();
for (HealthType type : types) {
switch (type) {
case LIVENESS:
initUnis(checks, livenessChecks, asyncLivenessChecks);
checks.addAll(livenessHealthRegistry.getChecks(healthChecksConfigs));
break;
case READINESS:
initUnis(checks, readinessChecks, asyncReadinessChecks);
checks.addAll(readinessHealthRegistry.getChecks(healthChecksConfigs));
break;
case WELLNESS:
initUnis(checks, wellnessChecks, asyncWellnessChecks);
checks.addAll(wellnessHealthRegistry.getChecks(healthChecksConfigs));
break;
case STARTUP:
initUnis(checks, startupChecks, asyncStartupChecks);
checks.addAll(startupHealthRegistry.getChecks(healthChecksConfigs));
break;
}
}
return getHealthAsync(checks);
}

private boolean additionalListsChanged(HealthType... types) {
Expand Down Expand Up @@ -467,7 +479,6 @@ private boolean additionalListsChanged(HealthType... types) {

private Uni<SmallRyeHealth> computeHealth(HealthType[] types) {
List<Uni<HealthCheckResponse>> checks = new ArrayList<>();

for (HealthType type : types) {
switch (type) {
case LIVENESS:
Expand All @@ -488,7 +499,6 @@ private Uni<SmallRyeHealth> computeHealth(HealthType[] types) {
break;
}
}

return getHealthAsync(checks);
}

Expand All @@ -508,7 +518,7 @@ private Uni<SmallRyeHealth> getHealthAsync(Collection<Uni<HealthCheckResponse>>
}

return Uni.combine().all().unis(healthCheckUnis)
.combinedWith(responses -> {
.with(responses -> {
JsonArrayBuilder results = jsonProvider.createArrayBuilder();
HealthCheckResponse.Status status = HealthCheckResponse.Status.UP;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package io.smallrye.health.registry;

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.eclipse.microprofile.health.HealthCheck;
import org.eclipse.microprofile.health.HealthCheckResponse;
Expand All @@ -16,31 +16,32 @@

public class HealthRegistryImpl implements HealthRegistry {

Map<String, Uni<HealthCheckResponse>> checks = new HashMap<>();
Map<String, HealthCheck> checks = new HashMap<>();
Map<String, AsyncHealthCheck> asyncChecks = new HashMap<>();
private boolean checksChanged = false;

AsyncHealthCheckFactory asyncHealthCheckFactory = new AsyncHealthCheckFactory();

@Override
public HealthRegistry register(String id, HealthCheck healthCheck) {
return register(id, asyncHealthCheckFactory.callSync(healthCheck));
return register(id, healthCheck, checks);
}

@Override
public HealthRegistry register(String id, AsyncHealthCheck asyncHealthCheck) {
return register(id, asyncHealthCheckFactory.callAsync(asyncHealthCheck));
return register(id, asyncHealthCheck, asyncChecks);
}

private HealthRegistry register(String id, Uni<HealthCheckResponse> healthCheckResponseUni) {
checks.put(id, healthCheckResponseUni);
private <T> HealthRegistry register(String id, T check, Map<String, T> checks) {
checks.put(id, check);
checksChanged = true;
return this;
}

@Override
public HealthRegistry remove(String id) {
try {
if (checks.remove(id) == null) {
if (checks.remove(id) == null && asyncChecks.remove(id) == null) {
throw new IllegalStateException(String.format("ID '%s' not found", id));
}
checksChanged = true;
Expand All @@ -51,9 +52,13 @@ public HealthRegistry remove(String id) {
}

public Collection<Uni<HealthCheckResponse>> getChecks(Map<String, Boolean> healthChecksConfigs) {
return Collections.unmodifiableCollection(checks.entrySet().stream()
var enabledChecks = checks.entrySet().stream()
.filter(e -> healthChecksConfigs.getOrDefault(e.getKey(), true))
.map(Map.Entry::getValue).collect(Collectors.toList()));
.map(e -> asyncHealthCheckFactory.callSync(e.getValue()));
var enabledAsyncChecks = asyncChecks.entrySet().stream()
.filter(e -> healthChecksConfigs.getOrDefault(e.getKey(), true))
.map(e -> asyncHealthCheckFactory.callAsync(e.getValue()));
return Stream.concat(enabledChecks, enabledAsyncChecks).collect(Collectors.toList());
}

public boolean checksChanged() {
Expand Down
6 changes: 6 additions & 0 deletions testsuite/experimental/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@
<scope>compile</scope>
</dependency>

<dependency>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
</dependency>

</dependencies>

<profiles>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.smallrye.health.deployment;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

import org.eclipse.microprofile.health.HealthCheck;
import org.eclipse.microprofile.health.HealthCheckResponse;

import io.smallrye.health.api.Wellness;

@Wellness
@ApplicationScoped
public class ContextHealthCheck implements HealthCheck {

@Inject
ContextInfo context;

@Override
public HealthCheckResponse call() {
return HealthCheckResponse.up(context.getValue());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.smallrye.health.deployment;

import jakarta.enterprise.context.RequestScoped;

@RequestScoped
public class ContextInfo {

private String value;

public String getValue() {
return value;
}

public void setValue(String value) {
this.value = value;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package io.smallrye.health.deployment;

import java.io.IOException;

import jakarta.inject.Inject;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.annotation.WebFilter;
import jakarta.servlet.http.HttpFilter;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

@WebFilter(urlPatterns = "*", filterName = "ContextInfoFilter")
public class RequestFilter extends HttpFilter {

public static final String TEST_CONTEXT_HEADER = "X-CONTEXT";

@Inject
ContextInfo contextInfo;

@Override
protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
throws IOException, ServletException {
var value = req.getHeader(TEST_CONTEXT_HEADER);
contextInfo.setValue(value);
super.doFilter(req, res, chain);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package io.smallrye.health.test;

import java.util.List;
import java.util.Map;

import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.container.test.api.RunAsClient;
import org.jboss.shrinkwrap.api.Archive;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.WebArchive;
import org.testng.Assert;
import org.testng.annotations.Test;

import io.smallrye.health.deployment.ContextHealthCheck;
import io.smallrye.health.deployment.ContextInfo;
import io.smallrye.health.deployment.RequestFilter;
import io.smallrye.mutiny.Uni;
import io.smallrye.mutiny.infrastructure.Infrastructure;

@RunAsClient
class ContextPropagatedHealthTest extends TCKBase {

@Deployment
public static Archive<WebArchive> getDeployment() {
return DeploymentUtils
.createWarFileWithClasses(ContextPropagatedHealthTest.class.getSimpleName(),
ContextHealthCheck.class, ContextInfo.class, RequestFilter.class, TCKBase.class)
.addAsManifestResource(
new StringAsset("io.smallrye.health.context.propagation=true"),
"microprofile-config.properties");
}

// got to repeat this test a couple of times to trigger potential race condition issue at least once.
@Test(invocationCount = 20)
void twoParallelRequests_shouldNotMixupContext() {
String contextValue = "test-value";
String otherContextValue = "other-test-value";
Uni<Response> response = Uni.createFrom().voidItem().emitOn(Infrastructure.getDefaultExecutor()).onItem()
.transform(v -> getUrlHealthContents(Map.of(RequestFilter.TEST_CONTEXT_HEADER, contextValue)));
Uni<Response> otherResponse = Uni.createFrom().voidItem().emitOn(Infrastructure.getDefaultExecutor()).onItem()
.transform(v -> getUrlHealthContents(Map.of(RequestFilter.TEST_CONTEXT_HEADER, otherContextValue)));

List<Response> responses = Uni.join().all(response, otherResponse).andCollectFailures().await().indefinitely();

String responseBody = responses.get(0).getBody().orElseThrow();
String otherResponseBody = responses.get(1).getBody().orElseThrow();
Assert.assertNotEquals(responseBody, otherResponseBody);
Assert.assertTrue(responseBody.contains(contextValue), responseBody);
Assert.assertTrue(otherResponseBody.contains(otherContextValue), otherResponseBody);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.io.StringReader;
import java.lang.reflect.Method;
import java.net.URI;
import java.util.Map;
import java.util.Optional;
import java.util.logging.Logger;

Expand Down Expand Up @@ -65,6 +66,10 @@ public void beforeMethod(Method method) {
LOG.info(String.format("Running test: %s#%s", method.getDeclaringClass().getSimpleName(), method.getName()));
}

Response getUrlHealthContents(Map<String, String> headers) {
return getUrlContents(this.uri + "/health", false, true, headers);
}

Response getUrlHealthContents() {
return getUrlContents(this.uri + "/health", false);
}
Expand Down Expand Up @@ -98,6 +103,10 @@ private Response getUrlContents(String theUrl, boolean useAuth) {
}

private Response getUrlContents(String theUrl, boolean useAuth, boolean followRedirects) {
return getUrlContents(theUrl, useAuth, followRedirects, Map.of());
}

private Response getUrlContents(String theUrl, boolean useAuth, boolean followRedirects, Map<String, String> headers) {

StringBuilder content = new StringBuilder();
int code;
Expand All @@ -117,8 +126,9 @@ private Response getUrlContents(String theUrl, boolean useAuth, boolean followRe
}

HttpClient client = builder.build();

HttpResponse response = client.execute(new HttpGet(theUrl));
var httpGet = new HttpGet(theUrl);
headers.forEach(httpGet::addHeader);
HttpResponse response = client.execute(httpGet);
code = response.getStatusLine().getStatusCode();

if (response.getEntity() != null) {
Expand Down
Loading