Skip to content

Conversation

dhh
Copy link
Member

@dhh dhh commented Sep 3, 2024

The default Rails health check is intended to be hit every second or so by Kamal to ensure that the application is still or has just become responsive. This clutters up the production logs and makes it hard to follow what's actually going on. This adds a new middleware called Rails::Rack::SilenceRequest that is used by a new config config.silence_healthcheck = path to prevent that.

@rails-bot rails-bot bot added the railties label Sep 3, 2024
@rails-bot rails-bot bot added the docs label Sep 3, 2024
Thanks to @skipkayhil for reminding me of Propshaft::QuietAssets which
does something very similar and does it just via env!
dhh and others added 2 commits September 3, 2024 15:59
Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>
@dhh dhh merged commit 66a9d11 into main Sep 3, 2024
5 checks passed
@dhh dhh deleted the silence-up-request branch September 3, 2024 23:20
@@ -196,10 +196,10 @@ def app
assert_includes middleware, "ActionDispatch::AssumeSSL"
end

test "ActionDispatch::SSL is present when force_ssl is set" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhh did you mean to delete this test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not. Not sure what happened there. Please do PR to restore!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AnalyzePlatypus
Copy link

Will this ship in Rails 8, or in an earlier point release?

@dhh
Copy link
Member Author

dhh commented Sep 9, 2024

  1. We don't back port features.

@cristianbica
Copy link
Member

@dhh how do you feel abstracting this to a config.silence_requests_paths = ['/up', '/others']?
I'm thinking at use-cases like an endpoint exporting some metrics, people running rack-mini-profiler in production or people just filtering out logging for some endpoints which just adds noise to logging.

@dhh
Copy link
Member Author

dhh commented Sep 10, 2024

@cristianbica Are any of those examples you mention problems you've actually hit in production? At what intervals are those requests running? I don't want to blanket rule out making this more generic, but I find that you're better off with something specific, unless you've proven without a doubt the need for something generic.

@adampope
Copy link

@dhh In our app we have an async request for red dot / badge counts that happens on most page loads. I recently wrote a very similar middleware to filter those out of the development logs as I got bored of scrolling back up the tail past the badge request to see details of the main request. Having something configurable like @cristianbica is suggesting, and being able to be different in different environments would certainly have been great for our real use case.

@cristianbica
Copy link
Member

@cristianbica Are any of those examples you mention problems you've actually hit in production? At what intervals are those requests running? I don't want to blanket rule out making this more generic, but I find that you're better off with something specific, unless you've proven without a doubt the need for something generic.

@dhh on my current projects I do not have a valid use-case to prove the need to generalize this functionality.

In the past projects I've implemented different solution to reduce logging (e.g. silencer gem for requests). The purpose was to reduce clutter in the logs and reduce costs. From what I remember:

  • we had an endpoint exposing sidekiq metrics called minutely
  • we had many webhook-like requests - those were recorded and processed in the background; logging was too verbose so there was no benefit to log them

Anyway, looking at the implementation in this PR, one could easy filter out logging for other paths by adding to application.rb multiple lines like this

config.middleware.insert_before ::Rails::Rack::Logger, :Rails::Rack::SilenceRequest, path: '/sidekiq_stats'

DanielaVelasquez pushed a commit to DanielaVelasquez/rails that referenced this pull request Oct 3, 2024
* Add SilenceRequest middleware

* Make config.silence_healthcheck a way to quiet "/up"
@morgoth
Copy link
Member

morgoth commented Nov 6, 2024

@dhh With thruster gem included by default, the "/up" requests are showing up in the logs anyway. Is there any way to silence that as well, as I don't see such possibility in thruster project yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants