diff --git a/lib/new_relic/agent.rb b/lib/new_relic/agent.rb index deafa8350b..944e6e6c78 100644 --- a/lib/new_relic/agent.rb +++ b/lib/new_relic/agent.rb @@ -285,11 +285,17 @@ def ignore_error_filter(&block) # if there is one. # # * exception is the exception which will be recorded - # * extra_params is a hash of name value pairs to appear alongside - # the exception in RPM. + # 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, 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. diff --git a/lib/new_relic/agent/error_collector.rb b/lib/new_relic/agent/error_collector.rb index 32ed398398..6023c168d5 100644 --- a/lib/new_relic/agent/error_collector.rb +++ b/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 + # * :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 + # + # 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') diff --git a/lib/new_relic/agent/instrumentation/metric_frame.rb b/lib/new_relic/agent/instrumentation/metric_frame.rb index 2da8adc1f9..53db57db69 100644 --- a/lib/new_relic/agent/instrumentation/metric_frame.rb +++ b/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: + # * :request => Request object to get the uri and referer + # * :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 + # 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) diff --git a/lib/new_relic/agent/instrumentation/rails/errors.rb b/lib/new_relic/agent/instrumentation/rails/errors.rb index 7229d95a4b..4cf807687a 100644 --- a/lib/new_relic/agent/instrumentation/rails/errors.rb +++ b/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