Skip to content

Commit

Permalink
Support Rails 7+ error reporter (#944) (#953)
Browse files Browse the repository at this point in the history
When the Rails version we're integrating with supports the Rails error
reporter, add a subscriber to it to report errors to AppSignal.

https://guides.rubyonrails.org/error_reporting.html

## Error reporting

We report the errors using `Appsignal.send_error` to report the errors
separately from the main request sample.

If the request did not fail with an error, because `Rails.error.handle`
was used to not reraise the error, it will not mark the request
transaction as having encountered an error, while the end-user saw none.

When `Rails.error.record` is used, we don't record the error in the
error handler. It will be reraised and handled by our other
instrumentations, like the Rails instrumentation. This prevents the
error from reported twice.

Using `Appsignal.send_error` is also a bit of a workaround to support
reporting multiple errors reported by Rails, even though we don't
support multiple errors on a transaction.

## Transaction namespace, action name and tags

To make sure the errors are reported in a somewhat similar grouping as
the request they belonged to, the error reporter subscriber will check
different levels of context.

First, it will copy the controller name from the Rails execution context
on the transaction, if available. This is done because we don't have
this information in the Rails instrumentation middleware beforehand, and
the action name will be `nil` at this time. This should result in the
same action name as from our Rails middleware.

Then it will copy the transaction namespace and action name from the
current transaction, to the transaction created by
`Appsignal.send_error`.

In this step the Active Job's job action name is also is picked up, as
we do have that available on the transaction. If people have set a
custom namespace or action name on the transaction, that is also copied
here, as well as the transaction tags.

If the namespace, action name or tags change after the error has been
reported by the Rails error reporter, we will not pick up that change.
People need to set this metadata on the request transaction in a
`before_action` callback, rather than a `after_action` callback in a
Rails controller.

### Transaction namespace and action name from other instrumentations

For other instrumentations, like Sidekiq, we do not currently report the
action name correctly. The error is still reported, but the action name
will be unset.

The same problem I fixed for Active Job jobs in #945 applies here. This
will need to be applied to other instrumentations, where possible. I
can't guarantee it can work for all instrumentations, even if we update
them.

### Custom namespace and action name

If, people want to report this error using a different namespace or
action, than the one inherited from the current transaction, they can
use the `appsignal` context to customize it for this one error.

```ruby
given_context = {
  :appsignal => { :namespace => "context", :action => "ContextAction" }
}
Rails.error.handle(context: given_context) do
  raise ExampleStandardError
end
```

### Custom tags

All other context values, that do not have another purpose that I know
off (like `controller` and `job`), will be set as tags. If people add
their own additional context, this will be added as tags.

```ruby
tags = { :abc => "def" }
Rails.error.handle(context: tags) do
  raise ExampleStandardError
end
```
  • Loading branch information
tombruijn authored Apr 12, 2023
1 parent a493fa2 commit 77ce4e3
Show file tree
Hide file tree
Showing 6 changed files with 314 additions and 1 deletion.
26 changes: 26 additions & 0 deletions .changesets/add-rails-error-reporter-support.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
bump: "patch"
type: "add"
---

Add Rails [error reporter](https://guides.rubyonrails.org/error_reporting.html) support. Errors reported using `Rails.error.handle` are tracked as separate errors in AppSignal. We rely on our other Rails instrumentation to report the errors reported with `Rails.error.record`.

The error is reported under the same controller/job name, on a best effort basis. It may not be 100% accurate. If `Rails.error.handle` is called within a Rails controller or Active Job job, it will copy the AppSignal transaction namespace, action name and tags from the current transaction to the transaction for the `Rails.error.handle` reported error. If you call `Appsignal.set_namespace`, `Appsignal.set_action` or `Appsignal.tag_request` after `Rails.error.handle`, those changes will not be reflected up in the already reported error.

It is also possible to customize the AppSignal namespace and action name for the reported error using the `appsignal` context:

```ruby
Rails.error.handle(:context => { :appsignal => { :namespace => "context", :action => "ContextAction" } }) do
raise "Test"
end
```

All other key-values are reported as tags:

```ruby
Rails.error.handle(:context => { :tag_key => "tag value" }) do
raise "Test"
end
```

Integration with the Rails error reporter is enabled by default. Disable this feature by setting the `enable_rails_error_reporter` config option to `false`.
3 changes: 3 additions & 0 deletions lib/appsignal/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class Config
:enable_nginx_metrics => false,
:enable_gvl_global_timer => true,
:enable_gvl_waiting_threads => true,
:enable_rails_error_reporter => true,
:endpoint => "https://push.appsignal.com",
:files_world_accessible => true,
:filter_parameters => [],
Expand Down Expand Up @@ -74,6 +75,7 @@ class Config
"APPSIGNAL_ENABLE_NGINX_METRICS" => :enable_nginx_metrics,
"APPSIGNAL_ENABLE_GVL_GLOBAL_TIMER" => :enable_gvl_global_timer,
"APPSIGNAL_ENABLE_GVL_WAITING_THREADS" => :enable_gvl_waiting_threads,
"APPSIGNAL_ENABLE_RAILS_ERROR_REPORTER" => :enable_rails_error_reporter,
"APPSIGNAL_FILES_WORLD_ACCESSIBLE" => :files_world_accessible,
"APPSIGNAL_FILTER_PARAMETERS" => :filter_parameters,
"APPSIGNAL_FILTER_SESSION_DATA" => :filter_session_data,
Expand Down Expand Up @@ -130,6 +132,7 @@ class Config
APPSIGNAL_ENABLE_NGINX_METRICS
APPSIGNAL_ENABLE_GVL_GLOBAL_TIMER
APPSIGNAL_ENABLE_GVL_WAITING_THREADS
APPSIGNAL_ENABLE_RAILS_ERROR_REPORTER
APPSIGNAL_FILES_WORLD_ACCESSIBLE
APPSIGNAL_INSTRUMENT_HTTP_RB
APPSIGNAL_INSTRUMENT_NET_HTTP
Expand Down
73 changes: 73 additions & 0 deletions lib/appsignal/integrations/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,79 @@ def self.initialize_appsignal(app)
)

Appsignal.start

if Appsignal.config[:enable_rails_error_reporter] && Rails.respond_to?(:error)
Rails.error
.subscribe(Appsignal::Integrations::RailsErrorReporterSubscriber)
end
end
end

# Report errors reported by the Rails error reporter.
#
# We only report that are not reraised by the error reporter, using
# `Rails.error.handle`.
# @api private
class RailsErrorReporterSubscriber
class << self
def report(error, handled:, severity:, context: {}, source: nil)
# Ignore not handled errors. They are reraised by the error reporter
# and are caught and recorded by our Rails middleware.
return unless handled

Appsignal.send_error(error) do |transaction|
namespace, action_name, tags = context_for(context.dup)
transaction.set_namespace(namespace) if namespace
transaction.set_action(action_name) if action_name

tags[:severity] = severity
tags[:source] = source.to_s if source
transaction.set_tags(tags)
end
end

private

def context_for(context)
tags = {}

appsignal_context = context.delete(:appsignal)
# Fetch the namespace and action name based on the Rails execution
# context.
controller = context.delete(:controller)
if controller
namespace = Appsignal::Transaction::HTTP_REQUEST
action_name = "#{controller.class.name}##{controller.action_name}"
end
# ActiveJob transaction naming relies on the current AppSignal
# transaction namespace and action name copying done after this.
context.delete(:job)

# Copy the transaction action name, namespace and other data from
# the currently active transaction, if not already set.
if Appsignal::Transaction.current?
current_transaction = Appsignal::Transaction.current
namespace = current_transaction.namespace

transaction_action = current_transaction.action
action_name = current_transaction.action if transaction_action

current_tags = current_transaction.tags
tags.merge!(current_tags) if current_tags
end

# Use the user override set in the context
if appsignal_context
context_namespace = appsignal_context[:namespace]
namespace = context_namespace if context_namespace

context_action_name = appsignal_context[:action]
action_name = context_action_name if context_action_name
end
tags.merge!(context)

[namespace, action_name, tags]
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/diagnose
1 change: 1 addition & 0 deletions spec/lib/appsignal/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
:enable_minutely_probes => true,
:enable_statsd => true,
:enable_nginx_metrics => false,
:enable_rails_error_reporter => true,
:endpoint => "https://push.appsignal.com",
:files_world_accessible => true,
:filter_parameters => [],
Expand Down
210 changes: 210 additions & 0 deletions spec/lib/appsignal/integrations/railtie_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
if DependencyHelper.rails_present?
require "action_mailer"

describe Appsignal::Integrations::Railtie do
context "after initializing the app" do
it "should call initialize_appsignal" do
Expand Down Expand Up @@ -54,6 +56,50 @@
expect(config.env).to eq "env_test"
end
end

if Rails.respond_to?(:error)
context "when Rails listens to `error`" do
class ErrorReporterMock
attr_reader :subscribers

def initialize
@subscribers = []
end

def subscribe(subscriber)
@subscribers << subscriber
end
end

let(:error_reporter) { ErrorReporterMock.new }
before do
allow(Rails).to receive(:error).and_return(error_reporter)
end

context "when enable_rails_error_reporter is enabled" do
it "subscribes to the error reporter" do
Appsignal::Integrations::Railtie.initialize_appsignal(app)

expect(error_reporter.subscribers).to eq([Appsignal::Integrations::RailsErrorReporterSubscriber])
end
end

context "when enable_rails_error_reporter is disabled" do
it "does not subscribe to the error reporter" do
ENV["APPSIGNAL_ENABLE_RAILS_ERROR_REPORTER"] = "false"
Appsignal::Integrations::Railtie.initialize_appsignal(app)

expect(error_reporter.subscribers).to_not eq([Appsignal::Integrations::RailsErrorReporterSubscriber])
end
end
end
else
context "when Rails does not listen to `error`" do
it "does not error trying to subscribe to the error reporter" do
Appsignal::Integrations::Railtie.initialize_appsignal(app)
end
end
end
end

describe ".initial_config" do
Expand All @@ -74,6 +120,170 @@
Appsignal::Integrations::Railtie.initialize_appsignal(app)
end
end

if Rails.respond_to?(:error)
describe "Rails error reporter" do
before do
Appsignal::Integrations::Railtie.initialize_appsignal(app)
start_agent
end
around { |example| keep_transactions { example.run } }

context "when error is not handled (reraises the error)" do
it "does nothing" do
expect do
Rails.error.record { raise ExampleStandardError }
end.to raise_error(ExampleStandardError)

expect(created_transactions).to be_empty
end
end

context "when error is handled (not reraised)" do
context "when a transaction is active" do
it "duplicates the transaction namespace, action and tags" do
current_transaction = http_request_transaction
current_transaction.set_namespace "custom"
current_transaction.set_action "CustomAction"
current_transaction.set_tags(
:duplicated_tag => "duplicated value"
)

with_current_transaction current_transaction do
Rails.error.handle { raise ExampleStandardError }

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"action" => "CustomAction",
"namespace" => "custom",
"error" => {
"name" => "ExampleStandardError",
"message" => "ExampleStandardError",
"backtrace" => kind_of(String)
},
"sample_data" => hash_including(
"tags" => {
"duplicated_tag" => "duplicated value",
"severity" => "warning"
}
)
)
end
end

