Skip to content

Commit

Permalink
Fix double logging in ActiveRecord::QueryLog
Browse files Browse the repository at this point in the history
Fixes #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.
  • Loading branch information
ghiculescu committed Oct 22, 2022
1 parent 0eb42ca commit f737b5e
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 2 deletions.
7 changes: 7 additions & 0 deletions actionpack/CHANGELOG.md
@@ -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`.

Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/railtie.rb
Expand Up @@ -112,7 +112,7 @@ class Railtie < Rails::Railtie # :nodoc:
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!(
Expand Down
7 changes: 7 additions & 0 deletions activejob/CHANGELOG.md
@@ -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*
Expand Down
2 changes: 1 addition & 1 deletion activejob/lib/active_job/railtie.rb
Expand Up @@ -72,7 +72,7 @@ class Railtie < Rails::Railtie # :nodoc:
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] }
Expand Down
23 changes: 23 additions & 0 deletions railties/test/application/query_logs_test.rb
Expand Up @@ -115,6 +115,18 @@ def app
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"

Expand All @@ -136,6 +148,17 @@ def app
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"
Expand Down

0 comments on commit f737b5e

Please sign in to comment.