Skip to content

Commit

Permalink
Merge pull request #662 from DataDog/fix/active_record_event_connecti…
Browse files Browse the repository at this point in the history
…on_serialization

Fix ActiveRecord :connection incompatibility with serialization libraries
  • Loading branch information
delner authored Dec 20, 2018
2 parents 8189a7c + 601fd80 commit ce66688
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 83 deletions.
2 changes: 1 addition & 1 deletion lib/ddtrace/contrib/active_record/events/sql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def span_name
end

def process(span, event, _id, payload)
config = Utils.connection_config(payload[:connection])
config = Utils.connection_config(payload[:connection], payload[:connection_id])
settings = Datadog.configuration[:active_record, config]
adapter_name = Datadog::Utils::Database.normalize_vendor(config[:adapter])
service_name = if settings.service_name != Datadog::Utils::Database::VENDOR_DEFAULT
Expand Down
2 changes: 0 additions & 2 deletions lib/ddtrace/contrib/active_record/patcher.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'ddtrace/contrib/patcher'
require 'ddtrace/contrib/active_record/patches/abstract_adapter'
require 'ddtrace/contrib/active_record/events'

module Datadog
Expand All @@ -18,7 +17,6 @@ def patched?
def patch
do_once(:active_record) do
begin
::ActiveRecord::ConnectionAdapters::AbstractAdapter.send(:include, Patches::AbstractAdapter)
Events.subscribe!
rescue StandardError => e
Datadog::Tracer.log.error("Unable to apply Active Record integration: #{e}")
Expand Down
72 changes: 0 additions & 72 deletions lib/ddtrace/contrib/active_record/patches/abstract_adapter.rb

This file was deleted.

37 changes: 30 additions & 7 deletions lib/ddtrace/contrib/active_record/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,37 @@ def self.adapter_port
connection_config[:port]
end

def self.connection_config(connection = nil)
connection.nil? ? default_connection_config : connection_config_from_connection(connection)
end
# In newer Rails versions, the `payload` contains both the `connection` and its `object_id` named `connection_id`.
#
# So, if rails is recent we'll have a direct access to the connection.
# Else, we'll find it thanks to the passed `connection_id`.
#
# See this PR for more details: https://github.com/rails/rails/pull/34602
#
def self.connection_config(connection = nil, connection_id = nil)
return default_connection_config if connection.nil? && connection_id.nil?

conn = if !connection.nil?
connection
# Rails 3.0 - 3.2
elsif Gem.loaded_specs['activerecord'].version < Gem::Version.new('4.0')
::ActiveRecord::Base
.connection_handler
.connection_pools
.values
.flat_map(&:connections)
.find { |c| c.object_id == connection_id }
# Rails 4.2+
else
::ActiveRecord::Base
.connection_handler
.connection_pool_list
.flat_map(&:connections)
.find { |c| c.object_id == connection_id }
end

# Typical of ActiveSupport::Notifications `sql.active_record`
def self.connection_config_from_connection(connection)
if connection.instance_variable_defined?(:@config)
connection.instance_variable_get(:@config)
if conn.instance_variable_defined?(:@config)
conn.instance_variable_get(:@config)
else
EMPTY_CONFIG
end
Expand Down
4 changes: 3 additions & 1 deletion spec/ddtrace/contrib/sinatra/activerecord_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ def migrate_db
ActiveRecord::Base.establish_connection(
adapter: 'sqlite3',
database: ':memory:'
)
).tap do |conn|
conn.connection.execute('SELECT 42')
end
end

let(:sinatra_span) { spans.first }
Expand Down

0 comments on commit ce66688

Please sign in to comment.