Skip to content

Commit

Permalink
Merge pull request #704 from newrelic/expected_errors
Browse files Browse the repository at this point in the history
Implement Expected/Ignored errors
  • Loading branch information
tannalynn authored Jun 24, 2021
2 parents 19b3eac + e93daff commit 98fb917
Show file tree
Hide file tree
Showing 9 changed files with 464 additions and 52 deletions.
1 change: 1 addition & 0 deletions lib/new_relic/agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module Agent
require 'new_relic/agent/sql_sampler'
require 'new_relic/agent/commands/thread_profiler_session'
require 'new_relic/agent/error_collector'
require 'new_relic/agent/error_filter'
require 'new_relic/agent/sampler'
require 'new_relic/agent/database'
require 'new_relic/agent/database_adapter'
Expand Down
51 changes: 50 additions & 1 deletion lib/new_relic/agent/configuration/default_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1297,7 +1297,56 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
:public => true,
:type => String,
:allowed_from_server => true,
:description => 'Specify a comma-delimited list of error classes that the agent should ignore.'
:dynamic_name => true,
:description => 'DEPRECATED; use `error_collector.ignore_classes` instead. Specify a comma-delimited list of error classes that the agent should ignore.'
},
:'error_collector.ignore_classes' => {
:default => [],
:public => true,
:type => Array,
:allowed_from_server => true,
:dynamic_name => true,
:description => 'A list of error classes that the agent should ignore. *Note: this setting cannot be set via environment variable.*'
},
:'error_collector.ignore_messages' => {
:default => {},
:public => true,
:type => Hash,
:allowed_from_server => true,
:dynamic_name => true,
:description => 'A map of error classes to a list of messages. When an error of one of the classes specified here occurs, if its error message contains one of the strings corresponding to it here, that error will be ignored. *Note: this setting cannot be set via environment variable.*'
},
:'error_collector.ignore_status_codes' => {
:default => '',
:public => true,
:type => String,
:allowed_from_server => true,
:dynamic_name => true,
:description => 'A comma separated list of status codes, possibly including ranges. Errors associated with these status codes, where applicable, will be ignored.'
},
:'error_collector.expected_classes' => {
:default => [],
:public => true,
:type => Array,
:allowed_from_server => true,
:dynamic_name => true,
:description => 'A list of error classes that the agent should treat as expected. *Note: this setting cannot be set via environment variable.*'
},
:'error_collector.expected_messages' => {
:default => {},
:public => true,
:type => Hash,
:allowed_from_server => true,
:dynamic_name => true,
:description => 'A map of error classes to a list of messages. When an error of one of the classes specified here occurs, if its error message contains one of the strings corresponding to it here, that error will be treated as expected. *Note: this setting cannot be set via environment variable.*'
},
:'error_collector.expected_status_codes' => {
:default => '',
:public => true,
:type => String,
:allowed_from_server => true,
:dynamic_name => true,
:description => 'A comma separated list of status codes, possibly including ranges. Errors associated with these status codes, where applicable, will be treated as expected.'
},
:'error_collector.max_backtrace_frames' => {
:default => 50,
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/configuration/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def replace_or_add_config(source)
was_finished = finished_configuring?

invoke_callbacks(:add, source)

case source
when SecurityPolicySource then @security_policy_source = source
when HighSecuritySource then @high_security_source = source
Expand Down Expand Up @@ -159,7 +160,6 @@ def register_callback(key, &proc)
def invoke_callbacks(direction, source)
return unless source
source.keys.each do |key|

if @cache[key] != source[key]
@callbacks[key].each do |proc|
if direction == :add
Expand Down
79 changes: 43 additions & 36 deletions lib/new_relic/agent/error_collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,18 @@ def initialize events
@error_trace_aggregator = ErrorTraceAggregator.new(MAX_ERROR_QUEUE_LENGTH)
@error_event_aggregator = ErrorEventAggregator.new events

# lookup of exception class names to ignore. Hash for fast access
@ignore = {}

initialize_ignored_errors(Agent.config[:'error_collector.ignore_errors'])

Agent.config.register_callback(:'error_collector.ignore_errors') do |ignore_errors|
initialize_ignored_errors(ignore_errors)
@error_filter = NewRelic::Agent::ErrorFilter.new

%w(
ignore_errors ignore_classes ignore_messages ignore_status_codes
expected_classes expected_messages expected_status_codes
).each do |w|
Agent.config.register_callback(:"error_collector.#{w}") do |value|
@error_filter.load_from_config(w, value)
end
end
end

def initialize_ignored_errors(ignore_errors)
@ignore.clear
ignore_errors = ignore_errors.split(",") if ignore_errors.is_a? String
ignore_errors.each { |error| error.strip! }
ignore(ignore_errors)
end

def enabled?
error_trace_aggregator.enabled? || error_event_aggregator.enabled?
end
Expand Down Expand Up @@ -76,30 +71,31 @@ def self.ignore_error_filter
defined?(@ignore_filter) ? @ignore_filter : nil
end

# errors is an array of Exception Class Names
#
def ignore(errors)
errors.each do |error|
@ignore[error] = true
::NewRelic::Agent.logger.debug("Ignoring errors of type '#{error}'")
end
@error_filter.ignore(errors)
end

def ignore?(ex, status_code = nil)
@error_filter.ignore?(ex, status_code)
end

def expect(errors)
@error_filter.expect(errors)
end

def expected?(ex, status_code = nil)
@error_filter.expected?(ex, status_code)
end

# Checks the provided error against the error filter, if there
# is an error filter
def filtered_by_error_filter?(error)
def ignored_by_filter_proc?(error)
respond_to?(:ignore_filter_proc) && !ignore_filter_proc(error)
end

# Checks the array of error names and the error filter against
# the provided error
def filtered_error?(error)
@ignore[error.class.name] || filtered_by_error_filter?(error)
end

# an error is ignored if it is nil or if it is filtered
def error_is_ignored?(error)
error && filtered_error?(error)
def error_is_ignored?(error, status_code = nil)
error && (@error_filter.ignore?(error, status_code) || ignored_by_filter_proc?(error))
rescue => e
NewRelic::Agent.logger.error("Error '#{error}' will NOT be ignored. Exception '#{e}' while determining whether to ignore or not.", e)
false
Expand Down Expand Up @@ -174,9 +170,16 @@ def increment_error_count!(state, exception, options={})
end
end

def skip_notice_error?(exception)
def increment_expected_error_count!(state, exception)
stats_engine = NewRelic::Agent.agent.stats_engine
stats_engine.record_unscoped_metrics(state, ['ErrorsExpected/all']) do |stats|
stats.increment_count
end
end

def skip_notice_error?(exception, status_code = nil)
disabled? ||
error_is_ignored?(exception) ||
error_is_ignored?(exception, status_code) ||
exception.nil? ||
exception_tagged_with?(EXCEPTION_TAG_IVAR, exception)
end
Expand Down Expand Up @@ -210,13 +213,17 @@ def notice_segment_error(segment, exception, options={})

# See NewRelic::Agent.notice_error for options and commentary
def notice_error(exception, options={}, span_id=nil)
return if skip_notice_error?(exception)
state = ::NewRelic::Agent::Tracer.state
transaction = state.current_transaction
status_code = transaction ? transaction.http_response_code : nil

tag_exception(exception)
return if skip_notice_error?(exception, status_code)

state = ::NewRelic::Agent::Tracer.state
tag_exception(exception)

unless options[:expected]
if options[:expected] || @error_filter.expected?(exception, status_code)
increment_expected_error_count!(state, exception)
else
increment_error_count!(state, exception, options)
end

Expand Down Expand Up @@ -258,7 +265,7 @@ def create_noticed_error(exception, options)
noticed_error.line_number = sense_method(exception, :line_number)
noticed_error.stack_trace = truncate_trace(extract_stack_trace(exception))

noticed_error.expected = !! options.delete(:expected)
noticed_error.expected = !!options.delete(:expected) || expected?(exception)

noticed_error.attributes_from_notice_error = options.delete(:custom_params) || {}

Expand Down
162 changes: 162 additions & 0 deletions lib/new_relic/agent/error_filter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
# encoding: utf-8
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.

module NewRelic
module Agent

# Handles loading of ignored and expected errors from the agent configuration, and
# determining at runtime whether an exception is ignored or expected.
class ErrorFilter

def initialize
reset
end

def reset
@ignore_errors, @ignore_classes, @expected_classes = [], [], []
@ignore_messages, @expected_messages = {}, {}
@ignore_status_codes, @expected_status_codes = [], []
end

def load_all
%i(
ignore_errors ignore_classes ignore_messages ignore_status_codes
expected_classes expected_messages expected_status_codes
).each { |setting| load_from_config(setting) }
end

def load_from_config(setting, value = nil)
errors = nil
new_value = value || fetch_agent_config(setting.to_sym)
case setting.to_sym
when :ignore_errors # Deprecated; only use if ignore_classes isn't present
errors = @ignore_errors = new_value.to_s.split(',').map!(&:strip)
when :ignore_classes
errors = @ignore_classes = new_value || []
when :ignore_messages
errors = @ignore_messages = new_value || {}
when :ignore_status_codes
errors = @ignore_status_codes = parse_status_codes(new_value) || []
when :expected_classes
errors = @expected_classes = new_value || []
when :expected_messages
errors = @expected_messages = new_value || {}
when :expected_status_codes
errors = @expected_status_codes = parse_status_codes(new_value) || []
end
log_filter(setting, errors)
end

def ignore?(ex, status_code = nil)
@ignore_classes.include?(ex.class.name) ||
(@ignore_classes.empty? && @ignore_errors.include?(ex.class.name)) ||
(@ignore_messages.keys.include?(ex.class.name) &&
@ignore_messages[ex.class.name].any? { |m| ex.message.include?(m) }) ||
@ignore_status_codes.include?(status_code.to_i)
end

def expected?(ex, status_code = nil)
@expected_classes.include?(ex.class.name) ||
(@expected_messages.keys.include?(ex.class.name) &&
@expected_messages[ex.class.name].any? { |m| ex.message.include?(m) }) ||
@expected_status_codes.include?(status_code.to_i)
end

def fetch_agent_config(cfg)
NewRelic::Agent.config[:"error_collector.#{cfg}"]
end

# A generic method for adding ignore filters manually. This is kept for compatibility
# with the previous ErrorCollector#ignore method, and adds some flexibility for adding
# different ignore/expected error types by examining each argument.
def ignore(*args)
args.each do |errors|
case errors
when Array
errors.each { |e| ignore(e) }
when Hash
@ignore_messages.update(errors)
log_filter(:ignore_messages, errors)
when String
if errors.match(/^[\d\,\-]+$/)
@ignore_status_codes |= parse_status_codes(errors)
log_filter(:ignore_status_codes, errors)
else
new_ignore_classes = errors.split(',').map!(&:strip)
@ignore_classes |= new_ignore_classes
log_filter(:ignore_classes, new_ignore_classes)
end
end
end
end

# See #ignore above.
def expect(*args)
args.each do |errors|
case errors
when Array
errors.each { |e| expect(e) }
when Hash
@expected_messages.update(errors)
log_filter(:expected_messages, errors)
when String
if errors.match(/^[\d\,\-]+$/)
@expected_status_codes |= parse_status_codes(errors)
log_filter(:expected_status_codes, errors)
else
new_expected_classes = errors.split(',').map!(&:strip)
@expected_classes |= new_expected_classes
log_filter(:expected_classes, new_expected_classes)
end
end
end
end

private

def log_filter(setting, errors)
case setting
when :ignore_errors, :ignore_classes
errors.each do |error|
::NewRelic::Agent.logger.debug("Ignoring errors of type '#{error}'")
end
when :ignore_messages
errors.each do |error,messages|
::NewRelic::Agent.logger.debug("Ignoring errors of type '#{error}' with messages: #{messages.join(',')}")
end
when :ignore_status_codes
::NewRelic::Agent.logger.debug("Ignoring errors associated with status codes: #{errors}")
when :expected_classes
errors.each do |error|
::NewRelic::Agent.logger.debug("Expecting errors of type '#{error}'")
end
when :expected_messages
errors.each do |error,messages|
::NewRelic::Agent.logger.debug("Expecting errors of type '#{error}' with messages: #{messages.join(',')}")
end
when :expected_status_codes
::NewRelic::Agent.logger.debug("Expecting errors associated with status codes: #{errors}")
end
end

def parse_status_codes(codes)
code_list = codes.is_a?(String) ? codes.split(',') : codes
result = []
code_list.each do |code|
m = code.match(/(\d{3})(-\d{3})?/)
if m.nil? || m[1].nil?
::NewRelic::Agent.logger.warn("Invalid HTTP status code: '#{code}'; ignoring config")
next
end
if m[2]
result += (m[1]..m[2].tr('-', '')).to_a.map(&:to_i)
else
result << m[1].to_i
end
end
result.uniq
end
end
end
end
Loading

0 comments on commit 98fb917

Please sign in to comment.