-
Notifications
You must be signed in to change notification settings - Fork 375
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
Changes from 24 commits
f1f4519
78ffd20
064a060
b02ec5c
6ed0c48
6aa98cb
c084b13
0162b04
eca5ffc
e48c1d4
792b9d4
dc7156b
b6a05a7
767d697
a9fc131
f9898f8
a2f678e
2e611dd
466a79a
9073be2
a78f501
b56c1c7
4ea9e6d
f80fe76
ff17388
48e1b79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
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| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
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| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!