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

healthcheck endpoint #93

Merged
merged 1 commit into from
Jun 22, 2022
Merged

healthcheck endpoint #93

merged 1 commit into from
Jun 22, 2022

Conversation

ajgon
Copy link
Contributor

@ajgon ajgon commented Oct 2, 2021

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.

@ajgon ajgon requested a review from krallin as a code owner October 2, 2021 13:14
@maciej-orba
Copy link

@krallin can you please include this PR in next release? its nice just for visibility that service responds and there are no deathlocks et.

@neurosnap
Copy link
Member

Greetings! Thanks for submitting this PR and sorry for the long wait. Is there a reason why you couldn't use the / endpoint for the healthcheck? It's a relatively small html response.

@ajgon
Copy link
Contributor Author

ajgon commented Jun 22, 2022

@neurosnap Even better, you can use HEAD / check and have no response at all (just the headers). Eventually I settled up with this solution, since it works in consul - I'm not sure if it works in other monitoring tools, thus GET may be still viable.

I agree, GET / is small, but still it requires way more work (relatively) to render this HTML than sending simple two-byte string. Also monitoring tool, doesn't care about response, but still needs resources to process it. I know, this may sound insignificant and irrelevant, but this stuff tends to add up, when you monitor multiple services very often.

So, at the end of the day, it's your call :) Most of the services, implement /health endpoint in one way or another, so I think it's the way to go. However, there are some other (like Gitea), which support HEAD / and call it a day.

@neurosnap
Copy link
Member

Fair enough, thanks for the input. I don't see any harm in including this endpoint.

@neurosnap neurosnap merged commit 48e7290 into aptible:master Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants