From e19e613a97b53901c84d7b77635dca9e384e6015 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Wed, 3 Jul 2024 11:19:01 +0200 Subject: [PATCH] Fix Padrino instrumentation causing boot loop Apply the same boot loop fix as for Sinatra for Padrino from PR #1105. For Sinatra apps noticed in our test setup that when I added a Sinatra app to the Rails app, and loaded the Sinatra instrumentation as specified in our docs it would get stuck in a boot loop. This was caused by the two different configs overwriting each other every time. Apply the same fix for Padrino by checking first if AppSignal has already started and skipping the auto start. --- .changesets/padrino-boot-loop.md | 7 +++++ lib/appsignal/integrations/padrino.rb | 8 +++-- .../appsignal/integrations/padrino_spec.rb | 31 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 .changesets/padrino-boot-loop.md diff --git a/.changesets/padrino-boot-loop.md b/.changesets/padrino-boot-loop.md new file mode 100644 index 000000000..e3433cc8d --- /dev/null +++ b/.changesets/padrino-boot-loop.md @@ -0,0 +1,7 @@ +--- +bump: patch +type: fix +--- + +Fix issue with AppSignal getting stuck in a boot loop when loading the Padrino integration with: `require "appsignal/integrations/padrino"` +This could happen in nested applications, like a Padrino app in a Rails app. AppSignal will now use the first config AppSignal starts with. diff --git a/lib/appsignal/integrations/padrino.rb b/lib/appsignal/integrations/padrino.rb index 0dce5dc9f..748989ded 100644 --- a/lib/appsignal/integrations/padrino.rb +++ b/lib/appsignal/integrations/padrino.rb @@ -13,9 +13,11 @@ def self.init Padrino.before_load do Appsignal.internal_logger.debug("Loading Padrino (#{Padrino::VERSION}) integration") - root = Padrino.mounted_root - Appsignal.config = Appsignal::Config.new(root, Padrino.env) - Appsignal.start + unless Appsignal.active? + root = Padrino.mounted_root + Appsignal.config = Appsignal::Config.new(root, Padrino.env) + Appsignal.start + end next unless Appsignal.active? diff --git a/spec/lib/appsignal/integrations/padrino_spec.rb b/spec/lib/appsignal/integrations/padrino_spec.rb index e04cd18eb..8c7c7a017 100644 --- a/spec/lib/appsignal/integrations/padrino_spec.rb +++ b/spec/lib/appsignal/integrations/padrino_spec.rb @@ -24,6 +24,37 @@ def uninstall_padrino_integration end end + context "when already active" do + before { allow(Appsignal).to receive(:active?).and_return(true) } + + it "does not start AppSignal again" do + expect(Appsignal::Config).to_not receive(:new) + expect(Appsignal).to_not receive(:start) + + Appsignal::Integrations::PadrinoPlugin.init + callbacks[:before_load].call + end + + it "adds the instrumentation middleware to Sinatra::Base" do + Appsignal::Integrations::PadrinoPlugin.init + callbacks[:before_load].call + + middlewares = Padrino.middleware + expect(middlewares).to include( + [Rack::Events, [[instance_of(Appsignal::Rack::EventHandler)]], nil] + ) + expect(middlewares).to include( + [ + Appsignal::Rack::SinatraBaseInstrumentation, + [ + :instrument_span_name => "process_action.padrino" + ], + nil + ] + ) + end + end + context "with active config" do before do ENV["APPSIGNAL_APP_NAME"] = "My Padrino app name"