diff --git a/CHANGELOG.md b/CHANGELOG.md index e32ad1a6a3..2e3e0733ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # New Relic Ruby Agent Release Notes + +## dev + +Version allows the agent to record additional response information on a transaction when middleware instrumentation is disabled and fixes a bug in `NewRelic::Rack::AgentHooks.needed?`. + + +- **Feature: Report transaction HTTP status codes when middleware instrumentation is disabled** + Previously, when `disable_middleware_instrumentation` was set to `true`, the agent would not record the value of the response code or content type on the transaction. This was due to the possibility that a middleware could alter the response, which would not be captured by the agent when the middleware instrumentation was disabled. However, based on customer feedback, the agent will now report the HTTP status code and content type on a transaction 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?` incorrectly used inverted logic. This has now been resolved, allowing AgentHooks to be installed when `disable_middleware_instrumentation` is set to true. [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. diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 5ca0b0828f..288afa49c4 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/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). + + + 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. + + DESCRIPTION }, :disable_samplers => { :default => false, diff --git a/lib/new_relic/rack/agent_hooks.rb b/lib/new_relic/rack/agent_hooks.rb index 1c0f948870..504a80432c 100644 --- a/lib/new_relic/rack/agent_hooks.rb +++ b/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) diff --git a/lib/new_relic/rack/agent_middleware.rb b/lib/new_relic/rack/agent_middleware.rb index 0850b977f5..a018ebfaf4 100644 --- a/lib/new_relic/rack/agent_middleware.rb +++ b/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 diff --git a/test/multiverse/suites/rack/http_response_code_test.rb b/test/multiverse/suites/rack/http_response_code_test.rb index 61800fbf0e..6a767b681f 100644 --- a/test/multiverse/suites/rack/http_response_code_test.rb +++ b/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 diff --git a/test/multiverse/suites/rack/response_content_type_test.rb b/test/multiverse/suites/rack/response_content_type_test.rb index 69a10b47d4..8b389c4b7a 100644 --- a/test/multiverse/suites/rack/response_content_type_test.rb +++ b/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