From daaf5fec9e55a26c97a18291dbc3d5c1b343a831 Mon Sep 17 00:00:00 2001 From: Emanuele Palazzetti Date: Fri, 26 Jan 2018 23:50:06 +0100 Subject: [PATCH] [active_record] decouple ActiveRecord and Sinatra integrations (#330) * [active_record] use internal tracer instead of coupling the implementation to Sinatra * [active_record] update the app name for the service details * [active_record] add standalone test for ActiveRecord --- Appraisals | 3 ++ Rakefile | 3 ++ gemfiles/contrib.gemfile | 1 + gemfiles/contrib_old.gemfile | 4 ++- lib/ddtrace/contrib/active_record/patcher.rb | 9 ++--- spec/ddtrace/contrib/active_record/app.rb | 36 +++++++++++++++++++ .../contrib/active_record/tracer_spec.rb | 36 +++++++++++++++++++ 7 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 spec/ddtrace/contrib/active_record/app.rb create mode 100644 spec/ddtrace/contrib/active_record/tracer_spec.rb diff --git a/Appraisals b/Appraisals index e8337ad262..4a6885ec2a 100644 --- a/Appraisals +++ b/Appraisals @@ -129,6 +129,7 @@ if RUBY_VERSION >= '2.2.2' && RUBY_PLATFORM != 'java' gem 'dalli' gem 'resque', '< 2.0' gem 'racecar', '>= 0.3.5' + gem 'mysql2', platform: :ruby end else appraise 'contrib-old' do @@ -146,5 +147,7 @@ else gem 'sucker_punch' gem 'dalli' gem 'resque', '< 2.0' + gem 'mysql2', '0.3.21', platform: :ruby + gem 'activerecord-mysql-adapter', platform: :ruby end end diff --git a/Rakefile b/Rakefile index 5363a97f86..9deebd726e 100644 --- a/Rakefile +++ b/Rakefile @@ -53,6 +53,7 @@ namespace :spec do :mongodb, :racecar, :resque, + :active_record, :dalli ].each do |contrib| RSpec::Core::RakeTask.new(contrib) do |t| @@ -220,9 +221,11 @@ task :ci do sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake test:sucker_punch' sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake test:resque' # RSpec + sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:active_record' sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:dalli' sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake spec:racecar' sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:dalli' + sh 'rvm $MRI_OLD_VERSIONS --verbose do appraisal contrib-old rake spec:active_record' when 2 sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake test:sidekiq' sh 'rvm $SIDEKIQ_OLD_VERSIONS --verbose do appraisal contrib-old rake test:sidekiq' diff --git a/gemfiles/contrib.gemfile b/gemfiles/contrib.gemfile index e48968883c..72b1c3adda 100644 --- a/gemfiles/contrib.gemfile +++ b/gemfiles/contrib.gemfile @@ -19,5 +19,6 @@ gem "sucker_punch" gem "dalli" gem "resque", "< 2.0" gem "racecar", ">= 0.3.5" +gem "mysql2", platform: :ruby gemspec path: "../" diff --git a/gemfiles/contrib_old.gemfile b/gemfiles/contrib_old.gemfile index 2cb8cc5f83..d29710d571 100644 --- a/gemfiles/contrib_old.gemfile +++ b/gemfiles/contrib_old.gemfile @@ -4,7 +4,7 @@ source "https://rubygems.org" gem "pry-nav", git: "https://github.com/nixme/pry-nav.git", branch: "master" gem "elasticsearch-transport" -gem "mongo" +gem "mongo", "< 2.5" gem "redis", "< 4.0" gem "hiredis" gem "rack", "1.4.7" @@ -17,5 +17,7 @@ gem "aws-sdk", "~> 2.0" gem "sucker_punch" gem "dalli" gem "resque", "< 2.0" +gem "mysql2", "0.3.21", platform: :ruby +gem "activerecord-mysql-adapter", platform: :ruby gemspec path: "../" diff --git a/lib/ddtrace/contrib/active_record/patcher.rb b/lib/ddtrace/contrib/active_record/patcher.rb index 0d1979c186..4011bd499a 100644 --- a/lib/ddtrace/contrib/active_record/patcher.rb +++ b/lib/ddtrace/contrib/active_record/patcher.rb @@ -6,6 +6,7 @@ module Patcher include Base register_as :active_record, auto_patch: false option :service_name + option :tracer, default: Datadog.tracer @patched = false @@ -57,22 +58,18 @@ def self.adapter_port @adapter_port ||= Datadog::Contrib::Rails::Utils.adapter_port end - def self.tracer - @tracer ||= Datadog.configuration[:sinatra][:tracer] - end - def self.database_service return @database_service if defined?(@database_service) @database_service = get_option(:service_name) || adapter_name - tracer.set_service_info(@database_service, 'sinatra', Ext::AppTypes::DB) + get_option(:tracer).set_service_info(@database_service, 'active_record', Ext::AppTypes::DB) @database_service end def self.sql(_name, start, finish, _id, payload) span_type = Datadog::Ext::SQL::TYPE - span = tracer.trace( + span = get_option(:tracer).trace( "#{adapter_name}.query", resource: payload.fetch(:sql), service: database_service, diff --git a/spec/ddtrace/contrib/active_record/app.rb b/spec/ddtrace/contrib/active_record/app.rb new file mode 100644 index 0000000000..ccbe4e96a7 --- /dev/null +++ b/spec/ddtrace/contrib/active_record/app.rb @@ -0,0 +1,36 @@ +require 'active_record' +require 'mysql2' + +logger = Logger.new(STDOUT) +logger.level = Logger::INFO + +# connecting to any kind of database is enough to test the integration +ActiveRecord::Base.establish_connection('mysql2://root:root@127.0.0.1:53306/mysql') + +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end + +class Article < ApplicationRecord +end + +# check if the migration has been executed +# MySQL JDBC drivers require that, otherwise we get a +# "Table '?' already exists" error +begin + Article.count() +rescue ActiveRecord::StatementInvalid + logger.info 'Executing database migrations' + ActiveRecord::Schema.define(version: 20161003090450) do + create_table 'articles', force: :cascade do |t| + t.string 'title' + t.datetime 'created_at', null: false + t.datetime 'updated_at', null: false + end + end +else + logger.info 'Database already exists; nothing to do' +end + +# force an access to prevent extra spans during tests +Article.count() diff --git a/spec/ddtrace/contrib/active_record/tracer_spec.rb b/spec/ddtrace/contrib/active_record/tracer_spec.rb new file mode 100644 index 0000000000..357b8587cd --- /dev/null +++ b/spec/ddtrace/contrib/active_record/tracer_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' +require 'ddtrace' + +require_relative 'app' + +RSpec.describe 'ActiveRecord instrumentation' do + let(:tracer) { ::Datadog::Tracer.new(writer: FauxWriter.new) } + + before(:each) do + Datadog.configure do |c| + c.use :active_record, tracer: tracer + end + end + + it 'calls the instrumentation when is used standalone' do + Article.count + spans = tracer.writer.spans + services = tracer.writer.services + + # expect service and trace is sent + expect(spans.size).to eq(1) + expect(services['mysql2']).to eq({'app'=>'active_record', 'app_type'=>'db'}) + + span = spans[0] + expect(span.service).to eq('mysql2') + expect(span.name).to eq('mysql2.query') + expect(span.span_type).to eq('sql') + expect(span.resource.strip).to eq('SELECT COUNT(*) FROM `articles`') + expect(span.get_tag('active_record.db.vendor')).to eq('mysql2') + expect(span.get_tag('active_record.db.name')).to eq('mysql') + expect(span.get_tag('active_record.db.cached')).to eq(nil) + expect(span.get_tag('out.host')).to eq('127.0.0.1') + expect(span.get_tag('out.port')).to eq('53306') + expect(span.get_tag('sql.query')).to eq(nil) + end +end