-
Notifications
You must be signed in to change notification settings - Fork 17
Allow user to skip logging based on host as well as path #15
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
Conversation
@@ -29,11 +29,11 @@ Gem::Specification.new do |spec| | |||
spec.add_dependency "nokogiri" | |||
spec.add_dependency "zeitwerk", ">= 2.0.0" | |||
|
|||
spec.add_development_dependency "sqlite3", "~> 1.4.0" | |||
spec.add_development_dependency "sqlite3", ">= 2.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wants to be > 2, though I didn't check which dependency it was that was dependent on this.
spec.add_development_dependency "pg", "~> 1.5.4" | ||
spec.add_development_dependency "standard", "~> 1.31" | ||
spec.add_development_dependency "rake", "~> 12.0" | ||
spec.add_development_dependency "rspec", "~> 3.0" | ||
spec.add_development_dependency "rspec-rails", "~> 7.1.0" | ||
spec.add_development_dependency "rack" | ||
spec.add_development_dependency "rack", "< 3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rack 3 has removed the ability to rewind Rack inputs, and so the tests all fail on this line when Rack 3 is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this very much. Thanks! I'll adjust a couple of bits and merge it
Thanks @coorasse! Glad I could help :) |
Closed via #16 |
Pushing this up to get some opinions on it.
We have a Rails app serving an API layer on
api.*
, and a web app onapp.*
. We're using routing constraints to determine which routes are accessible on which subdomain. At the moment, we're running into a similar issue to that in #8 (same manifestation, different cause), in that junk requests toapp.*.com/api/v1/some-path
are resulting in a 500 error.This PR allows the user to specify a host regex to determine whether a request should be logged, similar to the way the path regex functions currently. This allows us to skip these junk requests at the middleware layer to stop them from causing the stack overflows.
Probably still needs some README.md updates, but I can do these if this is going to be merged.