Merge pull request rails#51965 from rails/fxn/trx-sql.active_record

Include the current transaction in sql.active_record event payloads

Author: fxn

activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb

@@ -277,6 +277,7 @@ def cache_notification_info(sql, name, binds)
             type_casted_binds: -> { type_casted_binds(binds) },
             name: name,
             connection: self,
+            transaction: current_transaction,
             cached: true
           }
         end

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -1118,6 +1118,7 @@ def log(sql, name = "SQL", binds = [], type_casted_binds = [], statement_name =
             statement_name:    statement_name,
             async:             async,
             connection:        self,
+            transaction:       current_transaction,
             row_count:         0,
             &block
           )

activerecord/test/cases/instrumentation_test.rb

@@ -170,5 +170,36 @@ def test_payload_connection_with_query_cache_enabled
     ensure
       ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
     end
+
+    def test_payload_with_implicit_transaction
+      expected_transaction = Book.current_transaction
+
+      subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event|
+        if event.payload[:name] == "Book Count"
+          assert_same expected_transaction, event.payload[:transaction]
+        end
+      end
+
+      Book.count
+    ensure
+      ActiveSupport::Notifications.unsubscribe(subscriber)
+    end
+
+    def test_payload_with_explicit_transaction
+      expected_transaction = nil
+
+      subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event|
+        if event.payload[:name] == "Book Load"
+          assert_same expected_transaction, event.payload[:transaction]
+        end
+      end
+
+      Book.transaction do |transaction|
+        expected_transaction = transaction
+        Book.first
+      end
+    ensure
+      ActiveSupport::Notifications.unsubscribe(subscriber)
+    end
   end
 end

activerecord/test/cases/query_cache_test.rb

@@ -1067,4 +1067,40 @@ def test_query_cache_lru_eviction
 
     assert_equal @connection_1, @connection_2
   end
+
+  test "payload with implicit transaction" do
+    expected_transaction = Task.current_transaction
+
+    subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event|
+      if event.payload[:cached]
+        assert_same expected_transaction, event.payload[:transaction]
+      end
+    end
+
+    Task.cache do
+      2.times { Task.count }
+    end
+  ensure
+    ActiveSupport::Notifications.unsubscribe(subscriber)
+  end
+
+  test "payload with explicit transaction" do
+    expected_transaction = nil
+
+    subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |event|
+      if event.payload[:cached]
+        assert_same expected_transaction, event.payload[:transaction]
+      end
+    end
+
+    Task.transaction do |transaction|
+      expected_transaction = transaction
+
+      Task.cache do
+        2.times { Task.count }
+      end
+    end
+  ensure
+    ActiveSupport::Notifications.unsubscribe(subscriber)
+  end
 end

guides/source/active_support_instrumentation.md

@@ -360,6 +360,7 @@ The `:cache_hits` key is only included if the collection is rendered with `cache
 | `:sql`               | SQL statement                            |
 | `:name`              | Name of the operation                    |
 | `:connection`        | Connection object                        |
+| `:transaction`       | Current transaction                      |
 | `:binds`             | Bind parameters                          |
 | `:type_casted_binds` | Typecasted bind parameters               |
 | `:statement_name`    | SQL Statement name                       |
@@ -374,13 +375,18 @@ Adapters may add their own data as well.
   sql: "SELECT \"posts\".* FROM \"posts\" ",
   name: "Post Load",
   connection: <ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x00007f9f7a838850>,
+  transaction: <ActiveRecord::ConnectionAdapters::RealTransaction:0x0000000121b5d3e0>
   binds: [<ActiveModel::Attribute::WithCastValue:0x00007fe19d15dc00>],
   type_casted_binds: [11],
   statement_name: nil,
   row_count: 5
 }
 ```
 
+If there is no transaction started at the moment, `:transaction` has a null
+object with UUID `00000000-0000-0000-0000-000000000000` (the nil UUID). This may
+happen, for example, issuing a `SELECT` not wrapped in a transaction.
+
 #### `strict_loading_violation.active_record`
 
 This event is only emitted when [`config.active_record.action_on_strict_loading_violation`][] is set to `:log`.