Skip to content

Enable query log tags by default on development env#51342

Merged
byroot merged 1 commit intorails:mainfrom
MatheusRich:patch-1
Aug 11, 2024
Merged

Enable query log tags by default on development env#51342
byroot merged 1 commit intorails:mainfrom
MatheusRich:patch-1

Conversation

@MatheusRich
Copy link
Contributor

@MatheusRich MatheusRich commented Mar 18, 2024 •
edited
Loading

Motivation / Background

This can be used to trace troublesome SQL statements back to the application code that generated these statements. I feel like this is a good default value that helps with debugging and code discovery.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the railties label Mar 18, 2024
@MatheusRich
Copy link
Contributor Author

MatheusRich commented Mar 18, 2024 •
edited
Loading

@rafaelfranca thoughts? If this is approved, I will add tests and a CHANGELOG entry.

@rafaelfranca
Copy link
Member

rafaelfranca commented Mar 18, 2024

We already have another feature doing the same in development verbose_query_logs with this one pointing to the exact line of code that generated the query. Why do we need both?

@MatheusRich
Copy link
Contributor Author

MatheusRich commented Mar 19, 2024

@rafaelfranca verbose_query_logs might point to a model scope or method (that might be called inside a view), so it might not be evident which controller or job generated that query. It is also useful when using multiple databases because the query logs can identify which db is being used.

@rafaelfranca
Copy link
Member

rafaelfranca commented Mar 19, 2024

Sounds good let's get this finished up.

As a general direction, don't open draft PRs, they are just ignored, and I would actually go as far as automatically closing draft PRs. Either the PR is ready to be reviewed or send a message in the Rails forum. Our attention spam is very small, so if we see a draft PR we will just close the tab since it isn't ready and never come back.

@MatheusRich MatheusRich marked this pull request as ready for review March 22, 2024 14:58
@MatheusRich MatheusRich force-pushed the patch-1 branch 5 times, most recently from c0d08fe to 5da8395 Compare March 22, 2024 15:57
@rails-bot rails-bot bot added the docs label Mar 22, 2024
@MatheusRich
Copy link
Contributor Author

MatheusRich commented Mar 22, 2024

@rafaelfranca Thanks for letting me know! I think this is finished now.

Side note: should we add the guideline to not open draft PRs in the contributing docs?

@skipkayhil
Copy link
Member

skipkayhil commented Mar 25, 2024

How do query logs interact with prepared statements? My understanding was the two were not compatible and all db adapters except MySQL will use prepared statements by default.

stevepolitodesign pushed a commit to thoughtbot/suspenders that referenced this pull request Apr 5, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator
1711f5b 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342
@stevepolitodesign stevepolitodesign mentioned this pull request Apr 5, 2024
Merged
stevepolitodesign pushed a commit to thoughtbot/suspenders that referenced this pull request Apr 5, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator
96180f8 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342
stevepolitodesign pushed a commit to thoughtbot/suspenders that referenced this pull request Apr 5, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator
12ed7db 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342
stevepolitodesign pushed a commit to thoughtbot/suspenders that referenced this pull request Apr 5, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator
4b8894d 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342
stevepolitodesign pushed a commit to thoughtbot/suspenders that referenced this pull request Apr 5, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator
50f2a8d 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342
stevepolitodesign pushed a commit to thoughtbot/suspenders that referenced this pull request Apr 5, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator
cca73b5 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request Apr 5, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator (#1149)
7d666ca 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342

Co-authored-by: Steve Polito <stevepolito@hey.com>
stevepolitodesign added a commit to thoughtbot/suspenders that referenced this pull request May 10, 2024
@crackofdusk @stevepolitodesign
Introduce suspenders:development:environment generator (#1149)
383efcf 
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.

It's possible a future release of Rails will alleviate us from having to
do the following:

- enable `active_model.i18n_customize_full_message` which is being
  addressed in [#50406][]
- enable `active_record.query_log_tags_enabled` which is being addressed
  in [#51342][]

[#50406]: rails/rails#50406
[#51342]: rails/rails#51342

Co-authored-by: Steve Polito <stevepolito@hey.com>
@zzak
Copy link
Member

zzak commented May 20, 2024

How do query logs interact with prepared statements? My understanding was the two were not compatible and all db adapters except MySQL will use prepared statements by default.

@skipkayhil They don't. See: #48631

@MatheusRich
Copy link
Contributor Author

MatheusRich commented Jul 31, 2024

@rafaelfranca this is probably good to go

@MatheusRich @byroot
Enable query log tags by default on development env
e29ad0b 
This can be used to trace troublesome SQL statements back to the application code that generated these statements.
@byroot byroot merged commit c9c3047 into rails:main Aug 11, 2024
@MatheusRich MatheusRich deleted the patch-1 branch August 16, 2024 21:20
Earlopain added a commit to Earlopain/rails that referenced this pull request Nov 8, 2024
@Earlopain
Update guide docs for updated environment files
ff770ee 
* rails#51831
* rails#51342
* rails#52887

Probably more, these are the ones I noticed.
@Earlopain Earlopain mentioned this pull request Nov 8, 2024
Merged
Earlopain added a commit to Earlopain/rails that referenced this pull request Nov 15, 2024
@Earlopain
Update guide docs for updated environment files
083ca4e 
* rails#51831
* rails#51342

Probably more, these are the ones I noticed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

docs railties

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MatheusRich @rafaelfranca @skipkayhil @zzak @byroot