From 392b35e78eddef5970e39e13e4cfbcbbaceb6878 Mon Sep 17 00:00:00 2001
From: Albert Llop <mrsimo@gmail.com>
Date: Tue, 22 Oct 2024 22:24:15 +0200
Subject: [PATCH] fix!: activerecord find_by_sql spans on Rails 7.0+ (#1184)

* fix: activerecord find_by_sql spans on Rails 7.0+

* tests: fix create_table call

* tests: rubocop tweak

* feat: call spans that produce reads `query`

* Refactor patching to use define_method

---------

Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>
Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com>
---
 .../active_record/patches/querying.rb                |  8 +++++---
 .../active_record/patches/querying_test.rb           | 12 +++++++++---
 instrumentation/active_record/test/test_helper.rb    | 11 ++++++++++-
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/querying.rb b/instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/querying.rb
index 41280b1f1..094575ae4 100644
--- a/instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/querying.rb
+++ b/instrumentation/active_record/lib/opentelemetry/instrumentation/active_record/patches/querying.rb
@@ -18,9 +18,11 @@ class << base
 
           # Contains ActiveRecord::Querying to be patched
           module ClassMethods
-            def find_by_sql(...)
-              tracer.in_span("#{self}.find_by_sql") do
-                super
+            method_name = ::ActiveRecord.version >= Gem::Version.new('7.0.0') ? :_query_by_sql : :find_by_sql
+
+            define_method(method_name) do |*args, **kwargs|
+              tracer.in_span("#{self} query") do
+                super(*args, **kwargs)
               end
             end
 
diff --git a/instrumentation/active_record/test/instrumentation/active_record/patches/querying_test.rb b/instrumentation/active_record/test/instrumentation/active_record/patches/querying_test.rb
index 6da620512..879af50e6 100644
--- a/instrumentation/active_record/test/instrumentation/active_record/patches/querying_test.rb
+++ b/instrumentation/active_record/test/instrumentation/active_record/patches/querying_test.rb
@@ -15,12 +15,18 @@
 
   before { exporter.reset }
 
-  describe 'find_by_sql' do
+  describe 'query' do
     it 'traces' do
+      Account.create!
+
       User.find_by_sql('SELECT * FROM users')
+      Account.first.users.to_a
+
+      user_find_spans = spans.select { |s| s.name == 'User query' }
+      account_find_span = spans.find { |s| s.name == 'Account query' }
 
-      find_span = spans.find { |s| s.name == 'User.find_by_sql' }
-      _(find_span).wont_be_nil
+      _(user_find_spans.length).must_equal(2)
+      _(account_find_span).wont_be_nil
     end
   end
 end
diff --git a/instrumentation/active_record/test/test_helper.rb b/instrumentation/active_record/test/test_helper.rb
index 6cf6d2757..02751e7f6 100644
--- a/instrumentation/active_record/test/test_helper.rb
+++ b/instrumentation/active_record/test/test_helper.rb
@@ -34,8 +34,14 @@
   database: 'db/development.sqlite3'
 )
 
-# Create User model
+# Create ActiveRecord models
+class Account < ActiveRecord::Base
+  has_many :users
+end
+
 class User < ActiveRecord::Base
+  belongs_to :account
+
   validate :name_if_present
 
   scope :recently_created, -> { where('created_at > ?', Time.now - 3600) }
@@ -54,9 +60,12 @@ class SuperUser < ActiveRecord::Base; end
 # Simple migration to create a table to test against
 class CreateUserTable < ActiveRecord::Migration[migration_version]
   def change
+    create_table :accounts, &:timestamps
+
     create_table :users do |t|
       t.string 'name'
       t.integer 'counter'
+      t.references 'account'
       t.timestamps
     end