From 5443cf3ed3c3885555ac0968a901e64531e68e67 Mon Sep 17 00:00:00 2001 From: Emil Ong Date: Wed, 8 Nov 2023 23:09:37 +0000 Subject: [PATCH 1/2] Add query counts and maximum queries to log configuration --- README.md | 4 +++- lib/prosopite.rb | 20 +++++++++++++++--- test/test_queries.rb | 48 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index cd9e1a1..c160b6c 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,9 @@ Or install it yourself as: The preferred type of notifications can be configured with: -* `Prosopite.min_n_queries`: Minimum number of N queries to report per N+1 case. Defaults to 2. +* `Prosopite.min_n_queries`: Minimum number of N queries required to report per N+1 case. Defaults to 2. +* `Prosopite.display_max_n_queries`: Maximum number of queries to log per N+1 case. Defaults to all queries. Note that this does not change whether an N+1 is reported or not, only how many queries are logged. +* `Prosopite.display_query_count = true`: Include the count of queries for each N+1 case logged. * `Prosopite.raise = true`: Raise warnings as exceptions * `Prosopite.rails_logger = true`: Send warnings to the Rails log * `Prosopite.prosopite_logger = true`: Send warnings to `log/prosopite.log` diff --git a/lib/prosopite.rb b/lib/prosopite.rb index 5dee267..2db683e 100644 --- a/lib/prosopite.rb +++ b/lib/prosopite.rb @@ -18,7 +18,9 @@ class << self attr_accessor :allow_stack_paths, :ignore_queries, - :min_n_queries + :min_n_queries, + :display_max_n_queries, + :display_query_count def allow_list=(value) puts "Prosopite.allow_list= is deprecated. Use Prosopite.allow_stack_paths= instead." @@ -217,9 +219,21 @@ def send_notifications notifications_str = '' tc[:prosopite_notifications].each do |queries, kaller| - notifications_str << "N+1 queries detected:\n" + notifications_str << + if @display_query_count + "N+1 queries detected (#{queries.count}):\n" + else + "N+1 queries detected:\n" + end + + queries_to_display = + if @display_max_n_queries + queries.take(@display_max_n_queries) + else + queries + end - queries.each { |q| notifications_str << " #{q}\n" } + queries_to_display.each { |q| notifications_str << " #{q}\n" } notifications_str << "Call stack:\n" kaller = backtrace_cleaner.clean(kaller) diff --git a/test/test_queries.rb b/test/test_queries.rb index cf477fc..138cd3c 100644 --- a/test/test_queries.rb +++ b/test/test_queries.rb @@ -432,6 +432,54 @@ def test_min_n_queries Prosopite.min_n_queries = 2 end + def test_display_max_n_queries + log_output = StringIO.new + Prosopite.custom_logger = Logger.new(log_output) + Prosopite.display_max_n_queries = 3 + + # 20 chairs, 4 legs each + chairs = create_list(:chair, 20) + chairs.each { |c| create_list(:leg, 4, chair: c) } + + Prosopite.scan + Chair.last(20).each do |c| + c.legs.first + end + + assert_n_plus_one + + log_lines = log_output.string.split("\n").map(&:strip) + repeated_query = + %(SELECT "legs".* FROM "legs" WHERE "legs"."chair_id" = ? ORDER BY "legs"."id" ASC LIMIT ?) + + assert_equal log_lines.count(repeated_query), 3 + ensure + Prosopite.custom_logger = nil + Prosopite.display_max_n_queries = nil + end + + def test_display_query_count + log_output = StringIO.new + Prosopite.custom_logger = Logger.new(log_output) + Prosopite.display_query_count = true + + # 20 chairs, 4 legs each + chairs = create_list(:chair, 20) + chairs.each { |c| create_list(:leg, 4, chair: c) } + + Prosopite.scan + Chair.last(20).each do |c| + c.legs.first + end + + assert_n_plus_one + + assert_includes log_output.string, 'N+1 queries detected (20)' + ensure + Prosopite.custom_logger = nil + Prosopite.display_query_count = false + end + private def assert_n_plus_one assert_raises(Prosopite::NPlusOneQueriesError) do From 8e5a2245b2b4812513394a0c2ca02ccbbc4c205c Mon Sep 17 00:00:00 2001 From: Emil Ong Date: Fri, 8 Nov 2024 15:22:23 -0800 Subject: [PATCH 2/2] Reduce README diff --- README.md | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index cdb0815..0ec8a4f 100644 --- a/README.md +++ b/README.md @@ -115,16 +115,16 @@ Or install it yourself as: The preferred type of notifications can be configured with: -- `Prosopite.display_max_n_queries`: Maximum number of queries to log per N+1 case. Defaults to all queries. Note that this does not change whether an N+1 is reported or not, only how many queries are logged. -- `Prosopite.display_query_count = true`: Include the count of queries for each N+1 case logged. -- `Prosopite.min_n_queries`: Minimum number of N queries to report per N+1 case. Defaults to 2. -- `Prosopite.raise = true`: Raise warnings as exceptions. Defaults to `false`. -- `Prosopite.rails_logger = true`: Send warnings to the Rails log. Defaults to `false`. -- `Prosopite.prosopite_logger = true`: Send warnings to `log/prosopite.log`. Defaults to `false`. -- `Prosopite.stderr_logger = true`: Send warnings to STDERR. Defaults to `false`. -- `Prosopite.backtrace_cleaner = my_custom_backtrace_cleaner`: use a different [ActiveSupport::BacktraceCleaner](https://api.rubyonrails.org/classes/ActiveSupport/BacktraceCleaner.html). Defaults to `Rails.backtrace_cleaner`. -- `Prosopite.custom_logger = my_custom_logger`: Set a custom logger. See the following section for the details. Defaults to `false`. -- `Prosopite.enabled = true`: Enables or disables the gem. Defaults to `true`. +* `Prosopite.display_max_n_queries`: Maximum number of queries to log per N+1 case. Defaults to all queries. Note that this does not change whether an N+1 is reported or not, only how many queries are logged. +* `Prosopite.display_query_count = true`: Include the count of queries for each N+1 case logged. +* `Prosopite.min_n_queries`: Minimum number of N queries to report per N+1 case. Defaults to 2. +* `Prosopite.raise = true`: Raise warnings as exceptions. Defaults to `false`. +* `Prosopite.rails_logger = true`: Send warnings to the Rails log. Defaults to `false`. +* `Prosopite.prosopite_logger = true`: Send warnings to `log/prosopite.log`. Defaults to `false`. +* `Prosopite.stderr_logger = true`: Send warnings to STDERR. Defaults to `false`. +* `Prosopite.backtrace_cleaner = my_custom_backtrace_cleaner`: use a different [ActiveSupport::BacktraceCleaner](https://api.rubyonrails.org/classes/ActiveSupport/BacktraceCleaner.html). Defaults to `Rails.backtrace_cleaner`. +* `Prosopite.custom_logger = my_custom_logger`: Set a custom logger. See the following section for the details. Defaults to `false`. +* `Prosopite.enabled = true`: Enables or disables the gem. Defaults to `true`. ### Custom Logging Configuration @@ -167,7 +167,6 @@ class ApplicationController < ActionController::Base end end ``` - And the preferred notification channel should be configured: ```ruby @@ -224,7 +223,6 @@ end ``` ### Sidekiq - We also provide a middleware for sidekiq `6.5.0+` so that you can auto detect n+1 queries that may occur in a sidekiq job. You just need to add the following to your sidekiq initializer. @@ -240,7 +238,6 @@ end ``` For applications running sidekiq < `6.5.0` but want to add the snippet, you can guard the snippet with something like this and remove it once you upgrade sidekiq: - ```ruby if Sidekiq::VERSION >= '6.5.0' && (Rails.env.development? || Rails.env.test?) .....