-
Notifications
You must be signed in to change notification settings - Fork 13
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
Suggest default path /health
#27
Comments
MicroProfile Health is adding liveness and readiness checks on top of health checks. Given status code expectations and coalescence of health check outcomes, additional See Kubernetes for liveness and readiness definitions, but, basically, readiness failures pull the app out of loadbalancer rotation, and liveness failures kill the app instance in favor of a new instance. |
The decoupling of the wire format, described in this RFC, from the URI endpoint that it can be used at is intentional. For instance, in case of Kubernetes liveness/readiness checks both of those endpoints can use the format defined in the RFC. Which is what the reference implementation of the spec in Node does: https://github.com/inadarei/maikai#kubernetes-liveness-and-readiness-probes The URIs health check endpoints are mounted to, in practice, are already so different, suggesting some specific URI in this RFC would probably be counter-productive. Case in point: Kubernetes itself doesn't even suggest a standard URI for mounting readiness and liveness probes, instead letting users indicate their own path. |
P.S. for making discovery of health check links easier, please see the existing conversation here: #5 and the suggested, arguably preferred way of doing it. |
Status code expectations is another topic. Does Nonetheless, I think there's a lot of simple services which themselves would just have a single endpoint, and I think the RFC suggesting (not demanding) that endpoint to be " |
Based on already-established behaviors, health endpoint is the conduit to the main API so it doesn't always return HTTP 200 when it itself can respond, health endpoints return HTTP 4xx-5xx if the main API is having trouble. |
@inadarei Thanks for the clarification. Just to make sure I'm 100% clear on this and on the same page as you, here's a use case. A middleware which talks to a backend. The middleware could have two health endpoints, Suppose the middleware is just fine, In the following, suppose the middleware is just fine. Is that interpretation around how you intend health endpoints to deal with status codes correct? |
When it comes to suggestions of the response code, I would welcome something along the lines of suggesting to use 2xx responses when the system is in a "pass" state, and an error state (5xx) when the system is in a "fail" state. For "warn" states, it would be up to the system what health to indicate to the world. Typically load balancers have a "health check" that they check every few minutes, and "warn" might not be a good enough indicator to move traffic away from the service. |
Currently the spec defines "warn" as 2xx, meaning - load-balancers would NOT move traffic away, unless they dig deeper and investigate what the warning is about. If they do not understand as much (old, less "intelligent" networking tooling), by definition they should interpret warn as "ok", since it is not a problem yet. Which is why 2xx is suggested as the return status of "warn". |
Ah, I completely missed that part of the spec! 👍 |
Sorry to resurrect this, but I work on REST APIs for a healthcare company, and it is not unlikely that a I would suggest standardizing on a token which indicates the purpose of a health-check, but is somewhat less likely to be used as part of a public-facing API. I have begun using |
Same as @kalexmills and we were wondering if an addition to |
While the example suggests to the observant reader that it is a good idea to use
/health
as the default path to reach the health check endpoint, this is not mentioned anywhere in the text. May I suggest that the following paragraphs are added:I'm sure the wording can be improved, but I guess you get what I mean.
The text was updated successfully, but these errors were encountered: