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

[delayed_job] integration #444

Merged
merged 26 commits into from
Jul 16, 2018
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f1f4519
Delayed Job initial implementation
pawelchcki May 24, 2018
78ffd20
Working deplayed_job tracing
pawelchcki May 25, 2018
064a060
Base for delayed_job tests
pawelchcki Jun 5, 2018
b02ec5c
cleanup unnecessary trace setup code
pawelchcki Jun 11, 2018
6ed0c48
Update rakefile for delayed_job
pawelchcki Jun 11, 2018
6aa98cb
Add all necessary appraisals
pawelchcki Jun 11, 2018
c084b13
add delayed_job patcher tests
pawelchcki Jun 11, 2018
0162b04
Plugin spec
pawelchcki Jun 11, 2018
eca5ffc
add delayed job specs
pawelchcki Jun 12, 2018
e48c1d4
Rename test suite
pawelchcki Jun 12, 2018
792b9d4
Add class level comment in DelayedJob::Plugin
pawelchcki Jun 12, 2018
dc7156b
updated GettingStarted
pawelchcki Jun 12, 2018
b6a05a7
cleanup
pawelchcki Jun 12, 2018
767d697
fix tests running in 1.9.3
pawelchcki Jun 12, 2018
a9fc131
Flush on execution end + tests
pawelchcki Jun 26, 2018
f9898f8
use configuration instead of pin
pawelchcki Jun 28, 2018
a2f678e
remove useless comment
pawelchcki Jun 28, 2018
2e611dd
cleanup app initialization since we're using SqLite
pawelchcki Jun 28, 2018
466a79a
use rspec helper to setup active_record and schema
pawelchcki Jun 28, 2018
9073be2
Remove pin from delayedjob worker
pawelchcki Jun 29, 2018
a78f501
Add delayed job links
pawelchcki Jun 29, 2018
b56c1c7
Rename active_Record_setup to delayed_job_active_record
pawelchcki Jun 29, 2018
4ea9e6d
add missing specification
pawelchcki Jul 11, 2018
f80fe76
fix tests
pawelchcki Jul 11, 2018
ff17388
use only mocked classes to test delayed_job
pawelchcki Jul 16, 2018
48e1b79
remove unpatch methods
pawelchcki Jul 16, 2018
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
12 changes: 12 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ elsif Gem::Version.new('1.9.3') <= Gem::Version.new(RUBY_VERSION) \
gem 'activerecord-mysql-adapter', platform: :ruby
gem 'aws-sdk', '~> 2.0'
gem 'dalli'
gem 'delayed_job'
gem 'delayed_job_active_record'
gem 'elasticsearch-transport'
gem 'excon'
gem 'hiredis'
Expand Down Expand Up @@ -142,6 +144,8 @@ elsif Gem::Version.new('2.0.0') <= Gem::Version.new(RUBY_VERSION) \
gem 'activerecord-mysql-adapter', platform: :ruby
gem 'aws-sdk', '~> 2.0'
gem 'dalli'
gem 'delayed_job'
gem 'delayed_job_active_record'
gem 'elasticsearch-transport'
gem 'excon'
gem 'hiredis'
Expand Down Expand Up @@ -242,6 +246,8 @@ elsif Gem::Version.new('2.1.0') <= Gem::Version.new(RUBY_VERSION) \
gem 'activerecord-mysql-adapter', platform: :ruby
gem 'aws-sdk', '~> 2.0'
gem 'dalli'
gem 'delayed_job'
gem 'delayed_job_active_record'
gem 'elasticsearch-transport'
gem 'excon'
gem 'hiredis'
Expand Down Expand Up @@ -373,6 +379,8 @@ elsif Gem::Version.new('2.2.0') <= Gem::Version.new(RUBY_VERSION) \
gem 'activerecord', '< 5.1.5'
gem 'aws-sdk'
gem 'dalli'
gem 'delayed_job'
gem 'delayed_job_active_record'
gem 'elasticsearch-transport'
gem 'excon'
gem 'grape'
Expand Down Expand Up @@ -507,6 +515,8 @@ elsif Gem::Version.new('2.3.0') <= Gem::Version.new(RUBY_VERSION) \
gem 'activerecord', '< 5.1.5'
gem 'aws-sdk'
gem 'dalli'
gem 'delayed_job'
gem 'delayed_job_active_record'
gem 'elasticsearch-transport'
gem 'excon'
gem 'grape'
Expand Down Expand Up @@ -535,6 +545,8 @@ elsif Gem::Version.new('2.4.0') <= Gem::Version.new(RUBY_VERSION)
gem 'activerecord', '< 5.1.5'
gem 'aws-sdk'
gem 'dalli'
gem 'delayed_job'
gem 'delayed_job_active_record'
gem 'elasticsearch-transport'
gem 'excon'
gem 'grape'
Expand Down
7 changes: 7 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace :spec do
[
:active_model_serializers,
:active_record,
:delayed_job,
:active_support,
:aws,
:dalli,
Expand Down Expand Up @@ -220,6 +221,7 @@ task :ci do
# Contrib specs
sh 'bundle exec appraisal contrib-old rake spec:active_model_serializers'
sh 'bundle exec appraisal contrib-old rake spec:active_record'
sh 'bundle exec appraisal contrib-old rake spec:delayed_job'
sh 'bundle exec appraisal contrib-old rake spec:active_support'
sh 'bundle exec appraisal contrib-old rake spec:dalli'
sh 'bundle exec appraisal contrib-old rake spec:excon'
Expand Down Expand Up @@ -260,6 +262,7 @@ task :ci do
# Contrib specs
sh 'bundle exec appraisal contrib-old rake spec:active_model_serializers'
sh 'bundle exec appraisal contrib-old rake spec:active_record'
sh 'bundle exec appraisal contrib-old rake spec:delayed_job'
sh 'bundle exec appraisal contrib-old rake spec:active_support'
sh 'bundle exec appraisal contrib-old rake spec:dalli'
sh 'bundle exec appraisal contrib-old rake spec:excon'
Expand Down Expand Up @@ -303,6 +306,7 @@ task :ci do
# Contrib specs
sh 'bundle exec appraisal contrib-old rake spec:active_model_serializers'
sh 'bundle exec appraisal contrib-old rake spec:active_record'
sh 'bundle exec appraisal contrib-old rake spec:delayed_job'
sh 'bundle exec appraisal contrib-old rake spec:active_support'
sh 'bundle exec appraisal contrib-old rake spec:dalli'
sh 'bundle exec appraisal contrib-old rake spec:excon'
Expand Down Expand Up @@ -352,6 +356,7 @@ task :ci do
# Contrib specs
sh 'bundle exec appraisal contrib rake spec:active_model_serializers'
sh 'bundle exec appraisal contrib rake spec:active_record'
sh 'bundle exec appraisal contrib rake spec:delayed_job'
sh 'bundle exec appraisal contrib rake spec:active_support'
sh 'bundle exec appraisal contrib rake spec:dalli'
sh 'bundle exec appraisal contrib rake spec:excon'
Expand Down Expand Up @@ -412,6 +417,7 @@ task :ci do
# Contrib specs
sh 'bundle exec appraisal contrib rake spec:active_model_serializers'
sh 'bundle exec appraisal contrib rake spec:active_record'
sh 'bundle exec appraisal contrib rake spec:delayed_job'
sh 'bundle exec appraisal contrib rake spec:active_support'
sh 'bundle exec appraisal contrib rake spec:dalli'
sh 'bundle exec appraisal contrib rake spec:excon'
Expand Down Expand Up @@ -471,6 +477,7 @@ task :ci do
# Contrib specs
sh 'bundle exec appraisal contrib rake spec:active_model_serializers'
sh 'bundle exec appraisal contrib rake spec:active_record'
sh 'bundle exec appraisal contrib rake spec:delayed_job'
sh 'bundle exec appraisal contrib rake spec:active_support'
sh 'bundle exec appraisal contrib rake spec:dalli'
sh 'bundle exec appraisal contrib rake spec:excon'
Expand Down
23 changes: 23 additions & 0 deletions docs/GettingStarted.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ For descriptions of terminology used in APM, take a look at the [official docume
- [Active Record](#active-record)
- [AWS](#aws)
- [Dalli](#dalli)
- [DelayedJob](#delayedjob)
- [Elastic Search](#elastic-search)
- [Excon](#excon)
- [Faraday](#faraday)
Expand Down Expand Up @@ -247,6 +248,7 @@ For a list of available integrations, and their configuration options, please re
| Active Record | `active_record` | `>= 3.2, < 5.2` | *[Link](#active-record)* | *[Link](https://github.com/rails/rails/tree/master/activerecord)* |
| AWS | `aws` | `>= 2.0` | *[Link](#aws)* | *[Link](https://github.com/aws/aws-sdk-ruby)* |
| Dalli | `dalli` | `>= 2.7` | *[Link](#dalli)* | *[Link](https://github.com/petergoldstein/dalli)* |
| DelayedJob | `delayed_job` | `>= 4.1` | *[Link](#delayedjob)* | *[Link](https://github.com/collectiveidea/delayed_job)* |
| Elastic Search | `elasticsearch` | `>= 6.0` | *[Link](#elastic-search)* | *[Link](https://github.com/elastic/elasticsearch-ruby)* |
| Excon | `excon` | `>= 0.62` | *[Link](#excon)* | *[Link](https://github.com/excon/excon)* |
| Faraday | `faraday` | `>= 0.14` | *[Link](#faraday)* | *[Link](https://github.com/lostisland/faraday)* |
Expand Down Expand Up @@ -952,6 +954,27 @@ Where `options` is an optional `Hash` that accepts the following parameters:
| --- | --- | --- |
| ``service_name`` | Service name used for `sidekiq` instrumentation | sidekiq |

### DelayedJob
Copy link
Contributor

Choose a reason for hiding this comment

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

We're missing important documentation; this section needs to be added to the list of links above, and the table of supported integrations before that, with appropriate details and links to resources wherever required.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this has since been added; thanks!


The DelayedJob integration uses lifecycle hooks to trace the job executions.

You can enable it through `Datadog.configure`:

```ruby
require 'ddtrace'

Datadog.configure do |c|
c.use :delayed_job, options
end
```

Where `options` is an optional `Hash` that accepts the following parameters:

| Key | Description | Default |
| --- | --- | --- |
| ``service_name`` | Service name used for `DelayedJob` instrumentation | delayed_job |
| ``tracer`` | A ``Datadog::Tracer`` instance used to instrument the application. Usually you don't need to set that. | ``Datadog.tracer`` |

### Sinatra

The Sinatra integration traces requests and template rendering.
Expand Down
1 change: 1 addition & 0 deletions lib/ddtrace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ def configure(target = configuration, opts = {})
require 'ddtrace/contrib/dalli/patcher'
require 'ddtrace/contrib/rake/patcher'
require 'ddtrace/contrib/resque/patcher'
require 'ddtrace/contrib/delayed_job/patcher'
require 'ddtrace/contrib/racecar/patcher'
require 'ddtrace/contrib/sidekiq/patcher'
require 'ddtrace/contrib/excon/patcher'
Expand Down
45 changes: 45 additions & 0 deletions lib/ddtrace/contrib/delayed_job/patcher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
module Datadog
module Contrib
module DelayedJob
# DelayedJob integration
module Patcher
include Base
register_as :delayed_job

option :service_name, default: 'delayed_job'.freeze
option :tracer, default: Datadog.tracer

@patched = false

class << self
def patch
return @patched if patched? || !defined?(::Delayed)

require 'ddtrace/ext/app_types'
require_relative 'plugin'

add_instrumentation(::Delayed::Worker)
@patched = true
rescue => e
Tracer.log.error("Unable to apply DelayedJob integration: #{e}")
@patched
end

def patched?
@patched
end

private

def unpatch
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider that this patch is not actually "unpatchable". We should not communicate the expectation that you can "undo" patches, or that you should be able to "undo" patches, since modifying classes and constants is generally irreversible. Trying to support this would be useful for tests, but only for tests; if this kind of feature only needs to exist for tests, then in should exist in the test suite, not in our patcher API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point

@patched = false
end

def add_instrumentation(klass)
klass.plugins << Plugin
end
end
end
end
end
end
43 changes: 43 additions & 0 deletions lib/ddtrace/contrib/delayed_job/plugin.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require 'delayed/plugin'

module Datadog
module Contrib
module DelayedJob
# DelayedJob plugin that instruments invoke_job hook
class Plugin < Delayed::Plugin
def self.instrument(job, &block)
return block.call(job) unless tracer && tracer.enabled

tracer.trace('delayed_job'.freeze, service: configuration[:service_name], resource: job.name) do |span|
span.set_tag('delayed_job.id'.freeze, job.id)
span.set_tag('delayed_job.queue'.freeze, job.queue) if job.queue
span.set_tag('delayed_job.priority'.freeze, job.priority)
span.set_tag('delayed_job.attempts'.freeze, job.attempts)
span.span_type = Ext::AppTypes::WORKER

yield job
end
end

def self.flush(worker, &block)
yield worker

tracer.shutdown! if tracer && tracer.enabled
end

def self.configuration
Datadog.configuration[:delayed_job]
end

def self.tracer
configuration[:tracer]
end

callbacks do |lifecycle|
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice way to hook into the framework!

lifecycle.around(:invoke_job, &method(:instrument))
lifecycle.around(:execute, &method(:flush))
end
end
end
end
end
24 changes: 24 additions & 0 deletions spec/ddtrace/contrib/delayed_job/delayed_job_active_record.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
RSpec.configure do |c|
c.around(:example, :delayed_job_active_record) do |example|
Copy link
Contributor

Choose a reason for hiding this comment

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

Like the use of a trait here. 👍

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Schema.define(version: 2018_05_25_114131) do
create_table 'delayed_jobs', force: :cascade do |t|
t.integer 'priority', default: 0, null: false
t.integer 'attempts', default: 0, null: false
t.text 'handler', null: false
t.text 'last_error'
t.datetime 'run_at'
t.datetime 'locked_at'
t.datetime 'failed_at'
t.string 'locked_by'
t.string 'queue'
t.datetime 'created_at'
t.datetime 'updated_at'
end
end

example.run

ActiveRecord::Base.connection.close
end
end
32 changes: 32 additions & 0 deletions spec/ddtrace/contrib/delayed_job/patcher_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
require 'spec_helper'
require 'ddtrace'
require_relative 'delayed_job_active_record'

RSpec.describe Datadog::Contrib::DelayedJob::Patcher, :delayed_job_active_record do
Copy link
Contributor

Choose a reason for hiding this comment

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

I dig the use of the trait here to drive test configuration, but I think we still need to update the active_record_setup.rb file to match, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

describe '.patch' do
let(:worker_plugins) { [] }
let!(:delayed_worker_class) { class_double('Delayed::Worker', plugins: worker_plugins).as_stubbed_const }

before do
described_class.send(:unpatch)
end

context 'when delayed job is not present' do
before do
hide_const('Delayed')
end

it "shouldn't patch the code" do
expect { described_class.patch }.not_to(change { described_class.patched? })
end
end

it 'should patch the code' do
expect { described_class.patch }.to change { described_class.patched? }.from(false).to(true)
end

it 'add plugin to worker class' do
expect { described_class.patch }.to change { worker_plugins.first }.to be_truthy
end
end
end
Loading