it "overwrites duplicated tags with tags from context" do
current_transaction = http_request_transaction
current_transaction.set_tags(:tag1 => "duplicated value")

with_current_transaction current_transaction do
given_context = { :tag1 => "value1", :tag2 => "value2" }
Rails.error.handle(context: given_context) { raise ExampleStandardError }

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"sample_data" => hash_including(
"tags" => {
"tag1" => "value1",
"tag2" => "value2",
"severity" => "warning"
}
)
)
end
end

it "overwrites duplicated namespace and action with custom from context" do
current_transaction = http_request_transaction
current_transaction.set_namespace "custom"
current_transaction.set_action "CustomAction"

with_current_transaction current_transaction do
given_context = {
:appsignal => { :namespace => "context", :action => "ContextAction" }
}
Rails.error.handle(context: given_context) { raise ExampleStandardError }

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"namespace" => "context",
"action" => "ContextAction"
)
end
end
end

context "when no transaction is active" do
class ExampleRailsControllerMock
def action_name
"index"
end
end

class ExampleRailsJobMock
end

class ExampleRailsMailerMock < ActionMailer::MailDeliveryJob
def arguments
["ExampleRailsMailerMock", "send_mail"]
end
end

before do
clear_current_transaction!
end

it "fetches the action from the controller in the context" do
# The controller key is set by Rails when raised in a controller
given_context = { :controller => ExampleRailsControllerMock.new }
Rails.error.handle(context: given_context) { raise ExampleStandardError }

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"action" => "ExampleRailsControllerMock#index"
)
end

it "sets no action if no execution context is present" do
# The controller key is set by Rails when raised in a controller
Rails.error.handle { raise ExampleStandardError }

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"action" => nil
)
end
end

it "sets the error context as tags" do
given_context = {
:controller => ExampleRailsControllerMock.new, # Not set as tag
:job => ExampleRailsJobMock.new, # Not set as tag
:appsignal => { :something => "not used" }, # Not set as tag
:tag1 => "value1",
:tag2 => "value2"
}
Rails.error.handle(context: given_context) { raise ExampleStandardError }

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"sample_data" => hash_including(
"tags" => {
"tag1" => "value1",
"tag2" => "value2",
"severity" => "warning"
}
)
)
end
end
end
end
end
end
end

0 comments on commit 77ce4e3

Please sign in to comment.