Health implementation, initial commit

[Heiko Braun]

I've committed the initial code to the health repository, so we have something to discuss:

https://github.com/eclipse/microprofile-health

It's in it's early stages, but a draft of the API is included and I've got the RI in the pipeline to we can create the test coverage for the wireformat and protocol described at https://github.com/eclipse/microprofile-evolution-process/blob/master/proposals/0003-spec.md

Regards, Heiko

[Juraci Paixão Kröhling]

On 05/11/2017 09:04 AM, 'Heiko Braun' via MicroProfile wrote:
> It's in it's early stages, but a draft of the API is included and I've
> got the RI in the pipeline to we can create the test coverage for the
> wireformat and protocol described
> at https://github.com/eclipse/microprofile-evolution-process/blob/master/proposals/0003-spec.md

Looks very interesting! I wonder if it makes sense to have a distinction
between a health check for a "readiness check" and for a "liveness
check", as they have different semantics on platforms like
Kubernetes/OpenShift. HeikoR mentioned that this might have been
discussed in the past, but I couldn't find a thread/message discussing
this specific aspect.

Arguably, the readiness check could be handled by the container without
an application-specific logic if the application uses context listeners
to perform operations during the boot/deployment, so, perhaps it's just
a matter of making it explicit that the container should return a 503
status code when the server is still booting and/or the application is
still being deployed?

I'd also like to suggest using the status code "202 Accepted" on
appendix A, to indicate that a system is in a "degraded" state. For
instance, a system is able to receive requests and queue the
transactions, but the actual processing is delayed due to problems in a
downstream dependency. This might require a flag on the `@Health`
annotation, to state that a failure on the check doesn't mean the
application is down.

- Juca.

[Werner Keil]

Heiko,

Thanks for the update. Since JSR 353 (374 soon, it's about to go Final) aka JSON-P has always been a core component of MicroProfile, would it make sense to use it in "toJson" methods?;-) 

I am doing something similar for status and availability of service nodes in another Java EE related project and what's standard-related I also quoted a few bits on how to use the JSON-P JSR for the Health API Microservice Pattern, so I may be able to fork it and propose some things around it over the long weekend.

About the file header, I know this was a bit unlucky in the thread here, the "misleading" version using only plurals was presented as de-facto standard, but looking especially at the Config libraries: https://github.com/eclipse/microprofile-config/blob/master/api/src/main/java/org/eclipse/microprofile/config/Config.java 

changed it to

* Copyright (c) 2011-2017 Contributors to the Eclipse Foundation

*

* See the NOTICE file(s) distributed with this work for additional

* information regarding copyright ownership.

...

now. It is not a big deal or showstopper I agree, but calling the file exactly the same way it's common standard in the repositoriy (NOTICE, not NOTICES) seems to make most sense.

Werner

[John D. Ament]

Heiko,

Would it make sense to include a build job at https://ci.eclipse.org/microprofile/ ?  This way we can publish snapshots, would like to see what it takes to implement the API a bit easier.

In Status, instead of a single optional message would it make sense to have multiple messages?  I'm not a fan of Status knowing how to convert itself to JSON, very awkward from my POV.

The fact that the API also provides an impl is also weird.  It basically means I must use the shipped HealthStatus class.

The Javadocs for @Health should include what the method signature should look like.

Good first steps!

John

[Heiko Braun]

Thanks John and Werner for taking the time to review the initial commit. I am aware of the #toJson() implementation and agree that it needs to change. Whether we leverage JSON-P for that or externalise it completely is up for debate. I am tempted to follow John suggestion and remove it from the API completely.

[Emily Jiang]

I would like one scenario to be clarified. For unauthenticated users, what should be in the payload? Spring boot has this policy on HTTP health endpoint format and access restrictions. It sounds reasonable to me. Do we need to follow the similar approach or doing something different? We need to spec this.

Emily

[Heiko Braun]

Thanks for bringing thus up Emily. It's indeed unclear in the spec. If we limit the spec to the HTTP protocol only, then we can use a similar approach as outlined in the spring docs. 

 

[Werner Keil]

Heiko/all,

Thanks for the feedback and suggestions by others.

Keeping the API more or less implementation and format-neutral may be a good thing to consider.

While there is no need to standardize the API in its case, even for the commercial projects I mentioned, I try to separate abstract method calls like health(), healthReport() or similar and the JSON manifestatio which I also delegated there, so it could be similar here. Leaving at least in theory other formats like XML, YAML or similar open and implementation-specific.

Werner