-
Notifications
You must be signed in to change notification settings - Fork 398
allow resque service name to be configurable #221
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
allow resque service name to be configurable #221
Conversation
| service = ::Rails.configuration.datadog_trace.fetch(:resque_service) || 'resque' | ||
| pin = Datadog::Pin.get_from(Resque) | ||
| pin.tracer.set_service_info(pin.service, 'resque', Datadog::Ext::AppTypes::WORKER) | ||
| pin.tracer.set_service_info(pin.service, service, Datadog::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.
Thanks @drewbailey for the PR! Actually the set_service_info only sends metadata to change some UI behavior (i.e. is it a Web, Database or a Cache service?). Our current signature is: https://github.com/drewbailey/dd-trace-rb/blob/6c3ed6302d11432d2985b5a2f65a6e4e04b20cdd/lib/ddtrace/tracer.rb#L128-L139
where:
serviceis your service name, so what you choose with thePininstance; it's not related to what will be your service name (maybe we should document that better)appis the application name used to distinguish the framework/library. Examples arerails,resque,faradayand so on. Not strictly related to the service itselfapp_typeused to define if it's a worker, cache, web service etc. It's mostly used in the UI to filter services
Said that, if we miss the set_service_info call, nothing really changes in how we collect data. Anyway, can you check if having that in your Rails initializer works or not:
# config/initializers/datadog-tracer.rb
Rails.configuration.datadog_trace = {
auto_instrument: true,
# ....
}
# change the Resque service name (may need a require 'resque')
pin = Datadog::Pin.get_from(::Resque)
pin.service = 'resque-queues'Thank you again for the PR and any feedback you'll provide!
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.
Thanks for the reply @palazzem! That seems to work although it unfortunately needs to be guarded in the event that pin is nil when the app initializes. Would the following be acceptable?
require 'ddtrace/ext/app_types'
require 'resque'
module Datadog
module Contrib
module Resque
# Uses Resque job hooks to create traces
module ResqueJob
def around_perform(*args)
pin = Pin.get_from(::Resque)
pin.tracer.trace('resque.job', service: pin.service) do |span|
span.resource = name
span.span_type = pin.app_type
yield
span.service = pin.service
end
end
def after_perform(*args)
pin = Pin.get_from(::Resque)
pin.tracer.shutdown!
end
end
end
end
end
Resque.before_first_fork do
+ service = ::Rails.configuration.datadog_trace.fetch(:resque_service) || 'resque'
pin = Datadog::Pin.get_from(Resque)
+ pin.service = service
pin.tracer.set_service_info(pin.service, service, Datadog::Ext::AppTypes::WORKER)
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.
Probably yes, but would be preferable not having any Rails specific configuration in the integration. Mostly we prefer to keep each single integration Rails agnostic, though the current configuration system couples a lot of things (so your PR makes sense and is correct with our current implementation).
Anyway, we're rewriting the configuration system in #203 that allows exactly what you need without changing the resque_service from the Rails config. It would simply:
Datadog.configure do |c|
c.use :rails, # ... some Rails specific options
c.use :resque, service_name: 'resque-queues'
endThat PR should be ready very soon. Do you mind if we wait a bit more so we can update your contribution with the new configuration and see how does it look?
Also feel free to give your feedback to the PR above!
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.
Thanks for the explanation, makes sense! Looking forward to #203
Release Information
Version: 2.12.3
PR: #4079
Branch: master
Announcement
Hey everyone! :ruby: datadog v2.12.3 is out!
This release contains ...
See more information at: https://github.com/DataDog/dd-trace-rb/releases/tag/v2.12.3
Give it a try, and let us know what you think!
Release Process
Use the release script at https://github.com/DataDog/fast_castle.