Fix double logging in `ActiveRecord::QueryLog`

Fixes https://github.com/rails/rails/issues/46103

An issue exists if you set `config.active_record.query_log_tags` to an array that includes `:controller`, `:action`, or `:job`; the relevant item will get duplicated in the log line. This occured because the relevant railties would add the item to `config.active_record.query_log_tags` again during setup. This PR fixes that by only adding those items to the config if they aren't already set.

The issue proposed more documentation to work around this, but I think it's a bug and should be fixed directly.
This commit is contained in:
Alex Ghiculescu 2022-10-19 15:33:51 -05:00
parent 0eb42cae30
commit f737b5ec13
5 changed files with 39 additions and 2 deletions

View File

@ -1,3 +1,10 @@
* Don't double log the `controller` or `action` when using `ActiveRecord::QueryLog`
Previously if you set `config.active_record.query_log_tags` to an array that included
`:controller` or `:action`, that item would get logged twice. This bug has been fixed.
*Alex Ghiculescu*
* Add the following permissions policy directives: `hid`, `idle-detection`, `screen-wake-lock`,
`serial`, `sync-xhr`, `web-share`.

View File

@ -112,7 +112,7 @@ module ActionController
app.config.action_controller.log_query_tags_around_actions
if query_logs_tags_enabled
app.config.active_record.query_log_tags += [:controller, :action]
app.config.active_record.query_log_tags |= [:controller, :action]
ActiveSupport.on_load(:active_record) do
ActiveRecord::QueryLogs.taggings.merge!(

View File

@ -1,3 +1,10 @@
* Don't double log the `job` when using `ActiveRecord::QueryLog`
Previously if you set `config.active_record.query_log_tags` to an array that included
`:job`, the job name would get logged twice. This bug has been fixed.
*Alex Ghiculescu*
* Add support for Sidekiq's transaction-aware client
*Jonathan del Strother*

View File

@ -72,7 +72,7 @@ module ActiveJob
app.config.active_job.log_query_tags_around_perform
if query_logs_tags_enabled
app.config.active_record.query_log_tags << :job
app.config.active_record.query_log_tags |= [:job]
ActiveSupport.on_load(:active_record) do
ActiveRecord::QueryLogs.taggings[:job] = ->(context) { context[:job].class.name if context[:job] }

View File

@ -115,6 +115,18 @@ module ApplicationTests
assert_not_includes comment, "controller:users"
end
test "controller tags are not doubled up if already configured" do
add_to_config "config.active_record.query_log_tags_enabled = true"
add_to_config "config.active_record.query_log_tags = [ :action, :job, :controller, :pid ]"
boot_app
get "/"
comment = last_response.body.strip
assert_match(/\/\*action='index',controller='users',pid='\d+'\*\//, comment)
end
test "job perform method has tagging filters enabled by default" do
add_to_config "config.active_record.query_log_tags_enabled = true"
@ -136,6 +148,17 @@ module ApplicationTests
assert_not_includes comment, "UserJob"
end
test "job tags are not doubled up if already configured" do
add_to_config "config.active_record.query_log_tags_enabled = true"
add_to_config "config.active_record.query_log_tags = [ :action, :job, :controller, :pid ]"
boot_app
comment = UserJob.new.perform_now
assert_match(/\/\*job='UserJob',pid='\d+'\*\//, comment)
end
test "query cache is cleared between requests" do
add_to_config "config.active_record.query_log_tags_enabled = true"
add_to_config "config.active_record.cache_query_log_tags = true"