From 243d20acd68a9e59a01d74e17abb910691667b25 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Tue, 2 Jul 2024 21:32:53 +0200 Subject: [PATCH] Support nested webmachine apps Apply the same logic as we have in the AbstractMiddleware for webmachine apps. When a parent transaction is active, use that and don't create a new one. More importantly, don't close the active transaction. The webmachine instrumentation can't use the AbstractMiddleware, or any middleware, as this discouraged by the webmachine project. In practice I don't see this happening, but if someone happens to add an AppSignal EventHandler to the middleware stack in `config.ru`, we now support it. Part of #329 --- .changesets/support-nested-webmachine-apps.md | 6 +++++ lib/appsignal/integrations/webmachine.rb | 24 ++++++++++++------- .../appsignal/integrations/webmachine_spec.rb | 20 ++++++++++++++++ 3 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 .changesets/support-nested-webmachine-apps.md diff --git a/.changesets/support-nested-webmachine-apps.md b/.changesets/support-nested-webmachine-apps.md new file mode 100644 index 000000000..ddf1f6cd6 --- /dev/null +++ b/.changesets/support-nested-webmachine-apps.md @@ -0,0 +1,6 @@ +--- +bump: patch +type: add +--- + +Support nested webmachine apps. If webmachine apps are nested in other AppSignal instrumentation it will now report the webmachine instrumentation as part of the parent transaction, reporting more runtime of the request. diff --git a/lib/appsignal/integrations/webmachine.rb b/lib/appsignal/integrations/webmachine.rb index e0c738961..9e92d030f 100644 --- a/lib/appsignal/integrations/webmachine.rb +++ b/lib/appsignal/integrations/webmachine.rb @@ -5,20 +5,26 @@ module Integrations # @api private module WebmachineIntegration def run - transaction = Appsignal::Transaction.create( - SecureRandom.uuid, - Appsignal::Transaction::HTTP_REQUEST, - request, - :params_method => :query - ) - - transaction.set_action_if_nil("#{resource.class.name}##{request.method}") + has_parent_transaction = Appsignal::Transaction.current? + transaction = + if has_parent_transaction + Appsignal::Transaction.current + else + Appsignal::Transaction.create( + SecureRandom.uuid, + Appsignal::Transaction::HTTP_REQUEST, + request + ) + end Appsignal.instrument("process_action.webmachine") do super end + ensure + transaction.set_action_if_nil("#{resource.class.name}##{request.method}") + transaction.set_params_if_nil(request.query) - Appsignal::Transaction.complete_current! + Appsignal::Transaction.complete_current! unless has_parent_transaction end private diff --git a/spec/lib/appsignal/integrations/webmachine_spec.rb b/spec/lib/appsignal/integrations/webmachine_spec.rb index 901f0110e..8ed3531f8 100644 --- a/spec/lib/appsignal/integrations/webmachine_spec.rb +++ b/spec/lib/appsignal/integrations/webmachine_spec.rb @@ -86,6 +86,26 @@ def to_html expect(last_transaction).to be_completed expect(current_transaction?).to be_falsy end + + context "with parent transaction" do + let(:transaction) { http_request_transaction } + before { set_current_transaction(transaction) } + + it "sets the action" do + fsm.run + expect(last_transaction).to have_action("MyResource#GET") + end + + it "sets the params" do + fsm.run + last_transaction._sample + expect(last_transaction).to include_params("param1" => "value1", "param2" => "value2") + end + + it "does not close the transaction" do + expect(last_transaction).to_not be_completed + end + end end describe "#handle_exceptions" do