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

Built-in MP Rest Client interface for checking health status of remote services #216

Open
andymc12 opened this issue Nov 11, 2019 · 7 comments

Comments

@andymc12
Copy link

Could the API include a "client" package that contains a built-in MP Rest Client interface capable of checking the liveness/readiness of a given endpoint?

Maybe something like:

// intended to match the JSON schema of the health check response
public class Status {
    public static class Check {
        private String name; // with getters/setters
        private String status; // with getters/setters - this could also be an enum { UP, DOWN }?
        private Map<String, String> data; // with getters/setters
        // ...
    }
    private String status; // with getters/setters - and maybe an isUp() method that returns "UP".equals(status) ?
    private List<Check> checks;
}

and

@Path("/health")
@Produces(MediaType.APPLICATION_JSON)
public interface HealthCheck {

    @GET
    @Path("/live")
    Status getLiveStatus();

    @GET
    @Path("/ready")
    Status getReadinessStatus();

    @GET
    Status getStatus();
}

With those APIs a user could easily determine the status of a remote service with something like:

static boolean isUp(String baseURI) {
    HealthCheck hc = RestClientBuilder.newBuilder()
                                      .baseURI(URI.create(baseURI))
                                      .property("microprofile.rest.client.disable.default.mapper",true)
                                      .create(HealthCheck);
    Status s = hc.getStatus();
    if (!s.isUp()) {
        // log a warning why the service isn't up
        for (Check c : s.getChecks()) {
            if (!c.isUp()) {
                log.warning("for service {0}, check {1} is down", baseURI, c.getName());
            }
        }
    }
    return s.isUp();
}

Even that code could be part of the Rest Client interface as a static method. Then, if the client has MP Rest Client and MP Health Check in it's class path, they could quickly determine the health of a remote system in a uniform manner - i.e. HealthCheck.isUp(myAppURI).

@xstefank
Copy link
Member

I like the idea but I am not sure if the MP Health is the right place to host this rest client interface. MP Health provides an API to expose health checks to be called by other services. So this is intended the other way around. But maybe I am missing the use case.

@andymc12
Copy link
Author

I think the most common use case is just to query if a service is "up" or not. But having a client API with the spec is consistent with other specs - like JAX-RS which has the server and client APIs together.

Another use case might be for aggregated services. For example, service A may depend on service B and C. Service A's readiness (or overall status) may need to depend on B and C being up before it declares it's own readiness as up. For this scenario, I think it would be handy to have a built-in health check client.

@xstefank
Copy link
Member

Thanks @andymc12. You made valid points. If everyone else is ok with this, we can include this in the next release.

@donbourne
Copy link
Member

@andymc12 this seems like it would lead to very tight coupling between MP Health and MP REST client. If someone is running MP Health in an environment where they aren't using MP REST this would be unwanted bulk. Is there some other way to do this that doesn't make MP Health depend on MP REST Client?

@andymc12
Copy link
Author

@donbourne it should only be a compile-time dependency on MP Rest Client. There should not need to be any hard dependency at runtime, so a service or framework using MP Health Check should not need the MP Rest Client API or its implementation, unless that service/framework intends to use it. I can try to prototype this if you think it would help.

@derekm
Copy link
Contributor

derekm commented Nov 18, 2019

Here's some stuff I just built out for our internal use...

public interface RootHealthListResource {

    @Path("health")
    HealthListResource health();

}
public interface HealthListResource {

    @GET
    @Produces(MediaType.APPLICATION_JSON)
    HealthListResponse get();

}
public class HealthListResponse {

    public List<Check> checks;
    public HealthStatus outcome;

}
public class Check {

    public String name;
    public HealthStatus state;
    public Map<String, Object> data;

}
public enum HealthStatus {
    UP, DOWN;
}

With the above files, I've built a generic healthcheck that checks the health of any remote service upon which we depend:

public abstract class RemoteHealthCheckList implements HealthCheck {

    RemoteHealthCheckClient client;

    abstract String getConfigPrefix();

    @Override
    public HealthCheckResponse call() {
        client = new RemoteHealthCheckClient();
        HealthListResponse response = client.root().health().get();

        HealthCheckResponseBuilder builder = HealthCheckResponse
                .named(getConfigPrefix() + ":health");

        for (Check check : response.checks) {
            builder.withData(check.name, check.state.toString());
        }

        return builder
                .withData("target", client.getTarget().getUri().toString())
                .state(response.outcome == HealthStatus.UP)
                .build();
    }

    class RemoteHealthCheckClient extends BaseServiceClient<RootHealthListResource> {

        @Override
        protected Class<RootHealthListResource> getRootClass() {
            return RootHealthListResource.class;
        }

        @Override
        protected String getConfigPrefix() {
            return RemoteHealthCheckList.this.getConfigPrefix();
        }

    }

}

We primarily use Hammock, so we can set hammock.health.output.format=map to produce a checks JSON object instead of a checks JSON array, so I have a corresponding set of interfaces for when checks is a Map<String, Check> instead.

In an app, I get this healthcheck basically for free by doing:

public class StarterAppHealth extends RemoteHealthCheckMap {

    @Override
    public String getConfigPrefix() {
        return "starterApp";
    }

}

IMVSO (in my very strong opinion), this is the level of accessibility and feature support at which MicroProfile needs to be thinking, instead of leaving it up to everyone to reinvent these wheels on top of MicroProfile.

@donbourne
Copy link
Member

@andymc12 what you said about this just be a compile time dependency makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants