-
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
Shoryuken middleware tracer #577
Changes from 1 commit
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,73 @@ | ||
module Datadog | ||
module Contrib | ||
module Shoryuken | ||
# Tracer is a Shoryuken server-side middleware which traces executed jobs | ||
class Tracer | ||
DEFAULT_CONFIG = { | ||
enabled: true, | ||
shoryuken_service: 'shoryuken', | ||
tracer: Datadog.tracer, | ||
debug: false, | ||
trace_agent_hostname: Datadog::Writer::HOSTNAME, | ||
trace_agent_port: Datadog::Writer::PORT | ||
}.freeze | ||
|
||
def initialize(options = {}) | ||
# check if Rails configuration is available and use it to override | ||
# Shoryuken defaults | ||
rails_config = ::Rails.configuration.datadog_trace rescue {} | ||
base_config = DEFAULT_CONFIG.merge(rails_config) | ||
user_config = base_config.merge(options) | ||
@tracer = user_config[:tracer] | ||
@shoryuken_service = user_config[:shoryuken_service] | ||
|
||
# set Tracer status | ||
@tracer.enabled = user_config[:enabled] | ||
Datadog::Tracer.debug_logging = user_config[:debug] | ||
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. Why expose and set the debug logging flag here? Is a user not able to otherwise set this flag directly on the tracer in their configuration? |
||
|
||
# configure the Tracer instance | ||
@tracer.configure( | ||
hostname: user_config[:trace_agent_hostname], | ||
port: user_config[:trace_agent_port] | ||
) | ||
end | ||
|
||
def call(worker_instance, queue, sqs_msg, body) | ||
# If class is wrapping something else, the interesting resource info | ||
# is the underlying, wrapped class, and not the wrapper. | ||
|
||
# configure Shoryuken service | ||
tracer_info = { resource: worker_instance.class.name } | ||
if worker_instance.class.name.ends_with?('ShoryukenAdapter::JobWrapper') | ||
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. What's the nuance here between a Job and JobWrapper? How do these things relate? And how does it fit in with the rest of the framework? |
||
tracer_info[:job_id] = body['job_id'] || body[:job_id] | ||
tracer_info[:resource] = body['job_class'] || body[:job_class] | ||
tracer_info[:wrapped] = true | ||
else | ||
tracer_info[:job_id] = worker_instance.job_id | ||
end | ||
set_service_info(@shoryuken_service) | ||
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. Having this is good! It's a nuance that we require in our tracing solutions right now (although we eventually plan to remove this.) This call only needs to be made once though. I do see the |
||
|
||
@tracer.trace('shoryuken.job', service: @shoryuken_service, span_type: 'job') do |span| | ||
span.resource = tracer_info[:resource] | ||
span.set_tag('shoryuken.job.id', tracer_info[:job_id]) | ||
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. Always good to see extra tags like this. 👍 |
||
span.set_tag('shoryuken.job.queue', queue) | ||
span.set_tag('shoryuken.job.wrapped', 'true') if tracer_info[:wrapped] | ||
|
||
yield | ||
end | ||
end | ||
|
||
private | ||
|
||
def set_service_info(service) | ||
return if @tracer.services[service] | ||
@tracer.set_service_info( | ||
service, | ||
'shoryuken', | ||
Datadog::Ext::AppTypes::WORKER | ||
) | ||
end | ||
end | ||
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.
What version of
ddtrace
was this written against?Rails.configuration.datadog_trace
hasn't been used in a while, so I suspect we might want to rebase this PR off of the latest version. We are currently on 0.16.0 as of this comment.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.
Also, other question: does Shoryuken depend on Rails? Or can it operate independently of a Rails application?
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.
Shoryuken is independent of Rails
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.
The implementation of this middleware was based on Contrib::Sidekiq (ddtrace-0.9.2), but there is no direct dependency of Rails. I'm going to change.