Skip to content

Commit

Permalink
fix!: activerecord find_by_sql spans on Rails 7.0+ (#1184)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
Co-authored-by: Kayla Reopelle <[email protected]>
  • Loading branch information
3 people authored Oct 22, 2024
1 parent e3bd524 commit 392b35e
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 10 additions & 1 deletion instrumentation/active_record/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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

Expand Down

0 comments on commit 392b35e

Please sign in to comment.