From 23277ebc9f391cb9b39dfc2b2c74cbf19b8475e5 Mon Sep 17 00:00:00 2001 From: Emanuele Palazzetti Date: Mon, 30 Oct 2017 08:55:41 +0100 Subject: [PATCH 1/4] [rails] ActionController: trace directly instead of using Rails instrumentation framework --- .../contrib/rails/action_controller.rb | 14 ++------------ lib/ddtrace/contrib/rails/core_extensions.rb | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/lib/ddtrace/contrib/rails/action_controller.rb b/lib/ddtrace/contrib/rails/action_controller.rb index 8a4fad471d0..e6502c0c9cc 100644 --- a/lib/ddtrace/contrib/rails/action_controller.rb +++ b/lib/ddtrace/contrib/rails/action_controller.rb @@ -9,19 +9,9 @@ module ActionController def self.instrument # patch Rails core components Datadog::RailsActionPatcher.patch_action_controller - - # subscribe when the request processing starts - ::ActiveSupport::Notifications.subscribe('!datadog.start_processing.action_controller') do |*args| - start_processing(*args) - end - - # subscribe when the request processing has been completed - ::ActiveSupport::Notifications.subscribe('!datadog.finish_processing.action_controller') do |*args| - finish_processing(*args) - end end - def self.start_processing(_name, _start, _finish, _id, payload) + def self.start_processing(payload) # trace the execution tracer = ::Rails.configuration.datadog_trace.fetch(:tracer) service = ::Rails.configuration.datadog_trace.fetch(:default_controller_service) @@ -35,7 +25,7 @@ def self.start_processing(_name, _start, _finish, _id, payload) Datadog::Tracer.log.error(e.message) end - def self.finish_processing(_name, start, finish, _id, payload) + def self.finish_processing(payload) # retrieve the tracing context and the latest active span tracing_context = payload.fetch(:tracing_context) span = tracing_context[:dd_request_span] diff --git a/lib/ddtrace/contrib/rails/core_extensions.rb b/lib/ddtrace/contrib/rails/core_extensions.rb index 0fed9bdaee3..85f31302b91 100644 --- a/lib/ddtrace/contrib/rails/core_extensions.rb +++ b/lib/ddtrace/contrib/rails/core_extensions.rb @@ -143,24 +143,25 @@ def process_action_with_datadog(*args) # mutable payload with a tracing context that is used in two different # signals; it propagates the request span so that it can be finished # no matter what - raw_payload = { + payload = { controller: self.class.name, action: action_name, tracing_context: {} } - # emits two different signals that start and finish the trace; this approach - # mimics the original behavior that is available since Rails 3.0: - # - https://github.com/rails/rails/blob/3-0-stable/actionpack/lib/action_controller/metal/instrumentation.rb#L17-L35 - # - https://github.com/rails/rails/blob/5-1-stable/actionpack/lib/action_controller/metal/instrumentation.rb#L17-L39 - ActiveSupport::Notifications.instrument('!datadog.start_processing.action_controller', raw_payload) - - # process the request and finish the trace - ActiveSupport::Notifications.instrument('!datadog.finish_processing.action_controller', raw_payload) do |payload| + begin + # process and catch request exceptions + Datadog::Contrib::Rails::ActionController.start_processing(payload) result = process_action_without_datadog(*args) payload[:status] = response.status result + rescue Exception => e + payload[:exception] = [e.class.name, e.message] + payload[:exception_object] = e + raise e end + ensure + Datadog::Contrib::Rails::ActionController.finish_processing(payload) end alias_method :process_action_without_datadog, :process_action From b2e157e14c64b1ffd8d008ca54f820f97f7bfa72 Mon Sep 17 00:00:00 2001 From: Emanuele Palazzetti Date: Mon, 30 Oct 2017 09:29:47 +0100 Subject: [PATCH 2/4] [rails] ActionView: trace directly instead of using Rails instrumentation framework --- lib/ddtrace/contrib/rails/action_view.rb | 30 ++++---------------- lib/ddtrace/contrib/rails/core_extensions.rb | 21 ++++---------- 2 files changed, 10 insertions(+), 41 deletions(-) diff --git a/lib/ddtrace/contrib/rails/action_view.rb b/lib/ddtrace/contrib/rails/action_view.rb index 06ebb0d85cc..00f1a4103e7 100644 --- a/lib/ddtrace/contrib/rails/action_view.rb +++ b/lib/ddtrace/contrib/rails/action_view.rb @@ -7,30 +7,10 @@ module Rails module ActionView def self.instrument # patch Rails core components - Datadog::RailsRendererPatcher.patch_renderer() - - # subscribe when the template rendering starts - ::ActiveSupport::Notifications.subscribe('!datadog.start_render_template.action_view') do |*args| - start_render_template(*args) - end - - # subscribe when the template rendering has been processed - ::ActiveSupport::Notifications.subscribe('!datadog.finish_render_template.action_view') do |*args| - finish_render_template(*args) - end - - # subscribe when the partial rendering starts - ::ActiveSupport::Notifications.subscribe('!datadog.start_render_partial.action_view') do |*args| - start_render_partial(*args) - end - - # subscribe when the partial rendering has been processed - ::ActiveSupport::Notifications.subscribe('!datadog.finish_render_partial.action_view') do |*args| - finish_render_partial(*args) - end + Datadog::RailsRendererPatcher.patch_renderer end - def self.start_render_template(_name, _start, _finish, _id, payload) + def self.start_render_template(payload) # retrieve the tracing context tracing_context = payload.fetch(:tracing_context) @@ -42,7 +22,7 @@ def self.start_render_template(_name, _start, _finish, _id, payload) Datadog::Tracer.log.debug(e.message) end - def self.finish_render_template(_name, _start, _finish, _id, payload) + def self.finish_render_template(payload) # retrieve the tracing context and the latest active span tracing_context = payload.fetch(:tracing_context) span = tracing_context[:dd_rails_template_span] @@ -64,7 +44,7 @@ def self.finish_render_template(_name, _start, _finish, _id, payload) Datadog::Tracer.log.debug(e.message) end - def self.start_render_partial(_name, _start, _finish, _id, payload) + def self.start_render_partial(payload) # retrieve the tracing context tracing_context = payload.fetch(:tracing_context) @@ -75,7 +55,7 @@ def self.start_render_partial(_name, _start, _finish, _id, payload) Datadog::Tracer.log.debug(e.message) end - def self.finish_render_partial(_name, start, finish, _id, payload) + def self.finish_render_partial(payload) # retrieve the tracing context and the latest active span tracing_context = payload.fetch(:tracing_context) span = tracing_context[:dd_rails_partial_span] diff --git a/lib/ddtrace/contrib/rails/core_extensions.rb b/lib/ddtrace/contrib/rails/core_extensions.rb index 85f31302b91..0e544bf963e 100644 --- a/lib/ddtrace/contrib/rails/core_extensions.rb +++ b/lib/ddtrace/contrib/rails/core_extensions.rb @@ -27,11 +27,9 @@ def render_with_datadog(*args, &block) # context when a partial is rendered @tracing_context ||= {} if @tracing_context.empty? - ::ActiveSupport::Notifications.instrument( - '!datadog.start_render_template.action_view', - tracing_context: @tracing_context - ) + Datadog::Contrib::Rails::ActionView.start_render_template(tracing_context: @tracing_context) end + render_without_datadog(*args) rescue Exception => e # attach the exception to the tracing context if any @@ -39,10 +37,7 @@ def render_with_datadog(*args, &block) raise e ensure # ensure that the template `Span` is finished even during exceptions - ::ActiveSupport::Notifications.instrument( - '!datadog.finish_render_template.action_view', - tracing_context: @tracing_context - ) + Datadog::Contrib::Rails::ActionView.finish_render_template(tracing_context: @tracing_context) end def render_template_with_datadog(*args) @@ -90,10 +85,7 @@ def patch_partial_renderer(klass) def render_with_datadog(*args, &block) # create a tracing context and start the rendering span @tracing_context = {} - ::ActiveSupport::Notifications.instrument( - '!datadog.start_render_partial.action_view', - tracing_context: @tracing_context - ) + Datadog::Contrib::Rails::ActionView.start_render_partial(tracing_context: @tracing_context) render_without_datadog(*args) rescue Exception => e # attach the exception to the tracing context if any @@ -101,10 +93,7 @@ def render_with_datadog(*args, &block) raise e ensure # ensure that the template `Span` is finished even during exceptions - ::ActiveSupport::Notifications.instrument( - '!datadog.finish_render_partial.action_view', - tracing_context: @tracing_context - ) + Datadog::Contrib::Rails::ActionView.finish_render_partial(tracing_context: @tracing_context) end def render_partial_with_datadog(*args) From 5664f87948fcbdf89940a4f6b1a4f6a0555aded0 Mon Sep 17 00:00:00 2001 From: Emanuele Palazzetti Date: Mon, 30 Oct 2017 09:36:22 +0100 Subject: [PATCH 3/4] [rails] ActiveSupport: trace directly instead of using Rails instrumentation framework --- lib/ddtrace/contrib/rails/active_support.rb | 16 ++----- lib/ddtrace/contrib/rails/core_extensions.rb | 48 +++++++++++++++----- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/lib/ddtrace/contrib/rails/active_support.rb b/lib/ddtrace/contrib/rails/active_support.rb index da23bbdafba..341ae92e4b7 100644 --- a/lib/ddtrace/contrib/rails/active_support.rb +++ b/lib/ddtrace/contrib/rails/active_support.rb @@ -8,20 +8,10 @@ module Rails module ActiveSupport def self.instrument # patch Rails core components - Datadog::RailsCachePatcher.patch_cache_store() - - # subscribe when a cache read starts being processed - ::ActiveSupport::Notifications.subscribe('!datadog.start_cache_tracing.active_support') do |*args| - start_trace_cache(*args) - end - - # subscribe when a cache read has been processed - ::ActiveSupport::Notifications.subscribe('!datadog.finish_cache_tracing.active_support') do |*args| - finish_trace_cache(*args) - end + Datadog::RailsCachePatcher.patch_cache_store end - def self.start_trace_cache(_name, _start, _finish, _id, payload) + def self.start_trace_cache(payload) tracer = ::Rails.configuration.datadog_trace.fetch(:tracer) tracing_context = payload.fetch(:tracing_context) @@ -45,7 +35,7 @@ def self.start_trace_cache(_name, _start, _finish, _id, payload) Datadog::Tracer.log.debug(e.message) end - def self.finish_trace_cache(_name, _start, _finish, _id, payload) + def self.finish_trace_cache(payload) # retrieve the tracing context and continue the trace tracing_context = payload.fetch(:tracing_context) span = tracing_context[:dd_cache_span] diff --git a/lib/ddtrace/contrib/rails/core_extensions.rb b/lib/ddtrace/contrib/rails/core_extensions.rb index 0e544bf963e..762e52fafab 100644 --- a/lib/ddtrace/contrib/rails/core_extensions.rb +++ b/lib/ddtrace/contrib/rails/core_extensions.rb @@ -196,11 +196,17 @@ def read(*args, &block) tracing_context: {} } - ActiveSupport::Notifications.instrument('!datadog.start_cache_tracing.active_support', raw_payload) - - ActiveSupport::Notifications.instrument('!datadog.finish_cache_tracing.active_support', raw_payload) do + begin + # process and catch cache exceptions + Datadog::Contrib::Rails::ActiveSupport.start_trace_cache(raw_payload) read_without_datadog(*args, &block) + rescue Exception => e + payload[:exception] = [e.class.name, e.message] + payload[:exception_object] = e + raise e end + ensure + Datadog::Contrib::Rails::ActiveSupport.finish_trace_cache(raw_payload) end end end @@ -215,11 +221,17 @@ def fetch(*args, &block) tracing_context: {} } - ActiveSupport::Notifications.instrument('!datadog.start_cache_tracing.active_support', raw_payload) - - ActiveSupport::Notifications.instrument('!datadog.finish_cache_tracing.active_support', raw_payload) do + begin + # process and catch cache exceptions + Datadog::Contrib::Rails::ActiveSupport.start_trace_cache(raw_payload) fetch_without_datadog(*args, &block) + rescue Exception => e + payload[:exception] = [e.class.name, e.message] + payload[:exception_object] = e + raise e end + ensure + Datadog::Contrib::Rails::ActiveSupport.finish_trace_cache(raw_payload) end end end @@ -234,11 +246,17 @@ def write(*args, &block) tracing_context: {} } - ActiveSupport::Notifications.instrument('!datadog.start_cache_tracing.active_support', raw_payload) - - ActiveSupport::Notifications.instrument('!datadog.finish_cache_tracing.active_support', raw_payload) do + begin + # process and catch cache exceptions + Datadog::Contrib::Rails::ActiveSupport.start_trace_cache(raw_payload) write_without_datadog(*args, &block) + rescue Exception => e + payload[:exception] = [e.class.name, e.message] + payload[:exception_object] = e + raise e end + ensure + Datadog::Contrib::Rails::ActiveSupport.finish_trace_cache(raw_payload) end end end @@ -253,11 +271,17 @@ def delete(*args, &block) tracing_context: {} } - ActiveSupport::Notifications.instrument('!datadog.start_cache_tracing.active_support', raw_payload) - - ActiveSupport::Notifications.instrument('!datadog.finish_cache_tracing.active_support', raw_payload) do + begin + # process and catch cache exceptions + Datadog::Contrib::Rails::ActiveSupport.start_trace_cache(raw_payload) delete_without_datadog(*args, &block) + rescue Exception => e + payload[:exception] = [e.class.name, e.message] + payload[:exception_object] = e + raise e end + ensure + Datadog::Contrib::Rails::ActiveSupport.finish_trace_cache(raw_payload) end end end From ab51dd0e8a9a493b23b41fa01f545fe269009e4d Mon Sep 17 00:00:00 2001 From: Emanuele Palazzetti Date: Mon, 30 Oct 2017 10:06:01 +0100 Subject: [PATCH 4/4] [rails] update rubocop rules --- lib/ddtrace/contrib/rails/core_extensions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ddtrace/contrib/rails/core_extensions.rb b/lib/ddtrace/contrib/rails/core_extensions.rb index 762e52fafab..458f0102b26 100644 --- a/lib/ddtrace/contrib/rails/core_extensions.rb +++ b/lib/ddtrace/contrib/rails/core_extensions.rb @@ -2,7 +2,7 @@ module Datadog # RailsRendererPatcher contains function to patch Rails rendering libraries. # rubocop:disable Lint/RescueException # rubocop:disable Metrics/MethodLength - # rubocop:disable Metrics/BlockLength + # rubocop:disable Metrics/ModuleLength module RailsRendererPatcher module_function