-
Notifications
You must be signed in to change notification settings - Fork 373
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
Adds Resque integration #204
Conversation
770366d
to
47b0dd2
Compare
99485fd
to
aaae4f3
Compare
aaae4f3
to
628ded1
Compare
628ded1
to
990add8
Compare
Datadog::Pin.new(SERVICE, app_type: Ext::AppTypes::WORKER).onto(base) | ||
end | ||
|
||
# rubocop:disable Style/RedundantSelf |
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.
why can't we use simply follow rubocop suggestion?
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.
I initially thought that was needed to call for the base class. I've changed it!
# rubocop:disable Style/RedundantSelf | ||
def around_perform(*args) | ||
pin = self.datadog_pin | ||
pin.tracer.set_service_info(SERVICE, 'resque', Ext::AppTypes::WORKER) |
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 should not do this in the around_perform
, because it means making this calls before each execution. set_service_info
call must be done only once per initialization, so it would be great if we can find another hook or class initialization to send the service metadata.
def around_perform(*args) | ||
pin = self.datadog_pin | ||
pin.tracer.set_service_info(SERVICE, 'resque', Ext::AppTypes::WORKER) | ||
pin.tracer.trace('resque.job', service: SERVICE) do |span| |
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 service must be set according to the pin
, since users may change the service name later.
|
||
def after_perform(*args) | ||
pin = self.datadog_pin | ||
pin.tracer.shutdown! |
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.
👍
SERVICE = 'resque'.freeze | ||
|
||
def self.extended(base) | ||
Datadog::Pin.new(SERVICE, app_type: Ext::AppTypes::WORKER).onto(base) |
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 attach the Pin
to the base class, but we don't provide a clean way to change the service name. Can you write here an example of how I change the service name from the main app? Let's say I have Rails + Resque, how do I change the service name from resque
to my-app-at-resque
from a Rails initializer?
test/contrib/resque/hooks_test.rb
Outdated
@tracer = enable_test_tracer! | ||
end | ||
|
||
def test_successful_job |
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.
can we add a test where we change the service name using the Pin
?
647071e
to
990add8
Compare
8df9cfd
to
b09c73f
Compare
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.
Wonderful work @ryplo ! thank you!
Overview
Adds the Resque integration, using the built-in hooks. To create a Job that is traced during the execution, just extend our base class: