Merge pull request #1744 from newrelic/action_controller_subscriber_a…

…dditional_topic

Subscribe to additional Action controller topics

CHANGELOG.md

@@ -10,6 +10,7 @@ The upcoming release of the agent introduces additional Ruby on Rails instrument
 
   The agent now newly supports or has updated support for the following libraries:
 
+  - Action Controller [PR#1744](https://github.com/newrelic/newrelic-ruby-agent/pull/1744/)
   - Action Dispatch (for middleware) [PR#1745](https://github.com/newrelic/newrelic-ruby-agent/pull/1745)
   - Action Mailbox (for sending mail) [PR#1740](https://github.com/newrelic/newrelic-ruby-agent/pull/1740)
   - Action Mailer (for routing mail) [PR#1740](https://github.com/newrelic/newrelic-ruby-agent/pull/1740)
@@ -19,6 +20,7 @@ The upcoming release of the agent introduces additional Ruby on Rails instrument
 
   | Configuration name | Default | Behavior |
   | ----- | ----- | ----- |
+  | `disable_action_controller` | `false` | If `true`, disables Action Controller instrumentation. |
   | `disable_action_dispatch` | `false` | If `true`, disables Action Dispatch instrumentation. |
   | `disable_action_mailbox` | `false` | If `true`, disables Action Mailbox instrumentation. |
   | `disable_action_mailer` | `false` | If `true`, disables Action Mailer instrumentation. |

lib/new_relic/agent/configuration/default_source.rb

@@ -1147,6 +1147,13 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
           :allowed_from_server => false,
           :description => 'If `true`, disables Action Dispatch instrumentation.'
         },
+        :disable_action_controller => {
+          :default => false,
+          :public => true,
+          :type => Boolean,
+          :allowed_from_server => false,
+          :description => 'If `true`, disables Action Controller instrumentation.'
+        },
         :disable_action_mailbox => {
           :default => false,
           :public => true,

lib/new_relic/agent/instrumentation/action_controller_other_subscriber.rb

@@ -0,0 +1,39 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/instrumentation/notifications_subscriber'
+require 'new_relic/agent/instrumentation/ignore_actions'
+require 'new_relic/agent/parameter_filtering'
+
+module NewRelic
+  module Agent
+    module Instrumentation
+      class ActionControllerOtherSubscriber < NotificationsSubscriber
+        def add_segment_params(segment, payload)
+          segment.params[:filter] = payload[:filter] if payload[:filter]
+          segment.params[:keys] = payload[:keys] if payload[:keys]
+          segment.params[:original_path] = payload[:request].original_fullpath if payload[:request]
+
+          if payload[:context]
+            segment.params[:action] = payload[:context][:action]
+            segment.params[:controller] = payload[:context][:controller]
+          end
+        end
+
+        def metric_name(name, payload)
+          controller_name = controller_name_for_metric(payload)
+          "Ruby/ActionController#{"/#{controller_name}" if controller_name}/#{name.gsub(/\.action_controller/, '')}"
+        end
+
+        def controller_name_for_metric(payload)
+          # redirect_to
+          return payload[:request].controller_class.controller_path if payload[:request] && payload[:request].controller_class
+
+          # unpermitted_parameters
+          ::NewRelic::LanguageSupport.constantize(payload[:context][:controller]).controller_path if payload[:context] && payload[:context][:controller]
+        end
+      end
+    end
+  end
+end

lib/new_relic/agent/instrumentation/action_mailbox_subscriber.rb

@@ -8,50 +8,19 @@ module NewRelic
   module Agent
     module Instrumentation
       class ActionMailboxSubscriber < NotificationsSubscriber
-        def start(name, id, payload)
-          return unless state.is_execution_traced?
-
-          start_segment(name, id, payload)
-        rescue => e
-          log_notification_error(e, name, 'start')
-        end
-
-        def finish(name, id, payload)
-          return unless state.is_execution_traced?
-
-          finish_segment(id, payload)
-        rescue => e
-          log_notification_error(e, name, 'finish')
-        end
-
-        def start_segment(name, id, payload)
-          segment = Tracer.start_segment(name: metric_name(name, payload))
-          push_segment(id, segment)
-        end
-
-        def finish_segment(id, payload)
-          if segment = pop_segment(id)
-            if exception = exception_object(payload)
-              segment.notice_error(exception)
-            end
-            segment.finish
-          end
-        end
-
         def metric_name(name, payload)
           mailbox = payload[:mailbox].class.name
           method = method_from_name(name)
           "Ruby/ActionMailbox/#{mailbox}/#{method}"
         end
 
         PATTERN = /\A([^\.]*)\.action_mailbox\z/
-        UNKNOWN = "unknown".freeze
 
         METHOD_NAME_MAPPING = Hash.new do |h, k|
           if PATTERN =~ k
             h[k] = $1
           else
-            h[k] = UNKNOWN
+            h[k] = NewRelic::UNKNOWN
           end
         end
 

lib/new_relic/agent/instrumentation/action_mailer_subscriber.rb

@@ -15,14 +15,13 @@ class ActionMailerSubscriber < NotificationsSubscriber
         BASE_NAME = 'Ruby/ActionMailer'
         PAYLOAD_KEYS = %i[action data key mailer message_id perform_deliveries subject]
         PATTERN = /\A([^\.]+)\.action_mailer\z/
-        UNKNOWN = 'unknown'
         UNKNOWN_MAILER = %r{^#{BASE_NAME}/#{UNKNOWN}/}
 
         METHOD_NAME_MAPPING = Hash.new do |h, k|
           if PATTERN =~ k
             h[k] = $1
           else
-            h[k] = UNKNOWN
+            h[k] = NewRelic::UNKNOWN
           end
         end
 
@@ -54,7 +53,7 @@ def finish_segment(id, payload)
           return unless segment
 
           if segment.name.match?(UNKNOWN_MAILER) && payload.key?(:mailer)
-            segment.name.sub!(UNKNOWN_MAILER, "#{BASE_NAME}/#{payload[:mailer]}/")
+            segment.name = segment.name.sub(UNKNOWN_MAILER, "#{BASE_NAME}/#{payload[:mailer]}/")
           end
 
           PAYLOAD_KEYS.each do |key|

lib/new_relic/agent/instrumentation/action_view_subscriber.rb

@@ -10,28 +10,25 @@ module NewRelic
   module Agent
     module Instrumentation
       class ActionViewSubscriber < NotificationsSubscriber
-        def start(name, id, payload) # THREAD_LOCAL_ACCESS
+        def start_segment(name, id, payload)
           parent = segment_stack[id].last
           metric_name = format_metric_name(name, payload, parent)
 
           event = ActionViewEvent.new(metric_name, payload[:identifier])
-          if state.is_execution_traced? && recordable?(name, metric_name)
+
+          if recordable?(name, metric_name)
             event.finishable = Tracer.start_segment(name: metric_name)
           end
           push_segment(id, event)
-        rescue => e
-          log_notification_error(e, name, 'start')
         end
 
-        def finish(name, id, payload)
+        def finish_segment(id, payload)
           if segment = pop_segment(id)
             if exception = exception_object(payload)
               segment.notice_error(exception)
             end
             segment.finish
           end
-        rescue => e
-          log_notification_error(e, name, 'finish')
         end
 
         def format_metric_name(event_name, payload, parent)
@@ -61,12 +58,15 @@ def recordable?(event_name, metric_name)
         RENDER_TEMPLATE_EVENT_NAME = 'render_template.action_view'.freeze
         RENDER_PARTIAL_EVENT_NAME = 'render_partial.action_view'.freeze
         RENDER_COLLECTION_EVENT_NAME = 'render_collection.action_view'.freeze
+        RENDER_LAYOUT_EVENT_NAME = 'render_layout.action_view'.freeze
 
         def metric_action(name)
           case name
           when /#{RENDER_TEMPLATE_EVENT_NAME}$/o then 'Rendering'
           when RENDER_PARTIAL_EVENT_NAME then 'Partial'
           when RENDER_COLLECTION_EVENT_NAME then 'Partial'
+          when RENDER_LAYOUT_EVENT_NAME then 'Layout'
+          else NewRelic::UNKNOWN
           end
         end
 

lib/new_relic/agent/instrumentation/active_storage_subscriber.rb

@@ -8,36 +8,9 @@ module NewRelic
   module Agent
     module Instrumentation
       class ActiveStorageSubscriber < NotificationsSubscriber
-        def start(name, id, payload)
-          return unless state.is_execution_traced?
-
-          start_segment(name, id, payload)
-        rescue => e
-          log_notification_error(e, name, 'start')
-        end
-
-        def finish(name, id, payload)
-          return unless state.is_execution_traced?
-
-          finish_segment(id, payload)
-        rescue => e
-          log_notification_error(e, name, 'finish')
-        end
-
-        def start_segment(name, id, payload)
-          segment = Tracer.start_segment(name: metric_name(name, payload))
+        def add_segment_params(segment, payload)
           segment.params[:key] = payload[:key]
           segment.params[:exist] = payload[:exist] if payload.key?(:exist)
-          push_segment(id, segment)
-        end
-
-        def finish_segment(id, payload)
-          if segment = pop_segment(id)
-            if exception = exception_object(payload)
-              segment.notice_error(exception)
-            end
-            segment.finish
-          end
         end
 
         def metric_name(name, payload)
@@ -47,13 +20,12 @@ def metric_name(name, payload)
         end
 
         PATTERN = /\Aservice_([^\.]*)\.active_storage\z/
-        UNKNOWN = "unknown".freeze
 
         METHOD_NAME_MAPPING = Hash.new do |h, k|
           if PATTERN =~ k
             h[k] = $1
           else
-            h[k] = UNKNOWN
+            h[k] = NewRelic::UNKNOWN
           end
         end
 

lib/new_relic/agent/instrumentation/active_support_subscriber.rb

@@ -8,29 +8,6 @@ module NewRelic
   module Agent
     module Instrumentation
       class ActiveSupportSubscriber < NotificationsSubscriber
-        def start(name, id, payload)
-          return unless state.is_execution_traced?
-
-          start_segment(name, id, payload)
-        rescue => e
-          log_notification_error(e, name, 'start')
-        end
-
-        def finish(name, id, payload)
-          return unless state.is_execution_traced?
-
-          finish_segment(id, payload)
-        rescue => e
-          log_notification_error(e, name, 'finish')
-        end
-
-        def start_segment(name, id, payload)
-          segment = Tracer.start_segment(name: metric_name(name, payload))
-
-          add_segment_params(segment, payload)
-          push_segment(id, segment)
-        end
-
         def add_segment_params(segment, payload)
           segment.params[:key] = payload[:key]
           segment.params[:store] = payload[:store]
@@ -39,29 +16,19 @@ def add_segment_params(segment, payload)
           segment
         end
 
-        def finish_segment(id, payload)
-          if segment = pop_segment(id)
-            if exception = exception_object(payload)
-              segment.notice_error(exception)
-            end
-            segment.finish
-          end
-        end
-
         def metric_name(name, payload)
           store = payload[:store]
           method = method_from_name(name)
           "Ruby/ActiveSupport#{"/#{store}" if store}/#{method}"
         end
 
         PATTERN = /\Acache_([^\.]*)\.active_support\z/
-        UNKNOWN = "unknown".freeze
 
         METHOD_NAME_MAPPING = Hash.new do |h, k|
           if PATTERN =~ k
             h[k] = $1
           else
-            h[k] = UNKNOWN
+            h[k] = NewRelic::UNKNOWN
           end
         end
 

lib/new_relic/agent/instrumentation/notifications_subscriber.rb

@@ -47,6 +47,47 @@ def self.subscribe(pattern)
           end
         end
 
+        def start(name, id, payload)
+          return unless state.is_execution_traced?
+
+          start_segment(name, id, payload)
+        rescue => e
+          log_notification_error(e, name, 'start')
+        end
+
+        def finish(name, id, payload)
+          return unless state.is_execution_traced?
+
+          finish_segment(id, payload)
+        rescue => e
+          log_notification_error(e, name, 'finish')
+        end
+
+        def start_segment(name, id, payload)
+          segment = Tracer.start_segment(name: metric_name(name, payload))
+          add_segment_params(segment, payload)
+          push_segment(id, segment)
+        end
+
+        def finish_segment(id, payload)
+          if segment = pop_segment(id)
+            if exception = exception_object(payload)
+              segment.notice_error(exception)
+            end
+            segment.finish
+          end
+        end
+
+        # for subclasses
+        def add_segment_params(segment, payload)
+          # no op
+        end
+
+        # for subclasses
+        def metric_name(name, payload)
+          "Ruby/#{name}"
+        end
+
         def log_notification_error(error, name, event_type)
           # These are important enough failures that we want the backtraces
           # logged at error level, hence the explicit log_exception call.

lib/new_relic/agent/instrumentation/rails_notifications/action_controller.rb

@@ -3,6 +3,7 @@
 # frozen_string_literal: true
 
 require 'new_relic/agent/instrumentation/action_controller_subscriber'
+require 'new_relic/agent/instrumentation/action_controller_other_subscriber'
 require 'new_relic/agent/prepend_supportability'
 
 DependencyDetection.defer do
@@ -13,7 +14,8 @@
   end
 
   depends_on do
-    defined?(ActionController) && (defined?(ActionController::Base) || defined?(ActionController::API))
+    !NewRelic::Agent.config[:disable_action_controller] &&
+      defined?(ActionController) && (defined?(ActionController::Base) || defined?(ActionController::API))
   end
 
   executes do
@@ -29,5 +31,15 @@
 
     NewRelic::Agent::Instrumentation::ActionControllerSubscriber \
       .subscribe(/^process_action.action_controller$/)
+
+    subs = %w[send_file
+      send_data
+      redirect_to
+      halted_callback
+      unpermitted_parameters]
+
+    # have to double escape period because its going from string -> regex
+    NewRelic::Agent::Instrumentation::ActionControllerOtherSubscriber \
+      .subscribe(Regexp.new("^(#{subs.join('|')})\\.action_controller$"))
   end
 end