Record status codes when middleware instrumentation is disabled

CHANGELOG.md

@@ -1,5 +1,18 @@
 # New Relic Ruby Agent Release Notes
 
+
+## dev
+
+Version <dev> allows the agent to record the http status code on a transaction when middleware instrumentation is disabled and fixes a bug in `NewRelic::Rack::AgentHooks.needed?`.
+
+
+- **Feature: Transactions now report http status codes when middleware instrumentation is disabled**
+  Previously, if `disable_middleware_instrumentation` is set to `true`, the agent would not record the value of the response code on the transaction. This was due to the possibility that a middleware could be used that might alter the response, which would not be captured by the agent if the middleware instrumentation is disabled. However, based on customer feedback, the agent will now report the http status code on a transaction still when middleware instrumentation is disabled. [PR#2175](https://github.com/newrelic/newrelic-ruby-agent/pull/2175)
+
+- **Bugfix: Resolve inverted logic of `NewRelic::Rack::AgentHooks.needed?`**
+  Previously, `NewRelic::Rack::AgentHooks.needed?` was incorrectly using inverted logic. This has now been resolved. [PR#2175](https://github.com/newrelic/newrelic-ruby-agent/pull/2175)
+
+
 ## v9.4.2
 
 Version 9.4.2 of the agent re-addresses the 9.4.0 issue of `NoMethodError` seen when using the `uppy-s3_multipart` gem.

lib/new_relic/agent/configuration/default_source.rb

@@ -1212,7 +1212,13 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
           :public => true,
           :type => Boolean,
           :allowed_from_server => false,
-          :description => 'If `true`, the agent won\'t wrap third-party middlewares in instrumentation (regardless of whether they are installed via `Rack::Builder` or Rails).'
+          :description => <<~DESCRIPTION
+            If `true`, the agent won't wrap third-party middlewares in instrumentation (regardless of whether they are installed via `Rack::Builder` or Rails).
+
+              <Callout variant="important">
+                When middleware instrumentation is disabled, if an application is using middleware that could alter the response code, the http status code reported on the transaction may not reflect the altered value.
+              </Callout>
+          DESCRIPTION
         },
         :disable_samplers => {
           :default => false,

lib/new_relic/rack/agent_hooks.rb

@@ -23,7 +23,7 @@ module NewRelic::Rack
   #
   class AgentHooks < AgentMiddleware
     def self.needed?
-      !NewRelic::Agent.config[:disable_middleware_instrumentation]
+      NewRelic::Agent.config[:disable_middleware_instrumentation]
     end
 
     def traced_call(env)

lib/new_relic/rack/agent_middleware.rb

@@ -26,22 +26,6 @@ def build_transaction_name
         prefix = ::NewRelic::Agent::Instrumentation::ControllerInstrumentation::TransactionNamer.prefix_for_category(nil, @category)
         "#{prefix}#{self.class.name}/call"
       end
-
-      # If middleware tracing is disabled, we'll still inject our agent-specific
-      # middlewares, and still trace those, but we don't want to capture HTTP
-      # response codes, since middleware that's outside of ours might change the
-      # response code before it goes back to the client.
-      def capture_http_response_code(state, result)
-        return if NewRelic::Agent.config[:disable_middleware_instrumentation]
-
-        super
-      end
-
-      def capture_response_content_type(state, result)
-        return if NewRelic::Agent.config[:disable_middleware_instrumentation]
-
-        super
-      end
     end
   end
 end

test/multiverse/suites/rack/http_response_code_test.rb

@@ -35,17 +35,17 @@ def test_records_http_response_code_on_analytics_events
       assert_equal(302, get_last_analytics_event[2][:'http.statusCode'])
     end
 
-    def test_skips_http_response_code_if_middleware_tracing_disabled
+    def test_records_http_response_code_if_middleware_tracing_disabled
       with_config(:disable_middleware_instrumentation => true) do
         rsp = get('/', {'override-response-code' => 404})
 
         assert_equal(404, rsp.status)
-        refute get_last_analytics_event[2][:'http.statusCode']
+        assert get_last_analytics_event[2][:'http.statusCode']
 
         rsp = get('/', {'override-response-code' => 302})
 
         assert_equal(302, rsp.status)
-        refute get_last_analytics_event[2][:'http.statusCode']
+        assert get_last_analytics_event[2][:'http.statusCode']
       end
     end
   end

test/multiverse/suites/rack/response_content_type_test.rb

@@ -35,17 +35,17 @@ def test_records_response_content_type_on_analytics_events
       assert_equal('application/xml', get_last_analytics_event[2][:'response.headers.contentType'])
     end
 
-    def test_skips_response_content_type_if_middleware_tracing_disabled
+    def test_records_response_content_type_if_middleware_tracing_disabled
       with_config(:disable_middleware_instrumentation => true) do
         rsp = get('/', {'override-content-type' => 'application/json'})
 
         assert_equal('application/json', rsp.headers['Content-Type'])
-        refute get_last_analytics_event[2][:'response.headers.contentType']
+        assert get_last_analytics_event[2][:'response.headers.contentType']
 
         rsp = get('/', {'override-content-type' => 'application/xml'})
 
         assert_equal('application/xml', rsp.headers['Content-Type'])
-        refute get_last_analytics_event[2][:'response.headers.contentType']
+        assert get_last_analytics_event[2][:'response.headers.contentType']
       end
     end
   end