db explain plan refactor

lib/new_relic/agent/database.rb

@@ -90,6 +90,42 @@ def get_connection(config, &connector)
         ConnectionManager.instance.get_connection(config, &connector)
       end
 
+      def explain_this(statement, use_execute = false)
+        if supports_with_connection?
+          explain_this_using_with_connection(statement)
+        else
+          explain_this_using_adapter_connection(statement, use_execute)
+        end
+      rescue => e
+        NewRelic::Agent.logger.error("Couldn't fetch the explain plan for statement: #{e}")
+      end
+
+      def explain_this_using_with_connection(statement)
+        ::ActiveRecord::Base.with_connection do |conn|
+          conn.exec_query("EXPLAIN #{statement.sql}", "Explain #{statement.name}", statement.binds)
+        end
+      end
+
+      def explain_this_using_adapter_connection(statement, use_execute)
+        connection = get_connection(statement.config) do
+          ::ActiveRecord::Base.send(:"#{statement.config[:adapter]}_connection", statement.config)
+        end
+
+        if use_execute
+          connection.execute("EXPLAIN #{statement.sql}")
+        else
+          connection.exec_query("EXPLAIN #{statement.sql}", "Explain #{statement.name}", statement.binds)
+        end
+      end
+
+      # ActiveRecord v7.2.0 introduced with_connection
+      def supports_with_connection?
+        return @supports_with_connection if defined?(@supports_with_connection)
+
+        @supports_with_connection = defined?(::ActiveRecord::VERSION::STRING) &&
+          Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('7.2.0')
+      end
+
       def close_connections
         ConnectionManager.instance.close_connections
       end

lib/new_relic/agent/instrumentation/active_record.rb

@@ -9,14 +9,7 @@ module Agent
     module Instrumentation
       module ActiveRecord
         EXPLAINER = lambda do |statement|
-          connection = NewRelic::Agent::Database.get_connection(statement.config) do
-            ::ActiveRecord::Base.send("#{statement.config[:adapter]}_connection",
-              statement.config)
-          end
-          # the following line needs else branch coverage
-          if connection && connection.respond_to?(:execute) # rubocop:disable Style/SafeNavigation
-            return connection.execute("EXPLAIN #{statement.sql}")
-          end
+          NewRelic::Agent::Database.explain_this(statement, true)
         end
 
         def self.insert_instrumentation

lib/new_relic/agent/instrumentation/active_record_subscriber.rb

@@ -70,18 +70,7 @@ def finish(name, id, payload) # THREAD_LOCAL_ACCESS
         end
 
         def get_explain_plan(statement)
-          connection = NewRelic::Agent::Database.get_connection(statement.config) do
-            ::ActiveRecord::Base.send("#{statement.config[:adapter]}_connection",
-              statement.config)
-          end
-          # the following line needs else branch coverage
-          if connection && connection.respond_to?(:exec_query) # rubocop:disable Style/SafeNavigation
-            return connection.exec_query("EXPLAIN #{statement.sql}",
-              "Explain #{statement.name}",
-              statement.binds)
-          end
-        rescue => e
-          NewRelic::Agent.logger.debug("Couldn't fetch the explain plan for #{statement} due to #{e}")
+          NewRelic::Agent::Database.explain_this(statement)
         end
 
         def active_record_config(payload)

test/multiverse/suites/active_record_pg/active_record_test.rb

@@ -375,14 +375,15 @@ def test_metrics_for_direct_sql_other
   end
 
   def test_metrics_for_direct_sql_show
-    if supports_show_tables?
-      in_web_transaction do
-        conn = Order.connection
+    skip "Adapter does not support 'show tables'" unless supports_show_tables?
+
+    in_web_transaction do
+      Order.with_connection do |conn|
         conn.execute('show tables')
       end
-
-      assert_generic_rollup_metrics('show')
     end
+
+    assert_generic_rollup_metrics('show')
   end
 
   def test_still_records_metrics_in_error_cases
@@ -454,11 +455,10 @@ def test_gathers_explain_plans
 
       sample.prepare_to_send!
       explanations = sql_node.params[:explain_plan]
-      if supports_explain_plans?
-        refute_nil explanations, "No explains in node: #{sql_node}"
-        assert_equal(2, explanations.size,
-          "No explains in node: #{sql_node}")
-      end
+
+      refute_nil explanations, "No explains in node: #{sql_node}"
+      assert_equal(2, explanations.size,
+        "No explains in node: #{sql_node}")
     end
   end
 
@@ -593,11 +593,7 @@ def adapter
   end
 
   def supports_show_tables?
-    [:mysql, :postgres].include?(adapter)
-  end
-
-  def supports_explain_plans?
-    [:mysql, :postgres].include?(adapter)
+    [:mysql, :mysql2, :trilogy].include?(adapter)
   end
 
   def current_product

test/new_relic/agent/sql_sampler_test.rb

@@ -167,15 +167,17 @@ def test_harvest_should_aggregate_similar_queries
   end
 
   def test_harvest_should_collect_explain_plans
-    @connection.expects(:execute).with('EXPLAIN select * from test') \
-      .returns(dummy_mysql_explain_result({'header0' => 'foo0', 'header1' => 'foo1', 'header2' => 'foo2'}))
-    @connection.expects(:execute).with('EXPLAIN select * from test2') \
-      .returns(dummy_mysql_explain_result({'header0' => 'bar0', 'header1' => 'bar1', 'header2' => 'bar2'}))
-
     data = NewRelic::Agent::TransactionSqlData.new
     data.set_transaction_info('/c/a', 'guid')
     data.set_transaction_name('WebTransaction/Controller/c/a')
-    explainer = NewRelic::Agent::Instrumentation::ActiveRecord::EXPLAINER
+    explainer = proc do |statement|
+      hash = if statement.sql.match?('test2')
+        {'header0' => 'bar0', 'header1' => 'bar1', 'header2' => 'bar2'}
+      else
+        {'header0' => 'foo0', 'header1' => 'foo1', 'header2' => 'foo2'}
+      end
+      dummy_mysql_explain_result(hash)
+    end
     config = {:adapter => 'mysql'}
     queries = [
       NewRelic::Agent::SlowSql.new(NewRelic::Agent::Database::Statement.new('select * from test', config, explainer),

test/new_relic/agent/transaction/trace_node_test.rb

@@ -37,7 +37,7 @@ def test_to_s
     s.to_s
   end
 
-  def test_to_array
+  def test_explain_sql_raising_an_errorest_to_array
     parent = NewRelic::Agent::Transaction::TraceNode.new('Custom/test/parent', 1)
     parent.params[:test] = 'value'
     child = NewRelic::Agent::Transaction::TraceNode.new('Custom/test/child', 2)
@@ -290,12 +290,11 @@ def test_each_node_with_nest_tracking
 
   def test_explain_sql_raising_an_error
     s = NewRelic::Agent::Transaction::TraceNode.new('Custom/test/metric', Process.clock_gettime(Process::CLOCK_REALTIME))
-    config = {:adapter => 'mysql'}
     statement = NewRelic::Agent::Database::Statement.new('SELECT')
-    statement.config = config
+    statement.config = {:adapter => 'mysql'}
     statement.explainer = NewRelic::Agent::Instrumentation::ActiveRecord::EXPLAINER
     s.params = {:sql => statement}
-    NewRelic::Agent::Database.expects(:get_connection).with(config).raises(RuntimeError.new('whee'))
+    NewRelic::Agent::Database.expects(:explain_this).raises(RuntimeError.new('whee'))
     s.explain_sql
   end