From e0dfbe5461be27c803a6ed9837212981e8791496 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 17 Sep 2018 11:12:18 -0400 Subject: [PATCH 01/23] Refactored: Rack to use Datadog::Contrib::Integration. (#541) --- lib/ddtrace.rb | 2 +- .../contrib/rack/configuration/settings.rb | 39 ++++++++++++ lib/ddtrace/contrib/rack/ext.rb | 18 ++++++ lib/ddtrace/contrib/rack/integration.rb | 32 ++++++++++ lib/ddtrace/contrib/rack/middlewares.rb | 7 ++- lib/ddtrace/contrib/rack/patcher.rb | 62 +++++++------------ test/contrib/grape/rack_app.rb | 4 +- test/contrib/rack/helpers.rb | 2 +- test/contrib/rack/resource_name_test.rb | 9 ++- 9 files changed, 127 insertions(+), 48 deletions(-) create mode 100644 lib/ddtrace/contrib/rack/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/rack/ext.rb create mode 100644 lib/ddtrace/contrib/rack/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index fe62034006c..69ba78ca8c7 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -61,7 +61,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/mongodb/patcher' require 'ddtrace/contrib/mysql2/patcher' require 'ddtrace/contrib/racecar/patcher' -require 'ddtrace/contrib/rack/patcher' +require 'ddtrace/contrib/rack/integration' require 'ddtrace/contrib/rails/patcher' require 'ddtrace/contrib/rake/patcher' require 'ddtrace/contrib/redis/patcher' diff --git a/lib/ddtrace/contrib/rack/configuration/settings.rb b/lib/ddtrace/contrib/rack/configuration/settings.rb new file mode 100644 index 00000000000..1b77d9454eb --- /dev/null +++ b/lib/ddtrace/contrib/rack/configuration/settings.rb @@ -0,0 +1,39 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/rack/ext' + +module Datadog + module Contrib + module Rack + module Configuration + # Custom settings for the Rack integration + class Settings < Contrib::Configuration::Settings + DEFAULT_HEADERS = { + response: [ + 'Content-Type', + 'X-Request-ID' + ] + }.freeze + + option :application + option :distributed_tracing, default: false + option :headers, default: DEFAULT_HEADERS + option :middleware_names, default: false + option :quantize, default: {} + option :request_queuing, default: false + + option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| + get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::WEB) + value + end + + option :web_service_name, default: Ext::WEBSERVER_SERVICE_NAME, depends_on: [:tracer, :request_queuing] do |value| + if get_option(:request_queuing) + get_option(:tracer).set_service_info(value, Ext::WEBSERVER_APP, Datadog::Ext::AppTypes::WEB) + end + value + end + end + end + end + end +end diff --git a/lib/ddtrace/contrib/rack/ext.rb b/lib/ddtrace/contrib/rack/ext.rb new file mode 100644 index 00000000000..579af62781e --- /dev/null +++ b/lib/ddtrace/contrib/rack/ext.rb @@ -0,0 +1,18 @@ +module Datadog + module Contrib + module Rack + # Rack integration constants + module Ext + APP = 'rack'.freeze + SERVICE_NAME = 'rack'.freeze + WEBSERVER_APP = 'webserver'.freeze + WEBSERVER_SERVICE_NAME = 'web-server'.freeze + + RACK_ENV_REQUEST_SPAN = 'datadog.rack_request_span'.freeze + + SPAN_HTTP_SERVER_QUEUE = 'http_server.queue'.freeze + SPAN_REQUEST = 'rack.request'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/rack/integration.rb b/lib/ddtrace/contrib/rack/integration.rb new file mode 100644 index 00000000000..a82cec1590d --- /dev/null +++ b/lib/ddtrace/contrib/rack/integration.rb @@ -0,0 +1,32 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/rack/configuration/settings' +require 'ddtrace/contrib/rack/patcher' + +module Datadog + module Contrib + module Rack + # Description of Rack integration + class Integration + include Contrib::Integration + + register_as :rack, auto_patch: false + + def self.version + Gem.loaded_specs['rack'] && Gem.loaded_specs['rack'].version + end + + def self.present? + super && defined?(::Rack) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/rack/middlewares.rb b/lib/ddtrace/contrib/rack/middlewares.rb index 4c4c0b022a2..aa6620f74d1 100644 --- a/lib/ddtrace/contrib/rack/middlewares.rb +++ b/lib/ddtrace/contrib/rack/middlewares.rb @@ -1,6 +1,7 @@ require 'ddtrace/ext/app_types' require 'ddtrace/ext/http' require 'ddtrace/propagation/http_propagator' +require 'ddtrace/contrib/rack/ext' require 'ddtrace/contrib/rack/request_queue' module Datadog @@ -15,6 +16,8 @@ module Rack # information available at the Rack level. # rubocop:disable Metrics/ClassLength class TraceMiddleware + # DEPRECATED: Remove in 1.0 in favor of Datadog::Contrib::Rack::Ext::RACK_ENV_REQUEST_SPAN + # This constant will remain here until then, for backwards compatibility. RACK_REQUEST_SPAN = 'datadog.rack_request_span'.freeze def initialize(app) @@ -29,7 +32,7 @@ def compute_queue_time(env, tracer) return if request_start.nil? tracer.trace( - 'http_server.queue', + Ext::SPAN_HTTP_SERVER_QUEUE, start_time: request_start, service: Datadog.configuration[:rack][:web_service_name] ) @@ -56,7 +59,7 @@ def call(env) # start a new request span and attach it to the current Rack environment; # we must ensure that the span `resource` is set later - request_span = tracer.trace('rack.request', trace_options) + request_span = tracer.trace(Ext::SPAN_REQUEST, trace_options) env[RACK_REQUEST_SPAN] = request_span # Add deprecation warnings diff --git a/lib/ddtrace/contrib/rack/patcher.rb b/lib/ddtrace/contrib/rack/patcher.rb index 441fc0cb639..e2b6fd14d3c 100644 --- a/lib/ddtrace/contrib/rack/patcher.rb +++ b/lib/ddtrace/contrib/rack/patcher.rb @@ -3,47 +3,26 @@ module Contrib module Rack # Provides instrumentation for `rack` module Patcher - include Base + include Contrib::Patcher - DEFAULT_HEADERS = { - response: [ - 'Content-Type', - 'X-Request-ID' - ] - }.freeze + module_function - register_as :rack - option :tracer, default: Datadog.tracer - option :distributed_tracing, default: false - option :middleware_names, default: false - option :quantize, default: {} - option :application - option :service_name, default: 'rack', depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, 'rack', Ext::AppTypes::WEB) - value - end - option :request_queuing, default: false - option :web_service_name, default: 'web-server', depends_on: [:tracer, :request_queuing] do |value| - if get_option(:request_queuing) - get_option(:tracer).set_service_info(value, 'webserver', Ext::AppTypes::WEB) - end - value + def patched? + done?(:rack) end - option :headers, default: DEFAULT_HEADERS - - module_function def patch - unless patched? + # Patch middleware + do_once(:rack) do require_relative 'middlewares' - @patched = true end - if (!instance_variable_defined?(:@middleware_patched) || !@middleware_patched) \ - && get_option(:middleware_names) + # Patch middleware names + if !done?(:rack_middleware_names) && get_option(:middleware_names) if get_option(:application) - enable_middleware_names - @middleware_patched = true + do_once(:rack_middleware_names) do + patch_middleware_names + end else Datadog::Tracer.log.warn(%( Rack :middleware_names requires you to also pass :application. @@ -51,15 +30,11 @@ def patch e.g. use: :rack, middleware_names: true, application: my_rack_app).freeze) end end - - @patched || @middleware_patched + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Rack integration: #{e}") end - def patched? - @patched ||= false - end - - def enable_middleware_names + def patch_middleware_names retain_middleware_name(get_option(:application)) rescue => e # We can safely ignore these exceptions since they happen only in the @@ -81,9 +56,16 @@ def call(env) end end - following = middleware.instance_variable_get('@app') + following = if middleware.instance_variable_defined?('@app') + middleware.instance_variable_get('@app') + end + retain_middleware_name(following) end + + def get_option(option) + Datadog.configuration[:rack].get_option(option) + end end end end diff --git a/test/contrib/grape/rack_app.rb b/test/contrib/grape/rack_app.rb index 06df08f0dff..c8b62c129c7 100644 --- a/test/contrib/grape/rack_app.rb +++ b/test/contrib/grape/rack_app.rb @@ -50,7 +50,7 @@ def setup def teardown super # reset the configuration - Datadog.registry[:rack].reset_options! - Datadog.registry[:grape].reset_options! + Datadog.configuration[:rack].reset_options! + Datadog.configuration[:grape].reset_options! end end diff --git a/test/contrib/rack/helpers.rb b/test/contrib/rack/helpers.rb index 54da2cc2da3..3354ac4ecf4 100644 --- a/test/contrib/rack/helpers.rb +++ b/test/contrib/rack/helpers.rb @@ -116,6 +116,6 @@ def setup def teardown super # reset the configuration - Datadog.registry[:rack].reset_options! + Datadog.configuration[:rack].reset_options! end end diff --git a/test/contrib/rack/resource_name_test.rb b/test/contrib/rack/resource_name_test.rb index 9e5ded07da6..259b544bc34 100644 --- a/test/contrib/rack/resource_name_test.rb +++ b/test/contrib/rack/resource_name_test.rb @@ -13,8 +13,13 @@ def setup run BottomMiddleware.new end.to_app - remove_patch!(:rack) - Datadog.registry[:rack].instance_variable_set('@middleware_patched', false) + Datadog.registry[:rack].patcher.tap do |patcher| + if patcher.instance_variable_defined?(:@done_once) + patcher.instance_variable_get(:@done_once).delete(:rack) + patcher.instance_variable_get(:@done_once).delete(:rack_middleware_names) + end + end + Datadog.configuration.use( :rack, middleware_names: true, From 3df964b7889aa7e4384e43d1680a6793f2989437 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 17 Sep 2018 11:08:21 -0400 Subject: [PATCH 02/23] Refactored: Rails to use Datadog::Contrib::Integration. (#540) --- lib/ddtrace.rb | 2 +- .../contrib/rails/action_controller.rb | 10 +- lib/ddtrace/contrib/rails/active_support.rb | 18 +-- .../contrib/rails/configuration/settings.rb | 26 ++++ lib/ddtrace/contrib/rails/core_extensions.rb | 25 ++-- lib/ddtrace/contrib/rails/ext.rb | 30 +++++ lib/ddtrace/contrib/rails/framework.rb | 5 +- lib/ddtrace/contrib/rails/integration.rb | 37 ++++++ lib/ddtrace/contrib/rails/patcher.rb | 119 +++++++++--------- lib/ddtrace/contrib/rails/railtie.rb | 13 +- lib/ddtrace/contrib/redis/patcher.rb | 2 +- lib/ddtrace/ext/cache.rb | 8 -- spec/ddtrace/contrib/rails/railtie_spec.rb | 1 - .../contrib/rails/support/application.rb | 1 - test/contrib/rails/cache_test.rb | 4 +- test/contrib/rails/rack_middleware_test.rb | 2 +- test/contrib/rails/tracer_test.rb | 2 +- 17 files changed, 187 insertions(+), 118 deletions(-) create mode 100644 lib/ddtrace/contrib/rails/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/rails/ext.rb create mode 100644 lib/ddtrace/contrib/rails/integration.rb delete mode 100644 lib/ddtrace/ext/cache.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 69ba78ca8c7..831d658a670 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -62,7 +62,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/mysql2/patcher' require 'ddtrace/contrib/racecar/patcher' require 'ddtrace/contrib/rack/integration' -require 'ddtrace/contrib/rails/patcher' +require 'ddtrace/contrib/rails/integration' require 'ddtrace/contrib/rake/patcher' require 'ddtrace/contrib/redis/patcher' require 'ddtrace/contrib/resque/patcher' diff --git a/lib/ddtrace/contrib/rails/action_controller.rb b/lib/ddtrace/contrib/rails/action_controller.rb index 116d318ae32..7ad6ce23cee 100644 --- a/lib/ddtrace/contrib/rails/action_controller.rb +++ b/lib/ddtrace/contrib/rails/action_controller.rb @@ -1,5 +1,7 @@ require 'ddtrace/ext/http' require 'ddtrace/ext/errors' +require 'ddtrace/contrib/rack/ext' +require 'ddtrace/contrib/rails/ext' module Datadog module Contrib @@ -20,7 +22,7 @@ def self.start_processing(payload) tracer = Datadog.configuration[:rails][:tracer] service = Datadog.configuration[:rails][:controller_service] type = Datadog::Ext::HTTP::TYPE - span = tracer.trace('rails.action_controller', service: service, span_type: type) + span = tracer.trace(Ext::SPAN_ACTION_CONTROLLER, service: service, span_type: type) # attach the current span to the tracing context tracing_context = payload.fetch(:tracing_context) @@ -43,12 +45,12 @@ def self.finish_processing(payload) # Set the parent resource if it's a `rack.request` span, # but not if its an exception contoller. - if !span.parent.nil? && span.parent.name == 'rack.request' && !exception_controller?(payload) + if !span.parent.nil? && span.parent.name == Rack::Ext::SPAN_REQUEST && !exception_controller?(payload) span.parent.resource = span.resource end - span.set_tag('rails.route.action', payload.fetch(:action)) - span.set_tag('rails.route.controller', payload.fetch(:controller)) + span.set_tag(Ext::TAG_ROUTE_ACTION, payload.fetch(:action)) + span.set_tag(Ext::TAG_ROUTE_CONTROLLER, payload.fetch(:controller)) exception = payload[:exception_object] if exception.nil? diff --git a/lib/ddtrace/contrib/rails/active_support.rb b/lib/ddtrace/contrib/rails/active_support.rb index 0a94a70f967..6167dfbf7c1 100644 --- a/lib/ddtrace/contrib/rails/active_support.rb +++ b/lib/ddtrace/contrib/rails/active_support.rb @@ -1,5 +1,5 @@ require 'thread' -require 'ddtrace/ext/cache' +require 'ddtrace/contrib/rails/ext' module Datadog module Contrib @@ -24,16 +24,16 @@ def self.start_trace_cache(payload) # NOTE: the ``finish_trace_cache()`` is fired but it already has a safe-guard # to avoid any kind of issue. current_span = tracer.active_span - return if payload[:action] == 'GET'.freeze && - current_span.try(:name) == 'rails.cache'.freeze && - current_span.try(:resource) == 'GET'.freeze + return if payload[:action] == Ext::RESOURCE_CACHE_GET && + current_span.try(:name) == Ext::SPAN_CACHE && + current_span.try(:resource) == Ext::RESOURCE_CACHE_GET tracing_context = payload.fetch(:tracing_context) # create a new ``Span`` and add it to the tracing context service = Datadog.configuration[:rails][:cache_service] - type = Datadog::Ext::CACHE::TYPE - span = tracer.trace('rails.cache'.freeze, service: service, span_type: type) + type = Ext::SPAN_TYPE_CACHE + span = tracer.trace(Ext::SPAN_CACHE, service: service, span_type: type) span.resource = payload.fetch(:action) tracing_context[:dd_cache_span] = span rescue StandardError => e @@ -49,9 +49,9 @@ def self.finish_trace_cache(payload) begin # discard parameters from the cache_store configuration store, = *Array.wrap(::Rails.configuration.cache_store).flatten - span.set_tag('rails.cache.backend'.freeze, store) - cache_key = Datadog::Utils.truncate(payload.fetch(:key), Ext::CACHE::MAX_KEY_SIZE) - span.set_tag('rails.cache.key'.freeze, cache_key) + span.set_tag(Ext::TAG_CACHE_BACKEND, store) + cache_key = Datadog::Utils.truncate(payload.fetch(:key), Ext::QUANTIZE_CACHE_MAX_KEY_SIZE) + span.set_tag(Ext::TAG_CACHE_KEY, cache_key) span.set_error(payload[:exception]) if payload[:exception] ensure span.finish diff --git a/lib/ddtrace/contrib/rails/configuration/settings.rb b/lib/ddtrace/contrib/rails/configuration/settings.rb new file mode 100644 index 00000000000..257c8d5453d --- /dev/null +++ b/lib/ddtrace/contrib/rails/configuration/settings.rb @@ -0,0 +1,26 @@ +require 'ddtrace/contrib/configuration/settings' + +module Datadog + module Contrib + module Rails + module Configuration + # Custom settings for the Rails integration + class Settings < Contrib::Configuration::Settings + option :cache_service + option :controller_service + option :database_service, depends_on: [:service_name] do |value| + value.tap do + # Update ActiveRecord service name too + Datadog.configuration[:active_record][:service_name] = value + end + end + option :distributed_tracing, default: false + option :exception_controller, default: nil + option :middleware, default: true + option :middleware_names, default: false + option :template_base_path, default: 'views/' + end + end + end + end +end diff --git a/lib/ddtrace/contrib/rails/core_extensions.rb b/lib/ddtrace/contrib/rails/core_extensions.rb index 70ca8465856..16a623fdb5e 100644 --- a/lib/ddtrace/contrib/rails/core_extensions.rb +++ b/lib/ddtrace/contrib/rails/core_extensions.rb @@ -1,3 +1,5 @@ +require 'ddtrace/contrib/rails/ext' + module Datadog # RailsRendererPatcher contains function to patch Rails rendering libraries. # rubocop:disable Lint/RescueException @@ -6,11 +8,6 @@ module Datadog module RailsRendererPatcher include Datadog::Patcher - SPAN_NAME_RENDER_PARTIAL = 'rails.render_partial'.freeze - SPAN_NAME_RENDER_TEMPLATE = 'rails.render_template'.freeze - TAG_LAYOUT = 'rails.layout'.freeze - TAG_TEMPLATE_NAME = 'rails.template_name'.freeze - module_function def patch_renderer @@ -39,7 +36,7 @@ def render_with_datadog(*args, &block) render_without_datadog(*args, &block) else datadog_tracer.trace( - Datadog::RailsRendererPatcher::SPAN_NAME_RENDER_TEMPLATE, + Datadog::Contrib::Rails::Ext::SPAN_RENDER_TEMPLATE, span_type: Datadog::Ext::HTTP::TEMPLATE ) do |span| with_datadog_span(span) { render_without_datadog(*args, &block) } @@ -64,14 +61,14 @@ def render_template_with_datadog(*args) end if template_name active_datadog_span.set_tag( - Datadog::RailsRendererPatcher::TAG_TEMPLATE_NAME, + Datadog::Contrib::Rails::Ext::TAG_TEMPLATE_NAME, template_name ) end if layout active_datadog_span.set_tag( - Datadog::RailsRendererPatcher::TAG_LAYOUT, + Datadog::Contrib::Rails::Ext::TAG_LAYOUT, layout ) end @@ -119,7 +116,7 @@ def patch_partial_renderer(klass) klass.class_eval do def render_with_datadog(*args, &block) datadog_tracer.trace( - Datadog::RailsRendererPatcher::SPAN_NAME_RENDER_PARTIAL, + Datadog::Contrib::Rails::Ext::SPAN_RENDER_PARTIAL, span_type: Datadog::Ext::HTTP::TEMPLATE ) do |span| with_datadog_span(span) { render_without_datadog(*args) } @@ -131,7 +128,7 @@ def render_partial_with_datadog(*args) template_name = Datadog::Contrib::Rails::Utils.normalize_template_name(@template.try('identifier')) if template_name active_datadog_span.set_tag( - Datadog::RailsRendererPatcher::TAG_TEMPLATE_NAME, + Datadog::Contrib::Rails::Ext::TAG_TEMPLATE_NAME, template_name ) end @@ -228,7 +225,7 @@ def patch_cache_store_read def read(*args, &block) payload = { - action: 'GET', + action: Datadog::Contrib::Rails::Ext::RESOURCE_CACHE_GET, key: args[0], tracing_context: {} } @@ -256,7 +253,7 @@ def patch_cache_store_fetch def fetch(*args, &block) payload = { - action: 'GET', + action: Datadog::Contrib::Rails::Ext::RESOURCE_CACHE_GET, key: args[0], tracing_context: {} } @@ -284,7 +281,7 @@ def patch_cache_store_write def write(*args, &block) payload = { - action: 'SET', + action: Datadog::Contrib::Rails::Ext::RESOURCE_CACHE_SET, key: args[0], tracing_context: {} } @@ -312,7 +309,7 @@ def patch_cache_store_delete def delete(*args, &block) payload = { - action: 'DELETE', + action: Datadog::Contrib::Rails::Ext::RESOURCE_CACHE_DELETE, key: args[0], tracing_context: {} } diff --git a/lib/ddtrace/contrib/rails/ext.rb b/lib/ddtrace/contrib/rails/ext.rb new file mode 100644 index 00000000000..7fa8381e132 --- /dev/null +++ b/lib/ddtrace/contrib/rails/ext.rb @@ -0,0 +1,30 @@ +module Datadog + module Contrib + module Rails + # Rails integration constants + module Ext + APP = 'rails'.freeze + + QUANTIZE_CACHE_MAX_KEY_SIZE = 300 + + RESOURCE_CACHE_DELETE = 'DELETE'.freeze + RESOURCE_CACHE_GET = 'GET'.freeze + RESOURCE_CACHE_SET = 'SET'.freeze + + SPAN_ACTION_CONTROLLER = 'rails.action_controller'.freeze + SPAN_CACHE = 'rails.cache'.freeze + SPAN_RENDER_PARTIAL = 'rails.render_partial'.freeze + SPAN_RENDER_TEMPLATE = 'rails.render_template'.freeze + + SPAN_TYPE_CACHE = 'cache'.freeze + + TAG_CACHE_BACKEND = 'rails.cache.backend'.freeze + TAG_CACHE_KEY = 'rails.cache.key'.freeze + TAG_LAYOUT = 'rails.layout'.freeze + TAG_ROUTE_ACTION = 'rails.route.action'.freeze + TAG_ROUTE_CONTROLLER = 'rails.route.controller'.freeze + TAG_TEMPLATE_NAME = 'rails.template_name'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/rails/framework.rb b/lib/ddtrace/contrib/rails/framework.rb index 43cfd13693e..379709edc7b 100644 --- a/lib/ddtrace/contrib/rails/framework.rb +++ b/lib/ddtrace/contrib/rails/framework.rb @@ -5,6 +5,7 @@ require 'ddtrace/contrib/grape/endpoint' require 'ddtrace/contrib/rack/middlewares' +require 'ddtrace/contrib/rails/ext' require 'ddtrace/contrib/rails/core_extensions' require 'ddtrace/contrib/rails/action_controller' require 'ddtrace/contrib/rails/action_view' @@ -66,8 +67,8 @@ def self.activate_active_record!(config) def self.set_service_info!(config) tracer = config[:tracer] - tracer.set_service_info(config[:controller_service], 'rails', Ext::AppTypes::WEB) - tracer.set_service_info(config[:cache_service], 'rails', Ext::AppTypes::CACHE) + tracer.set_service_info(config[:controller_service], Ext::APP, Datadog::Ext::AppTypes::WEB) + tracer.set_service_info(config[:cache_service], Ext::APP, Datadog::Ext::AppTypes::CACHE) end end end diff --git a/lib/ddtrace/contrib/rails/integration.rb b/lib/ddtrace/contrib/rails/integration.rb new file mode 100644 index 00000000000..4a5abc23907 --- /dev/null +++ b/lib/ddtrace/contrib/rails/integration.rb @@ -0,0 +1,37 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/rails/configuration/settings' +require 'ddtrace/contrib/rails/patcher' + +module Datadog + module Contrib + module Rails + # Description of Rails integration + class Integration + include Contrib::Integration + + register_as :rails, auto_patch: false + + def self.version + Gem.loaded_specs['rails'] && Gem.loaded_specs['rails'].version + end + + def self.present? + defined?(::Rails) + end + + def self.compatible? + return false if ENV['DISABLE_DATADOG_RAILS'] + super && defined?(::Rails::VERSION) && ::Rails::VERSION::MAJOR.to_i >= 3 + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/rails/patcher.rb b/lib/ddtrace/contrib/rails/patcher.rb index 0aad1630635..273084deb1f 100644 --- a/lib/ddtrace/contrib/rails/patcher.rb +++ b/lib/ddtrace/contrib/rails/patcher.rb @@ -6,82 +6,75 @@ module Datadog module Contrib module Rails - # Patcher + # Patcher enables patching of 'rails' module. module Patcher - include Base - register_as :rails, auto_patch: true + include Contrib::Patcher - option :service_name - option :controller_service - option :cache_service - option :database_service, depends_on: [:service_name] do |value| - value.tap do - # Update ActiveRecord service name too - Datadog.configuration[:active_record][:service_name] = value - end - end - option :middleware, default: true - option :middleware_names, default: false - option :distributed_tracing, default: false - option :template_base_path, default: 'views/' - option :exception_controller, default: nil - option :tracer, default: Datadog.tracer + module_function - @patched = false + def patched? + done?(:rails) + end - class << self - def patch - return @patched if patched? || !compatible? + def patch + do_once(:rails) do + patch_before_intialize + patch_after_intialize + end + rescue => e + Datadog::Tracer.log.error("Unable to apply Rails integration: #{e}") + end - # Add a callback hook to add the trace middleware before the application initializes. - # Otherwise the middleware stack will be frozen. - do_once(:rails_before_initialize_hook) do - ::ActiveSupport.on_load(:before_initialize) do - # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. - if Datadog.configuration[:rails][:middleware] - # Add trace middleware - config.middleware.insert_before(0, Datadog::Contrib::Rack::TraceMiddleware) + def patch_before_intialize + ::ActiveSupport.on_load(:before_initialize) do + Datadog::Contrib::Rails::Patcher.before_intialize(self) + end + end - # Insert right after Rails exception handling middleware, because if it's before, - # it catches and swallows the error. If it's too far after, custom middleware can find itself - # between, and raise exceptions that don't end up getting tagged on the request properly. - # e.g lost stack trace. - config.middleware.insert_after( - ActionDispatch::ShowExceptions, - Datadog::Contrib::Rails::ExceptionMiddleware - ) - end - end - end + def before_intialize(app) + # Middleware must be added before the application is initialized. + # Otherwise the middleware stack will be frozen. + # Sometimes we don't want to activate middleware e.g. OpenTracing, etc. + add_middleware(app) if Datadog.configuration[:rails][:middleware] + end - # Add a callback hook to finish configuring the tracer after the application is initialized. - # We need to wait for some things, like application name, middleware stack, etc. - do_once(:rails_after_initialize_hook) do - ::ActiveSupport.on_load(:after_initialize) do - Datadog::Contrib::Rails::Framework.setup + def add_middleware(app) + # Add trace middleware + app.middleware.insert_before(0, Datadog::Contrib::Rack::TraceMiddleware) - # Add instrumentation to Rails components - Datadog::Contrib::Rails::ActionController.instrument - Datadog::Contrib::Rails::ActionView.instrument - Datadog::Contrib::Rails::ActiveSupport.instrument - end - end + # Insert right after Rails exception handling middleware, because if it's before, + # it catches and swallows the error. If it's too far after, custom middleware can find itself + # between, and raise exceptions that don't end up getting tagged on the request properly. + # e.g lost stack trace. + app.middleware.insert_after( + ActionDispatch::ShowExceptions, + Datadog::Contrib::Rails::ExceptionMiddleware + ) + end - @patched = true - rescue => e - Datadog::Tracer.log.error("Unable to apply Rails integration: #{e}") - @patched + def patch_after_intialize + ::ActiveSupport.on_load(:after_initialize) do + Datadog::Contrib::Rails::Patcher.after_intialize(self) end + end - def patched? - @patched - end + def after_intialize(app) + # Finish configuring the tracer after the application is initialized. + # We need to wait for some things, like application name, middleware stack, etc. + setup_tracer + instrument_rails + end - def compatible? - return if ENV['DISABLE_DATADOG_RAILS'] + # Configure Rails tracing with settings + def setup_tracer + Datadog::Contrib::Rails::Framework.setup + end - defined?(::Rails::VERSION) && ::Rails::VERSION::MAJOR.to_i >= 3 - end + # Add instrumentation to Rails components + def instrument_rails + Datadog::Contrib::Rails::ActionController.instrument + Datadog::Contrib::Rails::ActionView.instrument + Datadog::Contrib::Rails::ActiveSupport.instrument end end end diff --git a/lib/ddtrace/contrib/rails/railtie.rb b/lib/ddtrace/contrib/rails/railtie.rb index bb2a0ccdafc..e173b7cc005 100644 --- a/lib/ddtrace/contrib/rails/railtie.rb +++ b/lib/ddtrace/contrib/rails/railtie.rb @@ -6,19 +6,12 @@ module Datadog # Railtie class initializes class Railtie < Rails::Railtie # Add the trace middleware to the application stack - initializer 'datadog.add_middleware' do |app| - app.middleware.insert_before(0, Datadog::Contrib::Rack::TraceMiddleware) - # Insert right after Rails exception handling middleware, because if it's before, - # it catches and swallows the error. If it's too far after, custom middleware can find itself - # between, and raise exceptions that don't end up getting tagged on the request properly (e.g lost stack trace.) - app.middleware.insert_after(ActionDispatch::ShowExceptions, Datadog::Contrib::Rails::ExceptionMiddleware) + initializer 'datadog.before_intialize' do |app| + Datadog::Contrib::Rails::Patcher.before_intialize(app) end config.after_initialize do - Datadog::Contrib::Rails::Framework.setup - Datadog::Contrib::Rails::ActionController.instrument - Datadog::Contrib::Rails::ActionView.instrument - Datadog::Contrib::Rails::ActiveSupport.instrument + Datadog::Contrib::Rails::Patcher.after_intialize(self) end end end diff --git a/lib/ddtrace/contrib/redis/patcher.rb b/lib/ddtrace/contrib/redis/patcher.rb index 50b2c96e741..f1e9d11c117 100644 --- a/lib/ddtrace/contrib/redis/patcher.rb +++ b/lib/ddtrace/contrib/redis/patcher.rb @@ -28,7 +28,7 @@ def patch patch_redis_client @patched = true - RailsCachePatcher.reload_cache_store if Datadog.registry[:rails].patched? + RailsCachePatcher.reload_cache_store if Datadog.registry[:rails].patcher.patched? rescue StandardError => e Datadog::Tracer.log.error("Unable to apply Redis integration: #{e}") end diff --git a/lib/ddtrace/ext/cache.rb b/lib/ddtrace/ext/cache.rb deleted file mode 100644 index 6cd407eac0b..00000000000 --- a/lib/ddtrace/ext/cache.rb +++ /dev/null @@ -1,8 +0,0 @@ -module Datadog - module Ext - module CACHE - TYPE = 'cache'.freeze - MAX_KEY_SIZE = 300 - end - end -end diff --git a/spec/ddtrace/contrib/rails/railtie_spec.rb b/spec/ddtrace/contrib/rails/railtie_spec.rb index 2570bab73f7..60914eaf138 100644 --- a/spec/ddtrace/contrib/rails/railtie_spec.rb +++ b/spec/ddtrace/contrib/rails/railtie_spec.rb @@ -31,7 +31,6 @@ def index end before(:each) do - Datadog.registry[:rails].instance_variable_set(:@patched, false) Datadog.configure do |c| c.tracer hostname: ENV.fetch('TEST_DDAGENT_HOST', 'localhost') c.use :rails, rails_options if use_rails diff --git a/spec/ddtrace/contrib/rails/support/application.rb b/spec/ddtrace/contrib/rails/support/application.rb index fa0a00a2c4c..823b6622f43 100644 --- a/spec/ddtrace/contrib/rails/support/application.rb +++ b/spec/ddtrace/contrib/rails/support/application.rb @@ -4,7 +4,6 @@ include_context 'Rails base application' before do - Datadog.registry[:rails].instance_variable_set(:@patched, false) reset_rails_configuration! end diff --git a/test/contrib/rails/cache_test.rb b/test/contrib/rails/cache_test.rb index db39f0f46e5..2c84bb9db71 100644 --- a/test/contrib/rails/cache_test.rb +++ b/test/contrib/rails/cache_test.rb @@ -1,7 +1,7 @@ require 'helper' require 'contrib/rails/test_helper' require 'securerandom' -require 'ddtrace/ext/cache' +require 'ddtrace/contrib/rails/ext' class CacheTracingTest < ActionController::TestCase setup do @@ -97,7 +97,7 @@ class CacheTracingTest < ActionController::TestCase end def test_cache_key_truncation_regression - max_key_size = Datadog::Ext::CACHE::MAX_KEY_SIZE + max_key_size = Datadog::Contrib::Rails::Ext::QUANTIZE_CACHE_MAX_KEY_SIZE large_key = ''.ljust(max_key_size * 2, SecureRandom.hex) Rails.cache.write(large_key, 'foobar') diff --git a/test/contrib/rails/rack_middleware_test.rb b/test/contrib/rails/rack_middleware_test.rb index 31c1db53cd6..378a9b76978 100644 --- a/test/contrib/rails/rack_middleware_test.rb +++ b/test/contrib/rails/rack_middleware_test.rb @@ -13,7 +13,7 @@ class FullStackTest < ActionDispatch::IntegrationTest # this prevents the overhead to reinitialize the Rails application # and the Rack stack @tracer = get_test_tracer - Datadog.registry[:rails].reset_options! + Datadog.configuration[:rails].reset_options! Datadog.configuration[:rails][:tracer] = @tracer Datadog.configuration[:rails][:database_service] = get_adapter_name Datadog::Contrib::Rails::Framework.setup diff --git a/test/contrib/rails/tracer_test.rb b/test/contrib/rails/tracer_test.rb index 4d80b944034..b5ba8737d92 100644 --- a/test/contrib/rails/tracer_test.rb +++ b/test/contrib/rails/tracer_test.rb @@ -7,7 +7,7 @@ class TracerTest < ActionDispatch::IntegrationTest # don't pollute the global tracer @tracer = get_test_tracer @config = Datadog.configuration[:rails] - Datadog.registry[:rails].reset_options! + Datadog.configuration[:rails].reset_options! @config[:tracer] = @tracer end From 8074a42dcceaf0c5eabac423c7765c3758152877 Mon Sep 17 00:00:00 2001 From: David Elner Date: Thu, 20 Sep 2018 12:53:34 -0400 Subject: [PATCH 03/23] Refactored: Sidekiq to use Datadog::Contrib::Integration. (#545) --- lib/ddtrace.rb | 2 +- .../contrib/sidekiq/configuration/settings.rb | 15 ++++++++ lib/ddtrace/contrib/sidekiq/ext.rb | 19 ++++++++++ lib/ddtrace/contrib/sidekiq/integration.rb | 36 +++++++++++++++++++ lib/ddtrace/contrib/sidekiq/patcher.rb | 36 +++++++++---------- lib/ddtrace/contrib/sidekiq/tracer.rb | 20 +++++------ 6 files changed, 99 insertions(+), 29 deletions(-) create mode 100644 lib/ddtrace/contrib/sidekiq/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/sidekiq/ext.rb create mode 100644 lib/ddtrace/contrib/sidekiq/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 831d658a670..3ac6996ab0e 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -68,6 +68,6 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/resque/patcher' require 'ddtrace/contrib/rest_client/integration' require 'ddtrace/contrib/sequel/integration' -require 'ddtrace/contrib/sidekiq/patcher' +require 'ddtrace/contrib/sidekiq/integration' require 'ddtrace/contrib/sucker_punch/patcher' require 'ddtrace/monkey' diff --git a/lib/ddtrace/contrib/sidekiq/configuration/settings.rb b/lib/ddtrace/contrib/sidekiq/configuration/settings.rb new file mode 100644 index 00000000000..c6a1b90e196 --- /dev/null +++ b/lib/ddtrace/contrib/sidekiq/configuration/settings.rb @@ -0,0 +1,15 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/sidekiq/ext' + +module Datadog + module Contrib + module Sidekiq + module Configuration + # Custom settings for the Sidekiq integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/sidekiq/ext.rb b/lib/ddtrace/contrib/sidekiq/ext.rb new file mode 100644 index 00000000000..fd22cf7a19d --- /dev/null +++ b/lib/ddtrace/contrib/sidekiq/ext.rb @@ -0,0 +1,19 @@ +module Datadog + module Contrib + module Sidekiq + # Sidekiq integration constants + module Ext + APP = 'sidekiq'.freeze + SERVICE_NAME = 'sidekiq'.freeze + + SPAN_JOB = 'sidekiq.job'.freeze + + TAG_JOB_DELAY = 'sidekiq.job.delay'.freeze + TAG_JOB_ID = 'sidekiq.job.id'.freeze + TAG_JOB_QUEUE = 'sidekiq.job.queue'.freeze + TAG_JOB_RETRY = 'sidekiq.job.retry'.freeze + TAG_JOB_WRAPPER = 'sidekiq.job.wrapper'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/sidekiq/integration.rb b/lib/ddtrace/contrib/sidekiq/integration.rb new file mode 100644 index 00000000000..6e008426deb --- /dev/null +++ b/lib/ddtrace/contrib/sidekiq/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/sidekiq/configuration/settings' +require 'ddtrace/contrib/sidekiq/patcher' + +module Datadog + module Contrib + module Sidekiq + # Description of Sidekiq integration + class Integration + include Contrib::Integration + + register_as :sidekiq + + def self.version + Gem.loaded_specs['sidekiq'] && Gem.loaded_specs['sidekiq'].version + end + + def self.present? + super && defined?(::Sidekiq) + end + + def self.compatible? + super && version >= Gem::Version.new('4.0.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/sidekiq/patcher.rb b/lib/ddtrace/contrib/sidekiq/patcher.rb index 488c9114554..1f795aa3648 100644 --- a/lib/ddtrace/contrib/sidekiq/patcher.rb +++ b/lib/ddtrace/contrib/sidekiq/patcher.rb @@ -1,32 +1,32 @@ +require 'ddtrace/contrib/patcher' + module Datadog module Contrib module Sidekiq - # Provides instrumentation support for Sidekiq + # Patcher enables patching of 'sidekiq' module. module Patcher - include Base - VERSION_REQUIRED = Gem::Version.new('4.0.0') - register_as :sidekiq - option :service_name, default: 'sidekiq' - option :tracer, default: Datadog.tracer + include Contrib::Patcher module_function - def patch - return unless compatible? - - require_relative 'tracer' + def patched? + done?(:sidekiq) + end - ::Sidekiq.configure_server do |config| - config.server_middleware do |chain| - chain.add(Sidekiq::Tracer) + def patch + do_once(:sidekiq) do + begin + require 'ddtrace/contrib/sidekiq/tracer' + ::Sidekiq.configure_server do |config| + config.server_middleware do |chain| + chain.add(Sidekiq::Tracer) + end + end + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Sidekiq integration: #{e}") end end end - - def compatible? - defined?(::Sidekiq) && - Gem::Version.new(::Sidekiq::VERSION) >= VERSION_REQUIRED - end end end end diff --git a/lib/ddtrace/contrib/sidekiq/tracer.rb b/lib/ddtrace/contrib/sidekiq/tracer.rb index 267353c0bc2..7036ad1f16d 100644 --- a/lib/ddtrace/contrib/sidekiq/tracer.rb +++ b/lib/ddtrace/contrib/sidekiq/tracer.rb @@ -1,6 +1,7 @@ require 'sidekiq/api' require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/sidekiq/ext' module Datadog module Contrib @@ -8,9 +9,8 @@ module Sidekiq # Tracer is a Sidekiq server-side middleware which traces executed jobs class Tracer def initialize(options = {}) - config = Datadog.configuration[:sidekiq].merge(options) - @tracer = config[:tracer] - @sidekiq_service = config[:service_name] + @tracer = options[:tracer] || Datadog.configuration[:sidekiq][:tracer] + @sidekiq_service = options[:service_name] || Datadog.configuration[:sidekiq][:service_name] end def call(worker, job, queue) @@ -26,13 +26,13 @@ def call(worker, job, queue) service = sidekiq_service(resource_worker(resource)) set_service_info(service) - @tracer.trace('sidekiq.job', service: service, span_type: Datadog::Ext::AppTypes::WORKER) do |span| + @tracer.trace(Ext::SPAN_JOB, service: service, span_type: Datadog::Ext::AppTypes::WORKER) do |span| span.resource = resource - span.set_tag('sidekiq.job.id', job['jid']) - span.set_tag('sidekiq.job.retry', job['retry']) - span.set_tag('sidekiq.job.queue', job['queue']) - span.set_tag('sidekiq.job.wrapper', job['class']) if job['wrapped'] - span.set_tag('sidekiq.job.delay', 1000.0 * (Time.now.utc.to_f - job['enqueued_at'].to_f)) + span.set_tag(Ext::TAG_JOB_ID, job['jid']) + span.set_tag(Ext::TAG_JOB_RETRY, job['retry']) + span.set_tag(Ext::TAG_JOB_QUEUE, job['queue']) + span.set_tag(Ext::TAG_JOB_WRAPPER, job['class']) if job['wrapped'] + span.set_tag(Ext::TAG_JOB_DELAY, 1000.0 * (Time.now.utc.to_f - job['enqueued_at'].to_f)) yield end @@ -62,7 +62,7 @@ def set_service_info(service) return if @tracer.services[service] @tracer.set_service_info( service, - 'sidekiq', + Ext::APP, Datadog::Ext::AppTypes::WORKER ) end From 982b739fb0d5efd62b36ba682529b83e37f1f87b Mon Sep 17 00:00:00 2001 From: David Elner Date: Thu, 20 Sep 2018 12:54:04 -0400 Subject: [PATCH 04/23] Implement Resque integration configuration (#546) * Changed: ConfigurationHelpers#remove_patch! to work with Integration patchers. * Refactored: Resque to use Datadog::Contrib::Integration. --- lib/ddtrace.rb | 2 +- .../contrib/resque/configuration/settings.rb | 16 +++++ lib/ddtrace/contrib/resque/ext.rb | 13 ++++ lib/ddtrace/contrib/resque/integration.rb | 37 +++++++++++ lib/ddtrace/contrib/resque/patcher.rb | 63 ++++++++----------- lib/ddtrace/contrib/resque/resque_job.rb | 3 +- spec/support/configuration_helpers.rb | 14 ++++- 7 files changed, 107 insertions(+), 41 deletions(-) create mode 100644 lib/ddtrace/contrib/resque/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/resque/ext.rb create mode 100644 lib/ddtrace/contrib/resque/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 3ac6996ab0e..2ed8b1e7d00 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -65,7 +65,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/rails/integration' require 'ddtrace/contrib/rake/patcher' require 'ddtrace/contrib/redis/patcher' -require 'ddtrace/contrib/resque/patcher' +require 'ddtrace/contrib/resque/integration' require 'ddtrace/contrib/rest_client/integration' require 'ddtrace/contrib/sequel/integration' require 'ddtrace/contrib/sidekiq/integration' diff --git a/lib/ddtrace/contrib/resque/configuration/settings.rb b/lib/ddtrace/contrib/resque/configuration/settings.rb new file mode 100644 index 00000000000..1a7fe6bbff5 --- /dev/null +++ b/lib/ddtrace/contrib/resque/configuration/settings.rb @@ -0,0 +1,16 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/resque/ext' + +module Datadog + module Contrib + module Resque + module Configuration + # Custom settings for the Resque integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + option :workers, default: [] + end + end + end + end +end diff --git a/lib/ddtrace/contrib/resque/ext.rb b/lib/ddtrace/contrib/resque/ext.rb new file mode 100644 index 00000000000..a551705eaed --- /dev/null +++ b/lib/ddtrace/contrib/resque/ext.rb @@ -0,0 +1,13 @@ +module Datadog + module Contrib + module Resque + # Resque integration constants + module Ext + APP = 'resque'.freeze + SERVICE_NAME = 'resque'.freeze + + SPAN_JOB = 'resque.job'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/resque/integration.rb b/lib/ddtrace/contrib/resque/integration.rb new file mode 100644 index 00000000000..c5f7d812fd4 --- /dev/null +++ b/lib/ddtrace/contrib/resque/integration.rb @@ -0,0 +1,37 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/resque/configuration/settings' +require 'ddtrace/contrib/resque/patcher' + +module Datadog + module Contrib + module Resque + # Description of Resque integration + class Integration + include Contrib::Integration + + register_as :resque, auto_patch: true + + def self.version + Gem.loaded_specs['resque'] && Gem.loaded_specs['resque'].version + end + + def self.present? + super && defined?(::Resque) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + + class << self + # Globally-acccesible reference for pre-forking optimization + attr_accessor :sync_writer + end + end + end + end +end diff --git a/lib/ddtrace/contrib/resque/patcher.rb b/lib/ddtrace/contrib/resque/patcher.rb index a40750db17a..0e7a9f8c14f 100644 --- a/lib/ddtrace/contrib/resque/patcher.rb +++ b/lib/ddtrace/contrib/resque/patcher.rb @@ -1,49 +1,40 @@ +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/sidekiq/ext' + module Datadog module Contrib - # Namespace for `resque` integration module Resque - SERVICE = 'resque'.freeze - - class << self - # Globally-acccesible reference for pre-forking optimization - attr_accessor :sync_writer - end - - # Patcher for Resque integration - sets up the pin for the integration + # Patcher enables patching of 'resque' module. module Patcher - include Base - register_as :resque, auto_patch: true - option :service_name, default: SERVICE - - @patched = false - option :workers, default: [] - - class << self - def patch - return @patched if patched? || !defined?(::Resque) + include Contrib::Patcher - require 'ddtrace/ext/app_types' - require_relative 'resque_job' + module_function - add_pin - get_option(:workers).each { |worker| worker.extend(ResqueJob) } - @patched = true - rescue => e - Tracer.log.error("Unable to apply Resque integration: #{e}") - @patched - end + def patched? + done?(:resque) + end - def patched? - @patched + def patch + do_once(:resque) do + begin + require_relative 'resque_job' + add_pin + get_option(:workers).each { |worker| worker.extend(ResqueJob) } + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Resque integration: #{e}") + end end + end - private + def add_pin + Pin + .new(get_option(:service_name), app: Ext::APP, app_type: Datadog::Ext::AppTypes::WORKER) + .onto(::Resque) + end - def add_pin - Pin - .new(get_option(:service_name), app: 'resque', app_type: Ext::AppTypes::WORKER) - .onto(::Resque) - end + def get_option(option) + Datadog.configuration[:resque].get_option(option) end end end diff --git a/lib/ddtrace/contrib/resque/resque_job.rb b/lib/ddtrace/contrib/resque/resque_job.rb index d3eac5bd4bb..d83f55dcda8 100644 --- a/lib/ddtrace/contrib/resque/resque_job.rb +++ b/lib/ddtrace/contrib/resque/resque_job.rb @@ -1,5 +1,6 @@ require 'ddtrace/ext/app_types' require 'ddtrace/sync_writer' +require 'ddtrace/contrib/sidekiq/ext' require 'resque' module Datadog @@ -10,7 +11,7 @@ module ResqueJob def around_perform(*args) pin = Pin.get_from(::Resque) return yield unless pin && pin.tracer - pin.tracer.trace('resque.job', service: pin.service) do |span| + pin.tracer.trace(Ext::SPAN_JOB, service: pin.service) do |span| span.resource = name span.span_type = pin.app_type yield diff --git a/spec/support/configuration_helpers.rb b/spec/support/configuration_helpers.rb index 3419176b1c4..0556ff1d74a 100644 --- a/spec/support/configuration_helpers.rb +++ b/spec/support/configuration_helpers.rb @@ -1,7 +1,15 @@ module ConfigurationHelpers def remove_patch!(integration) - Datadog - .registry[integration] - .instance_variable_set('@patched', false) + if Datadog.registry[integration].respond_to?(:patcher) + Datadog.registry[integration].patcher.tap do |patcher| + if patcher.instance_variable_defined?(:@done_once) + patcher.instance_variable_get(:@done_once).delete(integration) + end + end + else + Datadog + .registry[integration] + .instance_variable_set('@patched', false) + end end end From 9089bb1d22c7785454b6bf9b28c9ba11cddf83af Mon Sep 17 00:00:00 2001 From: David Elner Date: Thu, 20 Sep 2018 12:54:47 -0400 Subject: [PATCH 05/23] Refactored: DelayedJob to use Datadog::Contrib::Integration. (#547) --- lib/ddtrace.rb | 2 +- .../delayed_job/configuration/settings.rb | 15 ++++++ lib/ddtrace/contrib/delayed_job/ext.rb | 18 +++++++ .../contrib/delayed_job/integration.rb | 32 +++++++++++++ lib/ddtrace/contrib/delayed_job/patcher.rb | 47 ++++++++----------- lib/ddtrace/contrib/delayed_job/plugin.rb | 13 ++--- .../contrib/delayed_job/patcher_spec.rb | 20 ++++---- 7 files changed, 102 insertions(+), 45 deletions(-) create mode 100644 lib/ddtrace/contrib/delayed_job/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/delayed_job/ext.rb create mode 100644 lib/ddtrace/contrib/delayed_job/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 2ed8b1e7d00..26122040c60 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -49,7 +49,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/aws/patcher' require 'ddtrace/contrib/concurrent_ruby/integration' require 'ddtrace/contrib/dalli/patcher' -require 'ddtrace/contrib/delayed_job/patcher' +require 'ddtrace/contrib/delayed_job/integration' require 'ddtrace/contrib/elasticsearch/patcher' require 'ddtrace/contrib/excon/patcher' require 'ddtrace/contrib/faraday/patcher' diff --git a/lib/ddtrace/contrib/delayed_job/configuration/settings.rb b/lib/ddtrace/contrib/delayed_job/configuration/settings.rb new file mode 100644 index 00000000000..668e0f735df --- /dev/null +++ b/lib/ddtrace/contrib/delayed_job/configuration/settings.rb @@ -0,0 +1,15 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/delayed_job/ext' + +module Datadog + module Contrib + module DelayedJob + module Configuration + # Custom settings for the DelayedJob integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/delayed_job/ext.rb b/lib/ddtrace/contrib/delayed_job/ext.rb new file mode 100644 index 00000000000..e1f77a8390b --- /dev/null +++ b/lib/ddtrace/contrib/delayed_job/ext.rb @@ -0,0 +1,18 @@ +module Datadog + module Contrib + module DelayedJob + # DelayedJob integration constants + module Ext + APP = 'delayed_job'.freeze + SERVICE_NAME = 'delayed_job'.freeze + + SPAN_JOB = 'delayed_job'.freeze + + TAG_ATTEMPTS = 'delayed_job.attempts'.freeze + TAG_ID = 'delayed_job.id'.freeze + TAG_PRIORITY = 'delayed_job.priority'.freeze + TAG_QUEUE = 'delayed_job.queue'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/delayed_job/integration.rb b/lib/ddtrace/contrib/delayed_job/integration.rb new file mode 100644 index 00000000000..cfa0bef790e --- /dev/null +++ b/lib/ddtrace/contrib/delayed_job/integration.rb @@ -0,0 +1,32 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/delayed_job/configuration/settings' +require 'ddtrace/contrib/delayed_job/patcher' + +module Datadog + module Contrib + module DelayedJob + # Description of DelayedJob integration + class Integration + include Contrib::Integration + + register_as :delayed_job + + def self.version + Gem.loaded_specs['delayed_job'] && Gem.loaded_specs['delayed_job'].version + end + + def self.present? + super && defined?(::Delayed) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/delayed_job/patcher.rb b/lib/ddtrace/contrib/delayed_job/patcher.rb index 82ffab4577f..0a94578d548 100644 --- a/lib/ddtrace/contrib/delayed_job/patcher.rb +++ b/lib/ddtrace/contrib/delayed_job/patcher.rb @@ -1,39 +1,32 @@ +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' + module Datadog module Contrib module DelayedJob - # DelayedJob integration + # Patcher enables patching of 'delayed_job' module. module Patcher - include Base - register_as :delayed_job - - option :service_name, default: 'delayed_job'.freeze - option :tracer, default: Datadog.tracer - - @patched = false + include Contrib::Patcher - class << self - def patch - return @patched if patched? || !defined?(::Delayed) + module_function - require 'ddtrace/ext/app_types' - require_relative 'plugin' - - add_instrumentation(::Delayed::Worker) - @patched = true - rescue => e - Tracer.log.error("Unable to apply DelayedJob integration: #{e}") - @patched - end + def patched? + done?(:delayed_job) + end - def patched? - @patched + def patch + do_once(:delayed_job) do + begin + require 'ddtrace/contrib/delayed_job/plugin' + add_instrumentation(::Delayed::Worker) + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply DelayedJob integration: #{e}") + end end + end - private - - def add_instrumentation(klass) - klass.plugins << Plugin - end + def add_instrumentation(klass) + klass.plugins << Plugin end end end diff --git a/lib/ddtrace/contrib/delayed_job/plugin.rb b/lib/ddtrace/contrib/delayed_job/plugin.rb index ccd76379b73..615fbab1a81 100644 --- a/lib/ddtrace/contrib/delayed_job/plugin.rb +++ b/lib/ddtrace/contrib/delayed_job/plugin.rb @@ -1,4 +1,5 @@ require 'delayed/plugin' +require 'ddtrace/contrib/delayed_job/ext' module Datadog module Contrib @@ -8,12 +9,12 @@ class Plugin < Delayed::Plugin def self.instrument(job, &block) return block.call(job) unless tracer && tracer.enabled - tracer.trace('delayed_job'.freeze, service: configuration[:service_name], resource: job.name) do |span| - span.set_tag('delayed_job.id'.freeze, job.id) - span.set_tag('delayed_job.queue'.freeze, job.queue) if job.queue - span.set_tag('delayed_job.priority'.freeze, job.priority) - span.set_tag('delayed_job.attempts'.freeze, job.attempts) - span.span_type = Ext::AppTypes::WORKER + tracer.trace(Ext::SPAN_JOB, service: configuration[:service_name], resource: job.name) do |span| + span.set_tag(Ext::TAG_ID, job.id) + span.set_tag(Ext::TAG_QUEUE, job.queue) if job.queue + span.set_tag(Ext::TAG_PRIORITY, job.priority) + span.set_tag(Ext::TAG_ATTEMPTS, job.attempts) + span.span_type = Datadog::Ext::AppTypes::WORKER yield job end diff --git a/spec/ddtrace/contrib/delayed_job/patcher_spec.rb b/spec/ddtrace/contrib/delayed_job/patcher_spec.rb index cbf0db8348f..d20269ff637 100644 --- a/spec/ddtrace/contrib/delayed_job/patcher_spec.rb +++ b/spec/ddtrace/contrib/delayed_job/patcher_spec.rb @@ -7,20 +7,18 @@ let(:worker_plugins) { [] } let!(:delayed_worker_class) { class_double('Delayed::Worker', plugins: worker_plugins).as_stubbed_const } - before do - described_class.instance_variable_set(:@patched, false) - end - - context 'when delayed job is not present' do - before do - hide_const('Delayed') - end - - it "shouldn't patch the code" do - expect { described_class.patch }.not_to(change { described_class.patched? }) + def remove_patch! + Datadog.registry[:delayed_job].patcher.tap do |patcher| + if patcher.instance_variable_defined?(:@done_once) + patcher.instance_variable_get(:@done_once).delete(:delayed_job) + end end end + # Prevents random order from breaking tests + before(:each) { remove_patch! } + after(:each) { remove_patch! } + it 'should patch the code' do expect { described_class.patch }.to change { described_class.patched? }.from(false).to(true) end From 41f16216739c91b7e2851e46550f863fc233e73a Mon Sep 17 00:00:00 2001 From: David Elner Date: Thu, 20 Sep 2018 12:55:11 -0400 Subject: [PATCH 06/23] Refactored: Racecar to use Datadog::Contrib::Integration. (#548) --- lib/ddtrace.rb | 2 +- .../contrib/racecar/configuration/settings.rb | 23 +++++++ lib/ddtrace/contrib/racecar/event.rb | 13 ++-- lib/ddtrace/contrib/racecar/events/batch.rb | 4 +- lib/ddtrace/contrib/racecar/events/message.rb | 4 +- lib/ddtrace/contrib/racecar/ext.rb | 21 ++++++ lib/ddtrace/contrib/racecar/integration.rb | 36 ++++++++++ lib/ddtrace/contrib/racecar/patcher.rb | 65 +++++++------------ spec/ddtrace/contrib/racecar/patcher_spec.rb | 4 +- 9 files changed, 118 insertions(+), 54 deletions(-) create mode 100644 lib/ddtrace/contrib/racecar/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/racecar/ext.rb create mode 100644 lib/ddtrace/contrib/racecar/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 26122040c60..b9da1004d9a 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -60,7 +60,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/integration' require 'ddtrace/contrib/mongodb/patcher' require 'ddtrace/contrib/mysql2/patcher' -require 'ddtrace/contrib/racecar/patcher' +require 'ddtrace/contrib/racecar/integration' require 'ddtrace/contrib/rack/integration' require 'ddtrace/contrib/rails/integration' require 'ddtrace/contrib/rake/patcher' diff --git a/lib/ddtrace/contrib/racecar/configuration/settings.rb b/lib/ddtrace/contrib/racecar/configuration/settings.rb new file mode 100644 index 00000000000..d2aa072f54d --- /dev/null +++ b/lib/ddtrace/contrib/racecar/configuration/settings.rb @@ -0,0 +1,23 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/racecar/ext' + +module Datadog + module Contrib + module Racecar + module Configuration + # Custom settings for the Rack integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + option :tracer, default: Datadog.tracer do |value| + (value || Datadog.tracer).tap do |v| + # Make sure to update tracers of all subscriptions + Events.subscriptions.each do |subscription| + subscription.tracer = v + end + end + end + end + end + end + end +end diff --git a/lib/ddtrace/contrib/racecar/event.rb b/lib/ddtrace/contrib/racecar/event.rb index d167e9391ed..35d8bd4cf67 100644 --- a/lib/ddtrace/contrib/racecar/event.rb +++ b/lib/ddtrace/contrib/racecar/event.rb @@ -1,4 +1,5 @@ require 'ddtrace/contrib/active_support/notifications/event' +require 'ddtrace/contrib/racecar/ext' module Datadog module Contrib @@ -35,12 +36,12 @@ def process(span, event, _id, payload) span.service = configuration[:service_name] span.resource = payload[:consumer_class] - span.set_tag('kafka.topic', payload[:topic]) - span.set_tag('kafka.consumer', payload[:consumer_class]) - span.set_tag('kafka.partition', payload[:partition]) - span.set_tag('kafka.offset', payload[:offset]) if payload.key?(:offset) - span.set_tag('kafka.first_offset', payload[:first_offset]) if payload.key?(:first_offset) - span.set_tag('kafka.message_count', payload[:message_count]) if payload.key?(:message_count) + span.set_tag(Ext::TAG_TOPIC, payload[:topic]) + span.set_tag(Ext::TAG_CONSUMER, payload[:consumer_class]) + span.set_tag(Ext::TAG_PARTITION, payload[:partition]) + span.set_tag(Ext::TAG_OFFSET, payload[:offset]) if payload.key?(:offset) + span.set_tag(Ext::TAG_FIRST_OFFSET, payload[:first_offset]) if payload.key?(:first_offset) + span.set_tag(Ext::TAG_MESSAGE_COUNT, payload[:message_count]) if payload.key?(:message_count) span.set_error(payload[:exception_object]) if payload[:exception_object] end diff --git a/lib/ddtrace/contrib/racecar/events/batch.rb b/lib/ddtrace/contrib/racecar/events/batch.rb index 96b6a276d42..3cfde89469f 100644 --- a/lib/ddtrace/contrib/racecar/events/batch.rb +++ b/lib/ddtrace/contrib/racecar/events/batch.rb @@ -1,3 +1,4 @@ +require 'ddtrace/contrib/racecar/ext' require 'ddtrace/contrib/racecar/event' module Datadog @@ -9,7 +10,6 @@ module Batch include Racecar::Event EVENT_NAME = 'process_batch.racecar'.freeze - SPAN_NAME = 'racecar.batch'.freeze module_function @@ -18,7 +18,7 @@ def event_name end def span_name - self::SPAN_NAME + Ext::SPAN_BATCH end end end diff --git a/lib/ddtrace/contrib/racecar/events/message.rb b/lib/ddtrace/contrib/racecar/events/message.rb index 85901b7242f..e48cfc1b404 100644 --- a/lib/ddtrace/contrib/racecar/events/message.rb +++ b/lib/ddtrace/contrib/racecar/events/message.rb @@ -1,3 +1,4 @@ +require 'ddtrace/contrib/racecar/ext' require 'ddtrace/contrib/racecar/event' module Datadog @@ -9,7 +10,6 @@ module Message include Racecar::Event EVENT_NAME = 'process_message.racecar'.freeze - SPAN_NAME = 'racecar.message'.freeze module_function @@ -18,7 +18,7 @@ def event_name end def span_name - self::SPAN_NAME + Ext::SPAN_MESSAGE end end end diff --git a/lib/ddtrace/contrib/racecar/ext.rb b/lib/ddtrace/contrib/racecar/ext.rb new file mode 100644 index 00000000000..a06add064eb --- /dev/null +++ b/lib/ddtrace/contrib/racecar/ext.rb @@ -0,0 +1,21 @@ +module Datadog + module Contrib + module Racecar + # Racecar integration constants + module Ext + APP = 'racecar'.freeze + SERVICE_NAME = 'racecar'.freeze + + SPAN_BATCH = 'racecar.batch'.freeze + SPAN_MESSAGE = 'racecar.message'.freeze + + TAG_CONSUMER = 'kafka.consumer'.freeze + TAG_FIRST_OFFSET = 'kafka.first_offset'.freeze + TAG_MESSAGE_COUNT = 'kafka.message_count'.freeze + TAG_OFFSET = 'kafka.offset'.freeze + TAG_PARTITION = 'kafka.partition'.freeze + TAG_TOPIC = 'kafka.topic'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/racecar/integration.rb b/lib/ddtrace/contrib/racecar/integration.rb new file mode 100644 index 00000000000..74fa7248d5d --- /dev/null +++ b/lib/ddtrace/contrib/racecar/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/racecar/configuration/settings' +require 'ddtrace/contrib/racecar/patcher' + +module Datadog + module Contrib + module Racecar + # Description of Racecar integration + class Integration + include Contrib::Integration + + register_as :racecar, auto_patch: false + + def self.version + Gem.loaded_specs['racecar'] && Gem.loaded_specs['racecar'].version + end + + def self.present? + super && defined?(::Racecar) + end + + def self.compatible? + super && defined?(::ActiveSupport::Notifications) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/racecar/patcher.rb b/lib/ddtrace/contrib/racecar/patcher.rb index fd190cfa4ab..2528325bddc 100644 --- a/lib/ddtrace/contrib/racecar/patcher.rb +++ b/lib/ddtrace/contrib/racecar/patcher.rb @@ -1,54 +1,37 @@ +require 'ddtrace/contrib/patcher' require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/racecar/ext' require 'ddtrace/contrib/racecar/events' module Datadog module Contrib module Racecar - # Provides instrumentation for `racecar` through ActiveSupport instrumentation signals + # Patcher enables patching of 'racecar' module. module Patcher - include Base + include Contrib::Patcher - register_as :racecar - option :service_name, default: 'racecar' - option :tracer, default: Datadog.tracer do |value| - (value || Datadog.tracer).tap do |v| - # Make sure to update tracers of all subscriptions - Events.subscriptions.each do |subscription| - subscription.tracer = v - end - end - end - - class << self - def patch - return patched? if patched? || !compatible? - - # Subscribe to Racecar events - Events.subscribe! - - # Set service info - configuration[:tracer].set_service_info( - configuration[:service_name], - 'racecar', - Ext::AppTypes::WORKER - ) - - @patched = true - end - - def patched? - return @patched if defined?(@patched) - @patched = false - end - - private + module_function - def configuration - Datadog.configuration[:racecar] - end + def patched? + done?(:racecar) + end - def compatible? - defined?(::Racecar) && defined?(::ActiveSupport::Notifications) + def patch + do_once(:racecar) do + begin + # Subscribe to Racecar events + Events.subscribe! + + # Set service info + configuration = Datadog.configuration[:racecar] + configuration[:tracer].set_service_info( + configuration[:service_name], + Ext::APP, + Datadog::Ext::AppTypes::WORKER + ) + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Racecar integration: #{e}") + end end end end diff --git a/spec/ddtrace/contrib/racecar/patcher_spec.rb b/spec/ddtrace/contrib/racecar/patcher_spec.rb index 9c2361d104c..2679b4f87f0 100644 --- a/spec/ddtrace/contrib/racecar/patcher_spec.rb +++ b/spec/ddtrace/contrib/racecar/patcher_spec.rb @@ -32,7 +32,7 @@ def all_spans end let(:racecar_span) do - all_spans.select { |s| s.name == Datadog::Contrib::Racecar::Events::Message::SPAN_NAME }.first + all_spans.select { |s| s.name == Datadog::Contrib::Racecar::Ext::SPAN_MESSAGE }.first end context 'that doesn\'t raise an error' do @@ -100,7 +100,7 @@ def all_spans end let(:racecar_span) do - all_spans.select { |s| s.name == Datadog::Contrib::Racecar::Events::Batch::SPAN_NAME }.first + all_spans.select { |s| s.name == Datadog::Contrib::Racecar::Ext::SPAN_BATCH }.first end context 'that doesn\'t raise an error' do From 16d9782add709d3e7dee842148fa766291ce3d01 Mon Sep 17 00:00:00 2001 From: David Elner Date: Thu, 20 Sep 2018 13:50:13 -0400 Subject: [PATCH 07/23] Refactored: SuckerPunch to use Datadog::Contrib::Integration. (#549) --- lib/ddtrace.rb | 2 +- .../sucker_punch/configuration/settings.rb | 15 +++++ lib/ddtrace/contrib/sucker_punch/ext.rb | 18 ++++++ .../contrib/sucker_punch/instrumentation.rb | 11 ++-- .../contrib/sucker_punch/integration.rb | 36 ++++++++++++ lib/ddtrace/contrib/sucker_punch/patcher.rb | 57 ++++++++----------- test/contrib/sucker_punch/patcher_test.rb | 8 +-- 7 files changed, 104 insertions(+), 43 deletions(-) create mode 100644 lib/ddtrace/contrib/sucker_punch/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/sucker_punch/ext.rb create mode 100644 lib/ddtrace/contrib/sucker_punch/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index b9da1004d9a..13353ff810b 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -69,5 +69,5 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/rest_client/integration' require 'ddtrace/contrib/sequel/integration' require 'ddtrace/contrib/sidekiq/integration' -require 'ddtrace/contrib/sucker_punch/patcher' +require 'ddtrace/contrib/sucker_punch/integration' require 'ddtrace/monkey' diff --git a/lib/ddtrace/contrib/sucker_punch/configuration/settings.rb b/lib/ddtrace/contrib/sucker_punch/configuration/settings.rb new file mode 100644 index 00000000000..03265243d2c --- /dev/null +++ b/lib/ddtrace/contrib/sucker_punch/configuration/settings.rb @@ -0,0 +1,15 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/sucker_punch/ext' + +module Datadog + module Contrib + module SuckerPunch + module Configuration + # Custom settings for the SuckerPunch integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/sucker_punch/ext.rb b/lib/ddtrace/contrib/sucker_punch/ext.rb new file mode 100644 index 00000000000..26b99a3cb4e --- /dev/null +++ b/lib/ddtrace/contrib/sucker_punch/ext.rb @@ -0,0 +1,18 @@ +module Datadog + module Contrib + module SuckerPunch + # SuckerPunch integration constants + module Ext + APP = 'sucker_punch'.freeze + SERVICE_NAME = 'sucker_punch'.freeze + + SPAN_PERFORM = 'sucker_punch.perform'.freeze + SPAN_PERFORM_ASYNC = 'sucker_punch.perform_async'.freeze + SPAN_PERFORM_IN = 'sucker_punch.perform_in'.freeze + + TAG_PERFORM_IN = 'sucker_punch.perform_in'.freeze + TAG_QUEUE = 'sucker_punch.queue'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/sucker_punch/instrumentation.rb b/lib/ddtrace/contrib/sucker_punch/instrumentation.rb index 410dac56276..67d238fb0a8 100644 --- a/lib/ddtrace/contrib/sucker_punch/instrumentation.rb +++ b/lib/ddtrace/contrib/sucker_punch/instrumentation.rb @@ -1,4 +1,5 @@ require 'sucker_punch' +require 'ddtrace/contrib/sucker_punch/ext' module Datadog module Contrib @@ -15,7 +16,7 @@ def __run_perform(*args) pin = Datadog::Pin.get_from(::SuckerPunch) pin.tracer.provider.context = Datadog::Context.new - __with_instrumentation('sucker_punch.perform') do |span| + __with_instrumentation(Ext::SPAN_PERFORM) do |span| span.resource = "PROCESS #{self}" __run_perform_without_datadog(*args) end @@ -25,7 +26,7 @@ def __run_perform(*args) alias_method :__perform_async, :perform_async def perform_async(*args) - __with_instrumentation('sucker_punch.perform_async') do |span| + __with_instrumentation(Ext::SPAN_PERFORM_ASYNC) do |span| span.resource = "ENQUEUE #{self}" __perform_async(*args) end @@ -33,9 +34,9 @@ def perform_async(*args) alias_method :__perform_in, :perform_in def perform_in(interval, *args) - __with_instrumentation('sucker_punch.perform_in') do |span| + __with_instrumentation(Ext::SPAN_PERFORM_IN) do |span| span.resource = "ENQUEUE #{self}" - span.set_tag('sucker_punch.perform_in', interval) + span.set_tag(Ext::TAG_PERFORM_IN, interval) __perform_in(interval, *args) end end @@ -48,7 +49,7 @@ def __with_instrumentation(name) pin.tracer.trace(name) do |span| span.service = pin.service span.span_type = pin.app_type - span.set_tag('sucker_punch.queue', to_s) + span.set_tag(Ext::TAG_QUEUE, to_s) yield span end end diff --git a/lib/ddtrace/contrib/sucker_punch/integration.rb b/lib/ddtrace/contrib/sucker_punch/integration.rb new file mode 100644 index 00000000000..7548e77ab12 --- /dev/null +++ b/lib/ddtrace/contrib/sucker_punch/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/sucker_punch/configuration/settings' +require 'ddtrace/contrib/sucker_punch/patcher' + +module Datadog + module Contrib + module SuckerPunch + # Description of SuckerPunch integration + class Integration + include Contrib::Integration + + register_as :sucker_punch, auto_patch: true + + def self.version + Gem.loaded_specs['sucker_punch'] && Gem.loaded_specs['sucker_punch'].version + end + + def self.present? + super && defined?(::SuckerPunch) + end + + def self.compatible? + super && Gem::Version.new(::SuckerPunch::VERSION) >= Gem::Version.new('2.0.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/sucker_punch/patcher.rb b/lib/ddtrace/contrib/sucker_punch/patcher.rb index 75843a1cc53..03083f77e31 100644 --- a/lib/ddtrace/contrib/sucker_punch/patcher.rb +++ b/lib/ddtrace/contrib/sucker_punch/patcher.rb @@ -1,53 +1,44 @@ +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/sucker_punch/ext' + module Datadog module Contrib module SuckerPunch - SERVICE = 'sucker_punch'.freeze - COMPATIBLE_WITH = Gem::Version.new('2.0.0') - - # Responsible for hooking the instrumentation into `sucker_punch` + # Patcher enables patching of 'sucker_punch' module. module Patcher - include Base - register_as :sucker_punch, auto_patch: true - option :service_name, default: SERVICE - - @patched = false + include Contrib::Patcher module_function - def patch - return @patched if patched? || !compatible? - - require 'ddtrace/ext/app_types' - require_relative 'exception_handler' - require_relative 'instrumentation' - - add_pin! - ExceptionHandler.patch! - Instrumentation.patch! - - @patched = true - rescue => e - Datadog::Tracer.log.error("Unable to apply SuckerPunch integration: #{e}") - @patched - end - def patched? - @patched + done?(:sucker_punch) end - def compatible? - return unless defined?(::SuckerPunch::VERSION) - - Gem::Version.new(::SuckerPunch::VERSION) >= COMPATIBLE_WITH + def patch + do_once(:sucker_punch) do + begin + require 'ddtrace/contrib/sucker_punch/exception_handler' + require 'ddtrace/contrib/sucker_punch/instrumentation' + + add_pin! + ExceptionHandler.patch! + Instrumentation.patch! + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply SuckerPunch integration: #{e}") + end + end end def add_pin! - Pin.new(get_option(:service_name), app: 'sucker_punch', app_type: Ext::AppTypes::WORKER).tap do |pin| + Pin.new(get_option(:service_name), app: Ext::APP, app_type: Datadog::Ext::AppTypes::WORKER).tap do |pin| pin.onto(::SuckerPunch) end end - private_class_method :compatible?, :add_pin! + def get_option(option) + Datadog.configuration[:sucker_punch].get_option(option) + end end end end diff --git a/test/contrib/sucker_punch/patcher_test.rb b/test/contrib/sucker_punch/patcher_test.rb index 5a68b1c331e..43491afd57c 100644 --- a/test/contrib/sucker_punch/patcher_test.rb +++ b/test/contrib/sucker_punch/patcher_test.rb @@ -35,7 +35,7 @@ def test_successful_job assert_equal('sucker_punch.perform', span.name) assert_equal('PROCESS DummyWorker', span.resource) assert_equal('DummyWorker', span.get_tag('sucker_punch.queue')) - refute_equal(Ext::Errors::STATUS, span.status) + refute_equal(Datadog::Ext::Errors::STATUS, span.status) end def test_failed_job @@ -47,9 +47,9 @@ def test_failed_job assert_equal('sucker_punch.perform', span.name) assert_equal('PROCESS DummyWorker', span.resource) assert_equal('DummyWorker', span.get_tag('sucker_punch.queue')) - assert_equal(Ext::Errors::STATUS, span.status) - assert_equal('ZeroDivisionError', span.get_tag(Ext::Errors::TYPE)) - assert_equal('divided by 0', span.get_tag(Ext::Errors::MSG)) + assert_equal(Datadog::Ext::Errors::STATUS, span.status) + assert_equal('ZeroDivisionError', span.get_tag(Datadog::Ext::Errors::TYPE)) + assert_equal('divided by 0', span.get_tag(Datadog::Ext::Errors::MSG)) end def test_async_enqueueing From 611bcdd58f131e9a0e7957b214779c1956a345af Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:06:38 -0400 Subject: [PATCH 08/23] Refactored: Rake to use Datadog::Contrib::Integration. (#554) --- lib/ddtrace.rb | 2 +- .../contrib/rake/configuration/settings.rb | 18 ++++++ lib/ddtrace/contrib/rake/ext.rb | 18 ++++++ lib/ddtrace/contrib/rake/instrumentation.rb | 15 +++-- lib/ddtrace/contrib/rake/integration.rb | 36 ++++++++++++ lib/ddtrace/contrib/rake/patcher.rb | 56 ++++++++----------- .../contrib/rake/instrumentation_spec.rb | 52 ++++++++--------- 7 files changed, 130 insertions(+), 67 deletions(-) create mode 100644 lib/ddtrace/contrib/rake/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/rake/ext.rb create mode 100644 lib/ddtrace/contrib/rake/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 13353ff810b..e756f81004f 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -63,7 +63,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/racecar/integration' require 'ddtrace/contrib/rack/integration' require 'ddtrace/contrib/rails/integration' -require 'ddtrace/contrib/rake/patcher' +require 'ddtrace/contrib/rake/integration' require 'ddtrace/contrib/redis/patcher' require 'ddtrace/contrib/resque/integration' require 'ddtrace/contrib/rest_client/integration' diff --git a/lib/ddtrace/contrib/rake/configuration/settings.rb b/lib/ddtrace/contrib/rake/configuration/settings.rb new file mode 100644 index 00000000000..90dc9d58d8a --- /dev/null +++ b/lib/ddtrace/contrib/rake/configuration/settings.rb @@ -0,0 +1,18 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/rake/ext' + +module Datadog + module Contrib + module Rake + module Configuration + # Custom settings for the Rake integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + option :tracer, default: Datadog.tracer + option :enabled, default: true + option :quantize, default: {} + end + end + end + end +end diff --git a/lib/ddtrace/contrib/rake/ext.rb b/lib/ddtrace/contrib/rake/ext.rb new file mode 100644 index 00000000000..602dd4f5ead --- /dev/null +++ b/lib/ddtrace/contrib/rake/ext.rb @@ -0,0 +1,18 @@ +module Datadog + module Contrib + module Rake + # Rake integration constants + module Ext + APP = 'rake'.freeze + SERVICE_NAME = 'rake'.freeze + + SPAN_INVOKE = 'rake.invoke'.freeze + SPAN_EXECUTE = 'rake.execute'.freeze + + TAG_EXECUTE_ARGS = 'rake.execute.args'.freeze + TAG_INVOKE_ARGS = 'rake.invoke.args'.freeze + TAG_TASK_ARG_NAMES = 'rake.task.arg_names'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/rake/instrumentation.rb b/lib/ddtrace/contrib/rake/instrumentation.rb index 41a60f734a7..d2d109baa1c 100644 --- a/lib/ddtrace/contrib/rake/instrumentation.rb +++ b/lib/ddtrace/contrib/rake/instrumentation.rb @@ -1,11 +1,10 @@ +require 'ddtrace/contrib/rake/ext' + module Datadog module Contrib module Rake # Instrumentation for Rake tasks module Instrumentation - SPAN_NAME_INVOKE = 'rake.invoke'.freeze - SPAN_NAME_EXECUTE = 'rake.execute'.freeze - def self.included(base) base.send(:prepend, InstanceMethods) end @@ -15,7 +14,7 @@ module InstanceMethods def invoke(*args) return super unless enabled? - tracer.trace(SPAN_NAME_INVOKE, span_options) do |span| + tracer.trace(Ext::SPAN_INVOKE, span_options) do |span| super annotate_invoke!(span, args) end @@ -26,7 +25,7 @@ def invoke(*args) def execute(args = nil) return super unless enabled? - tracer.trace(SPAN_NAME_EXECUTE, span_options) do |span| + tracer.trace(Ext::SPAN_EXECUTE, span_options) do |span| super annotate_execute!(span, args) end @@ -42,15 +41,15 @@ def shutdown_tracer! def annotate_invoke!(span, args) span.resource = name - span.set_tag('rake.task.arg_names', arg_names) - span.set_tag('rake.invoke.args', quantize_args(args)) unless args.nil? + span.set_tag(Ext::TAG_TASK_ARG_NAMES, arg_names) + span.set_tag(Ext::TAG_INVOKE_ARGS, quantize_args(args)) unless args.nil? rescue StandardError => e Datadog::Tracer.log.debug("Error while tracing Rake invoke: #{e.message}") end def annotate_execute!(span, args) span.resource = name - span.set_tag('rake.execute.args', quantize_args(args.to_hash)) unless args.nil? + span.set_tag(Ext::TAG_EXECUTE_ARGS, quantize_args(args.to_hash)) unless args.nil? rescue StandardError => e Datadog::Tracer.log.debug("Error while tracing Rake execute: #{e.message}") end diff --git a/lib/ddtrace/contrib/rake/integration.rb b/lib/ddtrace/contrib/rake/integration.rb new file mode 100644 index 00000000000..322a158fa2f --- /dev/null +++ b/lib/ddtrace/contrib/rake/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/rake/configuration/settings' +require 'ddtrace/contrib/rake/patcher' + +module Datadog + module Contrib + module Rake + # Description of Rake integration + class Integration + include Contrib::Integration + + register_as :rake + + def self.version + Gem.loaded_specs['rake'] && Gem.loaded_specs['rake'].version + end + + def self.present? + super && defined?(::Rake) + end + + def self.compatible? + super && RUBY_VERSION >= '2.0.0' + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/rake/patcher.rb b/lib/ddtrace/contrib/rake/patcher.rb index aac12728975..fb8f8cfedb4 100644 --- a/lib/ddtrace/contrib/rake/patcher.rb +++ b/lib/ddtrace/contrib/rake/patcher.rb @@ -1,51 +1,41 @@ +require 'ddtrace/contrib/patcher' require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/rake/ext' require 'ddtrace/contrib/rake/instrumentation' module Datadog module Contrib module Rake - # Patcher for Rake instrumentation + # Patcher enables patching of 'rake' module. module Patcher - include Base - - register_as :rake - option :service_name, default: 'rake' - option :tracer, default: Datadog.tracer - option :enabled, default: true - option :quantize, default: {} + include Contrib::Patcher module_function - def patch - return patched? if patched? || !compatible? - - patch_rake - - # Set service info - configuration[:tracer].set_service_info( - configuration[:service_name], - 'rake', - Ext::AppTypes::WORKER - ) - - @patched = true - end - def patched? - return @patched if defined?(@patched) - @patched = false + done?(:rake) end - def patch_rake - ::Rake::Task.send(:include, Instrumentation) - end - - def compatible? - RUBY_VERSION >= '2.0.0' && defined?(::Rake) + def patch + do_once(:rake) do + begin + # Add instrumentation patch to Rake task + ::Rake::Task.send(:include, Instrumentation) + + # Set service info + get_option(:tracer).set_service_info( + get_option(:service_name), + Ext::APP, + Datadog::Ext::AppTypes::WORKER + ) + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Rake integration: #{e}") + end + end end - def configuration - Datadog.configuration[:rake] + def get_option(option) + Datadog.configuration[:rake].get_option(option) end end end diff --git a/spec/ddtrace/contrib/rake/instrumentation_spec.rb b/spec/ddtrace/contrib/rake/instrumentation_spec.rb index bd250ebc3f3..5e8568f8d59 100644 --- a/spec/ddtrace/contrib/rake/instrumentation_spec.rb +++ b/spec/ddtrace/contrib/rake/instrumentation_spec.rb @@ -13,7 +13,7 @@ let(:span) { spans.first } before(:each) do - skip('Rake integration incompatible.') unless Datadog::Contrib::Rake::Patcher.compatible? + skip('Rake integration incompatible.') unless Datadog::Contrib::Rake::Integration.compatible? # Reset options (that might linger from other tests) Datadog.configuration[:rake].reset_options! @@ -74,8 +74,8 @@ def reset_task!(task_name) task.invoke(*args) end - let(:invoke_span) { spans.find { |s| s.name == described_class::SPAN_NAME_INVOKE } } - let(:execute_span) { spans.find { |s| s.name == described_class::SPAN_NAME_EXECUTE } } + let(:invoke_span) { spans.find { |s| s.name == Datadog::Contrib::Rake::Ext::SPAN_INVOKE } } + let(:execute_span) { spans.find { |s| s.name == Datadog::Contrib::Rake::Ext::SPAN_EXECUTE } } it do expect(spans).to have(2).items @@ -83,7 +83,7 @@ def reset_task!(task_name) describe '\'rake.invoke\' span' do it do - expect(invoke_span.name).to eq(described_class::SPAN_NAME_INVOKE) + expect(invoke_span.name).to eq(Datadog::Contrib::Rake::Ext::SPAN_INVOKE) expect(invoke_span.resource).to eq(task_name.to_s) expect(invoke_span.parent_id).to eq(0) end @@ -91,7 +91,7 @@ def reset_task!(task_name) describe '\'rake.execute\' span' do it do - expect(execute_span.name).to eq(described_class::SPAN_NAME_EXECUTE) + expect(execute_span.name).to eq(Datadog::Contrib::Rake::Ext::SPAN_EXECUTE) expect(execute_span.resource).to eq(task_name.to_s) expect(execute_span.parent_id).to eq(invoke_span.span_id) end @@ -124,15 +124,15 @@ def reset_task!(task_name) it_behaves_like 'a single task execution' do describe '\'rake.invoke\' span tags' do it do - expect(invoke_span.get_tag('rake.task.arg_names')).to eq([].to_s) - expect(invoke_span.get_tag('rake.invoke.args')).to eq(['?'].to_s) + expect(invoke_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_TASK_ARG_NAMES)).to eq([].to_s) + expect(invoke_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_INVOKE_ARGS)).to eq(['?'].to_s) end end describe '\'rake.execute\' span tags' do it do - expect(execute_span.get_tag('rake.task.arg_names')).to be nil - expect(execute_span.get_tag('rake.execute.args')).to eq({}.to_s) + expect(execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_TASK_ARG_NAMES)).to be nil + expect(execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_EXECUTE_ARGS)).to eq({}.to_s) end end end @@ -144,15 +144,17 @@ def reset_task!(task_name) it_behaves_like 'a single task execution' do describe '\'rake.invoke\' span tags' do it do - expect(invoke_span.get_tag('rake.task.arg_names')).to eq([:one, :two, :three].to_s) - expect(invoke_span.get_tag('rake.invoke.args')).to eq(['?'].to_s) + expect(invoke_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_TASK_ARG_NAMES)).to eq([:one, :two, :three].to_s) + expect(invoke_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_INVOKE_ARGS)).to eq(['?'].to_s) end end describe '\'rake.execute\' span tags' do it do - expect(execute_span.get_tag('rake.arg_names')).to be nil - expect(execute_span.get_tag('rake.execute.args')).to eq({ one: '?', two: '?', three: '?' }.to_s) + expect(execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_TASK_ARG_NAMES)).to be nil + expect(execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_EXECUTE_ARGS)).to eq( + { one: '?', two: '?', three: '?' }.to_s + ) end end end @@ -189,16 +191,16 @@ def reset_task!(task_name) task.invoke(*args) end - let(:invoke_span) { spans.find { |s| s.name == described_class::SPAN_NAME_INVOKE } } + let(:invoke_span) { spans.find { |s| s.name == Datadog::Contrib::Rake::Ext::SPAN_INVOKE } } let(:prerequisite_task_execute_span) do spans.find do |s| - s.name == described_class::SPAN_NAME_EXECUTE \ + s.name == Datadog::Contrib::Rake::Ext::SPAN_EXECUTE \ && s.resource == prerequisite_task_name.to_s end end let(:task_execute_span) do spans.find do |s| - s.name == described_class::SPAN_NAME_EXECUTE \ + s.name == Datadog::Contrib::Rake::Ext::SPAN_EXECUTE \ && s.resource == task_name.to_s end end @@ -209,31 +211,31 @@ def reset_task!(task_name) describe '\'rake.invoke\' span' do it do - expect(invoke_span.name).to eq(described_class::SPAN_NAME_INVOKE) + expect(invoke_span.name).to eq(Datadog::Contrib::Rake::Ext::SPAN_INVOKE) expect(invoke_span.resource).to eq(task_name.to_s) expect(invoke_span.parent_id).to eq(0) - expect(invoke_span.get_tag('rake.task.arg_names')).to eq([].to_s) - expect(invoke_span.get_tag('rake.invoke.args')).to eq(['?'].to_s) + expect(invoke_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_TASK_ARG_NAMES)).to eq([].to_s) + expect(invoke_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_INVOKE_ARGS)).to eq(['?'].to_s) end end describe 'prerequisite \'rake.execute\' span' do it do - expect(prerequisite_task_execute_span.name).to eq(described_class::SPAN_NAME_EXECUTE) + expect(prerequisite_task_execute_span.name).to eq(Datadog::Contrib::Rake::Ext::SPAN_EXECUTE) expect(prerequisite_task_execute_span.resource).to eq(prerequisite_task_name.to_s) expect(prerequisite_task_execute_span.parent_id).to eq(invoke_span.span_id) - expect(prerequisite_task_execute_span.get_tag('rake.task.arg_names')).to be nil - expect(prerequisite_task_execute_span.get_tag('rake.execute.args')).to eq({}.to_s) + expect(prerequisite_task_execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_TASK_ARG_NAMES)).to be nil + expect(prerequisite_task_execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_EXECUTE_ARGS)).to eq({}.to_s) end end describe 'task \'rake.execute\' span' do it do - expect(task_execute_span.name).to eq(described_class::SPAN_NAME_EXECUTE) + expect(task_execute_span.name).to eq(Datadog::Contrib::Rake::Ext::SPAN_EXECUTE) expect(task_execute_span.resource).to eq(task_name.to_s) expect(task_execute_span.parent_id).to eq(invoke_span.span_id) - expect(task_execute_span.get_tag('rake.task.arg_names')).to be nil - expect(task_execute_span.get_tag('rake.execute.args')).to eq({}.to_s) + expect(task_execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_TASK_ARG_NAMES)).to be nil + expect(task_execute_span.get_tag(Datadog::Contrib::Rake::Ext::TAG_EXECUTE_ARGS)).to eq({}.to_s) end end end From d96e6609d4d57087f703f8062e4393bbc6924f49 Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:07:59 -0400 Subject: [PATCH 09/23] Refactored: HTTP to use Datadog::Contrib::Integration. (#556) --- lib/ddtrace.rb | 2 +- lib/ddtrace/contrib/http/circuit_breaker.rb | 39 +++++++++++ .../contrib/http/configuration/settings.rb | 17 +++++ lib/ddtrace/contrib/http/ext.rb | 13 ++++ lib/ddtrace/contrib/http/integration.rb | 32 +++++++++ lib/ddtrace/contrib/http/patcher.rb | 68 +++---------------- spec/ddtrace/contrib/http/patcher_spec.rb | 4 +- 7 files changed, 114 insertions(+), 61 deletions(-) create mode 100644 lib/ddtrace/contrib/http/circuit_breaker.rb create mode 100644 lib/ddtrace/contrib/http/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/http/ext.rb create mode 100644 lib/ddtrace/contrib/http/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index e756f81004f..d8e102937cb 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -56,7 +56,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/grape/patcher' require 'ddtrace/contrib/graphql/patcher' require 'ddtrace/contrib/grpc/patcher' -require 'ddtrace/contrib/http/patcher' +require 'ddtrace/contrib/http/integration' require 'ddtrace/contrib/integration' require 'ddtrace/contrib/mongodb/patcher' require 'ddtrace/contrib/mysql2/patcher' diff --git a/lib/ddtrace/contrib/http/circuit_breaker.rb b/lib/ddtrace/contrib/http/circuit_breaker.rb new file mode 100644 index 00000000000..2727c8a1848 --- /dev/null +++ b/lib/ddtrace/contrib/http/circuit_breaker.rb @@ -0,0 +1,39 @@ + +module Datadog + module Contrib + module HTTP + # HTTP integration circuit breaker behavior + # For avoiding recursive traces. + module CircuitBreaker + def should_skip_tracing?(req, address, port, transport, pin) + # we don't want to trace our own call to the API (they use net/http) + # when we know the host & port (from the URI) we use it, else (most-likely + # called with a block) rely on the URL at the end. + if req.respond_to?(:uri) && req.uri + if req.uri.host.to_s == transport.hostname.to_s && + req.uri.port.to_i == transport.port.to_i + return true + end + elsif address && port && + address.to_s == transport.hostname.to_s && + port.to_i == transport.port.to_i + return true + end + # we don't want a "shotgun" effect with two nested traces for one + # logical get, and request is likely to call itself recursively + active = pin.tracer.active_span + return true if active && (active.name == Ext::SPAN_REQUEST) + false + end + + def should_skip_distributed_tracing?(pin) + if pin.config && pin.config.key?(:distributed_tracing) + return !pin.config[:distributed_tracing] + end + + !Datadog.configuration[:http][:distributed_tracing] + end + end + end + end +end diff --git a/lib/ddtrace/contrib/http/configuration/settings.rb b/lib/ddtrace/contrib/http/configuration/settings.rb new file mode 100644 index 00000000000..3609403a5dd --- /dev/null +++ b/lib/ddtrace/contrib/http/configuration/settings.rb @@ -0,0 +1,17 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/http/ext' + +module Datadog + module Contrib + module HTTP + module Configuration + # Custom settings for the HTTP integration + class Settings < Contrib::Configuration::Settings + option :distributed_tracing, default: false + option :service_name, default: Ext::SERVICE_NAME + option :tracer, default: Datadog.tracer + end + end + end + end +end diff --git a/lib/ddtrace/contrib/http/ext.rb b/lib/ddtrace/contrib/http/ext.rb new file mode 100644 index 00000000000..fbffd405fc2 --- /dev/null +++ b/lib/ddtrace/contrib/http/ext.rb @@ -0,0 +1,13 @@ +module Datadog + module Contrib + module HTTP + # HTTP integration constants + module Ext + APP = 'net/http'.freeze + SERVICE_NAME = 'net/http'.freeze + + SPAN_REQUEST = 'http.request'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/http/integration.rb b/lib/ddtrace/contrib/http/integration.rb new file mode 100644 index 00000000000..f1c764a1f33 --- /dev/null +++ b/lib/ddtrace/contrib/http/integration.rb @@ -0,0 +1,32 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/http/configuration/settings' +require 'ddtrace/contrib/http/patcher' +require 'ddtrace/contrib/http/circuit_breaker' + +module Datadog + module Contrib + # HTTP integration + module HTTP + extend CircuitBreaker + + # Description of HTTP integration + class Integration + include Contrib::Integration + + register_as :http, auto_patch: true + + def self.version + Gem::Version.new(RUBY_VERSION) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/http/patcher.rb b/lib/ddtrace/contrib/http/patcher.rb index 98da5b0e88b..a6803830fdd 100644 --- a/lib/ddtrace/contrib/http/patcher.rb +++ b/lib/ddtrace/contrib/http/patcher.rb @@ -1,63 +1,23 @@ -# requirements should be kept minimal as Patcher is a shared requirement. +require 'ddtrace/contrib/patcher' +require 'ddtrace/contrib/http/ext' module Datadog module Contrib # Datadog Net/HTTP integration. module HTTP - URL = 'http.url'.freeze - METHOD = 'http.method'.freeze - BODY = 'http.body'.freeze - - NAME = 'http.request'.freeze - APP = 'net/http'.freeze - SERVICE = 'net/http'.freeze - - module_function - - def should_skip_tracing?(req, address, port, transport, pin) - # we don't want to trace our own call to the API (they use net/http) - # when we know the host & port (from the URI) we use it, else (most-likely - # called with a block) rely on the URL at the end. - if req.respond_to?(:uri) && req.uri - if req.uri.host.to_s == transport.hostname.to_s && - req.uri.port.to_i == transport.port.to_i - return true - end - elsif address && port && - address.to_s == transport.hostname.to_s && - port.to_i == transport.port.to_i - return true - end - # we don't want a "shotgun" effect with two nested traces for one - # logical get, and request is likely to call itself recursively - active = pin.tracer.active_span() - return true if active && (active.name == NAME) - false - end - - def should_skip_distributed_tracing?(pin) - if pin.config && pin.config.key?(:distributed_tracing) - return !pin.config[:distributed_tracing] - end - - !Datadog.configuration[:http][:distributed_tracing] - end - # Patcher enables patching of 'net/http' module. module Patcher - include Base - register_as :http, auto_patch: true - option :distributed_tracing, default: false - option :service_name, default: SERVICE - option :tracer, default: Datadog.tracer - - @patched = false + include Contrib::Patcher module_function + def patched? + done?(:http) + end + # patch applies our patch if needed def patch - unless @patched + do_once(:http) do begin require 'uri' require 'ddtrace/pin' @@ -67,18 +27,10 @@ def patch require 'ddtrace/ext/distributed' patch_http - - @patched = true rescue StandardError => e Datadog::Tracer.log.error("Unable to apply net/http integration: #{e}") end end - @patched - end - - # patched? tells whether patch has been successfully applied - def patched? - @patched end # rubocop:disable Metrics/MethodLength @@ -94,7 +46,7 @@ def datadog_pin service = Datadog.configuration[:http][:service_name] tracer = Datadog.configuration[:http][:tracer] - Datadog::Pin.new(service, app: APP, app_type: Datadog::Ext::AppTypes::WEB, tracer: tracer) + Datadog::Pin.new(service, app: Ext::APP, app_type: Datadog::Ext::AppTypes::WEB, tracer: tracer) end end @@ -108,7 +60,7 @@ def request(req, body = nil, &block) # :yield: +response+ return request_without_datadog(req, body, &block) end - pin.tracer.trace(NAME) do |span| + pin.tracer.trace(Ext::SPAN_REQUEST) do |span| begin span.service = pin.service span.span_type = Datadog::Ext::HTTP::TYPE diff --git a/spec/ddtrace/contrib/http/patcher_spec.rb b/spec/ddtrace/contrib/http/patcher_spec.rb index bc2c4b7d4c6..affbd28c881 100644 --- a/spec/ddtrace/contrib/http/patcher_spec.rb +++ b/spec/ddtrace/contrib/http/patcher_spec.rb @@ -12,14 +12,14 @@ stub_request(:any, host) - Datadog.registry[:http].reset_options! + Datadog.configuration[:http].reset_options! Datadog.configure do |c| c.use :http, tracer: tracer end end let(:request_span) do - tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::HTTP::NAME } + tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::HTTP::Ext::SPAN_REQUEST } end describe 'with default configuration' do From 57c29fff95b026c047d1b0036bd0f065168e2fde Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:08:40 -0400 Subject: [PATCH 10/23] Refactored: AWS to use Datadog::Contrib::Integration. (#562) --- lib/ddtrace.rb | 2 +- .../contrib/aws/configuration/settings.rb | 15 ++++ lib/ddtrace/contrib/aws/ext.rb | 21 +++++ lib/ddtrace/contrib/aws/instrumentation.rb | 20 +++-- lib/ddtrace/contrib/aws/integration.rb | 32 ++++++++ lib/ddtrace/contrib/aws/patcher.rb | 82 +++++++++---------- 6 files changed, 121 insertions(+), 51 deletions(-) create mode 100644 lib/ddtrace/contrib/aws/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/aws/ext.rb create mode 100644 lib/ddtrace/contrib/aws/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index d8e102937cb..50e71b36d6b 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -46,7 +46,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/base' require 'ddtrace/contrib/active_model_serializers/patcher' require 'ddtrace/contrib/active_record/integration' -require 'ddtrace/contrib/aws/patcher' +require 'ddtrace/contrib/aws/integration' require 'ddtrace/contrib/concurrent_ruby/integration' require 'ddtrace/contrib/dalli/patcher' require 'ddtrace/contrib/delayed_job/integration' diff --git a/lib/ddtrace/contrib/aws/configuration/settings.rb b/lib/ddtrace/contrib/aws/configuration/settings.rb new file mode 100644 index 00000000000..c098f5e64c4 --- /dev/null +++ b/lib/ddtrace/contrib/aws/configuration/settings.rb @@ -0,0 +1,15 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/aws/ext' + +module Datadog + module Contrib + module Aws + module Configuration + # Custom settings for the AWS integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/aws/ext.rb b/lib/ddtrace/contrib/aws/ext.rb new file mode 100644 index 00000000000..639ed08c8b6 --- /dev/null +++ b/lib/ddtrace/contrib/aws/ext.rb @@ -0,0 +1,21 @@ +module Datadog + module Contrib + module Aws + # AWS integration constants + module Ext + APP = 'aws'.freeze + SERVICE_NAME = 'aws'.freeze + + SPAN_COMMAND = 'aws.command'.freeze + + TAG_AGENT = 'aws.agent'.freeze + TAG_OPERATION = 'aws.operation'.freeze + TAG_REGION = 'aws.region'.freeze + TAG_PATH = 'path'.freeze + TAG_HOST = 'host'.freeze + + TAG_DEFAULT_AGENT = 'aws-sdk-ruby'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/aws/instrumentation.rb b/lib/ddtrace/contrib/aws/instrumentation.rb index 164f0872817..d68e4ea586e 100644 --- a/lib/ddtrace/contrib/aws/instrumentation.rb +++ b/lib/ddtrace/contrib/aws/instrumentation.rb @@ -1,3 +1,5 @@ +require 'ddtrace/contrib/aws/ext' + module Datadog module Contrib module Aws @@ -15,7 +17,7 @@ def call(context) return @handler.call(context) unless pin && pin.enabled? - pin.tracer.trace(RESOURCE) do |span| + pin.tracer.trace(Ext::SPAN_COMMAND) do |span| result = @handler.call(context) annotate!(span, pin, ParsedContext.new(context)) result @@ -27,15 +29,15 @@ def call(context) def annotate!(span, pin, context) span.service = pin.service span.span_type = pin.app_type - span.name = RESOURCE + span.name = Ext::SPAN_COMMAND span.resource = context.safely(:resource) - span.set_tag('aws.agent', AGENT) - span.set_tag('aws.operation', context.safely(:operation)) - span.set_tag('aws.region', context.safely(:region)) - span.set_tag('path', context.safely(:path)) - span.set_tag('host', context.safely(:host)) - span.set_tag(Ext::HTTP::METHOD, context.safely(:http_method)) - span.set_tag(Ext::HTTP::STATUS_CODE, context.safely(:status_code)) + span.set_tag(Ext::TAG_AGENT, Ext::TAG_DEFAULT_AGENT) + span.set_tag(Ext::TAG_OPERATION, context.safely(:operation)) + span.set_tag(Ext::TAG_REGION, context.safely(:region)) + span.set_tag(Ext::TAG_PATH, context.safely(:path)) + span.set_tag(Ext::TAG_HOST, context.safely(:host)) + span.set_tag(Datadog::Ext::HTTP::METHOD, context.safely(:http_method)) + span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, context.safely(:status_code)) end end end diff --git a/lib/ddtrace/contrib/aws/integration.rb b/lib/ddtrace/contrib/aws/integration.rb new file mode 100644 index 00000000000..e9c6ab2f421 --- /dev/null +++ b/lib/ddtrace/contrib/aws/integration.rb @@ -0,0 +1,32 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/aws/configuration/settings' +require 'ddtrace/contrib/aws/patcher' + +module Datadog + module Contrib + module Aws + # Description of AWS integration + class Integration + include Contrib::Integration + + register_as :aws, auto_patch: true + + def self.version + Gem.loaded_specs['aws-sdk'] && Gem.loaded_specs['aws-sdk'].version + end + + def self.present? + super && defined?(::Seahorse::Client::Base) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/aws/patcher.rb b/lib/ddtrace/contrib/aws/patcher.rb index f280f34f0ba..90951a1b293 100644 --- a/lib/ddtrace/contrib/aws/patcher.rb +++ b/lib/ddtrace/contrib/aws/patcher.rb @@ -1,57 +1,57 @@ +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/aws/ext' + module Datadog module Contrib module Aws - AGENT = 'aws-sdk-ruby'.freeze - RESOURCE = 'aws.command'.freeze - - # Responsible for hooking the instrumentation into aws-sdk + # Patcher enables patching of 'aws' module. module Patcher - include Base - register_as :aws, auto_patch: true - option :service_name, default: 'aws' - - @patched = false - - class << self - def patch - return @patched if patched? || !defined?(Seahorse::Client::Base) + include Contrib::Patcher - require 'ddtrace/ext/app_types' - require 'ddtrace/contrib/aws/parsed_context' - require 'ddtrace/contrib/aws/instrumentation' - require 'ddtrace/contrib/aws/services' + module_function - add_pin - add_plugin(Seahorse::Client::Base, *loaded_constants) - - @patched = true - rescue => e - Datadog::Tracer.log.error("Unable to apply AWS integration: #{e}") - @patched - end + def patched? + done?(:aws) + end - def patched? - @patched + def patch + do_once(:aws) do + begin + require 'ddtrace/contrib/aws/parsed_context' + require 'ddtrace/contrib/aws/instrumentation' + require 'ddtrace/contrib/aws/services' + + add_pin + add_plugin(Seahorse::Client::Base, *loaded_constants) + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply AWS integration: #{e}") + end end + end - private + def add_pin + Pin + .new( + get_option(:service_name), + app: Ext::APP, + app_type: Datadog::Ext::AppTypes::WEB + ).onto(::Aws) + end - def add_pin - Pin - .new(get_option(:service_name), app: 'aws', app_type: Ext::AppTypes::WEB) - .onto(::Aws) - end + def add_plugin(*targets) + targets.each { |klass| klass.add_plugin(Instrumentation) } + end - def add_plugin(*targets) - targets.each { |klass| klass.add_plugin(Instrumentation) } + def loaded_constants + SERVICES.each_with_object([]) do |service, constants| + next if ::Aws.autoload?(service) + constants << ::Aws.const_get(service).const_get(:Client) rescue next end + end - def loaded_constants - SERVICES.each_with_object([]) do |service, constants| - next if ::Aws.autoload?(service) - constants << ::Aws.const_get(service).const_get(:Client) rescue next - end - end + def get_option(option) + Datadog.configuration[:aws].get_option(option) end end end From 4a5e3131215550928d5c6faa42e4554e3bd30a3c Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:09:06 -0400 Subject: [PATCH 11/23] Implement Mysql2 integration configuration (#559) * Refactored: Mysql2 to use Datadog::Contrib::Integration. * Fixed: Mysql2 tests not working locally. --- docker-compose.yml | 1 + lib/ddtrace.rb | 2 +- lib/ddtrace/contrib/mysql2/client.rb | 14 ++++---- .../contrib/mysql2/configuration/settings.rb | 16 +++++++++ lib/ddtrace/contrib/mysql2/ext.rb | 15 ++++++++ lib/ddtrace/contrib/mysql2/integration.rb | 32 +++++++++++++++++ lib/ddtrace/contrib/mysql2/patcher.rb | 34 +++++++------------ spec/ddtrace/contrib/mysql2/patcher_spec.rb | 2 +- 8 files changed, 86 insertions(+), 30 deletions(-) create mode 100644 lib/ddtrace/contrib/mysql2/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/mysql2/ext.rb create mode 100644 lib/ddtrace/contrib/mysql2/integration.rb diff --git a/docker-compose.yml b/docker-compose.yml index d1dcd7b4c02..5215531fe8b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -217,6 +217,7 @@ services: mysql: image: mysql:5.6 environment: + - MYSQL_DATABASE=$TEST_MYSQL_DB - MYSQL_ROOT_PASSWORD=$TEST_MYSQL_ROOT_PASSWORD - MYSQL_PASSWORD=$TEST_MYSQL_PASSWORD - MYSQL_USER=$TEST_MYSQL_USER diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 50e71b36d6b..51aee4dc2b1 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -59,7 +59,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/http/integration' require 'ddtrace/contrib/integration' require 'ddtrace/contrib/mongodb/patcher' -require 'ddtrace/contrib/mysql2/patcher' +require 'ddtrace/contrib/mysql2/integration' require 'ddtrace/contrib/racecar/integration' require 'ddtrace/contrib/rack/integration' require 'ddtrace/contrib/rails/integration' diff --git a/lib/ddtrace/contrib/mysql2/client.rb b/lib/ddtrace/contrib/mysql2/client.rb index 91a8498dba5..0a1ea487464 100644 --- a/lib/ddtrace/contrib/mysql2/client.rb +++ b/lib/ddtrace/contrib/mysql2/client.rb @@ -1,5 +1,7 @@ -require 'ddtrace/ext/sql' require 'ddtrace/ext/app_types' +require 'ddtrace/ext/net' +require 'ddtrace/ext/sql' +require 'ddtrace/contrib/mysql2/ext' module Datadog module Contrib @@ -34,13 +36,13 @@ module InstanceMethods end def query(sql, options = {}) - datadog_pin.tracer.trace('mysql2.query') do |span| + datadog_pin.tracer.trace(Ext::SPAN_QUERY) do |span| span.resource = sql span.service = datadog_pin.service span.span_type = Datadog::Ext::SQL::TYPE - span.set_tag('mysql2.db.name', query_options[:database]) - span.set_tag('out.host', query_options[:host]) - span.set_tag('out.port', query_options[:port]) + span.set_tag(Ext::TAG_DB_NAME, query_options[:database]) + span.set_tag(Datadog::Ext::NET::TARGET_HOST, query_options[:host]) + span.set_tag(Datadog::Ext::NET::TARGET_PORT, query_options[:port]) super(sql, options) end end @@ -48,7 +50,7 @@ def query(sql, options = {}) def datadog_pin @datadog_pin ||= Datadog::Pin.new( Datadog.configuration[:mysql2][:service_name], - app: 'mysql2', + app: Ext::APP, app_type: Datadog::Ext::AppTypes::DB, tracer: Datadog.configuration[:mysql2][:tracer] ) diff --git a/lib/ddtrace/contrib/mysql2/configuration/settings.rb b/lib/ddtrace/contrib/mysql2/configuration/settings.rb new file mode 100644 index 00000000000..27cde8bb78b --- /dev/null +++ b/lib/ddtrace/contrib/mysql2/configuration/settings.rb @@ -0,0 +1,16 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/mysql2/ext' + +module Datadog + module Contrib + module Mysql2 + module Configuration + # Custom settings for the Mysql2 integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + option :tracer, default: Datadog.tracer + end + end + end + end +end diff --git a/lib/ddtrace/contrib/mysql2/ext.rb b/lib/ddtrace/contrib/mysql2/ext.rb new file mode 100644 index 00000000000..1e6cfc50557 --- /dev/null +++ b/lib/ddtrace/contrib/mysql2/ext.rb @@ -0,0 +1,15 @@ +module Datadog + module Contrib + module Mysql2 + # Mysql2 integration constants + module Ext + APP = 'mysql2'.freeze + SERVICE_NAME = 'mysql2'.freeze + + SPAN_QUERY = 'mysql2.query'.freeze + + TAG_DB_NAME = 'mysql2.db.name'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/mysql2/integration.rb b/lib/ddtrace/contrib/mysql2/integration.rb new file mode 100644 index 00000000000..fd7b81aa91a --- /dev/null +++ b/lib/ddtrace/contrib/mysql2/integration.rb @@ -0,0 +1,32 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/mysql2/configuration/settings' +require 'ddtrace/contrib/mysql2/patcher' + +module Datadog + module Contrib + module Mysql2 + # Description of Mysql2 integration + class Integration + include Contrib::Integration + + register_as :mysql2 + + def self.version + Gem.loaded_specs['mysql2'] && Gem.loaded_specs['mysql2'].version + end + + def self.present? + super && defined?(::Mysql2) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/mysql2/patcher.rb b/lib/ddtrace/contrib/mysql2/patcher.rb index 27dbe65a393..24f9aa9e53b 100644 --- a/lib/ddtrace/contrib/mysql2/patcher.rb +++ b/lib/ddtrace/contrib/mysql2/patcher.rb @@ -1,37 +1,27 @@ +require 'ddtrace/contrib/patcher' require 'ddtrace/contrib/mysql2/client' module Datadog module Contrib module Mysql2 - # Mysql2 patcher + # Patcher enables patching of 'mysql2' module. module Patcher - include Base - - register_as :mysql2 - option :service_name, default: 'mysql2' - option :tracer, default: Datadog.tracer - - @patched = false + include Contrib::Patcher module_function - def patch - return @patched if patched? || !compatible? - - patch_mysql2_client - - @patched = true - rescue StandardError => e - Tracer.log.error("Unable to apply mysql2 integration: #{e}") - @patched - end - def patched? - @patched + done?(:mysql2) end - def compatible? - defined?(::Mysql2) + def patch + do_once(:mysql2) do + begin + patch_mysql2_client + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply mysql2 integration: #{e}") + end + end end def patch_mysql2_client diff --git a/spec/ddtrace/contrib/mysql2/patcher_spec.rb b/spec/ddtrace/contrib/mysql2/patcher_spec.rb index 8d883da6f32..44924f94219 100644 --- a/spec/ddtrace/contrib/mysql2/patcher_spec.rb +++ b/spec/ddtrace/contrib/mysql2/patcher_spec.rb @@ -19,7 +19,7 @@ let(:host) { ENV.fetch('TEST_MYSQL_HOST') { '127.0.0.1' } } let(:port) { ENV.fetch('TEST_MYSQL_PORT') { '3306' } } let(:database) { ENV.fetch('TEST_MYSQL_DB') { 'mysql' } } - let(:username) { ENV.fetch('TEST_MYSQL_USERNAME') { 'root' } } + let(:username) { ENV.fetch('TEST_MYSQL_USER') { 'root' } } let(:password) { ENV.fetch('TEST_MYSQL_PASSWORD') { 'root' } } let(:pin) { client.datadog_pin } From c754c2c415e22f2912eaf47d8b7b9a248a0c5368 Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:09:36 -0400 Subject: [PATCH 12/23] Refactored: Elasticsearch to use Datadog::Contrib::Integration. (#561) --- lib/ddtrace.rb | 2 +- .../elasticsearch/configuration/settings.rb | 16 +++++ lib/ddtrace/contrib/elasticsearch/ext.rb | 18 +++++ .../contrib/elasticsearch/integration.rb | 37 +++++++++++ lib/ddtrace/contrib/elasticsearch/patcher.rb | 66 ++++++++----------- 5 files changed, 99 insertions(+), 40 deletions(-) create mode 100644 lib/ddtrace/contrib/elasticsearch/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/elasticsearch/ext.rb create mode 100644 lib/ddtrace/contrib/elasticsearch/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 51aee4dc2b1..9154909e7a8 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -50,7 +50,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/concurrent_ruby/integration' require 'ddtrace/contrib/dalli/patcher' require 'ddtrace/contrib/delayed_job/integration' -require 'ddtrace/contrib/elasticsearch/patcher' +require 'ddtrace/contrib/elasticsearch/integration' require 'ddtrace/contrib/excon/patcher' require 'ddtrace/contrib/faraday/patcher' require 'ddtrace/contrib/grape/patcher' diff --git a/lib/ddtrace/contrib/elasticsearch/configuration/settings.rb b/lib/ddtrace/contrib/elasticsearch/configuration/settings.rb new file mode 100644 index 00000000000..5fd362d095e --- /dev/null +++ b/lib/ddtrace/contrib/elasticsearch/configuration/settings.rb @@ -0,0 +1,16 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/elasticsearch/ext' + +module Datadog + module Contrib + module Elasticsearch + module Configuration + # Custom settings for the Elasticsearch integration + class Settings < Contrib::Configuration::Settings + option :quantize, default: {} + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/elasticsearch/ext.rb b/lib/ddtrace/contrib/elasticsearch/ext.rb new file mode 100644 index 00000000000..ae34391d6ac --- /dev/null +++ b/lib/ddtrace/contrib/elasticsearch/ext.rb @@ -0,0 +1,18 @@ +module Datadog + module Contrib + module Elasticsearch + # Elasticsearch integration constants + module Ext + APP = 'elasticsearch'.freeze + SERVICE_NAME = 'elasticsearch'.freeze + + SPAN_QUERY = 'elasticsearch.query'.freeze + + TAG_BODY = 'elasticsearch.body'.freeze + TAG_METHOD = 'elasticsearch.method'.freeze + TAG_PARAMS = 'elasticsearch.params'.freeze + TAG_URL = 'elasticsearch.url'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/elasticsearch/integration.rb b/lib/ddtrace/contrib/elasticsearch/integration.rb new file mode 100644 index 00000000000..c0996dbe941 --- /dev/null +++ b/lib/ddtrace/contrib/elasticsearch/integration.rb @@ -0,0 +1,37 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/elasticsearch/configuration/settings' +require 'ddtrace/contrib/elasticsearch/patcher' + +module Datadog + module Contrib + module Elasticsearch + # Description of Elasticsearch integration + class Integration + include Contrib::Integration + + register_as :elasticsearch, auto_patch: true + + def self.version + Gem.loaded_specs['elasticsearch-transport'] \ + && Gem.loaded_specs['elasticsearch-transport'].version + end + + def self.present? + super && defined?(::Elasticsearch::Transport) + end + + def self.compatible? + super && version >= Gem::Version.new('1.0.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/elasticsearch/patcher.rb b/lib/ddtrace/contrib/elasticsearch/patcher.rb index 28ce02c98bf..1befbad9997 100644 --- a/lib/ddtrace/contrib/elasticsearch/patcher.rb +++ b/lib/ddtrace/contrib/elasticsearch/patcher.rb @@ -1,45 +1,34 @@ -# requirements should be kept minimal as Patcher is a shared requirement. +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' +require 'ddtrace/ext/net' +require 'ddtrace/contrib/elasticsearch/ext' module Datadog module Contrib module Elasticsearch - URL = 'elasticsearch.url'.freeze - METHOD = 'elasticsearch.method'.freeze - PARAMS = 'elasticsearch.params'.freeze - BODY = 'elasticsearch.body'.freeze - - SERVICE = 'elasticsearch'.freeze - - # Patcher enables patching of 'elasticsearch/transport' module. + # Patcher enables patching of 'elasticsearch' module. module Patcher - include Base - register_as :elasticsearch, auto_patch: true - option :service_name, default: SERVICE - option :quantize, default: {} - - @patched = false + include Contrib::Patcher module_function - # patch applies our patch if needed + def patched? + done?(:elasticsearch) + end + def patch - if !@patched && (defined?(::Elasticsearch::Transport::VERSION) && \ - Gem::Version.new(::Elasticsearch::Transport::VERSION) >= Gem::Version.new('1.0.0')) + do_once(:elasticsearch) do begin require 'uri' require 'json' require 'ddtrace/pin' - require 'ddtrace/ext/app_types' require 'ddtrace/contrib/elasticsearch/quantize' - patch_elasticsearch_transport_client() - - @patched = true + patch_elasticsearch_transport_client rescue StandardError => e - Datadog::Tracer.log.error("Unable to apply Elastic Search integration: #{e}") + Datadog::Tracer.log.error("Unable to apply Elasticsearch integration: #{e}") end end - @patched end # rubocop:disable Metrics/MethodLength @@ -54,7 +43,11 @@ def patch_elasticsearch_transport_client def initialize(*args, &block) service = Datadog.configuration[:elasticsearch][:service_name] - pin = Datadog::Pin.new(service, app: 'elasticsearch', app_type: Datadog::Ext::AppTypes::DB) + pin = Datadog::Pin.new( + service, + app: Datadog::Contrib::Elasticsearch::Ext::APP, + app_type: Datadog::Ext::AppTypes::DB + ) pin.onto(self) initialize_without_datadog(*args, &block) end @@ -74,29 +67,29 @@ def perform_request(*args) url = full_url.path response = nil - pin.tracer.trace('elasticsearch.query') do |span| + pin.tracer.trace(Datadog::Contrib::Elasticsearch::Ext::SPAN_QUERY) do |span| begin connection = transport.connections.first host = connection.host[:host] if connection port = connection.host[:port] if connection span.service = pin.service - span.span_type = Ext::AppTypes::DB + span.span_type = Datadog::Ext::AppTypes::DB # load JSON for the following fields unless they're already strings params = JSON.generate(params) if params && !params.is_a?(String) body = JSON.generate(body) if body && !body.is_a?(String) - span.set_tag(METHOD, method) - span.set_tag(URL, url) - span.set_tag(PARAMS, params) if params + span.set_tag(Datadog::Contrib::Elasticsearch::Ext::TAG_METHOD, method) + span.set_tag(Datadog::Contrib::Elasticsearch::Ext::TAG_URL, url) + span.set_tag(Datadog::Contrib::Elasticsearch::Ext::TAG_PARAMS, params) if params if body quantize_options = Datadog.configuration[:elasticsearch][:quantize] quantized_body = Datadog::Contrib::Elasticsearch::Quantize.format_body(body, quantize_options) - span.set_tag(BODY, quantized_body) + span.set_tag(Datadog::Contrib::Elasticsearch::Ext::TAG_BODY, quantized_body) end - span.set_tag('out.host', host) if host - span.set_tag('out.port', port) if port + span.set_tag(Datadog::Ext::NET::TARGET_HOST, host) if host + span.set_tag(Datadog::Ext::NET::TARGET_PORT, port) if port quantized_url = Datadog::Contrib::Elasticsearch::Quantize.format_url(url) span.resource = "#{method} #{quantized_url}" @@ -105,18 +98,13 @@ def perform_request(*args) ensure # the call is still executed response = perform_request_without_datadog(*args) - span.set_tag('http.status_code', response.status) + span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response.status) end end response end end end - - # patched? tells wether patch has been successfully applied - def patched? - @patched - end end end end From 9e68babbba5b92239f41c62d8822cfc55257c0ef Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:09:59 -0400 Subject: [PATCH 13/23] Refactored: Dalli to use Datadog::Contrib::Integration. (#560) --- lib/ddtrace.rb | 2 +- .../contrib/dalli/configuration/settings.rb | 15 ++++ lib/ddtrace/contrib/dalli/ext.rb | 15 ++++ lib/ddtrace/contrib/dalli/instrumentation.rb | 6 +- lib/ddtrace/contrib/dalli/integration.rb | 36 ++++++++++ lib/ddtrace/contrib/dalli/patcher.rb | 68 ++++++++----------- lib/ddtrace/contrib/dalli/quantize.rb | 6 +- spec/ddtrace/contrib/dalli/quantize_spec.rb | 4 +- 8 files changed, 105 insertions(+), 47 deletions(-) create mode 100644 lib/ddtrace/contrib/dalli/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/dalli/ext.rb create mode 100644 lib/ddtrace/contrib/dalli/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 9154909e7a8..b9c1c38e909 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -48,7 +48,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/active_record/integration' require 'ddtrace/contrib/aws/integration' require 'ddtrace/contrib/concurrent_ruby/integration' -require 'ddtrace/contrib/dalli/patcher' +require 'ddtrace/contrib/dalli/integration' require 'ddtrace/contrib/delayed_job/integration' require 'ddtrace/contrib/elasticsearch/integration' require 'ddtrace/contrib/excon/patcher' diff --git a/lib/ddtrace/contrib/dalli/configuration/settings.rb b/lib/ddtrace/contrib/dalli/configuration/settings.rb new file mode 100644 index 00000000000..613ea07fb63 --- /dev/null +++ b/lib/ddtrace/contrib/dalli/configuration/settings.rb @@ -0,0 +1,15 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/dalli/ext' + +module Datadog + module Contrib + module Dalli + module Configuration + # Custom settings for the Dalli integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/dalli/ext.rb b/lib/ddtrace/contrib/dalli/ext.rb new file mode 100644 index 00000000000..de2334d8d07 --- /dev/null +++ b/lib/ddtrace/contrib/dalli/ext.rb @@ -0,0 +1,15 @@ +module Datadog + module Contrib + module Dalli + # Dalli integration constants + module Ext + APP = 'dalli'.freeze + SERVICE_NAME = 'memcached'.freeze + + QUANTIZE_MAX_CMD_LENGTH = 100 + SPAN_COMMAND = 'memcached.command'.freeze + TAG_COMMAND = 'memcached.command'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/dalli/instrumentation.rb b/lib/ddtrace/contrib/dalli/instrumentation.rb index 35cf88331c9..1f775bd888f 100644 --- a/lib/ddtrace/contrib/dalli/instrumentation.rb +++ b/lib/ddtrace/contrib/dalli/instrumentation.rb @@ -1,5 +1,5 @@ -require_relative 'quantize' require 'ddtrace/ext/net' +require 'ddtrace/contrib/dalli/quantize' module Datadog module Contrib @@ -15,14 +15,14 @@ def patch! def request(op, *args) pin = Datadog::Pin.get_from(::Dalli) - pin.tracer.trace(Datadog::Contrib::Dalli::NAME) do |span| + pin.tracer.trace(Datadog::Contrib::Dalli::Ext::SPAN_COMMAND) do |span| span.resource = op.to_s.upcase span.service = pin.service span.span_type = pin.app_type span.set_tag(Datadog::Ext::NET::TARGET_HOST, hostname) span.set_tag(Datadog::Ext::NET::TARGET_PORT, port) cmd = Datadog::Contrib::Dalli::Quantize.format_command(op, args) - span.set_tag(Datadog::Contrib::Dalli::CMD_TAG, cmd) + span.set_tag(Datadog::Contrib::Dalli::Ext::TAG_COMMAND, cmd) __request(op, *args) end diff --git a/lib/ddtrace/contrib/dalli/integration.rb b/lib/ddtrace/contrib/dalli/integration.rb new file mode 100644 index 00000000000..627424006f5 --- /dev/null +++ b/lib/ddtrace/contrib/dalli/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/dalli/configuration/settings' +require 'ddtrace/contrib/dalli/patcher' + +module Datadog + module Contrib + module Dalli + # Description of Dalli integration + class Integration + include Contrib::Integration + + register_as :dalli, auto_patch: true + + def self.version + Gem.loaded_specs['dalli'] && Gem.loaded_specs['dalli'].version + end + + def self.present? + super && defined?(::Dalli) + end + + def self.compatible? + super && version > Gem::Version.new('2.0.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/dalli/patcher.rb b/lib/ddtrace/contrib/dalli/patcher.rb index 480463fc859..c5bfa615c3b 100644 --- a/lib/ddtrace/contrib/dalli/patcher.rb +++ b/lib/ddtrace/contrib/dalli/patcher.rb @@ -1,51 +1,43 @@ +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/dalli/ext' +require 'ddtrace/contrib/dalli/instrumentation' + module Datadog module Contrib module Dalli - COMPATIBLE_WITH = Gem::Version.new('2.0.0') - NAME = 'memcached.command'.freeze - CMD_TAG = 'memcached.command'.freeze - - # Responsible for hooking the instrumentation into `dalli` + # Patcher enables patching of 'dalli' module. module Patcher - include Base - register_as :dalli, auto_patch: true - option :service_name, default: 'memcached' - - @patched = false - - class << self - def patch - return @patched if patched? || !compatible? + include Contrib::Patcher - require 'ddtrace/ext/app_types' - require_relative 'instrumentation' + module_function - add_pin! - Instrumentation.patch! - - @patched = true - rescue => e - Tracer.log.error("Unable to apply Dalli integration: #{e}") - @patched - end + def patched? + done?(:dalli) + end - def patched? - @patched + def patch + do_once(:dalli) do + begin + add_pin! + Instrumentation.patch! + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Dalli integration: #{e}") + end end + end - private - - def compatible? - return unless defined?(::Dalli::VERSION) - - Gem::Version.new(::Dalli::VERSION) > COMPATIBLE_WITH - end + def add_pin! + Pin + .new( + get_option(:service_name), + app: Ext::APP, + app_type: Datadog::Ext::AppTypes::CACHE + ).onto(::Dalli) + end - def add_pin! - Pin - .new(get_option(:service_name), app: 'dalli', app_type: Ext::AppTypes::CACHE) - .onto(::Dalli) - end + def get_option(option) + Datadog.configuration[:dalli].get_option(option) end end end diff --git a/lib/ddtrace/contrib/dalli/quantize.rb b/lib/ddtrace/contrib/dalli/quantize.rb index 42ae6029475..41d6ab79d18 100644 --- a/lib/ddtrace/contrib/dalli/quantize.rb +++ b/lib/ddtrace/contrib/dalli/quantize.rb @@ -1,17 +1,17 @@ +require 'ddtrace/contrib/dalli/ext' + module Datadog module Contrib module Dalli # Quantize contains dalli-specic quantization tools. module Quantize - MAX_CMD_LENGTH = 100 - module_function def format_command(operation, args) placeholder = "#{operation} BLOB (OMITTED)" command = [operation, *args].join(' ').strip command = Utils.utf8_encode(command, binary: true, placeholder: placeholder) - Utils.truncate(command, MAX_CMD_LENGTH) + Utils.truncate(command, Ext::QUANTIZE_MAX_CMD_LENGTH) rescue => e Tracer.log.debug("Error sanitizing Dalli operation: #{e}") placeholder diff --git a/spec/ddtrace/contrib/dalli/quantize_spec.rb b/spec/ddtrace/contrib/dalli/quantize_spec.rb index 47d6ee3d73c..8f9356cacc9 100644 --- a/spec/ddtrace/contrib/dalli/quantize_spec.rb +++ b/spec/ddtrace/contrib/dalli/quantize_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Datadog::Contrib::Dalli::Quantize do describe '#format_command' do - subject { described_class.format_command(op, args) } + subject(:formatted_command) { described_class.format_command(op, args) } context 'output' do let(:op) { :set } @@ -19,7 +19,7 @@ let(:op) { :set } let(:args) { ['foo', 'A' * 100] } - it { expect(subject.size).to eq(described_class::MAX_CMD_LENGTH) } + it { expect(formatted_command.size).to eq(Datadog::Contrib::Dalli::Ext::QUANTIZE_MAX_CMD_LENGTH) } it { is_expected.to end_with('...') } it { is_expected.to eq('set foo ' + 'A' * 89 + '...') } end From 65c91f295c840ec3ae96f6f865aa0c3478f3cd62 Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:12:41 -0400 Subject: [PATCH 14/23] Refactored: Redis to use Datadog::Contrib::Integration. (#555) --- lib/ddtrace.rb | 2 +- lib/ddtrace/contrib/rails/core_extensions.rb | 2 +- .../contrib/redis/configuration/settings.rb | 16 +++++++ lib/ddtrace/contrib/redis/ext.rb | 19 ++++++++ lib/ddtrace/contrib/redis/integration.rb | 36 ++++++++++++++ lib/ddtrace/contrib/redis/patcher.rb | 48 ++++++++----------- lib/ddtrace/contrib/redis/tags.rb | 6 +-- lib/ddtrace/ext/redis.rb | 17 ------- 8 files changed, 96 insertions(+), 50 deletions(-) create mode 100644 lib/ddtrace/contrib/redis/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/redis/ext.rb create mode 100644 lib/ddtrace/contrib/redis/integration.rb delete mode 100644 lib/ddtrace/ext/redis.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index b9c1c38e909..b52ccd629eb 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -64,7 +64,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/rack/integration' require 'ddtrace/contrib/rails/integration' require 'ddtrace/contrib/rake/integration' -require 'ddtrace/contrib/redis/patcher' +require 'ddtrace/contrib/redis/integration' require 'ddtrace/contrib/resque/integration' require 'ddtrace/contrib/rest_client/integration' require 'ddtrace/contrib/sequel/integration' diff --git a/lib/ddtrace/contrib/rails/core_extensions.rb b/lib/ddtrace/contrib/rails/core_extensions.rb index 16a623fdb5e..1cba06f0258 100644 --- a/lib/ddtrace/contrib/rails/core_extensions.rb +++ b/lib/ddtrace/contrib/rails/core_extensions.rb @@ -332,7 +332,7 @@ def delete(*args, &block) def self.reload_cache_store redis = Datadog.registry[:redis] - return unless redis && redis.patched? + return unless redis && redis.patcher.patched? return unless defined?(::ActiveSupport::Cache::RedisStore) && defined?(::Rails.cache) && diff --git a/lib/ddtrace/contrib/redis/configuration/settings.rb b/lib/ddtrace/contrib/redis/configuration/settings.rb new file mode 100644 index 00000000000..1bb64aee6f3 --- /dev/null +++ b/lib/ddtrace/contrib/redis/configuration/settings.rb @@ -0,0 +1,16 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/redis/ext' + +module Datadog + module Contrib + module Redis + module Configuration + # Custom settings for the Redis integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + option :tracer, default: Datadog.tracer + end + end + end + end +end diff --git a/lib/ddtrace/contrib/redis/ext.rb b/lib/ddtrace/contrib/redis/ext.rb new file mode 100644 index 00000000000..b20bf7e3495 --- /dev/null +++ b/lib/ddtrace/contrib/redis/ext.rb @@ -0,0 +1,19 @@ +module Datadog + module Contrib + module Redis + # Redis integration constants + module Ext + APP = 'redis'.freeze + SERVICE_NAME = 'redis'.freeze + TYPE = 'redis'.freeze + + METRIC_PIPELINE_LEN = 'redis.pipeline_length'.freeze + + SPAN_COMMAND = 'redis.command'.freeze + + TAG_DB = 'out.redis_db'.freeze + TAG_RAW_COMMAND = 'redis.raw_command'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/redis/integration.rb b/lib/ddtrace/contrib/redis/integration.rb new file mode 100644 index 00000000000..1da05027513 --- /dev/null +++ b/lib/ddtrace/contrib/redis/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/redis/configuration/settings' +require 'ddtrace/contrib/redis/patcher' + +module Datadog + module Contrib + module Redis + # Description of Redis integration + class Integration + include Contrib::Integration + + register_as :redis, auto_patch: true + + def self.version + Gem.loaded_specs['redis'] && Gem.loaded_specs['redis'].version + end + + def self.present? + super && defined?(::Redis) + end + + def self.compatible? + !version.nil? && version >= Gem::Version.new('3.0.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/redis/patcher.rb b/lib/ddtrace/contrib/redis/patcher.rb index f1e9d11c117..fdea5885bbe 100644 --- a/lib/ddtrace/contrib/redis/patcher.rb +++ b/lib/ddtrace/contrib/redis/patcher.rb @@ -1,43 +1,35 @@ -# requirements should be kept minimal as Patcher is a shared requirement. +require 'ddtrace/contrib/patcher' +require 'ddtrace/contrib/redis/ext' module Datadog module Contrib module Redis - SERVICE = 'redis'.freeze - DRIVER = 'redis.driver'.freeze - # Patcher enables patching of 'redis' module. module Patcher - include Base - register_as :redis, auto_patch: true - option :service_name, default: SERVICE - option :tracer, default: Datadog.tracer - - @patched = false + include Contrib::Patcher module_function + def patched? + done?(:redis) + end + # patch applies our patch if needed def patch - if !@patched && compatible? + do_once(:redis) do begin # do not require these by default, but only when actually patching + require 'redis' require 'ddtrace/ext/app_types' require 'ddtrace/contrib/redis/tags' require 'ddtrace/contrib/redis/quantize' patch_redis_client - @patched = true RailsCachePatcher.reload_cache_store if Datadog.registry[:rails].patcher.patched? rescue StandardError => e Datadog::Tracer.log.error("Unable to apply Redis integration: #{e}") end end - @patched - end - - def compatible? - defined?(::Redis::VERSION) && Gem::Version.new(::Redis::VERSION) >= Gem::Version.new('3.0.0') end # rubocop:disable Metrics/MethodLength @@ -52,7 +44,12 @@ def patch_redis_client def initialize(*args) service = Datadog.configuration[:redis][:service_name] tracer = Datadog.configuration[:redis][:tracer] - pin = Datadog::Pin.new(service, app: 'redis', app_type: Datadog::Ext::AppTypes::DB, tracer: tracer) + pin = Datadog::Pin.new( + service, + app: Ext::APP, + app_type: Datadog::Ext::AppTypes::DB, + tracer: tracer + ) pin.onto(self) initialize_without_datadog(*args) end @@ -64,9 +61,9 @@ def call(*args, &block) return call_without_datadog(*args, &block) unless pin && pin.tracer response = nil - pin.tracer.trace('redis.command') do |span| + pin.tracer.trace(Datadog::Contrib::Redis::Ext::SPAN_COMMAND) do |span| span.service = pin.service - span.span_type = Datadog::Ext::Redis::TYPE + span.span_type = Datadog::Contrib::Redis::Ext::TYPE span.resource = Datadog::Contrib::Redis::Quantize.format_command_args(*args) Datadog::Contrib::Redis::Tags.set_common_tags(self, span) @@ -83,13 +80,13 @@ def call_pipeline(*args, &block) return call_pipeline_without_datadog(*args, &block) unless pin && pin.tracer response = nil - pin.tracer.trace('redis.command') do |span| + pin.tracer.trace(Datadog::Contrib::Redis::Ext::SPAN_COMMAND) do |span| span.service = pin.service - span.span_type = Datadog::Ext::Redis::TYPE + span.span_type = Datadog::Contrib::Redis::Ext::TYPE commands = args[0].commands.map { |c| Datadog::Contrib::Redis::Quantize.format_command_args(c) } span.resource = commands.join("\n") Datadog::Contrib::Redis::Tags.set_common_tags(self, span) - span.set_metric Datadog::Ext::Redis::PIPELINE_LEN, commands.length + span.set_metric Datadog::Contrib::Redis::Ext::METRIC_PIPELINE_LEN, commands.length response = call_pipeline_without_datadog(*args, &block) end @@ -98,11 +95,6 @@ def call_pipeline(*args, &block) end end end - - # patched? tells wether patch has been successfully applied - def patched? - @patched - end end end end diff --git a/lib/ddtrace/contrib/redis/tags.rb b/lib/ddtrace/contrib/redis/tags.rb index 18dd4c47164..e9881a1d099 100644 --- a/lib/ddtrace/contrib/redis/tags.rb +++ b/lib/ddtrace/contrib/redis/tags.rb @@ -1,5 +1,5 @@ require 'ddtrace/ext/net' -require 'ddtrace/ext/redis' +require 'ddtrace/contrib/redis/ext' module Datadog module Contrib @@ -11,8 +11,8 @@ module Tags def set_common_tags(client, span) span.set_tag Datadog::Ext::NET::TARGET_HOST, client.host span.set_tag Datadog::Ext::NET::TARGET_PORT, client.port - span.set_tag Datadog::Ext::Redis::DB, client.db - span.set_tag Datadog::Ext::Redis::RAW_COMMAND, span.resource + span.set_tag Ext::TAG_DB, client.db + span.set_tag Ext::TAG_RAW_COMMAND, span.resource end end end diff --git a/lib/ddtrace/ext/redis.rb b/lib/ddtrace/ext/redis.rb deleted file mode 100644 index 0686a12a368..00000000000 --- a/lib/ddtrace/ext/redis.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Datadog - module Ext - module Redis - # type of the spans - TYPE = 'redis'.freeze - - # net extension - DB = 'out.redis_db'.freeze - - # raw command - RAW_COMMAND = 'redis.raw_command'.freeze - - # pipeline length - PIPELINE_LEN = 'redis.pipeline_length'.freeze - end - end -end From cc9d368625a1ca037d99f28107a546d82ea9325e Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 26 Sep 2018 16:15:03 -0400 Subject: [PATCH 15/23] Refactored: MongoDB to use Datadog::Contrib::Integration. (#558) --- lib/ddtrace.rb | 2 +- .../contrib/mongodb/configuration/settings.rb | 18 ++++++++ lib/ddtrace/contrib/mongodb/ext.rb | 20 +++++++++ lib/ddtrace/contrib/mongodb/integration.rb | 36 ++++++++++++++++ lib/ddtrace/contrib/mongodb/patcher.rb | 42 +++++++------------ lib/ddtrace/contrib/mongodb/subscribers.rb | 23 +++++----- lib/ddtrace/ext/mongo.rb | 12 ------ 7 files changed, 102 insertions(+), 51 deletions(-) create mode 100644 lib/ddtrace/contrib/mongodb/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/mongodb/ext.rb create mode 100644 lib/ddtrace/contrib/mongodb/integration.rb delete mode 100644 lib/ddtrace/ext/mongo.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index b52ccd629eb..38d776b1b7f 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -58,8 +58,8 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/grpc/patcher' require 'ddtrace/contrib/http/integration' require 'ddtrace/contrib/integration' -require 'ddtrace/contrib/mongodb/patcher' require 'ddtrace/contrib/mysql2/integration' +require 'ddtrace/contrib/mongodb/integration' require 'ddtrace/contrib/racecar/integration' require 'ddtrace/contrib/rack/integration' require 'ddtrace/contrib/rails/integration' diff --git a/lib/ddtrace/contrib/mongodb/configuration/settings.rb b/lib/ddtrace/contrib/mongodb/configuration/settings.rb new file mode 100644 index 00000000000..89289e952a9 --- /dev/null +++ b/lib/ddtrace/contrib/mongodb/configuration/settings.rb @@ -0,0 +1,18 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/mongodb/ext' + +module Datadog + module Contrib + module MongoDB + module Configuration + # Custom settings for the MongoDB integration + class Settings < Contrib::Configuration::Settings + DEFAULT_QUANTIZE = { show: [:collection, :database, :operation] }.freeze + + option :quantize, default: DEFAULT_QUANTIZE + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/mongodb/ext.rb b/lib/ddtrace/contrib/mongodb/ext.rb new file mode 100644 index 00000000000..61cd119828b --- /dev/null +++ b/lib/ddtrace/contrib/mongodb/ext.rb @@ -0,0 +1,20 @@ +module Datadog + module Contrib + module MongoDB + # MongoDB integration constants + module Ext + APP = 'mongodb'.freeze + SERVICE_NAME = 'mongodb'.freeze + + SPAN_COMMAND = 'mongo.cmd'.freeze + SPAN_TYPE_COMMAND = 'mongodb'.freeze + + TAG_COLLECTION = 'mongodb.collection'.freeze + TAG_DB = 'mongodb.db'.freeze + TAG_OPERATION = 'mongodb.operation'.freeze + TAG_QUERY = 'mongodb.query'.freeze + TAG_ROWS = 'mongodb.rows'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/mongodb/integration.rb b/lib/ddtrace/contrib/mongodb/integration.rb new file mode 100644 index 00000000000..85db3eda972 --- /dev/null +++ b/lib/ddtrace/contrib/mongodb/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/mongodb/configuration/settings' +require 'ddtrace/contrib/mongodb/patcher' + +module Datadog + module Contrib + module MongoDB + # Description of MongoDB integration + class Integration + include Contrib::Integration + + register_as :mongo, auto_patch: true + + def self.version + Gem.loaded_specs['mongo'] && Gem.loaded_specs['mongo'].version + end + + def self.present? + super && defined?(::Mongo::Monitoring::Global) + end + + def self.compatible? + super && version >= Gem::Version.new('2.1.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/mongodb/patcher.rb b/lib/ddtrace/contrib/mongodb/patcher.rb index 2290cf2b31d..ddc55e1e9dd 100644 --- a/lib/ddtrace/contrib/mongodb/patcher.rb +++ b/lib/ddtrace/contrib/mongodb/patcher.rb @@ -1,56 +1,40 @@ -# requirements should be kept minimal as Patcher is a shared requirement. +require 'ddtrace/contrib/patcher' +require 'ddtrace/contrib/mongodb/ext' module Datadog module Contrib - # MongoDB module includes classes and functions to instrument MongoDB clients module MongoDB - APP = 'mongodb'.freeze - SERVICE = 'mongodb'.freeze - - # Patcher adds subscribers to the MongoDB driver so that each command is traced. + # Patcher enables patching of 'mongo' module. module Patcher - include Base - register_as :mongo, auto_patch: true - option :service_name, default: SERVICE - option :quantize, default: { show: [:collection, :database, :operation] } - - @patched = false + include Contrib::Patcher module_function def patched? - @patched + done?(:mongo) end def patch - # versions prior to 2.1.0 don't support the Monitoring API - if !@patched && (defined?(::Mongo::Monitoring::Global) && \ - Gem::Version.new(::Mongo::VERSION) >= Gem::Version.new('2.1.0')) + do_once(:mongo) do begin require 'ddtrace/pin' require 'ddtrace/ext/net' - require 'ddtrace/ext/mongo' require 'ddtrace/ext/app_types' + require 'ddtrace/contrib/mongodb/ext' require 'ddtrace/contrib/mongodb/parsers' require 'ddtrace/contrib/mongodb/subscribers' - patch_mongo_client() - add_mongo_monitoring() - - @patched = true + patch_mongo_client + add_mongo_monitoring rescue StandardError => e Datadog::Tracer.log.error("Unable to apply MongoDB integration: #{e}") end end - @patched end def add_mongo_monitoring # Subscribe to all COMMAND queries with our subscriber class - ::Mongo::Monitoring::Global.subscribe( - ::Mongo::Monitoring::COMMAND, - Datadog::Contrib::MongoDB::MongoCommandSubscriber.new - ) + ::Mongo::Monitoring::Global.subscribe(::Mongo::Monitoring::COMMAND, MongoCommandSubscriber.new) end def patch_mongo_client @@ -64,7 +48,11 @@ def initialize(*args, &blk) # attach the Pin instance initialize_without_datadog(*args, &blk) service = Datadog.configuration[:mongo][:service_name] - pin = Datadog::Pin.new(service, app: APP, app_type: Datadog::Ext::AppTypes::DB) + pin = Datadog::Pin.new( + service, + app: Datadog::Contrib::MongoDB::Ext::APP, + app_type: Datadog::Ext::AppTypes::DB + ) pin.onto(self) end diff --git a/lib/ddtrace/contrib/mongodb/subscribers.rb b/lib/ddtrace/contrib/mongodb/subscribers.rb index 8909b6479b3..13359392849 100644 --- a/lib/ddtrace/contrib/mongodb/subscribers.rb +++ b/lib/ddtrace/contrib/mongodb/subscribers.rb @@ -1,6 +1,8 @@ +require 'ddtrace/contrib/mongodb/ext' +require 'ddtrace/contrib/mongodb/parsers' + module Datadog module Contrib - # MongoDB module includes classes and functions to instrument MongoDB clients module MongoDB # `MongoCommandSubscriber` listens to all events from the `Monitoring` # system available in the Mongo driver. @@ -14,20 +16,19 @@ def started(event) # thread is involved in this execution so thread-local storage should be safe. Reference: # https://github.com/mongodb/mongo-ruby-driver/blob/master/lib/mongo/monitoring.rb#L70 # https://github.com/mongodb/mongo-ruby-driver/blob/master/lib/mongo/monitoring/publishable.rb#L38-L56 - span = pin.tracer.trace('mongo.cmd', service: pin.service, span_type: Datadog::Ext::Mongo::TYPE) + span = pin.tracer.trace(Ext::SPAN_COMMAND, service: pin.service, span_type: Ext::SPAN_TYPE_COMMAND) Thread.current[:datadog_mongo_span] = span # build a quantized Query using the Parser module - query = Datadog::Contrib::MongoDB - .query_builder(event.command_name, event.database_name, event.command) + query = MongoDB.query_builder(event.command_name, event.database_name, event.command) serialized_query = query.to_s # add operation tags; the full query is stored and used as a resource, # since it has been quantized and reduced - span.set_tag(Datadog::Ext::Mongo::DB, query['database']) - span.set_tag(Datadog::Ext::Mongo::COLLECTION, query['collection']) - span.set_tag(Datadog::Ext::Mongo::OPERATION, query['operation']) - span.set_tag(Datadog::Ext::Mongo::QUERY, serialized_query) + span.set_tag(Ext::TAG_DB, query['database']) + span.set_tag(Ext::TAG_COLLECTION, query['collection']) + span.set_tag(Ext::TAG_OPERATION, query['operation']) + span.set_tag(Ext::TAG_QUERY, serialized_query) span.set_tag(Datadog::Ext::NET::TARGET_HOST, event.address.host) span.set_tag(Datadog::Ext::NET::TARGET_PORT, event.address.port) @@ -47,7 +48,7 @@ def failed(event) ensure # whatever happens, the Span must be removed from the local storage and # it must be finished to prevent any leak - span.finish() unless span.nil? + span.finish unless span.nil? Thread.current[:datadog_mongo_span] = nil end @@ -57,13 +58,13 @@ def succeeded(event) # add fields that are available only after executing the query rows = event.reply.fetch('n', nil) - span.set_tag(Datadog::Ext::Mongo::ROWS, rows) unless rows.nil? + span.set_tag(Ext::TAG_ROWS, rows) unless rows.nil? rescue StandardError => e Datadog::Tracer.log.debug("error when handling MongoDB 'succeeded' event: #{e}") ensure # whatever happens, the Span must be removed from the local storage and # it must be finished to prevent any leak - span.finish() unless span.nil? + span.finish unless span.nil? Thread.current[:datadog_mongo_span] = nil end end diff --git a/lib/ddtrace/ext/mongo.rb b/lib/ddtrace/ext/mongo.rb deleted file mode 100644 index c817d5fcbf4..00000000000 --- a/lib/ddtrace/ext/mongo.rb +++ /dev/null @@ -1,12 +0,0 @@ -module Datadog - module Ext - module Mongo - DB = 'mongodb.db'.freeze - TYPE = 'mongodb'.freeze - ROWS = 'mongodb.rows'.freeze - QUERY = 'mongodb.query'.freeze - OPERATION = 'mongodb.operation'.freeze - COLLECTION = 'mongodb.collection'.freeze - end - end -end From d38661869121407af2175978a8ec8fe34a56430a Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 16:26:19 -0400 Subject: [PATCH 16/23] Refactored: Integration configurations to be more consistent. (#576) --- .../active_record/configuration/settings.rb | 5 +++-- .../active_record/events/instantiation.rb | 13 +++++------ .../contrib/active_record/events/sql.rb | 18 ++++++++------- lib/ddtrace/contrib/active_record/ext.rb | 22 +++++++++++++++++++ .../concurrent_ruby/configuration/settings.rb | 1 + lib/ddtrace/contrib/concurrent_ruby/ext.rb | 11 ++++++++++ .../contrib/concurrent_ruby/integration.rb | 10 ++++++--- .../contrib/concurrent_ruby/patcher.rb | 1 + .../contrib/racecar/configuration/settings.rb | 2 +- .../rest_client/configuration/settings.rb | 13 +++++------ lib/ddtrace/contrib/rest_client/ext.rb | 13 +++++++++++ .../contrib/rest_client/integration.rb | 10 ++++++++- lib/ddtrace/contrib/rest_client/patcher.rb | 2 +- .../contrib/rest_client/request_patch.rb | 19 ++++++++-------- .../contrib/sequel/configuration/settings.rb | 2 ++ lib/ddtrace/contrib/sequel/database.rb | 7 +++--- lib/ddtrace/contrib/sequel/dataset.rb | 5 +++-- lib/ddtrace/contrib/sequel/ext.rb | 15 +++++++++++++ lib/ddtrace/contrib/sequel/integration.rb | 4 +--- lib/ddtrace/contrib/sequel/patcher.rb | 1 - 20 files changed, 124 insertions(+), 50 deletions(-) create mode 100644 lib/ddtrace/contrib/active_record/ext.rb create mode 100644 lib/ddtrace/contrib/concurrent_ruby/ext.rb create mode 100644 lib/ddtrace/contrib/rest_client/ext.rb create mode 100644 lib/ddtrace/contrib/sequel/ext.rb diff --git a/lib/ddtrace/contrib/active_record/configuration/settings.rb b/lib/ddtrace/contrib/active_record/configuration/settings.rb index 6e65a00c663..c0140331b94 100644 --- a/lib/ddtrace/contrib/active_record/configuration/settings.rb +++ b/lib/ddtrace/contrib/active_record/configuration/settings.rb @@ -1,16 +1,17 @@ require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/active_record/ext' require 'ddtrace/contrib/active_record/utils' module Datadog module Contrib module ActiveRecord module Configuration - # Unique settings for ActiveRecord + # Custom settings for the ActiveRecord integration class Settings < Contrib::Configuration::Settings option :orm_service_name option :service_name, depends_on: [:tracer] do |value| (value || Utils.adapter_name).tap do |service_name| - tracer.set_service_info(service_name, 'active_record', Ext::AppTypes::DB) + tracer.set_service_info(service_name, Ext::APP, Datadog::Ext::AppTypes::DB) end end diff --git a/lib/ddtrace/contrib/active_record/events/instantiation.rb b/lib/ddtrace/contrib/active_record/events/instantiation.rb index f651f3727b8..6db609e2ff6 100644 --- a/lib/ddtrace/contrib/active_record/events/instantiation.rb +++ b/lib/ddtrace/contrib/active_record/events/instantiation.rb @@ -1,3 +1,4 @@ +require 'ddtrace/contrib/active_record/ext' require 'ddtrace/contrib/active_record/event' module Datadog @@ -9,8 +10,6 @@ module Instantiation include ActiveRecord::Event EVENT_NAME = 'instantiation.active_record'.freeze - SPAN_NAME = 'active_record.instantiation'.freeze - DEFAULT_SERVICE_NAME = 'active_record'.freeze module_function @@ -24,7 +23,7 @@ def event_name end def span_name - self::SPAN_NAME + Ext::SPAN_INSTANTIATION end def process(span, event, _id, payload) @@ -34,13 +33,13 @@ def process(span, event, _id, payload) elsif span.parent span.parent.service else - self::DEFAULT_SERVICE_NAME + Ext::SERVICE_NAME end span.resource = payload.fetch(:class_name) - span.span_type = 'custom' - span.set_tag('active_record.instantiation.class_name', payload.fetch(:class_name)) - span.set_tag('active_record.instantiation.record_count', payload.fetch(:record_count)) + span.span_type = Ext::SPAN_TYPE_INSTANTIATION + span.set_tag(Ext::TAG_INSTANTIATION_CLASS_NAME, payload.fetch(:class_name)) + span.set_tag(Ext::TAG_INSTANTIATION_RECORD_COUNT, payload.fetch(:record_count)) rescue StandardError => e Datadog::Tracer.log.debug(e.message) end diff --git a/lib/ddtrace/contrib/active_record/events/sql.rb b/lib/ddtrace/contrib/active_record/events/sql.rb index 96287c3efaf..e4f8da76ca5 100644 --- a/lib/ddtrace/contrib/active_record/events/sql.rb +++ b/lib/ddtrace/contrib/active_record/events/sql.rb @@ -1,3 +1,5 @@ +require 'ddtrace/ext/net' +require 'ddtrace/contrib/active_record/ext' require 'ddtrace/contrib/active_record/event' module Datadog @@ -9,7 +11,7 @@ module SQL include ActiveRecord::Event EVENT_NAME = 'sql.active_record'.freeze - SPAN_NAME = 'active_record.sql'.freeze + PAYLOAD_CACHE = 'CACHE'.freeze module_function @@ -18,7 +20,7 @@ def event_name end def span_name - self::SPAN_NAME + Ext::SPAN_SQL end def process(span, event, _id, payload) @@ -35,13 +37,13 @@ def process(span, event, _id, payload) # Find out if the SQL query has been cached in this request. This meta is really # helpful to users because some spans may have 0ns of duration because the query # is simply cached from memory, so the notification is fired with start == finish. - cached = payload[:cached] || (payload[:name] == 'CACHE') + cached = payload[:cached] || (payload[:name] == PAYLOAD_CACHE) - span.set_tag('active_record.db.vendor', adapter_name) - span.set_tag('active_record.db.name', config[:database]) - span.set_tag('active_record.db.cached', cached) if cached - span.set_tag('out.host', config[:host]) if config[:host] - span.set_tag('out.port', config[:port]) if config[:port] + span.set_tag(Ext::TAG_DB_VENDOR, adapter_name) + span.set_tag(Ext::TAG_DB_NAME, config[:database]) + span.set_tag(Ext::TAG_DB_CACHED, cached) if cached + span.set_tag(Datadog::Ext::NET::TARGET_HOST, config[:host]) if config[:host] + span.set_tag(Datadog::Ext::NET::TARGET_PORT, config[:port]) if config[:port] rescue StandardError => e Datadog::Tracer.log.debug(e.message) end diff --git a/lib/ddtrace/contrib/active_record/ext.rb b/lib/ddtrace/contrib/active_record/ext.rb new file mode 100644 index 00000000000..a9b3532551a --- /dev/null +++ b/lib/ddtrace/contrib/active_record/ext.rb @@ -0,0 +1,22 @@ +module Datadog + module Contrib + module ActiveRecord + # ActiveRecord integration constants + module Ext + APP = 'active_record'.freeze + SERVICE_NAME = 'active_record'.freeze + + SPAN_INSTANTIATION = 'active_record.instantiation'.freeze + SPAN_SQL = 'active_record.sql'.freeze + + SPAN_TYPE_INSTANTIATION = 'custom'.freeze + + TAG_DB_CACHED = 'active_record.db.cached'.freeze + TAG_DB_NAME = 'active_record.db.name'.freeze + TAG_DB_VENDOR = 'active_record.db.vendor'.freeze + TAG_INSTANTIATION_CLASS_NAME = 'active_record.instantiation.class_name'.freeze + TAG_INSTANTIATION_RECORD_COUNT = 'active_record.instantiation.record_count'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/concurrent_ruby/configuration/settings.rb b/lib/ddtrace/contrib/concurrent_ruby/configuration/settings.rb index 2f798da59f3..1978341898a 100644 --- a/lib/ddtrace/contrib/concurrent_ruby/configuration/settings.rb +++ b/lib/ddtrace/contrib/concurrent_ruby/configuration/settings.rb @@ -4,6 +4,7 @@ module Datadog module Contrib module ConcurrentRuby module Configuration + # Custom settings for the ConcurrentRuby integration class Settings < Contrib::Configuration::Settings # Add any custom ConcurrentRuby configuration or behavior here. end diff --git a/lib/ddtrace/contrib/concurrent_ruby/ext.rb b/lib/ddtrace/contrib/concurrent_ruby/ext.rb new file mode 100644 index 00000000000..561b55ab4ac --- /dev/null +++ b/lib/ddtrace/contrib/concurrent_ruby/ext.rb @@ -0,0 +1,11 @@ +module Datadog + module Contrib + module ConcurrentRuby + # ConcurrentRuby integration constants + module Ext + APP = 'concurrent-ruby'.freeze + SERVICE_NAME = 'concurrent-ruby'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/concurrent_ruby/integration.rb b/lib/ddtrace/contrib/concurrent_ruby/integration.rb index 760219a79e4..a58062c0272 100644 --- a/lib/ddtrace/contrib/concurrent_ruby/integration.rb +++ b/lib/ddtrace/contrib/concurrent_ruby/integration.rb @@ -5,14 +5,18 @@ module Datadog module Contrib module ConcurrentRuby - # Propagate Tracing context in Concurrent::Future + # Describes the ConcurrentRuby integration class Integration include Contrib::Integration register_as :concurrent_ruby - def self.compatible? - defined?(::Concurrent::Future) + def self.version + Gem.loaded_specs['concurrent-ruby'] && Gem.loaded_specs['concurrent-ruby'].version + end + + def self.present? + super && defined?(::Concurrent::Future) end def default_configuration diff --git a/lib/ddtrace/contrib/concurrent_ruby/patcher.rb b/lib/ddtrace/contrib/concurrent_ruby/patcher.rb index 43ccc644786..b0b07b59b94 100644 --- a/lib/ddtrace/contrib/concurrent_ruby/patcher.rb +++ b/lib/ddtrace/contrib/concurrent_ruby/patcher.rb @@ -25,6 +25,7 @@ def patch end end + # Propagate tracing context in Concurrent::Future def patch_future ::Concurrent::Future.send(:include, FuturePatch) end diff --git a/lib/ddtrace/contrib/racecar/configuration/settings.rb b/lib/ddtrace/contrib/racecar/configuration/settings.rb index d2aa072f54d..dff32f7bf2b 100644 --- a/lib/ddtrace/contrib/racecar/configuration/settings.rb +++ b/lib/ddtrace/contrib/racecar/configuration/settings.rb @@ -5,7 +5,7 @@ module Datadog module Contrib module Racecar module Configuration - # Custom settings for the Rack integration + # Custom settings for the Racecar integration class Settings < Contrib::Configuration::Settings option :service_name, default: Ext::SERVICE_NAME option :tracer, default: Datadog.tracer do |value| diff --git a/lib/ddtrace/contrib/rest_client/configuration/settings.rb b/lib/ddtrace/contrib/rest_client/configuration/settings.rb index 9d23fe71683..82d154834ad 100644 --- a/lib/ddtrace/contrib/rest_client/configuration/settings.rb +++ b/lib/ddtrace/contrib/rest_client/configuration/settings.rb @@ -1,20 +1,17 @@ require 'ddtrace/contrib/configuration/settings' -require 'ddtrace/contrib/active_record/utils' +require 'ddtrace/contrib/rest_client/ext' module Datadog module Contrib module RestClient module Configuration - # Unique settings for RestClient + # Custom settings for the RestClient integration class Settings < Contrib::Configuration::Settings - NAME = 'rest_client'.freeze - - option :service_name, default: NAME, depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, NAME, Ext::AppTypes::DB) + option :distributed_tracing, default: false + option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| + get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::DB) value end - - option :distributed_tracing, default: false end end end diff --git a/lib/ddtrace/contrib/rest_client/ext.rb b/lib/ddtrace/contrib/rest_client/ext.rb new file mode 100644 index 00000000000..b3a9727fbc1 --- /dev/null +++ b/lib/ddtrace/contrib/rest_client/ext.rb @@ -0,0 +1,13 @@ +module Datadog + module Contrib + module RestClient + # RestClient integration constants + module Ext + APP = 'rest_client'.freeze + SERVICE_NAME = 'rest_client'.freeze + + SPAN_REQUEST = 'rest_client.request'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/rest_client/integration.rb b/lib/ddtrace/contrib/rest_client/integration.rb index ff7bb5286f4..addb984bd43 100644 --- a/lib/ddtrace/contrib/rest_client/integration.rb +++ b/lib/ddtrace/contrib/rest_client/integration.rb @@ -10,8 +10,16 @@ class Integration include Contrib::Integration register_as :rest_client + def self.version + Gem.loaded_specs['rest-client'] && Gem.loaded_specs['rest-client'].version + end + + def self.present? + super && defined?(::RestClient::Request) + end + def self.compatible? - RUBY_VERSION >= '1.9.3' && defined?(::RestClient::Request) + super && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('1.9.3') end def default_configuration diff --git a/lib/ddtrace/contrib/rest_client/patcher.rb b/lib/ddtrace/contrib/rest_client/patcher.rb index 8e0a9be1e1a..0c721b3ccdf 100644 --- a/lib/ddtrace/contrib/rest_client/patcher.rb +++ b/lib/ddtrace/contrib/rest_client/patcher.rb @@ -1,7 +1,7 @@ module Datadog module Contrib module RestClient - # RestClient integration + # Patcher enables patching of 'rest_client' module. module Patcher include Contrib::Patcher diff --git a/lib/ddtrace/contrib/rest_client/request_patch.rb b/lib/ddtrace/contrib/rest_client/request_patch.rb index ff4b5126c6c..9088efa4796 100644 --- a/lib/ddtrace/contrib/rest_client/request_patch.rb +++ b/lib/ddtrace/contrib/rest_client/request_patch.rb @@ -1,14 +1,13 @@ require 'ddtrace/ext/net' require 'ddtrace/ext/distributed' require 'ddtrace/propagation/http_propagator' +require 'ddtrace/contrib/rest_client/ext' module Datadog module Contrib module RestClient # RestClient RequestPatch module RequestPatch - REQUEST_TRACE_NAME = 'rest_client.request'.freeze - def self.included(base) if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.0.0') base.class_eval do @@ -48,14 +47,14 @@ def execute(&block) def datadog_tag_request(uri, span) span.resource = method.to_s.upcase - span.set_tag(Ext::HTTP::URL, uri.path) - span.set_tag(Ext::HTTP::METHOD, method.to_s.upcase) - span.set_tag(Ext::NET::TARGET_HOST, uri.host) - span.set_tag(Ext::NET::TARGET_PORT, uri.port) + span.set_tag(Datadog::Ext::HTTP::URL, uri.path) + span.set_tag(Datadog::Ext::HTTP::METHOD, method.to_s.upcase) + span.set_tag(Datadog::Ext::NET::TARGET_HOST, uri.host) + span.set_tag(Datadog::Ext::NET::TARGET_PORT, uri.port) end def datadog_trace_request(uri) - span = datadog_configuration[:tracer].trace(REQUEST_TRACE_NAME, + span = datadog_configuration[:tracer].trace(Ext::SPAN_REQUEST, service: datadog_configuration[:service_name], span_type: Datadog::Ext::AppTypes::WEB) @@ -63,11 +62,11 @@ def datadog_trace_request(uri) response = yield(span) - span.set_tag(Ext::HTTP::STATUS_CODE, response.code) + span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response.code) response rescue ::RestClient::ExceptionWithResponse => e - span.set_error(e) if Ext::HTTP::ERROR_RANGE.cover?(e.http_code) - span.set_tag(Ext::HTTP::STATUS_CODE, e.http_code) + span.set_error(e) if Datadog::Ext::HTTP::ERROR_RANGE.cover?(e.http_code) + span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, e.http_code) raise e # rubocop:disable Lint/RescueException diff --git a/lib/ddtrace/contrib/sequel/configuration/settings.rb b/lib/ddtrace/contrib/sequel/configuration/settings.rb index 26a0e023a34..eb08daff09d 100644 --- a/lib/ddtrace/contrib/sequel/configuration/settings.rb +++ b/lib/ddtrace/contrib/sequel/configuration/settings.rb @@ -1,9 +1,11 @@ require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/sequel/ext' module Datadog module Contrib module Sequel module Configuration + # Custom settings for the Sequel integration class Settings < Contrib::Configuration::Settings # Add any custom Sequel settings or behavior here. end diff --git a/lib/ddtrace/contrib/sequel/database.rb b/lib/ddtrace/contrib/sequel/database.rb index 4357f490de6..ad193bd606a 100644 --- a/lib/ddtrace/contrib/sequel/database.rb +++ b/lib/ddtrace/contrib/sequel/database.rb @@ -1,5 +1,6 @@ require 'ddtrace/ext/sql' require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/sequel/ext' require 'ddtrace/contrib/sequel/utils' module Datadog @@ -18,11 +19,11 @@ def run(sql, options = ::Sequel::OPTS) response = nil - datadog_pin.tracer.trace('sequel.query') do |span| + datadog_pin.tracer.trace(Ext::SPAN_QUERY) do |span| span.service = datadog_pin.service span.resource = opts[:query] span.span_type = Datadog::Ext::SQL::TYPE - span.set_tag('sequel.db.vendor', adapter_name) + span.set_tag(Ext::TAG_DB_VENDOR, adapter_name) response = super(sql, options) end response @@ -31,7 +32,7 @@ def run(sql, options = ::Sequel::OPTS) def datadog_pin @pin ||= Datadog::Pin.new( Datadog.configuration[:sequel][:service_name] || adapter_name, - app: Integration::APP, + app: Ext::APP, app_type: Datadog::Ext::AppTypes::DB, tracer: Datadog.configuration[:sequel][:tracer] || Datadog.tracer ) diff --git a/lib/ddtrace/contrib/sequel/dataset.rb b/lib/ddtrace/contrib/sequel/dataset.rb index 4cde0a0c8f3..f0d6b2b2c7d 100644 --- a/lib/ddtrace/contrib/sequel/dataset.rb +++ b/lib/ddtrace/contrib/sequel/dataset.rb @@ -1,5 +1,6 @@ require 'ddtrace/ext/sql' require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/sequel/ext' require 'ddtrace/contrib/sequel/utils' module Datadog @@ -39,11 +40,11 @@ def trace_execute(super_method, sql, options, &block) opts = Utils.parse_opts(sql, options, db.opts) response = nil - datadog_pin.tracer.trace('sequel.query') do |span| + datadog_pin.tracer.trace(Ext::SPAN_QUERY) do |span| span.service = datadog_pin.service span.resource = opts[:query] span.span_type = Datadog::Ext::SQL::TYPE - span.set_tag('sequel.db.vendor', adapter_name) + span.set_tag(Ext::TAG_DB_VENDOR, adapter_name) response = super_method.call(sql, options, &block) end response diff --git a/lib/ddtrace/contrib/sequel/ext.rb b/lib/ddtrace/contrib/sequel/ext.rb new file mode 100644 index 00000000000..97a1bded869 --- /dev/null +++ b/lib/ddtrace/contrib/sequel/ext.rb @@ -0,0 +1,15 @@ +module Datadog + module Contrib + module Sequel + # Sequel integration constants + module Ext + APP = 'sequel'.freeze + SERVICE_NAME = 'sequel'.freeze + + SPAN_QUERY = 'sequel.query'.freeze + + TAG_DB_VENDOR = 'sequel.db.vendor'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/sequel/integration.rb b/lib/ddtrace/contrib/sequel/integration.rb index 84ae06971fc..158a86daec5 100644 --- a/lib/ddtrace/contrib/sequel/integration.rb +++ b/lib/ddtrace/contrib/sequel/integration.rb @@ -9,8 +9,6 @@ module Sequel class Integration include Contrib::Integration - APP = 'sequel'.freeze - register_as :sequel, auto_patch: false def self.version @@ -22,7 +20,7 @@ def self.present? end def self.compatible? - super && RUBY_VERSION >= '2.0.0' + super && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.0.0') end def default_configuration diff --git a/lib/ddtrace/contrib/sequel/patcher.rb b/lib/ddtrace/contrib/sequel/patcher.rb index 752da4b8053..bb54ecb5c92 100644 --- a/lib/ddtrace/contrib/sequel/patcher.rb +++ b/lib/ddtrace/contrib/sequel/patcher.rb @@ -6,7 +6,6 @@ module Datadog module Contrib module Sequel # Patcher enables patching of 'sequel' module. - # This is used in monkey.rb to manually apply patches module Patcher include Contrib::Patcher From 09988b3294aeb59a1c50f1aa3277b01de2ab9601 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 16:27:28 -0400 Subject: [PATCH 17/23] Refactored: ActiveModelSerializers to use Datadog::Contrib::Integration. (#568) --- lib/ddtrace.rb | 2 +- .../configuration/settings.rb | 23 +++++++ .../contrib/active_model_serializers/event.rb | 6 +- .../active_model_serializers/events/render.rb | 4 +- .../events/serialize.rb | 4 +- .../contrib/active_model_serializers/ext.rb | 17 +++++ .../active_model_serializers/integration.rb | 39 +++++++++++ .../active_model_serializers/patcher.rb | 67 +++++++------------ 8 files changed, 112 insertions(+), 50 deletions(-) create mode 100644 lib/ddtrace/contrib/active_model_serializers/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/active_model_serializers/ext.rb create mode 100644 lib/ddtrace/contrib/active_model_serializers/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 38d776b1b7f..575e1fadadb 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -44,7 +44,7 @@ def configure(target = configuration, opts = {}) end require 'ddtrace/contrib/base' -require 'ddtrace/contrib/active_model_serializers/patcher' +require 'ddtrace/contrib/active_model_serializers/integration' require 'ddtrace/contrib/active_record/integration' require 'ddtrace/contrib/aws/integration' require 'ddtrace/contrib/concurrent_ruby/integration' diff --git a/lib/ddtrace/contrib/active_model_serializers/configuration/settings.rb b/lib/ddtrace/contrib/active_model_serializers/configuration/settings.rb new file mode 100644 index 00000000000..6cd4686abfc --- /dev/null +++ b/lib/ddtrace/contrib/active_model_serializers/configuration/settings.rb @@ -0,0 +1,23 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/active_model_serializers/ext' + +module Datadog + module Contrib + module ActiveModelSerializers + module Configuration + # Custom settings for the ActiveModelSerializers integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + option :tracer, default: Datadog.tracer do |value| + (value || Datadog.tracer).tap do |v| + # Make sure to update tracers of all subscriptions + Events.subscriptions.each do |subscription| + subscription.tracer = v + end + end + end + end + end + end + end +end diff --git a/lib/ddtrace/contrib/active_model_serializers/event.rb b/lib/ddtrace/contrib/active_model_serializers/event.rb index 7aab0ecebb4..6d34750095d 100644 --- a/lib/ddtrace/contrib/active_model_serializers/event.rb +++ b/lib/ddtrace/contrib/active_model_serializers/event.rb @@ -1,4 +1,6 @@ +require 'ddtrace/ext/http' require 'ddtrace/contrib/active_support/notifications/event' +require 'ddtrace/contrib/active_model_serializers/ext' module Datadog module Contrib @@ -31,12 +33,12 @@ def process(span, event, _id, payload) # Set the resource name and serializer name res = resource(payload[:serializer]) span.resource = res - span.set_tag('active_model_serializers.serializer', res) + span.set_tag(Ext::TAG_SERIALIZER, res) span.span_type = Datadog::Ext::HTTP::TEMPLATE # Will be nil in 0.9 - span.set_tag('active_model_serializers.adapter', payload[:adapter].class) unless payload[:adapter].nil? + span.set_tag(Ext::TAG_ADAPTER, payload[:adapter].class) unless payload[:adapter].nil? end private diff --git a/lib/ddtrace/contrib/active_model_serializers/events/render.rb b/lib/ddtrace/contrib/active_model_serializers/events/render.rb index 28f2b198ba2..09d366fd419 100644 --- a/lib/ddtrace/contrib/active_model_serializers/events/render.rb +++ b/lib/ddtrace/contrib/active_model_serializers/events/render.rb @@ -1,3 +1,4 @@ +require 'ddtrace/contrib/active_model_serializers/ext' require 'ddtrace/contrib/active_model_serializers/event' module Datadog @@ -9,7 +10,6 @@ module Render include ActiveModelSerializers::Event EVENT_NAME = 'render.active_model_serializers'.freeze - SPAN_NAME = 'active_model_serializers.render'.freeze module_function @@ -23,7 +23,7 @@ def event_name end def span_name - self::SPAN_NAME + Ext::SPAN_RENDER end end end diff --git a/lib/ddtrace/contrib/active_model_serializers/events/serialize.rb b/lib/ddtrace/contrib/active_model_serializers/events/serialize.rb index 22ce802241d..7b277cc6ba1 100644 --- a/lib/ddtrace/contrib/active_model_serializers/events/serialize.rb +++ b/lib/ddtrace/contrib/active_model_serializers/events/serialize.rb @@ -1,3 +1,4 @@ +require 'ddtrace/contrib/active_model_serializers/ext' require 'ddtrace/contrib/active_model_serializers/event' module Datadog @@ -9,7 +10,6 @@ module Serialize include ActiveModelSerializers::Event EVENT_NAME = '!serialize.active_model_serializers'.freeze - SPAN_NAME = 'active_model_serializers.serialize'.freeze module_function @@ -26,7 +26,7 @@ def event_name end def span_name - self::SPAN_NAME + Ext::SPAN_SERIALIZE end end end diff --git a/lib/ddtrace/contrib/active_model_serializers/ext.rb b/lib/ddtrace/contrib/active_model_serializers/ext.rb new file mode 100644 index 00000000000..3ec4bdb1709 --- /dev/null +++ b/lib/ddtrace/contrib/active_model_serializers/ext.rb @@ -0,0 +1,17 @@ +module Datadog + module Contrib + module ActiveModelSerializers + # ActiveModelSerializers integration constants + module Ext + APP = 'active_model_serializers'.freeze + SERVICE_NAME = 'active_model_serializers'.freeze + + SPAN_RENDER = 'active_model_serializers.render'.freeze + SPAN_SERIALIZE = 'active_model_serializers.serialize'.freeze + + TAG_ADAPTER = 'active_model_serializers.adapter'.freeze + TAG_SERIALIZER = 'active_model_serializers.serializer'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/active_model_serializers/integration.rb b/lib/ddtrace/contrib/active_model_serializers/integration.rb new file mode 100644 index 00000000000..32f8083ac56 --- /dev/null +++ b/lib/ddtrace/contrib/active_model_serializers/integration.rb @@ -0,0 +1,39 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/active_model_serializers/configuration/settings' +require 'ddtrace/contrib/active_model_serializers/patcher' + +module Datadog + module Contrib + module ActiveModelSerializers + # Description of ActiveModelSerializers integration + class Integration + include Contrib::Integration + + register_as :active_model_serializers + + def self.version + Gem.loaded_specs['active_model_serializers'] \ + && Gem.loaded_specs['active_model_serializers'].version + end + + def self.present? + super && defined?(::ActiveModel::Serializer) + end + + def self.compatible? + super \ + && defined?(::ActiveSupport::Notifications) \ + && version >= Gem::Version.new('0.9.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/active_model_serializers/patcher.rb b/lib/ddtrace/contrib/active_model_serializers/patcher.rb index 569c443f6d8..35c994f81a2 100644 --- a/lib/ddtrace/contrib/active_model_serializers/patcher.rb +++ b/lib/ddtrace/contrib/active_model_serializers/patcher.rb @@ -1,60 +1,41 @@ +require 'ddtrace/contrib/patcher' require 'ddtrace/ext/app_types' -require 'ddtrace/ext/http' +require 'ddtrace/contrib/active_model_serializers/ext' require 'ddtrace/contrib/active_model_serializers/events' module Datadog module Contrib module ActiveModelSerializers - # Provides instrumentation for ActiveModelSerializers through ActiveSupport instrumentation signals + # Patcher enables patching of 'active_model_serializers' module. module Patcher - include Base + include Contrib::Patcher - VERSION_REQUIRED = Gem::Version.new('0.9.0') + module_function - register_as :active_model_serializers + def patched? + done?(:active_model_serializers) + end - option :service_name, default: 'active_model_serializers' - option :tracer, default: Datadog.tracer do |value| - (value || Datadog.tracer).tap do |v| - # Make sure to update tracers of all subscriptions - Events.subscriptions.each do |subscription| - subscription.tracer = v + def patch + do_once(:active_model_serializers) do + begin + # Subscribe to ActiveModelSerializers events + Events.subscribe! + + # Set service info + get_option(:tracer).set_service_info( + get_option(:service_name), + Ext::APP, + Datadog::Ext::AppTypes::WEB + ) + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply ActiveModelSerializers integration: #{e}") end end end - class << self - def patch - return patched? if patched? || !compatible? - - # Subscribe to ActiveModelSerializers events - Events.subscribe! - - # Set service info - configuration[:tracer].set_service_info( - configuration[:service_name], - 'active_model_serializers', - Ext::AppTypes::WEB - ) - - @patched = true - end - - def patched? - return @patched if defined?(@patched) - @patched = false - end - - private - - def configuration - Datadog.configuration[:active_model_serializers] - end - - def compatible? - Gem.loaded_specs['active_model_serializers'] && Gem.loaded_specs['activesupport'] \ - && Gem.loaded_specs['active_model_serializers'].version >= VERSION_REQUIRED - end + def get_option(option) + Datadog.configuration[:active_model_serializers].get_option(option) end end end From 8e27d4ba2d380685a656a18247ba734c4cfd5af0 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 16:40:59 -0400 Subject: [PATCH 18/23] Refactored: Sinatra to use Datadog::Contrib::Integration. (#575) --- docs/GettingStarted.md | 5 +- lib/ddtrace.rb | 1 + .../contrib/sinatra/configuration/settings.rb | 27 ++++++ lib/ddtrace/contrib/sinatra/env.rb | 7 +- lib/ddtrace/contrib/sinatra/ext.rb | 19 ++++ lib/ddtrace/contrib/sinatra/integration.rb | 36 ++++++++ lib/ddtrace/contrib/sinatra/patcher.rb | 33 +++++++ lib/ddtrace/contrib/sinatra/tracer.rb | 38 +------- .../contrib/sinatra/tracer_middleware.rb | 5 +- test/contrib/sinatra/first_test_app.rb | 11 --- test/contrib/sinatra/multi_app_test.rb | 41 +++++++-- test/contrib/sinatra/second_test_app.rb | 11 --- .../sinatra/tracer_activerecord_test.rb | 44 ++++----- test/contrib/sinatra/tracer_disabled_test.rb | 20 ++--- test/contrib/sinatra/tracer_test.rb | 89 ++++++++++--------- 15 files changed, 238 insertions(+), 149 deletions(-) create mode 100644 lib/ddtrace/contrib/sinatra/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/sinatra/ext.rb create mode 100644 lib/ddtrace/contrib/sinatra/integration.rb create mode 100644 lib/ddtrace/contrib/sinatra/patcher.rb delete mode 100644 test/contrib/sinatra/first_test_app.rb delete mode 100644 test/contrib/sinatra/second_test_app.rb diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index c0efa608e14..72d8542ef9d 100644 --- a/docs/GettingStarted.md +++ b/docs/GettingStarted.md @@ -1163,13 +1163,12 @@ Where `options` is an optional `Hash` that accepts the following parameters: The Sinatra integration traces requests and template rendering. -To start using the tracing client, make sure you import ``ddtrace`` and ``ddtrace/contrib/sinatra/tracer`` after -either ``sinatra`` or ``sinatra/base``: +To start using the tracing client, make sure you import ``ddtrace`` and ``use :sinatra`` after +either ``sinatra`` or ``sinatra/base``, and before you define your application/routes: ```ruby require 'sinatra' require 'ddtrace' -require 'ddtrace/contrib/sinatra/tracer' Datadog.configure do |c| c.use :sinatra, options diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 575e1fadadb..1aa145c13bd 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -69,5 +69,6 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/rest_client/integration' require 'ddtrace/contrib/sequel/integration' require 'ddtrace/contrib/sidekiq/integration' +require 'ddtrace/contrib/sinatra/integration' require 'ddtrace/contrib/sucker_punch/integration' require 'ddtrace/monkey' diff --git a/lib/ddtrace/contrib/sinatra/configuration/settings.rb b/lib/ddtrace/contrib/sinatra/configuration/settings.rb new file mode 100644 index 00000000000..048c6745401 --- /dev/null +++ b/lib/ddtrace/contrib/sinatra/configuration/settings.rb @@ -0,0 +1,27 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/sinatra/ext' + +module Datadog + module Contrib + module Sinatra + module Configuration + # Custom settings for the Sinatra integration + class Settings < Contrib::Configuration::Settings + DEFAULT_HEADERS = { + response: %w[Content-Type X-Request-ID] + }.freeze + + option :distributed_tracing, default: false + option :headers, default: DEFAULT_HEADERS + option :resource_script_names, default: false + + option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| + get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::WEB) + value + end + end + end + end + end +end diff --git a/lib/ddtrace/contrib/sinatra/env.rb b/lib/ddtrace/contrib/sinatra/env.rb index 097dc6040db..0b40b6fad4b 100644 --- a/lib/ddtrace/contrib/sinatra/env.rb +++ b/lib/ddtrace/contrib/sinatra/env.rb @@ -1,20 +1,19 @@ require 'ddtrace/ext/http' +require 'ddtrace/contrib/sinatra/ext' module Datadog module Contrib module Sinatra # Gets and sets trace information from a Rack Env module Env - ENV_SPAN = 'datadog.sinatra_request_span'.freeze - module_function def datadog_span(env) - env[ENV_SPAN] + env[Ext::RACK_ENV_REQUEST_SPAN] end def set_datadog_span(env, span) - env[ENV_SPAN] = span + env[Ext::RACK_ENV_REQUEST_SPAN] = span end def request_header_tags(env, headers) diff --git a/lib/ddtrace/contrib/sinatra/ext.rb b/lib/ddtrace/contrib/sinatra/ext.rb new file mode 100644 index 00000000000..0ab38150bc4 --- /dev/null +++ b/lib/ddtrace/contrib/sinatra/ext.rb @@ -0,0 +1,19 @@ +module Datadog + module Contrib + module Sinatra + # Sinatra integration constants + module Ext + APP = 'sinatra'.freeze + SERVICE_NAME = 'sinatra'.freeze + + RACK_ENV_REQUEST_SPAN = 'datadog.sinatra_request_span'.freeze + + SPAN_RENDER_TEMPLATE = 'sinatra.render_template'.freeze + SPAN_REQUEST = 'sinatra.request'.freeze + + TAG_ROUTE_PATH = 'sinatra.route.path'.freeze + TAG_TEMPLATE_NAME = 'sinatra.template_name'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/sinatra/integration.rb b/lib/ddtrace/contrib/sinatra/integration.rb new file mode 100644 index 00000000000..ff1774f69c5 --- /dev/null +++ b/lib/ddtrace/contrib/sinatra/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/sinatra/configuration/settings' +require 'ddtrace/contrib/sinatra/patcher' + +module Datadog + module Contrib + module Sinatra + # Description of Sinatra integration + class Integration + include Contrib::Integration + + register_as :sinatra + + def self.version + Gem.loaded_specs['sinatra'] && Gem.loaded_specs['sinatra'].version + end + + def self.present? + super && defined?(::Sinatra) + end + + def self.compatible? + super && version >= Gem::Version.new('1.4.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/sinatra/patcher.rb b/lib/ddtrace/contrib/sinatra/patcher.rb new file mode 100644 index 00000000000..fd28a0f61b0 --- /dev/null +++ b/lib/ddtrace/contrib/sinatra/patcher.rb @@ -0,0 +1,33 @@ +require 'ddtrace/contrib/patcher' + +module Datadog + module Contrib + module Sinatra + # Patcher enables patching of 'sinatra' module. + module Patcher + include Contrib::Patcher + + module_function + + def patched? + done?(:sinatra) + end + + def patch + do_once(:sinatra) do + begin + require 'ddtrace/contrib/sinatra/tracer' + register_tracer + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Sinatra integration: #{e}") + end + end + end + + def register_tracer + ::Sinatra.send(:register, Datadog::Contrib::Sinatra::Tracer) + end + end + end + end +end diff --git a/lib/ddtrace/contrib/sinatra/tracer.rb b/lib/ddtrace/contrib/sinatra/tracer.rb index ec27018c342..bf7fd2cc2e7 100644 --- a/lib/ddtrace/contrib/sinatra/tracer.rb +++ b/lib/ddtrace/contrib/sinatra/tracer.rb @@ -5,41 +5,16 @@ require 'ddtrace/ext/http' require 'ddtrace/propagation/http_propagator' +require 'ddtrace/contrib/sinatra/ext' require 'ddtrace/contrib/sinatra/tracer_middleware' require 'ddtrace/contrib/sinatra/env' -sinatra_vs = Gem::Version.new(Sinatra::VERSION) -sinatra_min_vs = Gem::Version.new('1.4.0') -if sinatra_vs < sinatra_min_vs - raise "sinatra version #{sinatra_vs} is not supported yet " \ - + "(supporting versions >=#{sinatra_min_vs})" -end - -Datadog::Tracer.log.info("activating instrumentation for sinatra #{sinatra_vs}") - module Datadog module Contrib module Sinatra # Datadog::Contrib::Sinatra::Tracer is a Sinatra extension which traces # requests. module Tracer - DEFAULT_HEADERS = { - response: %w[Content-Type X-Request-ID] - }.freeze - - include Base - register_as :sinatra - - option :service_name, default: 'sinatra', depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, 'sinatra', Ext::AppTypes::WEB) - value - end - - option :tracer, default: Datadog.tracer - option :resource_script_names, default: false - option :distributed_tracing, default: false - option :headers, default: DEFAULT_HEADERS - def route(verb, action, *) # Keep track of the route name when the app is instantiated for an # incoming request. @@ -62,10 +37,10 @@ def render(engine, data, *) output = '' tracer = Datadog.configuration[:sinatra][:tracer] if tracer.enabled - tracer.trace('sinatra.render_template', span_type: Datadog::Ext::HTTP::TEMPLATE) do |span| + tracer.trace(Ext::SPAN_RENDER_TEMPLATE, span_type: Datadog::Ext::HTTP::TEMPLATE) do |span| # If data is a string, it is a literal template and we don't # want to record it. - span.set_tag('sinatra.template_name', data) if data.is_a? Symbol + span.set_tag(Ext::TAG_TEMPLATE_NAME, data) if data.is_a? Symbol output = super end else @@ -97,7 +72,7 @@ def render(engine, data, *) end span.resource = "#{request.request_method} #{@datadog_route}" - span.set_tag('sinatra.route.path', @datadog_route) + span.set_tag(Ext::TAG_ROUTE_PATH, @datadog_route) span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response.status) span.set_error(env['sinatra.error']) if response.server_error? end @@ -106,8 +81,3 @@ def render(engine, data, *) end end end - -# rubocop:disable Style/Documentation -module Sinatra - register Datadog::Contrib::Sinatra::Tracer -end diff --git a/lib/ddtrace/contrib/sinatra/tracer_middleware.rb b/lib/ddtrace/contrib/sinatra/tracer_middleware.rb index 5b72340849a..984c854eee1 100644 --- a/lib/ddtrace/contrib/sinatra/tracer_middleware.rb +++ b/lib/ddtrace/contrib/sinatra/tracer_middleware.rb @@ -1,3 +1,4 @@ +require 'ddtrace/contrib/sinatra/ext' require 'ddtrace/contrib/sinatra/env' require 'ddtrace/contrib/sinatra/headers' @@ -6,8 +7,6 @@ module Contrib module Sinatra # Middleware used for automatically tagging configured headers and handle request span class TracerMiddleware - REQUEST_TRACE_NAME = 'sinatra.request'.freeze - def initialize(app) @app = app end @@ -21,7 +20,7 @@ def call(env) # Begin the trace tracer.trace( - REQUEST_TRACE_NAME, + Ext::SPAN_REQUEST, service: configuration[:service_name], span_type: Datadog::Ext::HTTP::TYPE ) do |span| diff --git a/test/contrib/sinatra/first_test_app.rb b/test/contrib/sinatra/first_test_app.rb deleted file mode 100644 index 62b844388e9..00000000000 --- a/test/contrib/sinatra/first_test_app.rb +++ /dev/null @@ -1,11 +0,0 @@ -require 'contrib/sinatra/tracer_test_base' - -class MultiAppTest < TracerTestBase - class FirstTestApp < Sinatra::Base - register Datadog::Contrib::Sinatra::Tracer - - get '/endpoint' do - '1' - end - end -end diff --git a/test/contrib/sinatra/multi_app_test.rb b/test/contrib/sinatra/multi_app_test.rb index 370a13dbc27..aebc9bebbf9 100644 --- a/test/contrib/sinatra/multi_app_test.rb +++ b/test/contrib/sinatra/multi_app_test.rb @@ -1,36 +1,61 @@ require 'contrib/sinatra/tracer_test_base' -require 'contrib/sinatra/first_test_app' -require 'contrib/sinatra/second_test_app' class MultiAppTest < TracerTestBase def app @use_multi_app ? multi_app : single_app end + def first_app + @first_app ||= Class.new(Sinatra::Base) do + register Datadog::Contrib::Sinatra::Tracer + + get '/endpoint' do + '1' + end + end + end + + def second_app + @second_app ||= Class.new(Sinatra::Base) do + register Datadog::Contrib::Sinatra::Tracer + + get '/endpoint' do + '2' + end + end + end + def multi_app - Rack::Builder.new do + app_one = first_app + app_two = second_app + + @multi_app ||= Rack::Builder.new do map '/one' do - run FirstTestApp + run app_one end map '/two' do - run SecondTestApp + run app_two end end.to_app end def single_app - FirstTestApp + first_app end def setup + @first_app = nil + @second_app = nil + @multi_app = nil @writer = FauxWriter.new - FirstTestApp.set :datadog_test_writer, @writer - SecondTestApp.set :datadog_test_writer, @writer tracer = Datadog::Tracer.new(writer: @writer, enabled: true) Datadog.configuration[:sinatra][:tracer] = tracer + first_app.set :datadog_test_writer, @writer + second_app.set :datadog_test_writer, @writer + super end diff --git a/test/contrib/sinatra/second_test_app.rb b/test/contrib/sinatra/second_test_app.rb deleted file mode 100644 index c4659822898..00000000000 --- a/test/contrib/sinatra/second_test_app.rb +++ /dev/null @@ -1,11 +0,0 @@ -require 'contrib/sinatra/tracer_test_base' - -class MultiAppTest < TracerTestBase - class SecondTestApp < Sinatra::Base - register Datadog::Contrib::Sinatra::Tracer - - get '/endpoint' do - '2' - end - end -end diff --git a/test/contrib/sinatra/tracer_activerecord_test.rb b/test/contrib/sinatra/tracer_activerecord_test.rb index 1398d4fa4f0..ea540fbe435 100644 --- a/test/contrib/sinatra/tracer_activerecord_test.rb +++ b/test/contrib/sinatra/tracer_activerecord_test.rb @@ -11,40 +11,40 @@ class ApplicationRecord < ActiveRecord::Base class Article < ApplicationRecord end - class TracerActiveRecordTestApp < Sinatra::Application - post '/request' do - conn = settings.datadog_test_conn - conn.connection.execute('SELECT 42') - '' - end + def app + @app ||= Class.new(Sinatra::Application) do + post '/request' do + conn = settings.datadog_test_conn + conn.connection.execute('SELECT 42') + '' + end - post '/cached_request' do - Article.cache do - # Do two queries (second should cache.) - Article.count - Article.count + post '/cached_request' do + Article.cache do + # Do two queries (second should cache.) + Article.count + Article.count + end end - end - get '/select_request' do - Article.all.entries.to_s + get '/select_request' do + Article.all.entries.to_s + end end end - def app - TracerActiveRecordTestApp - end - def setup - @writer = FauxWriter.new() - app().set :datadog_test_writer, @writer + @app = nil + @writer = FauxWriter.new tracer = Datadog::Tracer.new(writer: @writer) Datadog.configuration.use(:sinatra, tracer: tracer) + app.set :datadog_test_writer, @writer + conn = ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:') - app().set :datadog_test_conn, conn + app.set :datadog_test_conn, conn Datadog.configure do |c| c.tracer hostname: ENV.fetch('TEST_DDAGENT_HOST', 'localhost') @@ -71,7 +71,7 @@ def test_request post '/request' assert_equal(200, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_operator(2, :<=, spans.length, 'there should be at least 2 spans (span like "PRAGMA foreign_keys = ON" could appear)') diff --git a/test/contrib/sinatra/tracer_disabled_test.rb b/test/contrib/sinatra/tracer_disabled_test.rb index 935ca1865ff..bf47cee8dfd 100644 --- a/test/contrib/sinatra/tracer_disabled_test.rb +++ b/test/contrib/sinatra/tracer_disabled_test.rb @@ -2,23 +2,23 @@ require 'contrib/sinatra/tracer_test_base' class DisabledTracerTest < ::TracerTestBase - class DisabledTracerTestApp < Sinatra::Application - get '/request' do - 'hello world' - end - end - def app - DisabledTracerTestApp + @app ||= Class.new(Sinatra::Application) do + get '/request' do + 'hello world' + end + end end def setup - @writer = FauxWriter.new() - app().set :datadog_test_writer, @writer + @app = nil + @writer = FauxWriter.new tracer = Datadog::Tracer.new(writer: @writer, enabled: false) Datadog.configuration.use(:sinatra, tracer: tracer) + app.set :datadog_test_writer, @writer + super end @@ -26,7 +26,7 @@ def test_request get '/request' assert_equal(200, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(0, spans.length) end end diff --git a/test/contrib/sinatra/tracer_test.rb b/test/contrib/sinatra/tracer_test.rb index 53225c92db0..7bc7df0e4b6 100644 --- a/test/contrib/sinatra/tracer_test.rb +++ b/test/contrib/sinatra/tracer_test.rb @@ -2,48 +2,48 @@ # rubocop:disable Metrics/ClassLength class TracerTest < TracerTestBase - class TracerTestApp < Sinatra::Application - get '/request' do - headers['X-Request-Id'] = request.env['HTTP_X_REQUEST_ID'] - 'hello world' - end - - get '/bad-request' do - halt 400, 'bad request' - end - - get '/error' do - halt 500, 'server error' - end - - get '/exception' do - raise StandardError, 'something bad' - end - - get '/wildcard/*' do - params['splat'][0] - end - - get '/template' do - erb :msg, locals: { msg: 'hello' } - end - - get '/literal-template' do - erb '<%= msg %>', locals: { msg: 'hello' } - end - end - def app - TracerTestApp + @app ||= Class.new(Sinatra::Application) do + get '/request' do + headers['X-Request-Id'] = request.env['HTTP_X_REQUEST_ID'] + 'hello world' + end + + get '/bad-request' do + halt 400, 'bad request' + end + + get '/error' do + halt 500, 'server error' + end + + get '/exception' do + raise StandardError, 'something bad' + end + + get '/wildcard/*' do + params['splat'][0] + end + + get '/template' do + erb :msg, locals: { msg: 'hello' } + end + + get '/literal-template' do + erb '<%= msg %>', locals: { msg: 'hello' } + end + end end def setup - @writer = FauxWriter.new() - app().set :datadog_test_writer, @writer + @app = nil + @writer = FauxWriter.new tracer = Datadog::Tracer.new(writer: @writer, enabled: true) Datadog.configuration.use(:sinatra, tracer: tracer) + app.set :datadog_test_writer, @writer + super end @@ -54,7 +54,7 @@ def test_service_name get '/request' assert_equal(200, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(1, spans.length) span = spans[0] @@ -67,7 +67,7 @@ def test_request get '/request#foo?a=1' assert_equal(200, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(1, spans.length) span = spans[0] @@ -109,7 +109,7 @@ def test_bad_request get '/bad-request' assert_equal(400, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(1, spans.length) span = spans[0] @@ -128,7 +128,7 @@ def test_error get '/error' assert_equal(500, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(1, spans.length) span = spans[0] @@ -147,7 +147,7 @@ def test_exception get '/exception' assert_equal(500, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(1, spans.length) span = spans[0] @@ -166,7 +166,7 @@ def test_wildcard get '/wildcard/1/2/3' assert_equal(200, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(1, spans.length) span = spans[0] @@ -183,7 +183,7 @@ def test_template get '/template' assert_equal(200, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(3, spans.length) child1, child2, root = spans @@ -213,7 +213,7 @@ def test_literal_template get '/literal-template' assert_equal(200, last_response.status) - spans = @writer.spans() + spans = @writer.spans assert_equal(3, spans.length) child1, child2, root = spans @@ -293,6 +293,9 @@ def test_tagging_configured_connection_headers assert_equal(0, span.status) assert_nil(span.parent) ensure - Datadog.configuration.use(:sinatra, headers: Datadog::Contrib::Sinatra::Tracer::DEFAULT_HEADERS) + Datadog.configuration.use( + :sinatra, + headers: Datadog::Contrib::Sinatra::Configuration::Settings::DEFAULT_HEADERS + ) end end From a5ea7a103f53975fc597b58bc71b1fcb480aa5c5 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 16:50:33 -0400 Subject: [PATCH 19/23] Refactored: Excon to use Datadog::Contrib::Integration. (#569) --- lib/ddtrace.rb | 2 +- .../contrib/excon/configuration/settings.rb | 18 ++++++++ lib/ddtrace/contrib/excon/ext.rb | 13 ++++++ lib/ddtrace/contrib/excon/integration.rb | 32 ++++++++++++++ lib/ddtrace/contrib/excon/middleware.rb | 20 ++++----- lib/ddtrace/contrib/excon/patcher.rb | 44 ++++++------------- .../contrib/excon/instrumentation_spec.rb | 14 +++--- 7 files changed, 95 insertions(+), 48 deletions(-) create mode 100644 lib/ddtrace/contrib/excon/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/excon/ext.rb create mode 100644 lib/ddtrace/contrib/excon/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 1aa145c13bd..84dad44dea6 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -51,7 +51,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/dalli/integration' require 'ddtrace/contrib/delayed_job/integration' require 'ddtrace/contrib/elasticsearch/integration' -require 'ddtrace/contrib/excon/patcher' +require 'ddtrace/contrib/excon/integration' require 'ddtrace/contrib/faraday/patcher' require 'ddtrace/contrib/grape/patcher' require 'ddtrace/contrib/graphql/patcher' diff --git a/lib/ddtrace/contrib/excon/configuration/settings.rb b/lib/ddtrace/contrib/excon/configuration/settings.rb new file mode 100644 index 00000000000..73edcd8056c --- /dev/null +++ b/lib/ddtrace/contrib/excon/configuration/settings.rb @@ -0,0 +1,18 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/excon/ext' + +module Datadog + module Contrib + module Excon + module Configuration + # Custom settings for the Excon integration + class Settings < Contrib::Configuration::Settings + option :distributed_tracing, default: false + option :error_handler, default: nil + option :service_name, default: Ext::SERVICE_NAME + option :split_by_domain, default: false + end + end + end + end +end diff --git a/lib/ddtrace/contrib/excon/ext.rb b/lib/ddtrace/contrib/excon/ext.rb new file mode 100644 index 00000000000..b0804368407 --- /dev/null +++ b/lib/ddtrace/contrib/excon/ext.rb @@ -0,0 +1,13 @@ +module Datadog + module Contrib + module Excon + # Excon integration constants + module Ext + APP = 'excon'.freeze + SERVICE_NAME = 'excon'.freeze + + SPAN_REQUEST = 'excon.request'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/excon/integration.rb b/lib/ddtrace/contrib/excon/integration.rb new file mode 100644 index 00000000000..d1db84e9948 --- /dev/null +++ b/lib/ddtrace/contrib/excon/integration.rb @@ -0,0 +1,32 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/excon/configuration/settings' +require 'ddtrace/contrib/excon/patcher' + +module Datadog + module Contrib + module Excon + # Description of Excon integration + class Integration + include Contrib::Integration + + register_as :excon + + def self.version + Gem.loaded_specs['excon'] && Gem.loaded_specs['excon'].version + end + + def self.present? + super && defined?(::Excon) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/excon/middleware.rb b/lib/ddtrace/contrib/excon/middleware.rb index 8db7270acb3..a1d5500cfe0 100644 --- a/lib/ddtrace/contrib/excon/middleware.rb +++ b/lib/ddtrace/contrib/excon/middleware.rb @@ -3,26 +3,26 @@ require 'ddtrace/ext/net' require 'ddtrace/ext/distributed' require 'ddtrace/propagation/http_propagator' +require 'ddtrace/contrib/excon/ext' module Datadog module Contrib module Excon # Middleware implements an excon-middleware for ddtrace instrumentation class Middleware < ::Excon::Middleware::Base - SPAN_NAME = 'excon.request'.freeze DEFAULT_ERROR_HANDLER = lambda do |response| - Ext::HTTP::ERROR_RANGE.cover?(response[:status]) + Datadog::Ext::HTTP::ERROR_RANGE.cover?(response[:status]) end def initialize(stack, options = {}) super(stack) - @options = Datadog.configuration[:excon].merge(options) + @options = Datadog.configuration[:excon].to_h.merge(options) end def request_call(datum) begin unless datum.key?(:datadog_span) - tracer.trace(SPAN_NAME).tap do |span| + tracer.trace(Ext::SPAN_REQUEST).tap do |span| datum[:datadog_span] = span annotate!(span, datum) propagate!(span, datum) if distributed_tracing? @@ -97,11 +97,11 @@ def split_by_domain? def annotate!(span, datum) span.resource = datum[:method].to_s.upcase span.service = service_name(datum) - span.span_type = Ext::HTTP::TYPE - span.set_tag(Ext::HTTP::URL, datum[:path]) - span.set_tag(Ext::HTTP::METHOD, datum[:method].to_s.upcase) - span.set_tag(Ext::NET::TARGET_HOST, datum[:host]) - span.set_tag(Ext::NET::TARGET_PORT, datum[:port].to_s) + span.span_type = Datadog::Ext::HTTP::TYPE + span.set_tag(Datadog::Ext::HTTP::URL, datum[:path]) + span.set_tag(Datadog::Ext::HTTP::METHOD, datum[:method].to_s.upcase) + span.set_tag(Datadog::Ext::NET::TARGET_HOST, datum[:host]) + span.set_tag(Datadog::Ext::NET::TARGET_PORT, datum[:port].to_s) end def handle_response(datum) @@ -114,7 +114,7 @@ def handle_response(datum) if error_handler.call(response) span.set_error(["Error #{response[:status]}", response[:body]]) end - span.set_tag(Ext::HTTP::STATUS_CODE, response[:status]) + span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response[:status]) end span.set_error(datum[:error]) if datum.key?(:error) span.finish diff --git a/lib/ddtrace/contrib/excon/patcher.rb b/lib/ddtrace/contrib/excon/patcher.rb index 861142cc93d..9c95b137fd7 100644 --- a/lib/ddtrace/contrib/excon/patcher.rb +++ b/lib/ddtrace/contrib/excon/patcher.rb @@ -1,44 +1,28 @@ -require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/patcher' module Datadog module Contrib module Excon - # Responsible for hooking the instrumentation into Excon + # Patcher enables patching of 'excon' module. module Patcher - include Base - - DEFAULT_SERVICE = 'excon'.freeze - - register_as :excon - option :tracer, default: Datadog.tracer - option :service_name, default: DEFAULT_SERVICE - option :distributed_tracing, default: false - option :split_by_domain, default: false - option :error_handler, default: nil - - @patched = false + include Contrib::Patcher module_function - def patch - return @patched if patched? || !compatible? - - require 'ddtrace/contrib/excon/middleware' - - add_middleware - - @patched = true - rescue => e - Tracer.log.error("Unable to apply Excon integration: #{e}") - @patched - end - def patched? - @patched + done?(:excon) end - def compatible? - defined?(::Excon) + def patch + do_once(:excon) do + begin + require 'ddtrace/contrib/excon/middleware' + + add_middleware + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Excon integration: #{e}") + end + end end def add_middleware diff --git a/spec/ddtrace/contrib/excon/instrumentation_spec.rb b/spec/ddtrace/contrib/excon/instrumentation_spec.rb index f06dc1409b1..9ab1d64abf0 100644 --- a/spec/ddtrace/contrib/excon/instrumentation_spec.rb +++ b/spec/ddtrace/contrib/excon/instrumentation_spec.rb @@ -12,11 +12,11 @@ let(:configuration_options) { { tracer: tracer } } let(:request_span) do - tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::Excon::Middleware::SPAN_NAME } + tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::Excon::Ext::SPAN_REQUEST } end let(:all_request_spans) do - tracer.writer.spans(:keep).find_all { |span| span.name == Datadog::Contrib::Excon::Middleware::SPAN_NAME } + tracer.writer.spans(:keep).find_all { |span| span.name == Datadog::Contrib::Excon::Ext::SPAN_REQUEST } end before(:each) do @@ -76,8 +76,8 @@ it do expect(request_span).to_not be nil - expect(request_span.service).to eq(Datadog::Contrib::Excon::Patcher::DEFAULT_SERVICE) - expect(request_span.name).to eq(Datadog::Contrib::Excon::Middleware::SPAN_NAME) + expect(request_span.service).to eq(Datadog::Contrib::Excon::Ext::SERVICE_NAME) + expect(request_span.name).to eq(Datadog::Contrib::Excon::Ext::SPAN_REQUEST) expect(request_span.resource).to eq('GET') expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('GET') expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('200') @@ -93,8 +93,8 @@ subject!(:response) { connection.post(path: '/failure') } it do - expect(request_span.service).to eq(Datadog::Contrib::Excon::Patcher::DEFAULT_SERVICE) - expect(request_span.name).to eq(Datadog::Contrib::Excon::Middleware::SPAN_NAME) + expect(request_span.service).to eq(Datadog::Contrib::Excon::Ext::SERVICE_NAME) + expect(request_span.name).to eq(Datadog::Contrib::Excon::Ext::SPAN_REQUEST) expect(request_span.resource).to eq('POST') expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('POST') expect(request_span.get_tag(Datadog::Ext::HTTP::URL)).to eq('/failure') @@ -146,7 +146,7 @@ after(:each) { Datadog.configuration[:excon][:split_by_domain] = false } it do - expect(request_span.name).to eq(Datadog::Contrib::Excon::Middleware::SPAN_NAME) + expect(request_span.name).to eq(Datadog::Contrib::Excon::Ext::SPAN_REQUEST) expect(request_span.service).to eq('example.com') expect(request_span.resource).to eq('GET') end From c3f31edb67eb64026fd790d562b77ae4b82f14ae Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 16:50:54 -0400 Subject: [PATCH 20/23] Refactored: Grape to use Datadog::Contrib::Integration. (#571) --- lib/ddtrace.rb | 2 +- .../contrib/grape/configuration/settings.rb | 16 +++++++ lib/ddtrace/contrib/grape/endpoint.rb | 23 ++++----- lib/ddtrace/contrib/grape/ext.rb | 19 ++++++++ lib/ddtrace/contrib/grape/integration.rb | 36 ++++++++++++++ lib/ddtrace/contrib/grape/patcher.rb | 47 +++++++++---------- 6 files changed, 105 insertions(+), 38 deletions(-) create mode 100644 lib/ddtrace/contrib/grape/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/grape/ext.rb create mode 100644 lib/ddtrace/contrib/grape/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 84dad44dea6..88890360fcb 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -53,7 +53,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/elasticsearch/integration' require 'ddtrace/contrib/excon/integration' require 'ddtrace/contrib/faraday/patcher' -require 'ddtrace/contrib/grape/patcher' +require 'ddtrace/contrib/grape/integration' require 'ddtrace/contrib/graphql/patcher' require 'ddtrace/contrib/grpc/patcher' require 'ddtrace/contrib/http/integration' diff --git a/lib/ddtrace/contrib/grape/configuration/settings.rb b/lib/ddtrace/contrib/grape/configuration/settings.rb new file mode 100644 index 00000000000..e661b8a61cc --- /dev/null +++ b/lib/ddtrace/contrib/grape/configuration/settings.rb @@ -0,0 +1,16 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/ext/http' +require 'ddtrace/contrib/grape/ext' + +module Datadog + module Contrib + module Grape + module Configuration + # Custom settings for the Grape integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 1a2404da8d3..39c828c0fe5 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -1,6 +1,6 @@ require 'ddtrace/ext/http' require 'ddtrace/ext/errors' -require 'ddtrace/contrib/rack/middlewares' +require 'ddtrace/contrib/rack/ext' module Datadog module Contrib @@ -13,9 +13,6 @@ module Endpoint KEY_RENDER = 'datadog_grape_endpoint_render'.freeze def self.subscribe - # Grape is instrumented only if it's available - return unless defined?(::Grape) && defined?(::ActiveSupport::Notifications) - # subscribe when a Grape endpoint is hit ::ActiveSupport::Notifications.subscribe('endpoint_run.grape.start_process') do |*args| endpoint_start_process(*args) @@ -45,7 +42,7 @@ def self.endpoint_start_process(*) tracer = pin.tracer service = pin.service type = Datadog::Ext::HTTP::TYPE - tracer.trace('grape.endpoint_run', service: service, span_type: type) + tracer.trace(Ext::SPAN_ENDPOINT_RUN, service: service, span_type: type) Thread.current[KEY_RUN] = true rescue StandardError => e @@ -61,7 +58,7 @@ def self.endpoint_run(name, start, finish, id, payload) return unless pin && pin.enabled? tracer = pin.tracer - span = tracer.active_span() + span = tracer.active_span return unless span begin @@ -72,8 +69,8 @@ def self.endpoint_run(name, start, finish, id, payload) span.resource = resource # set the request span resource if it's a `rack.request` span - request_span = payload[:env][Datadog::Contrib::Rack::TraceMiddleware::RACK_REQUEST_SPAN] - if !request_span.nil? && request_span.name == 'rack.request' + request_span = payload[:env][Datadog::Contrib::Rack::Ext::RACK_ENV_REQUEST_SPAN] + if !request_span.nil? && request_span.name == Datadog::Contrib::Rack::Ext::SPAN_REQUEST request_span.resource = resource end @@ -81,8 +78,8 @@ def self.endpoint_run(name, start, finish, id, payload) span.set_error(payload[:exception_object]) unless payload[:exception_object].nil? # override the current span with this notification values - span.set_tag('grape.route.endpoint', api_view) - span.set_tag('grape.route.path', path) + span.set_tag(Ext::TAG_ROUTE_ENDPOINT, api_view) + span.set_tag(Ext::TAG_ROUTE_PATH, path) ensure span.start_time = start span.finish(finish) @@ -102,7 +99,7 @@ def self.endpoint_start_render(*) tracer = pin.tracer service = pin.service type = Datadog::Ext::HTTP::TYPE - tracer.trace('grape.endpoint_render', service: service, span_type: type) + tracer.trace(Ext::SPAN_ENDPOINT_RENDER, service: service, span_type: type) Thread.current[KEY_RENDER] = true rescue StandardError => e @@ -118,7 +115,7 @@ def self.endpoint_render(name, start, finish, id, payload) return unless pin && pin.enabled? tracer = pin.tracer - span = tracer.active_span() + span = tracer.active_span return unless span # catch thrown exceptions @@ -151,7 +148,7 @@ def self.endpoint_run_filters(name, start, finish, id, payload) begin # catch thrown exceptions span.set_error(payload[:exception_object]) unless payload[:exception_object].nil? - span.set_tag('grape.filter.type', type.to_s) + span.set_tag(Ext::TAG_FILTER_TYPE, type.to_s) ensure span.start_time = start span.finish(finish) diff --git a/lib/ddtrace/contrib/grape/ext.rb b/lib/ddtrace/contrib/grape/ext.rb new file mode 100644 index 00000000000..95f22a41ea2 --- /dev/null +++ b/lib/ddtrace/contrib/grape/ext.rb @@ -0,0 +1,19 @@ +module Datadog + module Contrib + module Grape + # Grape integration constants + module Ext + APP = 'grape'.freeze + SERVICE_NAME = 'grape'.freeze + + SPAN_ENDPOINT_RENDER = 'grape.endpoint_render'.freeze + SPAN_ENDPOINT_RUN = 'grape.endpoint_run'.freeze + SPAN_ENDPOINT_RUN_FILTERS = 'grape.endpoint_run_filters'.freeze + + TAG_FILTER_TYPE = 'grape.filter.type'.freeze + TAG_ROUTE_ENDPOINT = 'grape.route.endpoint'.freeze + TAG_ROUTE_PATH = 'grape.route.path'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/grape/integration.rb b/lib/ddtrace/contrib/grape/integration.rb new file mode 100644 index 00000000000..635b6685630 --- /dev/null +++ b/lib/ddtrace/contrib/grape/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/grape/configuration/settings' +require 'ddtrace/contrib/grape/patcher' + +module Datadog + module Contrib + module Grape + # Description of Grape integration + class Integration + include Contrib::Integration + + register_as :grape, auto_patch: true + + def self.version + Gem.loaded_specs['grape'] && Gem.loaded_specs['grape'].version + end + + def self.present? + super && defined?(::Grape) + end + + def self.compatible? + super && defined?(::ActiveSupport::Notifications) + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/grape/patcher.rb b/lib/ddtrace/contrib/grape/patcher.rb index 4020c5e9d98..401ff25755f 100644 --- a/lib/ddtrace/contrib/grape/patcher.rb +++ b/lib/ddtrace/contrib/grape/patcher.rb @@ -1,48 +1,43 @@ +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/grape/ext' + module Datadog module Contrib module Grape - SERVICE = 'grape'.freeze - - # Patcher that introduces more instrumentation for Grape endpoints, so that - # new signals are executed at the beginning of each step (filters, render and run) + # Patcher enables patching of 'grape' module. module Patcher - include Base - register_as :grape, auto_patch: true - option :service_name, default: SERVICE - - @patched = false + include Contrib::Patcher module_function def patched? - @patched + done?(:grape) end def patch - if !@patched && defined?(::Grape) + do_once(:grape) do begin - # do not require these by default, but only when actually patching - require 'ddtrace' - require 'ddtrace/ext/app_types' require 'ddtrace/contrib/grape/endpoint' - @patched = true - # patch all endpoints - patch_endpoint_run() - patch_endpoint_render() + # Patch all endpoints + patch_endpoint_run + patch_endpoint_render - # attach a PIN object globally and set the service once - service = get_option(:service_name) - pin = Datadog::Pin.new(service, app: 'grape', app_type: Datadog::Ext::AppTypes::WEB) + # Attach a PIN object globally and set the service once + pin = Datadog::Pin.new( + get_option(:service_name), + app: Ext::APP, + app_type: Datadog::Ext::AppTypes::WEB + ) pin.onto(::Grape) - # subscribe to ActiveSupport events - Datadog::Contrib::Grape::Endpoint.subscribe() + # Subscribe to ActiveSupport events + Datadog::Contrib::Grape::Endpoint.subscribe rescue StandardError => e Datadog::Tracer.log.error("Unable to apply Grape integration: #{e}") end end - @patched end def patch_endpoint_run @@ -69,6 +64,10 @@ def generate_api_method(*params, &block) end end end + + def get_option(option) + Datadog.configuration[:grape].get_option(option) + end end end end From ffb31fc2c5abe8f9eba041d5041a102c0a5cade9 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 16:51:41 -0400 Subject: [PATCH 21/23] Refactored: gRPC to use Datadog::Contrib::Integration. (#563) --- lib/ddtrace.rb | 2 +- .../contrib/grpc/configuration/settings.rb | 16 +++++ .../grpc/datadog_interceptor/client.rb | 7 ++- .../grpc/datadog_interceptor/server.rb | 7 ++- lib/ddtrace/contrib/grpc/ext.rb | 14 +++++ lib/ddtrace/contrib/grpc/integration.rb | 38 ++++++++++++ lib/ddtrace/contrib/grpc/patcher.rb | 60 ++++++++----------- lib/ddtrace/ext/grpc.rb | 7 --- .../grpc/datadog_interceptor/client_spec.rb | 2 +- .../grpc/datadog_interceptor/server_spec.rb | 2 +- .../contrib/grpc/interception_context_spec.rb | 6 +- 11 files changed, 109 insertions(+), 52 deletions(-) create mode 100644 lib/ddtrace/contrib/grpc/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/grpc/ext.rb create mode 100644 lib/ddtrace/contrib/grpc/integration.rb delete mode 100644 lib/ddtrace/ext/grpc.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 88890360fcb..62604d19d8e 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -55,8 +55,8 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/faraday/patcher' require 'ddtrace/contrib/grape/integration' require 'ddtrace/contrib/graphql/patcher' -require 'ddtrace/contrib/grpc/patcher' require 'ddtrace/contrib/http/integration' +require 'ddtrace/contrib/grpc/integration' require 'ddtrace/contrib/integration' require 'ddtrace/contrib/mysql2/integration' require 'ddtrace/contrib/mongodb/integration' diff --git a/lib/ddtrace/contrib/grpc/configuration/settings.rb b/lib/ddtrace/contrib/grpc/configuration/settings.rb new file mode 100644 index 00000000000..5dc40822b8c --- /dev/null +++ b/lib/ddtrace/contrib/grpc/configuration/settings.rb @@ -0,0 +1,16 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/contrib/grpc/ext' + +module Datadog + module Contrib + module GRPC + module Configuration + # Custom settings for the gRPC integration + class Settings < Contrib::Configuration::Settings + option :service_name, default: Ext::SERVICE_NAME + option :tracer, default: Datadog.tracer + end + end + end + end +end diff --git a/lib/ddtrace/contrib/grpc/datadog_interceptor/client.rb b/lib/ddtrace/contrib/grpc/datadog_interceptor/client.rb index 80ca2f85d3e..b926f155661 100644 --- a/lib/ddtrace/contrib/grpc/datadog_interceptor/client.rb +++ b/lib/ddtrace/contrib/grpc/datadog_interceptor/client.rb @@ -1,3 +1,6 @@ +require 'ddtrace/ext/http' +require 'ddtrace/contrib/grpc/ext' + module Datadog module Contrib module GRPC @@ -11,12 +14,12 @@ def trace(keywords) keywords[:metadata] ||= {} options = { - span_type: Datadog::Ext::GRPC::TYPE, + span_type: Datadog::Ext::HTTP::TYPE, service: datadog_pin.service_name, resource: format_resource(keywords[:method]) } - tracer.trace('grpc.client', options) do |span| + tracer.trace(Ext::SPAN_CLIENT, options) do |span| annotate!(span, keywords[:metadata]) yield diff --git a/lib/ddtrace/contrib/grpc/datadog_interceptor/server.rb b/lib/ddtrace/contrib/grpc/datadog_interceptor/server.rb index df4954355dc..1f452f6eeba 100644 --- a/lib/ddtrace/contrib/grpc/datadog_interceptor/server.rb +++ b/lib/ddtrace/contrib/grpc/datadog_interceptor/server.rb @@ -1,3 +1,6 @@ +require 'ddtrace/ext/http' +require 'ddtrace/contrib/grpc/ext' + module Datadog module Contrib module GRPC @@ -10,7 +13,7 @@ module DatadogInterceptor class Server < Base def trace(keywords) options = { - span_type: Datadog::Ext::GRPC::TYPE, + span_type: Datadog::Ext::HTTP::TYPE, service: datadog_pin.service_name, resource: format_resource(keywords[:method]) } @@ -18,7 +21,7 @@ def trace(keywords) set_distributed_context!(tracer, metadata) - tracer.trace('grpc.service', options) do |span| + tracer.trace(Ext::SPAN_SERVICE, options) do |span| annotate!(span, metadata) yield diff --git a/lib/ddtrace/contrib/grpc/ext.rb b/lib/ddtrace/contrib/grpc/ext.rb new file mode 100644 index 00000000000..372edf262ed --- /dev/null +++ b/lib/ddtrace/contrib/grpc/ext.rb @@ -0,0 +1,14 @@ +module Datadog + module Contrib + module GRPC + # gRPC integration constants + module Ext + APP = 'grpc'.freeze + SERVICE_NAME = 'grpc'.freeze + + SPAN_CLIENT = 'grpc.client'.freeze + SPAN_SERVICE = 'grpc.service'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/grpc/integration.rb b/lib/ddtrace/contrib/grpc/integration.rb new file mode 100644 index 00000000000..0c6bd94fd67 --- /dev/null +++ b/lib/ddtrace/contrib/grpc/integration.rb @@ -0,0 +1,38 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/grpc/configuration/settings' +require 'ddtrace/contrib/grpc/patcher' + +module Datadog + module Contrib + module GRPC + # Description of gRPC integration + class Integration + include Contrib::Integration + + register_as :grpc, auto_patch: true + + def self.version + Gem.loaded_specs['grpc'] && Gem.loaded_specs['grpc'].version + end + + def self.present? + super && defined?(::GRPC) + end + + def self.compatible? + super \ + && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.0') \ + && version >= Gem::Version.new('0.10.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/grpc/patcher.rb b/lib/ddtrace/contrib/grpc/patcher.rb index 5c4b7618493..f2d1fe78be0 100644 --- a/lib/ddtrace/contrib/grpc/patcher.rb +++ b/lib/ddtrace/contrib/grpc/patcher.rb @@ -1,60 +1,50 @@ -# requirements should be kept minimal as Patcher is a shared requirement. +require 'ddtrace/contrib/patcher' +require 'ddtrace/contrib/grpc/ext' module Datadog module Contrib module GRPC - SERVICE = 'grpc'.freeze - # Patcher enables patching of 'grpc' module. module Patcher - include Base - register_as :grpc, auto_patch: true - option :tracer, default: Datadog.tracer - option :service_name, default: SERVICE - - @patched = false + include Contrib::Patcher module_function - def patch - return false unless compatible? - return @patched if @patched - - require 'ddtrace/ext/grpc' - require 'ddtrace/propagation/grpc_propagator' - require 'ddtrace/contrib/grpc/datadog_interceptor' - require 'ddtrace/contrib/grpc/intercept_with_datadog' - - add_pin - prepend_interceptor - - @patched = true - rescue StandardError => e - Datadog::Tracer.log.error("Unable to apply gRPC integration: #{e}") - ensure - @patched - end - - def compatible? - defined?(::GRPC::VERSION) && Gem::Version.new(::GRPC::VERSION) >= Gem::Version.new('0.10.0') + def patched? + done?(:grpc) end - def patched? - @patched + def patch + do_once(:grpc) do + begin + require 'ddtrace/propagation/grpc_propagator' + require 'ddtrace/contrib/grpc/datadog_interceptor' + require 'ddtrace/contrib/grpc/intercept_with_datadog' + + add_pin + prepend_interceptor + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply gRPC integration: #{e}") + end + end end def add_pin Pin.new( get_option(:service_name), - app: 'grpc', - app_type: 'grpc', + app: Ext::APP, + app_type: Datadog::Ext::AppTypes::WEB, tracer: get_option(:tracer) ).onto(::GRPC) end def prepend_interceptor ::GRPC::InterceptionContext - .prepend(Datadog::Contrib::GRPC::InterceptWithDatadog) + .send(:prepend, Datadog::Contrib::GRPC::InterceptWithDatadog) + end + + def get_option(option) + Datadog.configuration[:grpc].get_option(option) end end end diff --git a/lib/ddtrace/ext/grpc.rb b/lib/ddtrace/ext/grpc.rb deleted file mode 100644 index e55131fc95e..00000000000 --- a/lib/ddtrace/ext/grpc.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Datadog - module Ext - module GRPC - TYPE = 'grpc'.freeze - end - end -end diff --git a/spec/ddtrace/contrib/grpc/datadog_interceptor/client_spec.rb b/spec/ddtrace/contrib/grpc/datadog_interceptor/client_spec.rb index 3d05c49b531..3e3313af5b6 100644 --- a/spec/ddtrace/contrib/grpc/datadog_interceptor/client_spec.rb +++ b/spec/ddtrace/contrib/grpc/datadog_interceptor/client_spec.rb @@ -46,7 +46,7 @@ shared_examples 'span data contents' do specify { expect(span.name).to eq 'grpc.client' } - specify { expect(span.span_type).to eq 'grpc' } + specify { expect(span.span_type).to eq 'http' } specify { expect(span.service).to eq 'rspec' } specify { expect(span.resource).to eq 'myservice.endpoint' } specify { expect(span.get_tag('error.stack')).to be_nil } diff --git a/spec/ddtrace/contrib/grpc/datadog_interceptor/server_spec.rb b/spec/ddtrace/contrib/grpc/datadog_interceptor/server_spec.rb index e56da246196..f4768d2068b 100644 --- a/spec/ddtrace/contrib/grpc/datadog_interceptor/server_spec.rb +++ b/spec/ddtrace/contrib/grpc/datadog_interceptor/server_spec.rb @@ -15,7 +15,7 @@ shared_examples 'span data contents' do specify { expect(span.name).to eq 'grpc.service' } - specify { expect(span.span_type).to eq 'grpc' } + specify { expect(span.span_type).to eq 'http' } specify { expect(span.service).to eq 'rspec' } specify { expect(span.resource).to eq 'my.server.endpoint' } specify { expect(span.get_tag('error.stack')).to be_nil } diff --git a/spec/ddtrace/contrib/grpc/interception_context_spec.rb b/spec/ddtrace/contrib/grpc/interception_context_spec.rb index bbe4307cdba..de00f4f5fe2 100644 --- a/spec/ddtrace/contrib/grpc/interception_context_spec.rb +++ b/spec/ddtrace/contrib/grpc/interception_context_spec.rb @@ -19,7 +19,7 @@ context 'when intercepting on the client' do shared_examples 'span data contents' do specify { expect(span.name).to eq 'grpc.client' } - specify { expect(span.span_type).to eq 'grpc' } + specify { expect(span.span_type).to eq 'http' } specify { expect(span.service).to eq 'rspec' } specify { expect(span.resource).to eq 'myservice.endpoint' } specify { expect(span.get_tag('error.stack')).to be_nil } @@ -60,7 +60,7 @@ specify do expect(span.name).to eq 'grpc.client' - expect(span.span_type).to eq 'grpc' + expect(span.span_type).to eq 'http' expect(span.service).to eq 'rspec' expect(span.resource).to eq 'myservice.endpoint' expect(span.get_tag('error.stack')).to be_nil @@ -84,7 +84,7 @@ context 'when intercepting on the server' do shared_examples 'span data contents' do specify { expect(span.name).to eq 'grpc.service' } - specify { expect(span.span_type).to eq 'grpc' } + specify { expect(span.span_type).to eq 'http' } specify { expect(span.service).to eq 'rspec' } specify { expect(span.resource).to eq 'my.server.endpoint' } specify { expect(span.get_tag('error.stack')).to be_nil } From f9fed671c2f8382c6a1e6d7ce6fd50a0ef2456a2 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 16:58:47 -0400 Subject: [PATCH 22/23] Refactored: Faraday to use Datadog::Contrib::Integration. (#570) --- lib/ddtrace.rb | 2 +- .../contrib/faraday/configuration/settings.rb | 22 +++++ lib/ddtrace/contrib/faraday/ext.rb | 13 +++ lib/ddtrace/contrib/faraday/integration.rb | 36 +++++++ lib/ddtrace/contrib/faraday/middleware.rb | 19 ++-- lib/ddtrace/contrib/faraday/patcher.rb | 93 ++++++++----------- .../contrib/faraday/middleware_spec.rb | 12 +-- 7 files changed, 127 insertions(+), 70 deletions(-) create mode 100644 lib/ddtrace/contrib/faraday/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/faraday/ext.rb create mode 100644 lib/ddtrace/contrib/faraday/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 62604d19d8e..75f0925921a 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -52,7 +52,7 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/delayed_job/integration' require 'ddtrace/contrib/elasticsearch/integration' require 'ddtrace/contrib/excon/integration' -require 'ddtrace/contrib/faraday/patcher' +require 'ddtrace/contrib/faraday/integration' require 'ddtrace/contrib/grape/integration' require 'ddtrace/contrib/graphql/patcher' require 'ddtrace/contrib/http/integration' diff --git a/lib/ddtrace/contrib/faraday/configuration/settings.rb b/lib/ddtrace/contrib/faraday/configuration/settings.rb new file mode 100644 index 00000000000..b7cc1a8758e --- /dev/null +++ b/lib/ddtrace/contrib/faraday/configuration/settings.rb @@ -0,0 +1,22 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/ext/http' +require 'ddtrace/contrib/faraday/ext' + +module Datadog + module Contrib + module Faraday + module Configuration + # Custom settings for the Faraday integration + class Settings < Contrib::Configuration::Settings + DEFAULT_ERROR_HANDLER = lambda do |env| + Datadog::Ext::HTTP::ERROR_RANGE.cover?(env[:status]) + end + + option :distributed_tracing, default: false + option :error_handler, default: DEFAULT_ERROR_HANDLER + option :service_name, default: Ext::SERVICE_NAME + end + end + end + end +end diff --git a/lib/ddtrace/contrib/faraday/ext.rb b/lib/ddtrace/contrib/faraday/ext.rb new file mode 100644 index 00000000000..8b90d447aef --- /dev/null +++ b/lib/ddtrace/contrib/faraday/ext.rb @@ -0,0 +1,13 @@ +module Datadog + module Contrib + module Faraday + # Faraday integration constants + module Ext + APP = 'faraday'.freeze + SERVICE_NAME = 'faraday'.freeze + + SPAN_REQUEST = 'faraday.request'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/faraday/integration.rb b/lib/ddtrace/contrib/faraday/integration.rb new file mode 100644 index 00000000000..9d6aeb5e289 --- /dev/null +++ b/lib/ddtrace/contrib/faraday/integration.rb @@ -0,0 +1,36 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/faraday/configuration/settings' +require 'ddtrace/contrib/faraday/patcher' + +module Datadog + module Contrib + module Faraday + # Description of Faraday integration + class Integration + include Contrib::Integration + + register_as :faraday, auto_patch: true + + def self.version + Gem.loaded_specs['faraday'] && Gem.loaded_specs['faraday'].version + end + + def self.present? + super && defined?(::Faraday) + end + + def self.compatible? + super && version < Gem::Version.new('1.0.0') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/faraday/middleware.rb b/lib/ddtrace/contrib/faraday/middleware.rb index fa53fbc25ec..e3e4558a484 100644 --- a/lib/ddtrace/contrib/faraday/middleware.rb +++ b/lib/ddtrace/contrib/faraday/middleware.rb @@ -2,23 +2,24 @@ require 'ddtrace/ext/http' require 'ddtrace/ext/net' require 'ddtrace/propagation/http_propagator' +require 'ddtrace/contrib/faraday/ext' module Datadog module Contrib module Faraday # Middleware implements a faraday-middleware for ddtrace instrumentation class Middleware < ::Faraday::Middleware - include Ext::DistributedTracing + include Datadog::Ext::DistributedTracing def initialize(app, options = {}) super(app) - @options = Datadog.configuration[:faraday].merge(options) + @options = Datadog.configuration[:faraday].to_h.merge(options) @tracer = Pin.get_from(::Faraday).tracer setup_service! end def call(env) - tracer.trace(NAME) do |span| + tracer.trace(Ext::SPAN_REQUEST) do |span| annotate!(span, env) propagate!(span, env) if options[:distributed_tracing] && tracer.enabled app.call(env).on_complete { |resp| handle_response(span, resp) } @@ -32,11 +33,11 @@ def call(env) def annotate!(span, env) span.resource = env[:method].to_s.upcase span.service = service_name(env) - span.span_type = Ext::HTTP::TYPE - span.set_tag(Ext::HTTP::URL, env[:url].path) - span.set_tag(Ext::HTTP::METHOD, env[:method].to_s.upcase) - span.set_tag(Ext::NET::TARGET_HOST, env[:url].host) - span.set_tag(Ext::NET::TARGET_PORT, env[:url].port) + span.span_type = Datadog::Ext::HTTP::TYPE + span.set_tag(Datadog::Ext::HTTP::URL, env[:url].path) + span.set_tag(Datadog::Ext::HTTP::METHOD, env[:method].to_s.upcase) + span.set_tag(Datadog::Ext::NET::TARGET_HOST, env[:url].host) + span.set_tag(Datadog::Ext::NET::TARGET_PORT, env[:url].port) end def handle_response(span, env) @@ -44,7 +45,7 @@ def handle_response(span, env) span.set_error(["Error #{env[:status]}", env[:body]]) end - span.set_tag(Ext::HTTP::STATUS_CODE, env[:status]) + span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, env[:status]) end def propagate!(span, env) diff --git a/lib/ddtrace/contrib/faraday/patcher.rb b/lib/ddtrace/contrib/faraday/patcher.rb index 0d390cbfaef..4cc556618e5 100644 --- a/lib/ddtrace/contrib/faraday/patcher.rb +++ b/lib/ddtrace/contrib/faraday/patcher.rb @@ -1,71 +1,56 @@ +require 'ddtrace/contrib/patcher' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/faraday/ext' + module Datadog module Contrib module Faraday - COMPATIBLE_UNTIL = Gem::Version.new('1.0.0') - SERVICE = 'faraday'.freeze - NAME = 'faraday.request'.freeze - - # Responsible for hooking the instrumentation into faraday + # Patcher enables patching of 'faraday' module. module Patcher - include Base + include Contrib::Patcher - register_as :faraday, auto_patch: true + module_function - DEFAULT_ERROR_HANDLER = lambda do |env| - Ext::HTTP::ERROR_RANGE.cover?(env[:status]) + def patched? + done?(:faraday) end - option :service_name, default: SERVICE - option :distributed_tracing, default: false - option :error_handler, default: DEFAULT_ERROR_HANDLER - option :tracer, default: Datadog.tracer - - @patched = false - - class << self - def patch - return @patched if patched? || !compatible? - - require 'ddtrace/ext/app_types' - require 'ddtrace/contrib/faraday/middleware' - - add_pin - add_middleware - - @patched = true - rescue => e - Tracer.log.error("Unable to apply Faraday integration: #{e}") - @patched - end - - def patched? - @patched - end + def patch + do_once(:faraday) do + begin + require 'ddtrace/contrib/faraday/middleware' - def register_service(name) - get_option(:tracer).set_service_info(name, 'faraday', Ext::AppTypes::WEB) + add_pin + add_middleware + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply Faraday integration: #{e}") + end end + end - private - - def compatible? - return unless defined?(::Faraday::VERSION) + def add_pin + Pin.new( + get_option(:service_name), + app: Ext::APP, + app_type: Datadog::Ext::AppTypes::WEB, + tracer: get_option(:tracer) + ).onto(::Faraday) + end - Gem::Version.new(::Faraday::VERSION) < COMPATIBLE_UNTIL - end + def add_middleware + ::Faraday::Middleware.register_middleware(ddtrace: Middleware) + end - def add_pin - Pin.new( - get_option(:service_name), - app: 'faraday', - app_type: Ext::AppTypes::WEB, - tracer: get_option(:tracer) - ).onto(::Faraday) - end + def register_service(name) + get_option(:tracer).set_service_info( + name, + Ext::APP, + Datadog::Ext::AppTypes::WEB + ) + end - def add_middleware - ::Faraday::Middleware.register_middleware(ddtrace: Middleware) - end + def get_option(option) + Datadog.configuration[:faraday].get_option(option) end end end diff --git a/spec/ddtrace/contrib/faraday/middleware_spec.rb b/spec/ddtrace/contrib/faraday/middleware_spec.rb index e87f7761d27..04dc8bc6547 100644 --- a/spec/ddtrace/contrib/faraday/middleware_spec.rb +++ b/spec/ddtrace/contrib/faraday/middleware_spec.rb @@ -22,7 +22,7 @@ let(:configuration_options) { { tracer: tracer } } let(:request_span) do - tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::Faraday::NAME } + tracer.writer.spans(:keep).find { |span| span.name == Datadog::Contrib::Faraday::Ext::SPAN_REQUEST } end before(:each) do @@ -51,8 +51,8 @@ it do expect(request_span).to_not be nil - expect(request_span.service).to eq(Datadog::Contrib::Faraday::SERVICE) - expect(request_span.name).to eq(Datadog::Contrib::Faraday::NAME) + expect(request_span.service).to eq(Datadog::Contrib::Faraday::Ext::SERVICE_NAME) + expect(request_span.name).to eq(Datadog::Contrib::Faraday::Ext::SPAN_REQUEST) expect(request_span.resource).to eq('GET') expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('GET') expect(request_span.get_tag(Datadog::Ext::HTTP::STATUS_CODE)).to eq('200') @@ -68,8 +68,8 @@ subject!(:response) { client.post('/failure') } it do - expect(request_span.service).to eq(Datadog::Contrib::Faraday::SERVICE) - expect(request_span.name).to eq(Datadog::Contrib::Faraday::NAME) + expect(request_span.service).to eq(Datadog::Contrib::Faraday::Ext::SERVICE_NAME) + expect(request_span.name).to eq(Datadog::Contrib::Faraday::Ext::SPAN_REQUEST) expect(request_span.resource).to eq('POST') expect(request_span.get_tag(Datadog::Ext::HTTP::METHOD)).to eq('POST') expect(request_span.get_tag(Datadog::Ext::HTTP::URL)).to eq('/failure') @@ -103,7 +103,7 @@ let(:middleware_options) { { split_by_domain: true } } it do - expect(request_span.name).to eq(Datadog::Contrib::Faraday::NAME) + expect(request_span.name).to eq(Datadog::Contrib::Faraday::Ext::SPAN_REQUEST) expect(request_span.service).to eq('example.com') expect(request_span.resource).to eq('GET') end From fa7327b00626a60909a863dac655112b769af69a Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 1 Oct 2018 17:04:10 -0400 Subject: [PATCH 23/23] Refactored: GraphQL to use Datadog::Contrib::Integration. (#572) --- lib/ddtrace.rb | 4 +- .../contrib/graphql/configuration/settings.rb | 20 +++++ lib/ddtrace/contrib/graphql/ext.rb | 11 +++ lib/ddtrace/contrib/graphql/integration.rb | 38 ++++++++++ lib/ddtrace/contrib/graphql/patcher.rb | 74 +++++++++---------- spec/ddtrace/contrib/graphql/tracer_spec.rb | 3 +- 6 files changed, 106 insertions(+), 44 deletions(-) create mode 100644 lib/ddtrace/contrib/graphql/configuration/settings.rb create mode 100644 lib/ddtrace/contrib/graphql/ext.rb create mode 100644 lib/ddtrace/contrib/graphql/integration.rb diff --git a/lib/ddtrace.rb b/lib/ddtrace.rb index 75f0925921a..471e21619b4 100644 --- a/lib/ddtrace.rb +++ b/lib/ddtrace.rb @@ -54,9 +54,9 @@ def configure(target = configuration, opts = {}) require 'ddtrace/contrib/excon/integration' require 'ddtrace/contrib/faraday/integration' require 'ddtrace/contrib/grape/integration' -require 'ddtrace/contrib/graphql/patcher' -require 'ddtrace/contrib/http/integration' +require 'ddtrace/contrib/graphql/integration' require 'ddtrace/contrib/grpc/integration' +require 'ddtrace/contrib/http/integration' require 'ddtrace/contrib/integration' require 'ddtrace/contrib/mysql2/integration' require 'ddtrace/contrib/mongodb/integration' diff --git a/lib/ddtrace/contrib/graphql/configuration/settings.rb b/lib/ddtrace/contrib/graphql/configuration/settings.rb new file mode 100644 index 00000000000..fa47f4c575d --- /dev/null +++ b/lib/ddtrace/contrib/graphql/configuration/settings.rb @@ -0,0 +1,20 @@ +require 'ddtrace/contrib/configuration/settings' +require 'ddtrace/ext/app_types' +require 'ddtrace/contrib/graphql/ext' + +module Datadog + module Contrib + module GraphQL + module Configuration + # Custom settings for the GraphQL integration + class Settings < Contrib::Configuration::Settings + option :schemas + option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| + get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::WEB) + value + end + end + end + end + end +end diff --git a/lib/ddtrace/contrib/graphql/ext.rb b/lib/ddtrace/contrib/graphql/ext.rb new file mode 100644 index 00000000000..8bcbd266a6c --- /dev/null +++ b/lib/ddtrace/contrib/graphql/ext.rb @@ -0,0 +1,11 @@ +module Datadog + module Contrib + module GraphQL + # GraphQL integration constants + module Ext + APP = 'ruby-graphql'.freeze + SERVICE_NAME = 'ruby-graphql'.freeze + end + end + end +end diff --git a/lib/ddtrace/contrib/graphql/integration.rb b/lib/ddtrace/contrib/graphql/integration.rb new file mode 100644 index 00000000000..5628748b4de --- /dev/null +++ b/lib/ddtrace/contrib/graphql/integration.rb @@ -0,0 +1,38 @@ +require 'ddtrace/contrib/integration' +require 'ddtrace/contrib/graphql/configuration/settings' +require 'ddtrace/contrib/graphql/patcher' + +module Datadog + module Contrib + module GraphQL + # Description of GraphQL integration + class Integration + include Contrib::Integration + + register_as :graphql, auto_patch: true + + def self.version + Gem.loaded_specs['graphql'] && Gem.loaded_specs['graphql'].version + end + + def self.present? + super && defined?(::GraphQL) + end + + def self.compatible? + super \ + && defined?(::GraphQL::Tracing::DataDogTracing) \ + && version >= Gem::Version.new('1.7.9') + end + + def default_configuration + Configuration::Settings.new + end + + def patcher + Patcher + end + end + end + end +end diff --git a/lib/ddtrace/contrib/graphql/patcher.rb b/lib/ddtrace/contrib/graphql/patcher.rb index c302f72350d..f397ea61c83 100644 --- a/lib/ddtrace/contrib/graphql/patcher.rb +++ b/lib/ddtrace/contrib/graphql/patcher.rb @@ -1,63 +1,55 @@ -require 'ddtrace/ext/app_types' -require 'ddtrace/ext/http' +require 'ddtrace/contrib/patcher' module Datadog module Contrib module GraphQL # Provides instrumentation for `graphql` through the GraphQL tracing framework module Patcher - include Base - register_as :graphql + include Contrib::Patcher - option :tracer, default: Datadog.tracer - option :service_name, default: 'ruby-graphql', depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, 'ruby-graphql', Ext::AppTypes::WEB) - value - end - option :schemas + module_function - class << self - def patch - return patched? if patched? || !compatible? || get_option(:schemas).nil? + def patched? + done?(:graphql) + end - get_option(:schemas).each { |s| patch_schema!(s) } + def patch + return if get_option(:schemas).nil? - @patched = true + do_once(:graphql) do + begin + require 'ddtrace/ext/app_types' + require 'ddtrace/ext/http' + get_option(:schemas).each { |s| patch_schema!(s) } + rescue StandardError => e + Datadog::Tracer.log.error("Unable to apply GraphQL integration: #{e}") + end end + end - def patch_schema!(schema) - tracer = get_option(:tracer) - service_name = get_option(:service_name) - - if schema.respond_to?(:use) - schema.use( + def patch_schema!(schema) + tracer = get_option(:tracer) + service_name = get_option(:service_name) + + if schema.respond_to?(:use) + schema.use( + ::GraphQL::Tracing::DataDogTracing, + tracer: tracer, + service: service_name + ) + else + schema.define do + use( ::GraphQL::Tracing::DataDogTracing, tracer: tracer, service: service_name ) - else - schema.define do - use( - ::GraphQL::Tracing::DataDogTracing, - tracer: tracer, - service: service_name - ) - end end end + end - def patched? - return @patched if defined?(@patched) - @patched = false - end - - private - - def compatible? - defined?(::GraphQL) \ - && defined?(::GraphQL::Tracing::DataDogTracing) \ - && Gem.loaded_specs['graphql'].version >= Gem::Version.new('1.7.9') - end + def get_option(option) + Datadog.configuration[:graphql].get_option(option) end end end diff --git a/spec/ddtrace/contrib/graphql/tracer_spec.rb b/spec/ddtrace/contrib/graphql/tracer_spec.rb index 3df876ccabb..32c2e8af07a 100644 --- a/spec/ddtrace/contrib/graphql/tracer_spec.rb +++ b/spec/ddtrace/contrib/graphql/tracer_spec.rb @@ -3,6 +3,7 @@ require 'ddtrace' RSpec.describe 'GraphQL patcher' do + include ConfigurationHelpers include_context 'GraphQL test schema' # GraphQL generates tons of warnings. @@ -24,7 +25,7 @@ def pop_spans RSpec.shared_examples 'Schema patcher' do before(:each) do - Datadog.configuration[:graphql].instance_variable_get(:@integration).instance_variable_set(:@patched, false) + remove_patch!(:graphql) Datadog.configure do |c| c.use :graphql, service_name: 'graphql-test',