Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors inbox fingerprinting #1858

Merged
merged 14 commits into from
Mar 13, 2023
34 changes: 34 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,39 @@
# New Relic Ruby Agent Release Notes

## error_fingerprinting_and_user_tracking
fallwith marked this conversation as resolved.
Show resolved Hide resolved

Version x.x.x of the agent delivers support for two new [errors inbox](https://docs.newrelic.com/docs/errors-inbox/errors-inbox/) features: error fingerprinting and user tracking.

- **Error fingerprinting**

A new `set_error_group_callback` public API method now exists that will accept a user defined proc. The proc will be invoked for each noticed error and whenever it returns a string, that string will be used as the error group name for the error and will take precedence over any server-side grouping that takes place with the New Relic errors inbox. This gives users much greater control over the grouping of their errors.

The customer defined proc will be expected to receive exactly one input argument, a hash. The hash contains the following:

| Key | Value |
| ---------------------| ---------------------------------------------------------------------------- |
| `:error` | The Ruby error class instance. Offers `#class`, `#message`, and `#backtrace` |
| `:customAttributes` | Any customer defined customed attributed for the current transaction |
| `:'request.uri'` | The current requeset URI if available |
| `:'http.statusCode'` | The HTTP status code (200, 404, etc.) if available |
| `:'http.method'` | The HTTP method (GET, PUT, etc.) if available |
| `:'error.expected'` | Whether (true) or not (false) the error was expected |
| `:'options'` | The options hash passed to `NewRelic::Agent.notice_error |

The callback only needs to be set once per initialization of the New Relic agent.

Example usage:

```
proc = proc { |hash| "Access" if hash[:'http.statusCode'].to_s == '401' }
NewRelic::Agent.set_error_group_callback(proc)
```

- **User tracking**

A new `set_user_id` public API method now exists that will accept a string representation of a user id and associate that user id with the current transaction. Transactions and errors will then have a new `enduser.id` agent attribute associated with them. This will allow agent users to tag transactions and errors as belonging to given user ids in support of greater filtering and alerting capabilities.


## dev

Upcoming version identifies the Amazon Timesteam data store, removes Distributed Tracing warnings from agent logs when using Sidekiq, fixes a bug regarding logged request headers, and is tested against the recently released JRuby 9.4.2.0.
Expand Down
44 changes: 44 additions & 0 deletions lib/new_relic/agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ class SerializationError < StandardError; end
# placeholder name used when we cannot determine a transaction's name
UNKNOWN_METRIC = '(unknown)'.freeze

attr_reader :error_group_callback

@agent = nil
@error_group_callback = nil
@logger = nil
@tracer_lock = Mutex.new
@tracer_queue = []
Expand Down Expand Up @@ -293,6 +296,47 @@ def notice_error(exception, options = {})
nil # don't return a noticed error data structure. it can only hurt.
end

# Set a callback proc for determining an error's error group name
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
#
# @param [Proc] the callback proc
#
# Typically this method should be called only once to set a callback for
fallwith marked this conversation as resolved.
Show resolved Hide resolved
# use with all noticed errors. If it is called multiple times, each new
# callback given will replace the old one.
#
# The proc will be called with a single hash as its input argument and is
# expected to return a string representing the desired error group.
#
# see https://docs.newrelic.com/docs/errors-inbox/errors-inbox/#groups
#
# The hash passed to the customer defined callback proc has the following
# keys:
#
# :error => The Ruby error class instance, likely inheriting from
# StandardError. Call `#class`, `#message`, and `#backtrace` on
# the error object to retrieve the error's class, message, and
# backtrace.
# :customAttributes => Any customer defined custom attributes that have been
# associated with the current transaction.
# :'request.uri' => The current request URI if available
# :'http.statusCode' => The HTTP status code (200, 404, etc.) if available
# :'http.method' => The HTTP method (GET, PUT, etc.) if available
# :'error.expected' => Whether (true) or not (false) the error was expected
# :options => The options hash passed to `NewRelic::Agent.notice_error`
#
# @api public
#
def set_error_group_callback(callback_proc)
unless callback_proc.is_a?(Proc)
NewRelic::Agent.logger.error("#{self}.#{__method__}: expected an argument of type Proc, " \
"got #{callback_proc.class}")
return
end

record_api_supportability_metric(:set_error_group_callback)
@error_group_callback = callback_proc
end

# @!endgroup

# @!group Recording custom Insights events
Expand Down
28 changes: 28 additions & 0 deletions lib/new_relic/agent/error_collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ def create_noticed_error(exception, options)
# get treated as custom attributes, so merge them into that hash.
noticed_error.attributes_from_notice_error.merge!(options)

update_error_group_name(noticed_error, exception, options)

noticed_error
end

Expand All @@ -306,6 +308,32 @@ def drop_buffered_data
@error_event_aggregator.reset!
nil
end

private

def update_error_group_name(noticed_error, exception, options)
return unless error_group_callback
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved

callback_hash = build_customer_callback_hash(noticed_error, exception, options)
result = error_group_callback.call(callback_hash)
noticed_error.error_group = result
rescue StandardError => e
NewRelic::Agent.logger.error("Failed to obtain error group from customer callback: #{e.class} - #{e.message}")
end

def build_customer_callback_hash(noticed_error, exception, options)
{error: exception,
customAttributes: noticed_error.custom_attributes,
'request.uri': noticed_error.request_uri,
'http.statusCode': noticed_error.agent_attributes[:'http.statusCode'],
'http.method': noticed_error.intrinsic_attributes[:'http.method'],
'error.expected': noticed_error.expected,
options: options}
end

def error_group_callback
NewRelic::Agent.error_group_callback
end
end
end
end
7 changes: 4 additions & 3 deletions lib/new_relic/agent/error_trace_aggregator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ def notice_agent_error(exception)
# Already seen this class once? Bail!
return if @errors.any? { |err| err.exception_class_name == exception.class.name }

trace = exception.backtrace || caller.dup
noticed_error = NewRelic::NoticedError.new('NewRelic/AgentError', exception)
noticed_error.stack_trace = trace
noticed_error = NewRelic::Agent.instance.error_collector.create_noticed_error(exception,
{metric: 'NewRelic/AgentError'})
noticed_error.stack_trace = caller.dup unless exception.backtrace
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved

@errors << noticed_error
end
rescue => e
Expand Down
20 changes: 17 additions & 3 deletions lib/new_relic/noticed_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class NewRelic::NoticedError
:stack_trace, :attributes_from_notice_error, :attributes,
:expected

attr_reader :exception_id, :is_internal
attr_reader :error_group, :exception_id, :is_internal

STRIPPED_EXCEPTION_REPLACEMENT_MESSAGE = "Message removed by New Relic 'strip_exception_messages' setting"
UNKNOWN_ERROR_CLASS_NAME = 'Error'
Expand All @@ -27,6 +27,8 @@ class NewRelic::NoticedError

DESTINATION = NewRelic::Agent::AttributeFilter::DST_ERROR_COLLECTOR

AGENT_ATTRIBUTE_ERROR_GROUP = :'error.group.name'

ERROR_PREFIX_KEY = 'error'
ERROR_MESSAGE_KEY = "#{ERROR_PREFIX_KEY}.message"
ERROR_CLASS_KEY = "#{ERROR_PREFIX_KEY}.class"
Expand Down Expand Up @@ -141,9 +143,15 @@ def build_error_attributes
end

def build_agent_attributes(merged_attributes)
return NewRelic::EMPTY_HASH unless @attributes
return error_group_hash unless @attributes

@attributes.agent_attributes_for(DESTINATION).merge(error_group_hash)
end

def error_group_hash
return NewRelic::EMPTY_HASH unless error_group

@attributes.agent_attributes_for(DESTINATION)
{AGENT_ATTRIBUTE_ERROR_GROUP => error_group}
end

def build_intrinsic_attributes
Expand Down Expand Up @@ -185,4 +193,10 @@ def extract_class_name_and_message_from(exception)
@message = exception.to_s
end
end

def error_group=(name)
return if name.nil? || name.empty?

@error_group = name
end
end
1 change: 1 addition & 0 deletions lib/new_relic/supportability_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module SupportabilityHelper
:record_metric,
:recording_web_transaction?,
:require_test_helper,
:set_error_group_callabck,
fallwith marked this conversation as resolved.
Show resolved Hide resolved
:set_sql_obfuscator,
:set_transaction_name,
:shutdown,
Expand Down
66 changes: 66 additions & 0 deletions test/new_relic/agent/error_collector_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,72 @@ def test_segment_error_exclude_block
assert_nothing_harvested_for_segment_errors
end

def test_build_customer_callback_hash
custom_attributes = {billie_eilish: :bored}
agent_attributes = {'http.statusCode': :bleachers__dont_take_the_money}
intrinsic_attributes = {'http.method': :walk_the_moon__anna_sun}
request_uri = :alternative_music
options = {taylor_swift: :maroon}
expected = true
error = StandardError.new

noticed_error = NewRelic::NoticedError.new(:watermelon, error)
noticed_error.instance_variable_set(:@processed_attributes,
{NewRelic::NoticedError::USER_ATTRIBUTES => custom_attributes,
NewRelic::NoticedError::AGENT_ATTRIBUTES => agent_attributes,
NewRelic::NoticedError::INTRINSIC_ATTRIBUTES => intrinsic_attributes})
noticed_error.request_uri = request_uri
noticed_error.expected = expected
hash = @error_collector.send(:build_customer_callback_hash, noticed_error, error, options)

assert_equal hash[:error], error
assert_equal hash[:customAttributes], custom_attributes
assert_equal hash[:'request.uri'], request_uri
assert_equal hash[:'http.statusCode'], agent_attributes[:'http.statusCode']
assert_equal hash[:'http.method'], intrinsic_attributes[:'http.method']
assert_equal hash[:'error.expected'], expected
assert_equal hash[:options], options
end

def test_update_error_group_name_returns_nil_unless_a_callback_has_been_registered
# because the build_customer_callback_hash method will call
# #custom_attributes on the noticed error object, and we're passing in
# nil for it, it would error out if the early return we're testing for
# did not return
@error_collector.stub(:error_group_callback, nil) do
assert_nil @error_collector.send(:update_error_group_name, nil, nil, nil)
end
end

def test_update_error_group_name_updates_the_error_group_name
error = ArgumentError.new
error_group = 'lucky tiger'
noticed_error = NewRelic::NoticedError.new(:watermelon, error)
NewRelic::Agent.set_error_group_callback(proc { |hash| error_group if error.is_a?(ArgumentError) })
@error_collector.send(:update_error_group_name, noticed_error, error, {})

assert_equal error_group, noticed_error.error_group
ensure
NewRelic::Agent.remove_instance_variable(:@error_group_callback)
end

def test_update_error_group_logs_if_an_error_is_rescued
skip_unless_minitest5_or_above

logger = MiniTest::Mock.new
# have #error return a phony value just for the purpose of being able
# to supply this test with at least 1 assertion. really, the #verify
# call to the mock should suffice, but it's nice to have assertions
phony_return = :yep_i_was_indeed_called
logger.expect :error, phony_return, [/Failed to obtain/]
@error_collector.stub(:error_group_callback, -> { raise 'kaboom' }) do
NewRelic::Agent.stub :logger, logger do
assert_equal phony_return, @error_collector.send(:update_error_group_name, nil, nil, nil)
end
end
logger.verify
end

private

# Segment errors utilize the error_collector's filtering for LASP
Expand Down
18 changes: 18 additions & 0 deletions test/new_relic/agent/error_trace_aggregator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,24 @@ def test_errors_not_harvested_when_changing_from_enabled_to_disabled
end
end

def test_noticed_internal_errors_invoke_the_error_group_callback
message = 'ton up'
group = 'cafe racer'
proc = proc { |hash| group if hash[:error].message.match?(message) }
NewRelic::Agent.set_error_group_callback(proc)

error = DifficultToDebugAgentError.new(message)
aggregator = NewRelic::Agent::ErrorTraceAggregator.new(1148)
aggregator.notice_agent_error(error)
noticed_error = aggregator.harvest!.first

assert_equal group, noticed_error.error_group
ensure
NewRelic::Agent.remove_instance_variable(:@error_group_callback)
end

private

def create_noticed_error(exception, options = {})
path = options.delete(:metric)
noticed_error = NewRelic::NoticedError.new(path, exception)
Expand Down
52 changes: 52 additions & 0 deletions test/new_relic/agent_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,58 @@ def test_modules_and_classes_return_name_properly
end
end

def test_set_error_group_callback
lambda = -> { 'Hello, World!' }
NewRelic::Agent.set_error_group_callback(lambda)

assert_equal lambda,
NewRelic::Agent.instance_variable_get(:@error_group_callback),
'Supplied error group callback proc was not found to be set.'
end

def test_set_error_group_callback_can_be_called_multiple_times
procs = [proc { 'one' }, proc { 'two' }, proc { 'three' }]
procs.each { |proc| NewRelic::Agent.set_error_group_callback(proc) }

assert_equal procs.last,
NewRelic::Agent.instance_variable_get(:@error_group_callback),
'Supplied error group callback proc was not found to be set.'
end

def test_set_error_group_callback_rejects_non_proc
skip_unless_minitest5_or_above

NewRelic::Agent.instance_variable_set(:@error_group_callback, nil)

mock_logger = MiniTest::Mock.new
mock_logger.expect :error, nil, [/expected an argument of type Proc/]

NewRelic::Agent.stub(:logger, mock_logger) do
NewRelic::Agent.set_error_group_callback([])
end

mock_logger.verify
assert_nil NewRelic::Agent.instance_variable_get(:@error_group_callback)
end

def test_error_group_callback_is_exposed
callback = 'lucky tiger'
NewRelic::Agent.instance_variable_set(:@error_group_callback, callback)

assert_equal callback, NewRelic::Agent.error_group_callback
ensure
NewRelic::Agent.remove_instance_variable(:@error_group_callback)
end

def test_successful_set_error_group_callback_api_invocation_produces_supportability_metrics
called = false
verification_proc = proc { |name| called = true if name == :set_error_group_callback }
NewRelic::Agent.stub :record_api_supportability_metric, verification_proc do
NewRelic::Agent.set_error_group_callback(proc {})
end
assert called
end

private

def with_unstarted_agent
Expand Down
Loading