Merged changes from trunk:

* Add disable_dj flag in newrelic.
* Ensure the DJ sampler does not start outside of DJ processes
* Sample memory every minute instead of polling on the sampler thread
* Sample the mongrel queue on the controller instrumentation instead of a sampler
* Fix missing URI in errors and transaction traces

CHANGELOG

@@ -1,4 +1,5 @@
 v2.10.3
+  * optimization to reduce overhead: move background samplers into foreground thread
   * change default config file to ignore RoutingErrors
   * moved the background task instrumentation into a separate tab in the RPM UI
   * allow override of the RPM application name via NEWRELIC_APP_NAME environment variable
@@ -9,11 +10,16 @@ v2.10.3
   * fix problem with apdex recording
   * switch to jeweler for gem building
   * minor fixes, test improvements, doc and rakefile improvements
+  * fix incompatibility with Hoptoad where Hoptoad was not getting errors handled by New Relic
+  * many other bug fixes and documentation improvements
+
 v2.10.2.1.
   * fix bugs with Sinatra app instrumentation 
   * minor doc updates
+
 v2.10.2.0.
   * beta release of 2.10
+
 v2.10.1.
   * rack support, including metal; ignores 404s; requires a module inclusion (see docs)
   * sinatra support, displays actions named by the URI pattern matched
@@ -47,6 +53,7 @@ v2.10.1.
   * added # of GC calls to GC instrumentation
   * renamed the dispatcher metric
   * refactored stats_engine code for readability
+  * optimization: reduce wakeup times for harvest thread
 
 v2.10.0.
   * support unicorn

lib/new_relic/agent.rb

@@ -103,7 +103,6 @@ module Agent
 
     require 'new_relic/agent/samplers/cpu_sampler'
     require 'new_relic/agent/samplers/memory_sampler'
-    require 'new_relic/agent/samplers/mongrel_sampler'
     require 'new_relic/agent/samplers/object_sampler'
     require 'new_relic/agent/samplers/delayed_job_lock_sampler'
     require 'set'
@@ -289,7 +288,7 @@ def ignore_error_filter(&block)
     # * <tt>extra_params</tt> is a hash of name value pairs to appear alongside
     #   the exception in RPM.
     #
-    def notice_error(exception, extra_params = {})
+    def notice_error(exception, extra_params = nil)
       NewRelic::Agent::Instrumentation::MetricFrame.notice_error(exception, extra_params)
     end
 

lib/new_relic/agent/agent.rb

@@ -278,8 +278,7 @@ def launch_worker_thread
           control.log! "Unable to establish connection with the server.  Run with log level set to debug for more information."
         rescue Exception => e
           @connected = false
-          control.log! e, :error
-          control.log! e.backtrace.join("\n  "), :error
+          log.error "Terminating worker loop: #{e.class.name}: #{e}\n  #{e.backtrace.join("\n  ")}"
         end
       end
       @worker_thread['newrelic_label'] = 'Worker Loop'

lib/new_relic/agent/error_collector.rb

@@ -40,32 +40,38 @@ def ignore_error_filter(&block)
     # errors is an array of Exception Class Names
     #
     def ignore(errors)
-      errors.each { |error| @ignore[error] = true; log.debug("Ignoring error: '#{error}'") }
+      errors.each { |error| @ignore[error] = true; log.debug("Ignoring errors of type '#{error}'") }
     end
 
-    def notice_error(exception, request=nil, action_path=nil, filtered_params={})
+    # Notice the error with the given available options:
+    #
+    # * :uri => The request path, minus any request params or query string.
+    # * :referer => The URI of the referer
+    # * :metric => The metric name associated with the transaction
+    # * :request_params => Request parameters, already filtered if necessary
+    # * :custom_params => Custom parameters
+    def notice_error(exception, options={})
       return unless @enabled
       return if @ignore[exception.class.name] 
       if @ignore_filter
         exception = @ignore_filter.call(exception)
         return if exception.nil?
       end
+
+      action_path     = options[:metric] || NewRelic::Agent.instance.stats_engine.scope_name || ''
+      filtered_params = options[:request_params] || {}
+
       NewRelic::Agent.get_stats("Errors/all").increment_count
       return unless NewRelic::Agent.instance.should_send_errors
 
-      action_path ||= NewRelic::Agent.instance.stats_engine.scope_name || ''
-
       data = {}
 
