add "as: :rails_health_check" to health check path #47217
Conversation
This makes the path discoverable even if the name or even the controller action changes.
| NOTE: This endpoint does not reflect the status of all of your service's dependencies, such as the database or redis cluster. Replace "rails/heath#show" with your own controller action if you have application specific needs. |
There was a problem hiding this comment.
@rubys My only knit is that we should be using "application's dependencies" here, I know it's bikeshedding but we refer to them as Rails Apps everywhere so trying to be consistent. When I first wrote this part, I also used "service" coincidentally 😂
I'm going to update this part, but appreciate you updating the docs here, it re-affirms my instincts about not putting too much cruft in a health check :)
There was a problem hiding this comment.
@zzak go for it. Those weren't my words, so I didn't change them.
I'm afraid that we are going to need to agree to disagree about "putting too much cruft in a health check". 😄 Over the next few days I'll be writing up a blog post about health checks and fly.io. I'll be putting everything in a positive light, but the net of it will be that we provide tcp checks (i.e. "is the app listening on the socket?") without any need for any app support. Adding a http check (e.g., with a minimal "/up" endpoint) does add some value, but it is minimal. Routing around regional failures require a bit more, and there things like easymon or rails-healthcheck come in handy.
When the post is ready, it will be posted here: https://fly.io/ruby-dispatch/
There was a problem hiding this comment.
And the promised post is finally live: https://fly.io/ruby-dispatch/health-checks/
Also have generator comment out Rails 7.1 health check. See also: * superfly/flyctl#1678 * rails/rails#47217 * rails/rails#46936
This makes the path discoverable even if the name or even the controller action changes.
Motivation / Background
Adding health checks (#46936) is a good idea. We are already seeing people varying the name. Or providing different implementations. All of that is fine, as long as the health check endpoint is discoverable. This pull request provides a way to discover the endpoint using something along the lines of:
Detail
This Pull Request adds
as: :rails_health_checkto health check pathAdditional information
Rails 7.1 provides a
rails/health#showcontroller action that does no application specific checks, which is goodness and satisfies many needs. The guide before this pull request goes further and recommends against providing application specific checks, which may be going too far.A real world example, that actually happened at fly.io. A customer application was deployed in multiple geographic regions, something we support and highly recommend. That application happened to depend on S3, which is by no means atypical. It came to pass that a S3 regional outage occurred - something that isn't common, but not unheard of.
Health checks in instances of this application in affected regions started to fail. Fly's infrastructure used this information to route requests to instances where the request would succeed. The application remained up during this outage.
This is a scenario that I would very much like to support. And the cost of doing so to Rails is minimal: a named route and change in the guide from saying "don't do it" to something along the lines of "think carefully before you proceed".
cc: @dhh @rafaelfranca
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]It is not clear to me whether a CHANGELOG entry is needed for this change.