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

add sidekiq integration #79

Merged
merged 1 commit into from
Mar 2, 2017
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 .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,6 @@ Metrics/CyclomaticComplexity:

Metrics/PerceivedComplexity:
Max: 15

Lint/UnusedMethodArgument:
Enabled: false
1 change: 1 addition & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,5 @@ appraise "contrib" do
gem "hiredis"
gem "rack-test"
gem "sinatra"
gem "sidekiq"
end
6 changes: 4 additions & 2 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require 'appraisal'
require 'yard'

namespace :test do
task all: [:main, :rails, :railsredis, :elasticsearch, :http, :redis, :sinatra, :monkey]
task all: [:main, :rails, :railsredis, :elasticsearch, :http, :redis, :sidekiq, :sinatra, :monkey]

Rake::TestTask.new(:main) do |t|
t.libs << %w(test lib)
Expand All @@ -30,7 +30,7 @@ namespace :test do
t.test_files = FileList['test/contrib/rails/**/*redis*_test.rb']
end

[:elasticsearch, :http, :redis, :sinatra].each do |contrib|
[:elasticsearch, :http, :redis, :sinatra, :sidekiq].each do |contrib|
Rake::TestTask.new(contrib) do |t|
t.libs << %w(test lib)
t.test_files = FileList["test/contrib/#{contrib}/*_test.rb"]
Expand All @@ -49,6 +49,7 @@ Rake::TestTask.new(:benchmark) do |t|
end

RuboCop::RakeTask.new(:rubocop) do |t|
t.options << ['-D']
t.patterns = ['lib/**/*.rb', 'test/**/*.rb', 'Gemfile', 'Rakefile']
end

Expand Down Expand Up @@ -114,6 +115,7 @@ task :ci do
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake test:http'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake test:redis'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake test:sinatra'
sh 'rvm $MRI_VERSIONS --verbose do appraisal contrib rake test:sidekiq'
when 2
sh 'rvm $RAILS_VERSIONS --verbose do appraisal rails3-postgres rake test:rails'
sh 'rvm $RAILS_VERSIONS --verbose do appraisal rails3-mysql2 rake test:rails'
Expand Down
45 changes: 45 additions & 0 deletions docs/GettingStarted.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,47 @@ Net::HTTP module.

content = Net::HTTP.get(URI('http://127.0.0.1/index.html'))

### Sidekiq

The Sidekiq integration is a server-side middleware which will trace job
executions. It can be added as any other Sidekiq middleware:

require 'sidekiq'
require 'ddtrace'
require 'ddtrace/contrib/sidekiq/tracer'