-      data[:request_params] = normalize_params(filtered_params) if NewRelic::Control.instance.capture_params
+      data[:request_params] = normalize_params(options[:request_params]) if NewRelic::Control.instance.capture_params && options[:request_params]
 
-      data[:custom_params] = normalize_params(NewRelic::Agent::Instrumentation::MetricFrame.custom_parameters) 
-
-      data[:request_uri] = request.path if request
-      data[:request_uri] ||= ""
+      data[:custom_params] = normalize_params(options[:custom_params]) if options[:custom_params]
 
-      data[:request_referer] = request.referer if request
-      data[:request_referer] ||= ""
+      data[:request_uri] = options[:uri] || ''
+      data[:request_referer] = options[:referer] || ''
 
       data[:rails_root] = NewRelic::Control.instance.root
 
@@ -87,8 +93,8 @@ def notice_error(exception, request=nil, action_path=nil, filtered_params={})
       noticed_error = NewRelic::NoticedError.new(action_path, data, exception)
 
       @lock.synchronize do
-        if @errors.length >= MAX_ERROR_QUEUE_LENGTH
-          log.warn("The error reporting queue has reached #{MAX_ERROR_QUEUE_LENGTH}. This error will not be reported to RPM: #{exception.message}")
+        if @errors.length == MAX_ERROR_QUEUE_LENGTH
+          log.warn("The error reporting queue has reached #{MAX_ERROR_QUEUE_LENGTH}. The error detail for this and subsequent errors will not be transmitted to RPM until the queued errors have been sent: #{exception.message}")
         else
           @errors << noticed_error
         end

lib/new_relic/agent/instrumentation/controller_instrumentation.rb

@@ -300,20 +300,23 @@ def _push_metric_frame(args) # :nodoc:
       frame_data = NewRelic::Agent::Instrumentation::MetricFrame.current(true)
 
       frame_data.apdex_start ||= _detect_upstream_wait(frame_data.start)
-
+      _record_queue_length 
       # If a block was passed in, then the arguments represent options for the instrumentation,
       # not app method arguments.
       if args.any?
-        frame_data.force_flag = args.last.is_a?(Hash) && args.last[:force]
+        if options = args.last.is_a?(Hash) && args.last
+          frame_data.force_flag = options[:force] 
+          frame_data.request = options[:request] if options[:request]
+        end
         category, path, available_params = _convert_args_to_path(args)
       else
         category = 'Controller'
         path = newrelic_metric_path
-        available_params = self.respond_to?(:params) ? self.params : {} 
+        available_params = self.respond_to?(:params) ? self.params : {}
+        frame_data.request = self.request if self.respond_to? :request
       end
       frame_data.push(category, path)
       frame_data.filtered_params = (respond_to? :filter_parameters) ? filter_parameters(available_params) : available_params
-      frame_data.available_request ||= (respond_to? :request) ? request : nil
       frame_data
     end
 
@@ -351,13 +354,25 @@ def _is_filtered?(key)
         true
       end
     end
+    # Take a guess at a measure representing the number of requests waiting in mongrel
+    # or heroku.
+    def _record_queue_length
+      if newrelic_request_headers
+        if queue_depth = newrelic_request_headers['HTTP_X_HEROKU_QUEUE_DEPTH']
+          queue_depth = queue_depth.to_i rescue nil
+        elsif mongrel = NewRelic::Control.instance.local_env.mongrel
+          # Always subtrace 1 for the active mongrel
+          queue_depth = [mongrel.workers.list.length.to_i - 1, 0].max rescue nil
+        end
+        NewRelic::Agent.agent.stats_engine.get_stats_no_scope('Mongrel/Queue Length').trace_call(queue_depth) if queue_depth
+      end
+    end
 
     def _detect_upstream_wait(now)
       if newrelic_request_headers
         if entry_time = newrelic_request_headers['HTTP_X_REQUEST_START']
-          if queue_depth = newrelic_request_headers['HTTP_X_HEROKU_QUEUE_DEPTH']
+          if newrelic_request_headers['HTTP_X_HEROKU_QUEUE_DEPTH'] # this is a heroku measure
             http_entry_time = entry_time.to_f / 1e3
-            _record_heroku_queue_depth(queue_depth)
           else # apache / nginx
             apache_parsed_time = entry_time[/t=(\d+)/, 1]
             http_entry_time = apache_parsed_time.to_f/1e6 if apache_parsed_time
