Allow setting the uri and referer manually on the notice_error

lib/new_relic/agent.rb

@@ -285,11 +285,17 @@ def ignore_error_filter(&block)
     # if there is one.
     # 
     # * <tt>exception</tt> is the exception which will be recorded
-    # * <tt>extra_params</tt> is a hash of name value pairs to appear alongside
-    #   the exception in RPM.
+    # Options:
+    # * <tt>:uri</tt> => The request path, minus any request params or query string.
+    # * <tt>:referer</tt> => The URI of the referer
+    # * <tt>:metric</tt> => The metric name associated with the transaction
+    # * <tt>:request_params</tt> => Request parameters, already filtered if necessary
+    # * <tt>:custom_params</tt> => Custom parameters
     #
-    def notice_error(exception, extra_params = nil)
-      NewRelic::Agent::Instrumentation::MetricFrame.notice_error(exception, extra_params)
+    # Anything left over is treated as custom params
+    #
+    def notice_error(exception, options={})
+      NewRelic::Agent::Instrumentation::MetricFrame.notice_error(exception, options)
     end
 
     # Add parameters to the current transaction trace on the call stack.

lib/new_relic/agent/error_collector.rb

@@ -45,11 +45,13 @@ def ignore(errors)
 
     # 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
+    # * <tt>:uri</tt> => The request path, minus any request params or query string.
+    # * <tt>:referer</tt> => The URI of the referer
+    # * <tt>:metric</tt> => The metric name associated with the transaction
+    # * <tt>:request_params</tt> => Request parameters, already filtered if necessary
+    # * <tt>:custom_params</tt> => Custom parameters
+    #
+    # If anything is left over, it's added to custom params
     def notice_error(exception, options={})
       return unless @enabled
       return if @ignore[exception.class.name] 
@@ -58,23 +60,22 @@ def notice_error(exception, options={})
         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
-      
+
       data = {}
-
-      data[:request_params] = normalize_params(options[:request_params]) if NewRelic::Control.instance.capture_params && options[:request_params]
+      data[:request_uri] = options.delete(:uri) || ''
+      data[:request_referer] = options.delete(:referer) || ''
 
-      data[:custom_params] = normalize_params(options[:custom_params]) if options[:custom_params]
-
-      data[:request_uri] = options[:uri] || ''
-      data[:request_referer] = options[:referer] || ''
+      action_path     = options.delete(:metric) || NewRelic::Agent.instance.stats_engine.scope_name || ''
+      request_params = options.delete(:request_params)
+      custom_params = options.delete(:custom_params) || {}
+      # If anything else is left over, treat it like a custom param:
+      custom_params.merge! options
 
+      data[:request_params] = normalize_params(request_params) if NewRelic::Control.instance.capture_params && request_params
+      data[:custom_params] = normalize_params(custom_params) unless custom_params.empty?
       data[:rails_root] = NewRelic::Control.instance.root
-
       data[:file_name] = exception.file_name if exception.respond_to?('file_name')
       data[:line_number] = exception.line_number if exception.respond_to?('line_number')
 

lib/new_relic/agent/instrumentation/metric_frame.rb

@@ -63,22 +63,14 @@ def self.abort_transaction!
 
     # 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?(:path) then @request.path
-        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
+      @uri ||= self.class.uri_from_request(@request) unless @request.nil?
     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.split('?').first
+      @referer ||= self.class.referer_from_request(@request)
     end
-    
+
     # Call this to ensure that the current transaction is not saved
     def abort_transaction!
       NewRelic::Agent.instance.transaction_sampler.ignore_transaction
@@ -130,19 +122,37 @@ def pop
     end
 
     # If we have an active metric frame, notice the error and increment the error metric.
-    def self.notice_error(e, additional_params=nil)
+    # Options:
+    # * <tt>:request</tt> => Request object to get the uri and referer
+    # * <tt>:uri</tt> => The request path, minus any request params or query string.
+    # * <tt>:referer</tt> => The URI of the referer
+    # * <tt>:metric</tt> => The metric name associated with the transaction
+    # * <tt>:request_params</tt> => Request parameters, already filtered if necessary
+    # * <tt>:custom_params</tt> => Custom parameters
+    # Anything left over is treated as custom params
+
+    def self.notice_error(e, options={})
+      if request = options.delete(:request)
+        options[:referer] = referer_from_request(request)
+        options[:uri] = uri_from_request(request)
+      end
       if current
-        current.notice_error(e, additional_params)
+        current.notice_error(e, options)
       else
-        NewRelic::Agent.instance.error_collector.notice_error(e, :custom_params => additional_params)
+        NewRelic::Agent.instance.error_collector.notice_error(e, options)
       end
     end
 
-    def notice_error(e, additional_params=nil)
+    # Do not call this.  Invoke the class method instead.
+    def notice_error(e, options={}) # :nodoc:
       params = custom_parameters
-      params = params.merge(additional_params) if additional_params
+      options[:referer] = referer if referer
+      options[:request_params] = filtered_params if filtered_params 
+      options[:uri] = uri if uri
+      options[:metric] = metric_name
+      options.merge!(custom_parameters)
       if exception != e
-        NewRelic::Agent.instance.error_collector.notice_error(e, :referer => referer, :uri => uri, :metric => metric_name, :request_params => filtered_params, :custom_params => params)
+        NewRelic::Agent.instance.error_collector.notice_error(e, options)
         self.exception = e
       end
     end
@@ -225,6 +235,26 @@ def recording_web_transaction?(cat = category)
       0 == cat.index("Controller")
     end
 
+    # Make a safe attempt to get the referer from a request object, generally successful when
+    # it's a Rack request.
+    def self.referer_from_request(request)
+      if request && request.respond_to?(:referer)
+        request.referer.to_s.split('?').first
+      end
+    end
+
+    # Make a safe attempt to get the URI, without the host and query string.
+    def self.uri_from_request(request)
+      approximate_uri = case
+        when request.respond_to?(:fullpath) then request.fullpath
+        when request.respond_to?(:path) then request.path
+        when request.respond_to?(:request_uri) then request.request_uri
+        when request.respond_to?(:uri) then request.uri
+        when request.respond_to?(:url) then request.url
+      end
+      return approximate_uri[%r{^(https?://.*?)?(/[^?]*)}, 2] || '/' if approximate_uri
+    end 
+
     private
 
     def update_apdex(stat, duration, failed)

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

@@ -1,16 +1,16 @@
 
 ActionController::Base.class_eval do
 
-  # Make a note of an exception associated with the currently executin
+  # Make a note of an exception associated with the currently executing
   # controller action.  Note that this used to be available on Object
   # but we replaced that global method with NewRelic::Agent#notice_error.
   # Use that one outside of controller actions.
   def newrelic_notice_error(exception, custom_params = {})
-    NewRelic::Agent::Instrumentation::MetricFrame.notice_error exception, custom_params
+    NewRelic::Agent::Instrumentation::MetricFrame.notice_error exception, :custom_params => custom_params, :request => request
   end
 
   def rescue_action_with_newrelic_trace(exception)
-    NewRelic::Agent::Instrumentation::MetricFrame.notice_error exception
+    NewRelic::Agent::Instrumentation::MetricFrame.notice_error exception, :request => request 
     rescue_action_without_newrelic_trace exception
   end