Sidekiq.configure_server do |config|
config.server_middleware do |chain|
chain.add(Datadog::Contrib::Sidekiq::Tracer, debug: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that the add method appends the middleware at the bottom of the chain right? don't we have a way to keep it as the first one? in this way we can trace other middlewares execution time. If we can't, a suggestion in the docs is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently the last added middlewares are executed first, and add() append at the end.

end
end

#### Configure the tracer

To modify the default configuration, simply pass arguments to the middleware.
For example, to change the default service name and activate the debug mode:

Sidekiq.configure_server do |config|
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I'm wondering what happen when we use Sidekiq with Rails. The current status is that if we install both Sidekiq and Rails, when Sidekiq is executed via bundle exec ... the Rails instrumentation is activated and Sidekiq Redis calls are traced (even the x seconds polling). Is it the expected behavior? I think that this middleware must be the only way to instrument the library so we may investigate and remove the side-effect of Rails instrumentation.

Also I was thinking about providing a good pattern (I think docs are enough) to instrument Sidekiq inside the Rails initializer so that our tracing configurations are all together. In this way Sidekiq is still separated from Rails instrumentation but users can easily follow our docs to share the Rails configuration with Sidekiq.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sidekiq uses the redis library, so its calls will be traced by the redis contrib. The sidekiq contrib traces job executions, so I think we're good.

config.server_middleware do |chain|
chain.add(Datadog::Contrib::Sidekiq::Tracer,
default_service: 'my_app', debug: true)
end
end

Available settings are:

* ``enabled``: define if the ``tracer`` is enabled or not. If set to
``false``, the code is still instrumented but no spans are sent to the local
trace agent.
* ``default_service``: set the service name used when tracing application
requests. Defaults to ``sidekiq``.
* ``tracer``: set the tracer to use. Usually you don't need to change that
value unless you're already using a different initialized tracer somewhere
else.
* ``debug``: set to ``true`` to enable debug logging.
* ``trace_agent_hostname``: set the hostname of the trace agent.
* ``trace_agent_port``: set the port the trace agent is listening on.

## Advanced usage

### Manual Instrumentation
Expand Down Expand Up @@ -360,6 +401,10 @@ The currently supported web server are:

Currently we are supporting Sinatra >= 1.4.0.

#### Sidekiq versions

Currently we are supporting Sidekiq >= 4.0.0.

### Glossary

* ``Service``: The name of a set of processes that do the same job. Some examples are ``datadog-web-app`` or ``datadog-metrics-db``.
Expand Down
54 changes: 54 additions & 0 deletions lib/ddtrace/contrib/sidekiq/tracer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@

require 'sidekiq/api'

require 'ddtrace/ext/app_types'

sidekiq_vs = Gem::Version.new(Sidekiq::VERSION)
sidekiq_min_vs = Gem::Version.new('4.0.0')
if sidekiq_vs < sidekiq_min_vs
raise "sidekiq version #{sidekiq_vs} is not supported yet " \
+ "(supporting versions >=#{sidekiq_min_vs})"
end

Datadog::Tracer.log.info("activating instrumentation for sidekiq #{sidekiq_vs}")

module Datadog
module Contrib
module Sidekiq
# Middleware is a Sidekiq server-side middleware which traces executed
# jobs.
class Tracer
def initialize(options)
@enabled = options.fetch(:enabled, true)
@default_service = options.fetch(:default_service, 'sidekiq')
@tracer = options.fetch(:tracer, Datadog.tracer)
@debug = options.fetch(:debug, false)
@trace_agent_hostname = options.fetch(:trace_agent_hostname,
Datadog::Writer::HOSTNAME)
@trace_agent_port = options.fetch(:trace_agent_port,
Datadog::Writer::PORT)

Datadog::Tracer.debug_logging = @debug

@tracer.enabled = @enabled
@tracer.set_service_info(@default_service, 'sidekiq',
Datadog::Ext::AppTypes::WORKER)
end

def call(worker, job, queue)
return yield unless @enabled

@tracer.trace('sidekiq.job',
service: @default_service, span_type: 'job') do |span|
span.resource = job['class']
span.set_tag('sidekiq.job.id', job['jid'])
span.set_tag('sidekiq.job.retry', job['retry'])
span.set_tag('sidekiq.job.queue', job['queue'])

yield
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/ddtrace/ext/app_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module AppTypes
WEB = 'web'.freeze
DB = 'db'.freeze
CACHE = 'cache'.freeze
WORKER = 'worker'.freeze
end
end
end
26 changes: 26 additions & 0 deletions test/contrib/sidekiq/disabled_tracer_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

require 'contrib/sidekiq/tracer_test_base'

class DisabledTracerTest < TracerTestBase
class EmptyWorker
include Sidekiq::Worker

def perform; end
end

def setup
super

Sidekiq::Testing.server_middleware do |chain|
chain.add(Datadog::Contrib::Sidekiq::Tracer,
tracer: @tracer, enabled: false)
end
end

def test_empty
EmptyWorker.perform_async()

spans = @writer.spans()
assert_equal(0, spans.length)
end
end
63 changes: 63 additions & 0 deletions test/contrib/sidekiq/tracer_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@

require 'contrib/sidekiq/tracer_test_base'

class TracerTest < TracerTestBase
class TestError < StandardError; end

class EmptyWorker
include Sidekiq::Worker

def perform(); end
end

class ErrorWorker
include Sidekiq::Worker

def perform
raise TestError, 'job error'
end
end

def setup
super

Sidekiq::Testing.server_middleware do |chain|
chain.add(Datadog::Contrib::Sidekiq::Tracer,
tracer: @tracer, enabled: true)
end
end

def test_empty
EmptyWorker.perform_async()

spans = @writer.spans()
assert_equal(1, spans.length)

span = spans[0]
assert_equal('sidekiq', span.service)
assert_equal('TracerTest::EmptyWorker', span.resource)
assert_equal('default', span.get_tag('sidekiq.job.queue'))
assert_equal(0, span.status)
assert_nil(span.parent)
end

# rubocop:disable Lint/HandleExceptions
def test_error
begin
ErrorWorker.perform_async()
rescue TestError
end

spans = @writer.spans()
assert_equal(1, spans.length)

span = spans[0]
assert_equal('sidekiq', span.service)
assert_equal('TracerTest::ErrorWorker', span.resource)
assert_equal('default', span.get_tag('sidekiq.job.queue'))
assert_equal(1, span.status)
assert_equal('job error', span.get_tag(Datadog::Ext::Errors::MSG))
assert_equal('TracerTest::TestError', span.get_tag(Datadog::Ext::Errors::TYPE))
assert_nil(span.parent)
end
end
27 changes: 27 additions & 0 deletions test/contrib/sidekiq/tracer_test_base.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

require 'sidekiq/testing'
require 'ddtrace'
require 'ddtrace/contrib/sidekiq/tracer'
require 'helper'

class TracerTestBase < Minitest::Test
REDIS_HOST = '127.0.0.1'.freeze()
REDIS_PORT = 46379

def setup
@writer = FauxWriter.new()
@tracer = Datadog::Tracer.new(writer: @writer)

redis_url = "redis://#{REDIS_HOST}:#{REDIS_PORT}"

Sidekiq.configure_client do |config|
config.redis = { url: redis_url }
end

Sidekiq.configure_server do |config|
config.redis = { url: redis_url }
end

Sidekiq::Testing.inline!
end
end