Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[active_record] decouple ActiveRecord and Sinatra integrations #330

Merged
merged 3 commits into from
Jan 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
3 changes: 3 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ namespace :spec do
:mongodb,
:racecar,
:resque,
:active_record,
:dalli
].each do |contrib|
RSpec::Core::RakeTask.new(contrib) do |t|
Expand Down Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions gemfiles/contrib.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ gem "sucker_punch"
gem "dalli"
gem "resque", "< 2.0"
gem "racecar", ">= 0.3.5"
gem "mysql2", platform: :ruby

gemspec path: "../"
4 changes: 3 additions & 1 deletion gemfiles/contrib_old.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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: "../"
9 changes: 3 additions & 6 deletions lib/ddtrace/contrib/active_record/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -57,22 +58,18 @@ def self.adapter_port
@adapter_port ||= Datadog::Contrib::Rails::Utils.adapter_port
end

def self.tracer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So normally as this is quite internal, nobody was using this. I let you decide wether it's worth mentioning in the release notes, but I'd say no.

@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,
Expand Down
36 changes: 36 additions & 0 deletions spec/ddtrace/contrib/active_record/app.rb
Original file line number Diff line number Diff line change
@@ -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()
36 changes: 36 additions & 0 deletions spec/ddtrace/contrib/active_record/tracer_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be considered a regression test, since undoing the patch doesn't make it pass

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