From e0dfbe5461be27c803a6ed9837212981e8791496 Mon Sep 17 00:00:00 2001 From: David Elner Date: Mon, 17 Sep 2018 11:12:18 -0400 Subject: [PATCH] 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 fe62034006..69ba78ca8c 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 0000000000..1b77d9454e --- /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 0000000000..579af62781 --- /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 0000000000..a82cec1590 --- /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 4c4c0b022a..aa6620f74d 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 441fc0cb63..e2b6fd14d3 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 06df08f0df..c8b62c129c 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 54da2cc2da..3354ac4ecf 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 9e5ded07da..259b544bc3 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,