RUBY-1267 RubyProf based on transaction

This replaces the old ControllerInstrumentation based method that we were using
to divert to a RubyProf based profiling of the method instead of tracing.

By hooking into our transaction events, this should work across Rails versions
seamlessly without duplicating work for Rails 4.x.

This also removed the old fronting of these variables via NewRelic::Control,
which is nice to tidy up.

lib/new_relic/agent/configuration/default_source.rb

@@ -147,6 +147,17 @@ def self.developer_mode
           Proc.new { NewRelic::Agent.config[:developer] }
         end
 
+        def self.profiling_available
+          Proc.new {
+            begin
+              require 'ruby-prof'
+              true
+            rescue LoadError
+              false
+            end
+          }
+        end
+
         def self.monitor_mode
           Proc.new { NewRelic::Agent.config[:enabled] }
         end
@@ -283,6 +294,18 @@ def self.monitor_mode
           :type => Boolean,
           :description => 'Alternative method of enabling developer_mode.'
         },
+        :'profiling.available' => {
+          :default => DefaultSource.profiling_available,
+          :public => false,
+          :type => Boolean,
+          :description => 'Determines if ruby-prof is available for developer mode profiling.'
+        },
+        :'profiling.enabled' => {
+          :default => false,
+          :public => false,
+          :type => Boolean,
+          :description => 'Determines at runtime whether developer mode should be profiling or not.'
+        },
         :apdex_t => {
           :default => 0.5,
           :public => true,

lib/new_relic/agent/instrumentation/action_controller_subscriber.rb

@@ -9,11 +9,6 @@ module Instrumentation
       class ActionControllerSubscriber < EventedSubscriber
         def initialize
           super
-          initialize_request_reset
-          initialize_profiling
-        end
-
-        def initialize_request_reset
           NewRelic::Agent.instance.events.subscribe(:before_call) do |env|
 
             request = begin
@@ -27,21 +22,6 @@ def initialize_request_reset
           end
         end
 
-        def initialize_profiling
-          return unless defined?(::RubyProf)
-
-          NewRelic::Agent.instance.events.subscribe(:start_transaction) do
-            ::RubyProf.start if NewRelic::Control.instance.profiling?
-          end
-
-          NewRelic::Agent.instance.events.subscribe(:transaction_finishing) do
-            if NewRelic::Control.instance.profiling?
-              profile = ::RubyProf.stop
-              NewRelic::Agent.instance.transaction_sampler.notice_profile(profile)
-            end
-          end
-        end
-
         def start(name, id, payload)
           request = TransactionState.get.request
           event = ControllerEvent.new(name, Time.now, nil, id, payload, request)

lib/new_relic/agent/instrumentation/controller_instrumentation.rb

@@ -322,9 +322,6 @@ def perform_action_with_newrelic_trace(*args, &block)
             end
           end
 
-          control = NewRelic::Control.instance
-          return perform_action_with_newrelic_profile(args, &block) if control.profiling?
-
           txn = _start_transaction(block_given? ? args : [])
           begin
             options = { :force => txn.force_flag, :transaction => true }
@@ -413,39 +410,6 @@ def ignore_enduser?
 
         private
 
-        # Profile the instrumented call.  Dev mode only.  Experimental
-        # - should definitely not be used on production applications
-        def perform_action_with_newrelic_profile(args)
-          txn = _start_transaction(block_given? ? args : [])
-          val = nil
-          options = { :metric => true }
-
-          _, expected_scope = NewRelic::Agent::MethodTracer::TraceExecutionScoped.trace_execution_scoped_header(options, txn.start_time.to_f)
-
-          NewRelic::Agent.disable_all_tracing do
-            # turn on profiling
-            profile = RubyProf.profile do
-              if block_given?
-                val = yield
-              else
-                val = perform_action_without_newrelic_trace(*args)
-              end
-            end
-            NewRelic::Agent.instance.transaction_sampler.notice_profile profile
-          end
-
-          return val
-        ensure
-          end_time = Time.now
-          txn.freeze_name
-          metric_names = Array(recorded_metrics(txn))
-          txn_name = metric_names.shift
-
-          NewRelic::Agent::MethodTracer::TraceExecutionScoped.trace_execution_scoped_footer(txn.start_time.to_f, txn_name, metric_names, expected_scope, options, end_time.to_f)
-
-          txn = Transaction.stop(txn_name, end_time)
-        end
-
         # Write a transaction onto a thread local if there isn't already one there.
         # If there is one, just update it.
         def _start_transaction(args) # :nodoc:

lib/new_relic/agent/instrumentation/rubyprof.rb

@@ -0,0 +1,26 @@
+# encoding: utf-8
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/rpm/blob/master/LICENSE for complete details.
+
+DependencyDetection.defer do
+  named :rubyprof
+
+  depends_on do
+    defined?(::RubyProf)
+  end
+
+  executes do
+    NewRelic::Agent.instance.events.subscribe(:start_transaction) do
+      if NewRelic::Agent.config[:'profiling.enabled']
+        ::RubyProf.start
+      end
+    end
+
+    NewRelic::Agent.instance.events.subscribe(:transaction_finishing) do
+      if NewRelic::Agent.config[:'profiling.enabled']
+        profile = ::RubyProf.stop
+        NewRelic::Agent.instance.transaction_sampler.notice_profile(profile)
+      end
+    end
+  end
+end

lib/new_relic/control.rb

@@ -16,7 +16,6 @@
 require 'net/https'
 require 'logger'
 require 'new_relic/control/frameworks'
-require 'new_relic/control/profiling'
 require 'new_relic/control/server_methods'
 require 'new_relic/control/instrumentation'
 require 'new_relic/control/class_methods'
@@ -25,7 +24,6 @@
 require 'new_relic/agent'
 require 'new_relic/delayed_job_injection'
 
-
 module NewRelic
 
   # The Control is a singleton responsible for the startup and
@@ -39,12 +37,8 @@ class Control
     # done in a subfile for load order purposes
     # extend ClassMethods
     # include InstanceMethods
-    # include Profiling
     # include Configuration
     # include ServerMethods
     # include Instrumentation
-
-
-
   end
 end

lib/new_relic/rack/developer_mode.rb

@@ -121,8 +121,15 @@ def explain_sql
         render(:explain_sql)
       end
 
+      PROFILE_CONFIG = { :'profiling.enabled' => true }
+
       def profile
-        NewRelic::Control.instance.profiling = params['start'] == 'true'
+        if params['start'] == 'true'
+          NewRelic::Agent.config.apply_config(PROFILE_CONFIG) unless NewRelic::Agent.config[:'profiling.enabled']
+        else
+          NewRelic::Agent.config.remove_config(PROFILE_CONFIG) if NewRelic::Agent.config[:'profiling.enabled']
+        end
+
         index
       end
 

test/helpers/file_searching.rb

@@ -6,7 +6,7 @@ module NewRelic
   module TestHelpers
     module FileSearching
       def all_rb_files
-        pattern = File.expand_path(gem_root + "/**/*.rb")
+        pattern = File.expand_path(gem_root + "/**/*.{rb,rhtml}")
         Dir[pattern]
       end
 

ui/views/newrelic/index.rhtml

@@ -35,9 +35,9 @@
 
         <%= link_to_if @samples.size > 0, "Clear Transactions (#{@samples.size})", 'reset' %><br/>
         <%= link_to "List Threads", 'threads' %><br/>
-		<% if NewRelic::Control.instance.profiling_available? %>
+		<% if NewRelic::Agent.config[:'profiling.available'] %>
 		  <p><b>Profiling available:</b>
-		  <% if NewRelic::Control.instance.profiling? %>
+		  <% if NewRelic::Agent.config[:'profiling.enabled'] %>
             <%= link_to "Stop Profiling", 'profile?stop=true'%>
 	      <% else %>
             <%= link_to "Start Profiling", 'profile?start=true' %>