@@ -374,11 +389,6 @@ def _detect_upstream_wait(now)
       http_entry_time || now
     end
 
-    def _record_heroku_queue_depth(header)
-      length_stat = NewRelic::Agent.agent.stats_engine.get_stats_no_scope('Mongrel/Queue Length')
-      length_stat.trace_call(header.to_i)
-    end
-
     def _dispatch_stat
       NewRelic::Agent.agent.stats_engine.get_stats_no_scope 'HttpDispatcher'  
     end

lib/new_relic/agent/instrumentation/delayed_job_instrumentation.rb

@@ -1,6 +1,6 @@
 require 'new_relic/agent/instrumentation/controller_instrumentation'
 
-if defined?(Delayed::Job)
+if defined?(Delayed::Job) and not NewRelic::Control.instance['disable_dj']
   module NewRelic
     module Agent
       module Instrumentation

lib/new_relic/agent/instrumentation/merb/errors.rb

@@ -1,7 +1,8 @@
 # Hook in the notification to merb
 error_notifier = Proc.new {
   if request.exceptions #check that there's actually an exception
-    NewRelic::Agent.agent.error_collector.notice_error(request.exceptions.first, request, "#{params[:controller]}/#{params[:action]}", params)
+    # Note, this assumes we have already captured the other information such as uri and params in the MetricFrame.
+    NewRelic::Agent::Instrumentation::MetricFrame.notice_error(request.exceptions.first)
   end
 }
 Merb::Dispatcher::DefaultException.before error_notifier

lib/new_relic/agent/instrumentation/metric_frame.rb

@@ -7,9 +7,9 @@
 module NewRelic::Agent::Instrumentation
   class MetricFrame 
     attr_accessor :start, :apdex_start, :exception, 
-                :filtered_params, :available_request, :force_flag, 
+                :filtered_params, :force_flag, 
                 :jruby_cpu_start, :process_cpu_start, :database_metric_name
-
+                
     # Return the currently active metric frame, or nil.  Call with +true+
     # to create a new metric frame if one is not already on the thread.
     def self.current(create_if_empty=nil)
@@ -55,6 +55,31 @@ def push(category, path)
     def self.abort_transaction!
       current.abort_transaction! if current
     end
