From 7a0ccf04a5ebf43024c2d2b128dcb1c0c59963fc Mon Sep 17 00:00:00 2001 From: "Jason R. Clark" Date: Mon, 3 Feb 2014 13:47:22 -0800 Subject: [PATCH] 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. --- .../agent/configuration/default_source.rb | 23 ++++++++++++ .../action_controller_subscriber.rb | 20 ----------- .../controller_instrumentation.rb | 36 ------------------- .../agent/instrumentation/rubyprof.rb | 26 ++++++++++++++ lib/new_relic/control.rb | 6 ---- lib/new_relic/control/profiling.rb | 29 --------------- lib/new_relic/rack/developer_mode.rb | 9 ++++- test/helpers/file_searching.rb | 2 +- ui/views/newrelic/index.rhtml | 4 +-- 9 files changed, 60 insertions(+), 95 deletions(-) create mode 100644 lib/new_relic/agent/instrumentation/rubyprof.rb delete mode 100644 lib/new_relic/control/profiling.rb diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 3e19d515b9..70f7072e3b 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/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, diff --git a/lib/new_relic/agent/instrumentation/action_controller_subscriber.rb b/lib/new_relic/agent/instrumentation/action_controller_subscriber.rb index fa6040b322..a929c1e620 100644 --- a/lib/new_relic/agent/instrumentation/action_controller_subscriber.rb +++ b/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) diff --git a/lib/new_relic/agent/instrumentation/controller_instrumentation.rb b/lib/new_relic/agent/instrumentation/controller_instrumentation.rb index 08f6dac5e5..914a7acd87 100644 --- a/lib/new_relic/agent/instrumentation/controller_instrumentation.rb +++ b/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: diff --git a/lib/new_relic/agent/instrumentation/rubyprof.rb b/lib/new_relic/agent/instrumentation/rubyprof.rb new file mode 100644 index 0000000000..57f9facad4 --- /dev/null +++ b/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 diff --git a/lib/new_relic/control.rb b/lib/new_relic/control.rb index 891b8b1695..daa5934ace 100644 --- a/lib/new_relic/control.rb +++ b/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 diff --git a/lib/new_relic/control/profiling.rb b/lib/new_relic/control/profiling.rb deleted file mode 100644 index c0966fb066..0000000000 --- a/lib/new_relic/control/profiling.rb +++ /dev/null @@ -1,29 +0,0 @@ -# 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. - -module NewRelic - class Control - module Profiling - - # A flag used in dev mode to indicate if profiling is available - def profiling? - @profiling - end - - def profiling_available? - @profiling_available ||= - begin - require 'ruby-prof' - true - rescue LoadError; end - end - # Set the flag for capturing profiles in dev mode. If RubyProf is not - # loaded a true value is ignored. - def profiling=(val) - @profiling = profiling_available? && val && defined?(RubyProf) - end - end - include Profiling - end -end diff --git a/lib/new_relic/rack/developer_mode.rb b/lib/new_relic/rack/developer_mode.rb index 21a0cbc4c0..3edf2e3b1e 100644 --- a/lib/new_relic/rack/developer_mode.rb +++ b/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 diff --git a/test/helpers/file_searching.rb b/test/helpers/file_searching.rb index 0a0d68c09c..7a97c27f0b 100644 --- a/test/helpers/file_searching.rb +++ b/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 diff --git a/ui/views/newrelic/index.rhtml b/ui/views/newrelic/index.rhtml index 0b12bb9bb2..c1518462d3 100644 --- a/ui/views/newrelic/index.rhtml +++ b/ui/views/newrelic/index.rhtml @@ -35,9 +35,9 @@ <%= link_to_if @samples.size > 0, "Clear Transactions (#{@samples.size})", 'reset' %>
<%= link_to "List Threads", 'threads' %>
- <% if NewRelic::Control.instance.profiling_available? %> + <% if NewRelic::Agent.config[:'profiling.available'] %>

Profiling available: - <% 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' %>