Fix duplicate response body instrumentation events (#1194)

Multiple response body instrumentation events could show up if between
two AppSignal instrumentation middlewares there is another middleware
that wraps around the response body too. That way we couldn't detect if
the response body was of a certain type anymore, leading to duplicate
response body instrumentation events.

To fix this, add another flag to the Rack request env to flag that the
response is already instrumented and we don't need to apply the
instrumentation again.

.changesets/don-t-instrument-response-bodies-twice.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+type: fix
+---
+
+Fix instrument events for response bodies appearing twice. When multiple instrumentation middleware were mounted in an application, it could create duplicate `process_response_body.rack` events.

lib/appsignal/rack.rb

@@ -3,6 +3,12 @@
 module Appsignal
   # @api private
   module Rack
+    APPSIGNAL_TRANSACTION = "appsignal.transaction"
+    APPSIGNAL_EVENT_HANDLER_ID = "appsignal.event_handler_id"
+    APPSIGNAL_EVENT_HANDLER_HAS_ERROR = "appsignal.event_handler.error"
+    APPSIGNAL_RESPONSE_INSTRUMENTED = "appsignal.response_instrumentation_active"
+    RACK_AFTER_REPLY = "rack.after_reply"
+
     class Utils
       # Fetch the queue start time from the request environment.
       #

lib/appsignal/rack/abstract_middleware.rb

@@ -91,9 +91,10 @@ def instrument_app_call(env, transaction)
       def call_app(env, transaction)
         status, headers, obody = @app.call(env)
         body =
-          if obody.is_a? Appsignal::Rack::BodyWrapper
+          if env[Appsignal::Rack::APPSIGNAL_RESPONSE_INSTRUMENTED]
             obody
           else
+            env[Appsignal::Rack::APPSIGNAL_RESPONSE_INSTRUMENTED] = true
             # Instrument response body and closing of the response body
             Appsignal::Rack::BodyWrapper.wrap(obody, transaction)
           end

lib/appsignal/rack/event_handler.rb

@@ -2,11 +2,6 @@
 
 module Appsignal
   module Rack
-    APPSIGNAL_TRANSACTION = "appsignal.transaction"
-    APPSIGNAL_EVENT_HANDLER_ID = "appsignal.event_handler_id"
-    APPSIGNAL_EVENT_HANDLER_HAS_ERROR = "appsignal.event_handler.error"
-    RACK_AFTER_REPLY = "rack.after_reply"
-
     # Instrumentation middleware using Rack's Events module.
     #
     # We recommend using this in combination with the

spec/lib/appsignal/rack/abstract_middleware_spec.rb

@@ -400,11 +400,12 @@ def session
           expect(response_events).to eq(1)
         end
 
-        context "when response body is already a BodyWrapper subclass" do
+        context "when the response body is already instrumented" do
           let(:body) { Appsignal::Rack::BodyWrapper.wrap(["hello!"], transaction) }
           let(:app) { DummyApp.new { [200, {}, body] } }
 
           it "doesn't wrap the body again" do
+            env[Appsignal::Rack::APPSIGNAL_RESPONSE_INSTRUMENTED] = true
             _status, _headers, body = make_request
             expect(body).to eq(body)