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

Add guides section on verbose query logs to Debugging #34257

Merged
merged 1 commit into from Oct 19, 2018

Conversation

@olivierlacan
Copy link
Contributor

@olivierlacan olivierlacan commented Oct 19, 2018

Since this is a useful tool in debugging it made sense to document its existence and usage, especially in the console where it's disabled by default.

I also amended the Sending Messages section right above since it wasn't displaying query callers. Since they're enabled by default in 5.2 it felt appropriate to show them.

I might have to go through other guides to make sure we display them when appropriate if showing some logging.

Rendered Preview

image

@rails-bot rails-bot bot added the docs label Oct 19, 2018
@rails-bot
Copy link

@rails-bot rails-bot commented Oct 19, 2018

r? @schneems

(@rails-bot has picked a reviewer for you, use r? to override)

@schneems
Copy link
Member

@schneems schneems commented Oct 19, 2018

Hey! 👋 thanks for the PR!

Overall I like the idea of calling out this debugging feature! My only concern is that people might think that “random” is a method provided by active record if they are skimming. Can we be more explicit about calling it that it’s a user supplied method?

You can add “CI skip” to your commit to skip the tests.

@olivierlacan olivierlacan force-pushed the olivierlacan:verbose-query-logs-guides branch 2 times, most recently from 0d70bda to 6073d5d Oct 19, 2018
Since this is a useful tool in debugging it made sense to document
its existence and usage, especially in the console where it's disabled
by default.

[ci skip]
@olivierlacan olivierlacan force-pushed the olivierlacan:verbose-query-logs-guides branch from 6073d5d to 8c3a228 Oct 19, 2018
@olivierlacan
Copy link
Contributor Author

@olivierlacan olivierlacan commented Oct 19, 2018

@schneems Looks like it was [ci skip] (brackets required) anywhere in the message title or body, so I accidentally triggered a few builds. My bad.

Great idea for the method. Changed it to Article.pamplemousse because now there's no way to think it could come from ActiveRecord... although it really really should. 😄

image

@schneems
Copy link
Member

@schneems schneems commented Oct 19, 2018

😂 love it. Thanks for the example!

@schneems schneems merged commit e3111c1 into rails:master Oct 19, 2018
1 check passed
1 check passed
codeclimate All good!
Details
=> #<Comment id: 2, author: "1", body: "Well, actually...", article_id: 1, created_at: "2018-10-19 00:56:10", updated_at: "2018-10-19 00:56:10">
```

Below each database statement you can see arrows pointing to the specific source filename (and line number) of the method that resulted in a database call. This can help you identity and address performance problems caused by N+1 queries: single database queries that generates multiple additional queries.

This comment has been minimized.

@nickcoyne

nickcoyne Nov 4, 2018
Contributor

Just noticed a typo here: identity should be identify.

@olivierlacan
Copy link
Contributor Author

@olivierlacan olivierlacan commented Nov 4, 2018

nickcoyne added a commit to nickcoyne/rails that referenced this pull request Nov 4, 2018
Fix typo
Just a small typo fix for the recently merged rails#34257
This was referenced Nov 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants