-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Fix database runtimes on production log #24963
Fix database runtimes on production log #24963
Conversation
Rails default production configuration establishes "info" as log level. Due to the changes included on commit 191facc, db runtimes were not being collected if the log level was different than "debug", and 0.0 ms was the runtime reported on production logs.
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @kaspth (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
r? @eileencodes |
self.class.runtime += event.duration | ||
return unless logger.debug? | ||
|
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 think this was on purpose
Thanks! I had made the change on purpose but if I remember correctly it wasn't an actual performance hit for integration tests, just something we noticed was weird when we were fixing performance there. |
…tion-logs Fix database runtimes on production log
Did this make 5.0.1? I'm still seeing the broken behaviour. |
I checked and no this didn't make it into 5.0.1. While it was backported to 5-0-stable it wasn't back-ported to the 5-0-1 branch and didn't make it into the release. It will be in 5.0.2 and 5.1.0 though. |
OK, thanks!
… On 21 Dec 2016, at 15:26, Eileen M. Uchitelle ***@***.***> wrote:
I checked and no this didn't make it into 5.0.1. While it was backported to 5-0-stable it wasn't back-ported to the 5-0-1 branch and didn't make it into the release. It will be in 5.0.2 and 5.1.0 though.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Summary
Rails default production configuration establishes
info
as log level. Due to the changes included on 191facc, database runtimes are not being collected if the log level is different thandebug
. This produces that production log reportsdb=0.0ms
as database runtime.Other Information
Sorry, I tried to include a test to cover this case, but I was not sure about how to do it or where to put it (probably here https://github.com/rails/rails/blob/master/actionpack/test/controller/log_subscriber_test.rb)
/cc @eileencodes