From d0db72b77e8404f5725565f06c64bfab0bb84dab Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Mon, 3 Jun 2024 12:07:47 +0200 Subject: [PATCH] Report request response status for Rails apps We can now report the response status as a `response_status` tag and metric. Previously, due to the position of the Rails middleware, this was unreliable and wouldn't always report the real status code if other middleware in the stack that are run after ours returned another status. This is part of issue #183, but only implements it now for Rails apps and apps using the `Appsignal::Rack::EventHandler` manually. We'll need to update other instrumentations for Rack, Sinatra, Padrino, Grape, etc. to use this new EventHandler, so they can also benefit from this new response status tag and metric. --- ...port-response-status-for-rails-requests.md | 8 ++++++++ lib/appsignal/rack/event_handler.rb | 9 ++++++++- spec/lib/appsignal/rack/event_handler_spec.rb | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 .changesets/report-response-status-for-rails-requests.md diff --git a/.changesets/report-response-status-for-rails-requests.md b/.changesets/report-response-status-for-rails-requests.md new file mode 100644 index 000000000..ecb46c9cc --- /dev/null +++ b/.changesets/report-response-status-for-rails-requests.md @@ -0,0 +1,8 @@ +--- +bump: patch +type: add +--- + +Report the response status for Rails requests as the `response_status` tag on samples, e.g. 200, 301, 500. This tag is visible on the sample detail page. + +The response status is also reported as the `response_status` metric. diff --git a/lib/appsignal/rack/event_handler.rb b/lib/appsignal/rack/event_handler.rb index 0332d6997..171fc2ec9 100644 --- a/lib/appsignal/rack/event_handler.rb +++ b/lib/appsignal/rack/event_handler.rb @@ -55,14 +55,21 @@ def on_error(request, _response, error) end end - def on_finish(request, _response) + def on_finish(request, response) self.class.safe_execution("Appsignal::Rack::EventHandler#on_finish") do transaction = request.env[APPSIGNAL_TRANSACTION] return unless transaction transaction.finish_event("process_request.rack", "", "") transaction.set_action_if_nil("#{request.request_method} #{request.path}") + transaction.set_tags(:response_status => response.status) transaction.set_http_or_background_queue_start + Appsignal.increment_counter( + :response_status, + 1, + :status => response.status, + :namespace => format_namespace(transaction.namespace) + ) # Make sure the current transaction is always closed when the request # is finished diff --git a/spec/lib/appsignal/rack/event_handler_spec.rb b/spec/lib/appsignal/rack/event_handler_spec.rb index e314fc109..12e609ce1 100644 --- a/spec/lib/appsignal/rack/event_handler_spec.rb +++ b/spec/lib/appsignal/rack/event_handler_spec.rb @@ -175,6 +175,25 @@ def on_finish ) end + it "sets the response status as a tag" do + on_start + on_finish + + expect(last_transaction.to_h).to include( + "sample_data" => hash_including( + "tags" => { "response_status" => 200 } + ) + ) + end + + it "increments the response status counter for response status" do + expect(Appsignal).to receive(:increment_counter) + .with(:response_status, 1, :status => 200, :namespace => :web) + + on_start + on_finish + end + it "logs an error in case of an error" do expect(Appsignal::Transaction) .to receive(:complete_current!).and_raise(ExampleStandardError, "oh no")