Skip to content

Conversation

@drewbailey
Copy link
Contributor

@drewbailey drewbailey commented Oct 10, 2017

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.

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)
Copy link
Contributor

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:

  • service is your service name, so what you choose with the Pin instance; it's not related to what will be your service name (maybe we should document that better)
  • app is the application name used to distinguish the framework/library. Examples are rails, resque, faraday and so on. Not strictly related to the service itself
  • app_type used 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!

Copy link
Contributor Author

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

Copy link
Contributor

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'
end

That 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!

Copy link
Contributor Author

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

@drewbailey drewbailey closed this Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants