caronc
healthcheck endpoint
Description:
This PR adds a /health endpoint which does nothing more than returning plaintext OK response. It is useful for service discovery tools (consul/kubernetes etc.), to quickly say, that service is alive and running. Currently, to achieve that, I had to ping / which returned a lot of unnecessary data.
Checklist
- [x] The code change is tested and works locally.
- [x] There is no commented out code in this PR.
- [x] No lint errors (use
flake8) - [x] tests added
Is the excess data that is returned in / that much overhead for the check? I mean, I don't see the problem here with your PR at all; it just seems kind of redundant, that's all. :man_shrugging:
I would think that it's just the 200 code response that is most important and that can be achieved from the / response letting you know everything is okay?
I could see more value in a /health URL if the link was also testing if the database service was responding okay (but there is no database). Can you think of an example as to why the response would ever return an ERROR vs just hard coding the word OK in the response? I'm just thinking out loud here as a way to bring more value to your /health check.
What are your thoughts?
Yes, usually health endpoints perform some internal checks (albeit, very "light" ones, since the endpoint can be called rather often) if everything is okay, and then they return 200/500 code. The body itself is less important - OK word is somehow a "standard" just for the sake of people who are testing this manually via browser for whatever reason. Since here there is nothing much to check (or I wasn't aware of it), I'm returning OK directly.
After I did this PR, it hit me, that there is another way of checking the health - which is way simpler, doesn't need any extra code and usually works. It's by using HEAD header. HEAD is basically GET without body, which is perfect for this (no response overhead). In apprise case - it works out of the box as well. However, I only saw one single service, which recommends healthchecking this way (gitea), other ones usually doesn't provide health checks at all (meaning even HEAD is blocked), or they use GET /health//healthz/ping etc. I'm not sure why it's the case - maybe some service discovery tools doesn't support HEAD header?
Anyway, I agree completely. We can get rid of this PR in favor of HEAD header - I'm using consul as a service discovery tool, which supports it - so it's fine by me. However I'm not sure, what is the golden "standard" here (if any) - as most of the services for whatever reason implement GET /health in one way or another...
However I'm not sure, what is the golden "standard" here (if any) - as most of the services for whatever reason implement GET /health in one way or another...
This is key. Again, just to be clear: i have no problem with your PR at all. :+1: . I absolutely love to see people using the tool and suggesting ways to improve it. :heart: If you think it would make everyone lives easier using a specific platform (you stated Kubernetes as an example) then I don't see a problem. You're teaching me here too (as i'm a bit of neckbeard trying to adapt to proper standards, etc :wink: ).
With respect to your comment above: is there actually some sort of standard that everyone follows and uses? I mean a /health response that returns OK today (allowing people to hook up to it) is fine if down the road it would be updated to help reflect the change down the road. In the event a database should ever gets implemented, the / would also likely return a 500 error as some of it's content may or may not be retrieved from there too (keeping up with the redundancy).
No worries, I'm a bit neckbeard too ;)
Regarding DB example - what if at some point, you decide to serve some static content at / and forget about it? Then / would return 200 despite of DB being dead (and killing other endpoints). But I think we're overthinking it here - as I said HEAD / sounds very reasonable to me - I just don't know why, nobody (except gitea) doesn't do it this way...
Regarding standards - there is one draft I'm aware of: https://inadarei.github.io/rfc-healthcheck/ - however, personally I don't like it. Firstly, it returns a lot of unnecessary noise, secondly - I've never seen it implemented anywhere (and I self-host and play with services all the time).
So in the end, I think it's a question to your neckbearded self which approach do you like better :smile:.
I think you're right with your comments. I think I like the HEAD approach better for now and we can cross the /health when the time is needed (your static example is a good point). :+1: