Supported nested Rack EventHandlers (#1101)

Author: tombruijn

.changesets/support-nested-rack-eventhandlers.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+type: change
+---
+
+Support apps that have multiple Appsignal::Rack::EventHandler-s in the middleware stack.

lib/appsignal/rack/event_handler.rb

@@ -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
@@ -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,
@@ -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
 
@@ -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
 
@@ -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
 

spec/lib/appsignal/rack/event_handler_spec.rb

@@ -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
@@ -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
 
@@ -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
@@ -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
 
@@ -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
@@ -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