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

Supported nested Rack EventHandlers #1101

Merged
merged 1 commit into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changesets/support-nested-rack-eventhandlers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: change
---

Support apps that have multiple Appsignal::Rack::EventHandler-s in the middleware stack.
21 changes: 21 additions & 0 deletions lib/appsignal/rack/event_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module Appsignal
module Rack
APPSIGNAL_TRANSACTION = "appsignal.transaction"
APPSIGNAL_EVENT_HANDLER_ID = "appsignal.event_handler_id"
RACK_AFTER_REPLY = "rack.after_reply"

class EventHandler
Expand All @@ -16,8 +17,22 @@ def self.safe_execution(name)
)
end

attr_reader :id

def initialize
@id = SecureRandom.uuid
end

def request_handler?(given_id)
id == given_id
end

def on_start(request, _response)
event_handler = self
self.class.safe_execution("Appsignal::Rack::EventHandler#on_start") do
request.env[APPSIGNAL_EVENT_HANDLER_ID] ||= id
return unless request_handler?(request.env[APPSIGNAL_EVENT_HANDLER_ID])

transaction = Appsignal::Transaction.create(
SecureRandom.uuid,
Appsignal::Transaction::HTTP_REQUEST,
Expand All @@ -29,6 +44,8 @@ def on_start(request, _response)
request.env[RACK_AFTER_REPLY] << proc do
Appsignal::Rack::EventHandler
.safe_execution("Appsignal::Rack::EventHandler's after_reply") do
next unless event_handler.request_handler?(request.env[APPSIGNAL_EVENT_HANDLER_ID])

transaction.finish_event("process_request.rack", "", "")
transaction.set_http_or_background_queue_start

Expand All @@ -48,6 +65,8 @@ def on_start(request, _response)

def on_error(request, _response, error)
self.class.safe_execution("Appsignal::Rack::EventHandler#on_error") do
return unless request_handler?(request.env[APPSIGNAL_EVENT_HANDLER_ID])

transaction = request.env[APPSIGNAL_TRANSACTION]
return unless transaction

Expand All @@ -57,6 +76,8 @@ def on_error(request, _response, error)

def on_finish(request, response)
self.class.safe_execution("Appsignal::Rack::EventHandler#on_finish") do
return unless request_handler?(request.env[APPSIGNAL_EVENT_HANDLER_ID])

transaction = request.env[APPSIGNAL_TRANSACTION]
return unless transaction

Expand Down
40 changes: 37 additions & 3 deletions spec/lib/appsignal/rack/event_handler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@
let(:response) { nil }
let(:log_stream) { StringIO.new }
let(:log) { log_contents(log_stream) }
let(:event_handler_instance) { described_class.new }
before do
start_agent
Appsignal.internal_logger = test_logger(log_stream)
end
around { |example| keep_transactions { example.run } }

def on_start
described_class.new.on_start(request, response)
event_handler_instance.on_start(request, response)
end

describe "#on_start" do
Expand All @@ -34,6 +35,14 @@ def on_start
expect(Appsignal::Transaction.current).to eq(last_transaction)
end

context "when the handler is nested in another EventHandler" do
it "does not create a new transaction in the nested EventHandler" do
on_start
expect { described_class.new.on_start(request, response) }
.to_not(change { created_transactions.length })
end
end

it "registers transaction on the request environment" do
on_start

Expand Down Expand Up @@ -87,7 +96,7 @@ def on_start

describe "#on_error" do
def on_error(error)
described_class.new.on_error(request, response, error)
event_handler_instance.on_error(request, response, error)
end

it "reports the error" do
Expand All @@ -103,6 +112,15 @@ def on_error(error)
)
end

context "when the handler is nested in another EventHandler" do
it "does not report the error on the transaction" do
on_start
described_class.new.on_error(request, response, ExampleStandardError.new("the error"))

expect(last_transaction.to_h).to include("error" => nil)
end
end

it "logs an error in case of an internal error" do
on_start

Expand All @@ -122,7 +140,7 @@ def on_error(error)
let(:response) { Rack::Events::BufferedResponse.new(200, {}, ["body"]) }

def on_finish
described_class.new.on_finish(request, response)
event_handler_instance.on_finish(request, response)
end

it "doesn't do anything without a transaction" do
Expand Down Expand Up @@ -155,6 +173,22 @@ def on_finish
)
)
expect(last_transaction.ext.queue_start).to eq(queue_start_time)
expect(last_transaction).to be_completed
end

context "when the handler is nested in another EventHandler" do
it "does not complete the transaction" do
on_start
described_class.new.on_finish(request, response)

expect(last_transaction.to_h).to include(
"action" => nil,
"metadata" => {},
"sample_data" => {},
"events" => []
)
expect(last_transaction).to_not be_completed
end
end

it "doesn't set the action name if already set" do
Expand Down