+
+    # Give the current metric frame a request context.  Use this to 
+    # get the URI and referer.  The request is interpreted loosely
+    # as a Rack::Request or an ActionController::AbstractRequest.
+    def request=(request)
+      @request = request
+    end
+
+    # For the current web transaction, return the path of the URI minus the host part and query string, or nil.
+    def uri
+      return @uri if @uri || @request.nil?
+      approximate_uri = case
+        when @request.respond_to?(:fullpath) then @request.fullpath
+        when @request.respond_to?(:uri) then @request.uri
+        when @request.respond_to?(:url) then @request.url
+      end
+      @uri = approximate_uri[%r{^(https?://.*?)?(/[^?]*)}, 2] || '/' if approximate_uri
+    end
+
+    # For the current web transaction, return the full referer, minus the host string, or nil.
+    def referer
+      return @referer if @referer || @request.nil? || !@request.respond_to?(:referer) || !@request.referer 
+      @referer = @request.referer[%r{^[^?]+}] 
+    end
+
 
     # Call this to ensure that the current transaction is not saved
     def abort_transaction!
@@ -66,7 +91,7 @@ def start_transaction
       NewRelic::Agent.instance.stats_engine.start_transaction metric_name
       # Only push the transaction context info once, on entry:
       if @path_stack.size == 1
-        NewRelic::Agent.instance.transaction_sampler.notice_transaction(metric_name, available_request, filtered_params)
+        NewRelic::Agent.instance.transaction_sampler.notice_transaction(metric_name, uri, filtered_params)
       end
     end
 
@@ -107,17 +132,17 @@ def pop
     end
 
     # If we have an active metric frame, notice the error and increment the error metric.
-    def self.notice_error(e, custom_params={})
+    def self.notice_error(e, custom_params=nil)
       if current
         current.notice_error(e, custom_params)
       else
-        NewRelic::Agent.instance.error_collector.notice_error(e, nil, nil, custom_params)
+        NewRelic::Agent.instance.error_collector.notice_error(e, :custom_params => custom_params)
       end
     end
 
-    def notice_error(e, custom_params={})
+    def notice_error(e, custom_params=nil)
       if exception != e
-        NewRelic::Agent.instance.error_collector.notice_error(e, nil, metric_name, filtered_params.merge(custom_params))
+        NewRelic::Agent.instance.error_collector.notice_error(e, :referer => referer, :uri => uri, :metric => metric_name, :request_params => filtered_params, :custom_params => custom_params)
         self.exception = e
       end
     end

lib/new_relic/agent/instrumentation/net.rb

@@ -1,18 +1,14 @@
 if defined? Net::HTTP
   Net::HTTP.class_eval do
     def request_with_newrelic_trace(*args, &block)
+      metrics = ["External/#{@address}/Net::HTTP/#{args[0].method}","External/#{@address}/all"]
       if NewRelic::Agent::Instrumentation::MetricFrame.recording_web_transaction?
-        self.class.trace_execution_scoped(["External/#{@address}/Net::HTTP/#{args[0].method}",
-                                           "External/#{@address}/all",
-                                           "External/allWeb"]) do
-          request_without_newrelic_trace(*args, &block)
-        end
+        metrics << "External/allWeb"
       else
-        self.class.trace_execution_scoped(["External/#{@address}/Net::HTTP/#{args[0].method}", 
-                                             "External/#{@address}/all",
-                                             "External/allOther"]) do
-          request_without_newrelic_trace(*args, &block)
-        end
+        metrics << "External/allOther"
+      end
+      self.class.trace_execution_scoped metrics do
+        request_without_newrelic_trace(*args, &block)
       end
     end
     alias request_without_newrelic_trace request

lib/new_relic/agent/instrumentation/rack.rb

@@ -52,9 +52,9 @@ module Instrumentation
       #
       # == Cascading or chained calls
       #
-      # You should avoid instrumenting nested calls.  So if you are using Rack::Cascade
-      # to fall through the action, or you are chaining through to the next middleware 
-      # which will have it's own controller instrumentation, then you will want to
+      # Calls which return a 404 will not have transactions recorded, but 
+      # any calls to instrumented frameworks like ActiveRecord will still be
+      # captured even if the result is a 404.  To avoid this you need to
       # instrument only when you are the endpoint.
       #
       # In these cases, you should not include or extend the Rack module but instead
@@ -77,11 +77,11 @@ module Instrumentation
       #      
       module Rack
         def newrelic_request_headers
-          @newrelic_env
+          @newrelic_request.env
         end
         def call_with_newrelic(*args)
-          @newrelic_env = args.first
-          perform_action_with_newrelic_trace(:category => :rack, :params => {:uri => @newrelic_env['REQUEST_PATH']}) do
+          @newrelic_request = ::Rack::Request.new(args.first)
+          perform_action_with_newrelic_trace(:category => :rack, :request => @newrelic_request) do
             result = call_without_newrelic(*args)
             # Ignore cascaded calls
             MetricFrame.abort_transaction! if result.first == 404

lib/new_relic/agent/samplers/delayed_job_lock_sampler.rb

@@ -4,6 +4,8 @@ module Samplers
   class DelayedJobLockSampler < NewRelic::Agent::Sampler
     def initialize
       super :delayed_job_lock
+      raise Unsupported, "DJ instrumentation disabled" if NewRelic::Control.instance['disable_dj'] 
+      raise Unsupported, "No DJ worker present" if NewRelic::DelayedJobInjection.worker_name
     end
 
     def stats
@@ -19,7 +21,7 @@ def worker_name
     end
 
     def locked_jobs
-      Delayed::Job.count(:conditions => {:locked_by => worker_name})
+      Delayed::Job.count(:conditions => {:locked_by => NewRelic::DelayedJobInjection.worker_name})
     end
 
     def self.supported_on_this_platform?

lib/new_relic/agent/samplers/memory_sampler.rb

@@ -31,14 +31,11 @@ def initialize
       raise Unsupported, "Unsupported platform for getting memory: #{platform}" if @sampler.nil?
       raise Unsupported, "Unable to run #{@sampler}" unless @sampler.can_run?
     end
+
     def self.supported_on_this_platform?
       defined?(JRuby) or platform =~ /linux|darwin9|darwin10|freebsd|solaris/
     end
 
-    def self.use_harvest_sampler?
-      false
-    end
-
     def self.platform
       if RUBY_PLATFORM =~ /java/
         %x[uname -s].downcase