From 5459a021d7d4bbbd09a0dcbdf5f3af7bf861b6f5 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Mon, 17 Jun 2024 15:21:56 +0200 Subject: [PATCH] Report request response status for Rails apps (#1090) 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 f3b97a99d..5ed08d549 100644 --- a/lib/appsignal/rack/event_handler.rb +++ b/lib/appsignal/rack/event_handler.rb @@ -55,13 +55,20 @@ 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_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 cf125d388..beb42627c 100644 --- a/spec/lib/appsignal/rack/event_handler_spec.rb +++ b/spec/lib/appsignal/rack/event_handler_spec.rb @@ -183,6 +183,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")