Add loader mechanism for integrations

For those integrations that require apps to manually require the AppSignal integration, introduce a new loader mechanism. Instead requiring a `require "appsignal/integrations/sinatra"` file, call `Appsignal.load(:sinatra)`. The "appsignal/integrations/sinatra" does too much. It creates a config, starts AppSignal and adds instrumentation to the library (Sinatra). When apps are composed of multiple frameworks, like Rails, Sinatra, Grape, Padrino, these integration file requires overwrite the previously set config and starts AppSignal multiple times. This can cause boot loop issues, which I've addressed before to a point. This mechanism will work better because it splits things up. ```ruby # Before require "appsignal/integrations/sinatra" # After require "appsignal" Appsignal.load(:sinatra) Appsignal.start ``` This will allow apps to load the integrations for multiple libraries, like Sinatra and Padrino, and add custom config between the integrations being loaded, applying their default config, and AppSignal being started. ```ruby # Before require "appsignal/integrations/sinatra" # After require "appsignal" Appsignal.load(:sinatra) Appsignal.load(:padrino) Appsignal.config = Appsignal::Config.new(...) Appsignal.start ``` The loader and initial configs are a bit mixed in order what is applied first. That's because the initial config should be deprecated. The loader default's env and root path is applied after the initial config's env and root path. The config options are applied before the initial config for now. This change does not send the loader config source in the diagnose report, as those are set in the app, and the diagnose CLI command doesn't load the app, so those would always be empty.
appsignal · Jul 17, 2024 · 35fff8c · 35fff8c
1 parent 03731e5
commit 35fff8c
Show file tree

Hide file tree

Showing 27 changed files with 1,116 additions and 747 deletions.
diff --git a/.changesets/update-sinatra-integration-setup.md b/.changesets/update-sinatra-integration-setup.md
@@ -0,0 +1,31 @@
+---
+bump: minor
+type: change
+---
+
+Update the Sinatra, Padrino, Grape and Hanami integration setup for applications. Before this change a "appsignal/integrations/sinatra" file would need to be required to load the AppSignal integration for Sinatra. Similar requires exist for other libraries. This has changed to a new DSL.
+
+This new DSL makes starting AppSignal more predictable when loading multiple integrations, like those for Sinatra, Padrino, Grape and Hanami.
+
+```ruby
+# Sinatra example
+# Before
+require "appsignal/integrations/sinatra"
+
+# After
+require "appsignal"
+
+Appsignal.load(:sinatra)
+Appsignal.start
+```
+
+The `require "appsignal/integrations/sinatra"` will still work, but is deprecated in this release.
+
+See the documentation for the specific libraries for the latest on how to integrate AppSignal.
+
+- [Grape](https://docs.appsignal.com/ruby/integrations/grape.html)
+- [Hanami](https://docs.appsignal.com/ruby/integrations/hanami.html)
+- [Padrino](https://docs.appsignal.com/ruby/integrations/padrino.html)
+- [Sinatra](https://docs.appsignal.com/ruby/integrations/sinatra.html)
+
+When using a combination of the libraries listed above, read our [integration guide](https://docs.appsignal.com/ruby/instrumentation/integrating-appsignal.html) on how to load and configure AppSignal for multiple integrations at once.
diff --git a/lib/appsignal.rb b/lib/appsignal.rb
@@ -114,6 +114,7 @@ def start
           config.write_to_environment
           Appsignal::Extension.start
           Appsignal::Hooks.load_hooks
+          Appsignal::Loaders.start
 
           if config[:enable_allocation_tracking] && !Appsignal::System.jruby?
             Appsignal::Extension.install_allocation_event_hook
@@ -163,6 +164,38 @@ def forked
       Appsignal::Extension.start
     end
 
+    # Load an AppSignal integration.
+    #
+    # Load one of the supported integrations via our loader system.
+    # This will set config defaults and integratie with the library if
+    # AppSignal is active upon start.
+    #
+    # @example Load Sinatra integrations
+    #   # First load the integration
+    #   Appsignal.load(:sinatra)
+    #   # Start AppSignal
+    #   Appsignal.start
+    #
+    # @example Load Sinatra integrations and define custom config
+    #   # First load the integration
+    #   Appsignal.load(:sinatra)
+    #
+    #   # Customize config
+    #   Appsignal.configure do |config|
+    #     config.ignore_actions = ["GET /ping"]
+    #   end
+    #
+    #
+    #   # Start AppSignal
+    #   Appsignal.start
+    #
+    # @param integration_name [String, Symbol] Name of the integration to load.
+    # @return [void]
+    # @since 3.12.0
+    def load(integration_name)
+      Loaders.load(integration_name)
+    end
+
     # @api private
     def get_server_state(key)
       Appsignal::Extension.get_server_state(key)
@@ -314,6 +347,7 @@ def const_missing(name)
   end
 end
 
+require "appsignal/loaders"
 require "appsignal/environment"
 require "appsignal/system"
 require "appsignal/utils"

diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb
@@ -10,6 +10,16 @@ module Appsignal
   class Config
     include Appsignal::Utils::StdoutAndLoggerMessage
 
+    # @api private
+    def self.loader_defaults
+      @loader_defaults ||= []
+    end
+
+    # @api private
+    def self.add_loader_defaults(name, options)
+      loader_defaults << [name, options]
+    end
+
     # @api private
     DEFAULT_CONFIG = {
       :activejob_report_errors => "all",
@@ -223,6 +233,21 @@ def initialize(
       # Set config based on the system
       @system_config = detect_from_system
       merge(system_config)
+
+      # Set defaults from loaders in reverse order so the first register
+      # loader's defaults overwrite all others
+      self.class.loader_defaults.reverse.each do |(_loader_name, loader_defaults)|
+        defaults = loader_defaults.compact.dup
+        # Overwrite root path
+        loader_path = defaults.delete(:root_path)
+        @root_path = loader_path if loader_path
+        # Overwrite env
+        loader_env = defaults.delete(:env)
+        @env = loader_env.to_s if loader_env
+        # Merge with the config loaded so far
+        merge(defaults)
+      end
+
       # Initial config
       @initial_config = initial_config
       merge(initial_config)

diff --git a/lib/appsignal/integrations/grape.rb b/lib/appsignal/integrations/grape.rb
@@ -3,6 +3,13 @@
 require "appsignal"
 require "appsignal/rack/grape_middleware"
 
+Appsignal::Utils::StdoutAndLoggerMessage.warning(
+  "The 'require \"appsignal/integrations/grape\"' file require integration " \
+    "method is deprecated. " \
+    "Please follow the Grape setup guide in our docs for the new method: " \
+    "https://docs.appsignal.com/ruby/integrations/grape.html"
+)
+
 Appsignal.internal_logger.debug("Loading Grape integration")
 
 module Appsignal

diff --git a/lib/appsignal/integrations/hanami.rb b/lib/appsignal/integrations/hanami.rb
@@ -1,48 +1,13 @@
 # frozen_string_literal: true
 
 require "appsignal"
-require "appsignal/rack/hanami_middleware"
 
-module Appsignal
-  module Integrations
-    # @api private
-    module HanamiPlugin
-      def self.init
-        Appsignal.internal_logger.debug("Loading Hanami integration")
+Appsignal::Utils::StdoutAndLoggerMessage.warning(
+  "The 'require \"appsignal/integrations/hanami\"' file require integration " \
+    "method is deprecated. " \
+    "Please follow the Hanami setup guide in our docs for the new method: " \
+    "https://docs.appsignal.com/ruby/integrations/hanami.html"
+)
 
-        hanami_app_config = ::Hanami.app.config
-
-        unless Appsignal.active?
-          Appsignal.config = Appsignal::Config.new(
-            hanami_app_config.root || Dir.pwd,
-            hanami_app_config.env
-          )
-          Appsignal.start
-        end
-
-        return unless Appsignal.active?
-
-        hanami_app_config.middleware.use(
-          ::Rack::Events,
-          [Appsignal::Rack::EventHandler.new]
-        )
-        hanami_app_config.middleware.use(Appsignal::Rack::HanamiMiddleware)
-
-        ::Hanami::Action.prepend Appsignal::Integrations::HanamiIntegration
-      end
-    end
-
-    # @api private
-    module HanamiIntegration
-      def call(env)
-        super
-      ensure
-        transaction = env[::Appsignal::Rack::APPSIGNAL_TRANSACTION]
-
-        transaction&.set_action_if_nil(self.class.name)
-      end
-    end
-  end
-end
-
-Appsignal::Integrations::HanamiPlugin.init unless Appsignal.testing?
+Appsignal.load(:hanami)
+Appsignal.start
diff --git a/lib/appsignal/integrations/padrino.rb b/lib/appsignal/integrations/padrino.rb
@@ -1,78 +1,13 @@
 # frozen_string_literal: true
 
 require "appsignal"
-require "appsignal/rack/sinatra_instrumentation"
 
-module Appsignal
-  module Integrations
-    # @api private
-    module PadrinoPlugin
-      def self.init
-        Padrino::Application.prepend Appsignal::Integrations::PadrinoIntegration
+Appsignal::Utils::StdoutAndLoggerMessage.warning(
+  "The 'require \"appsignal/integrations/padrino\"' file require integration " \
+    "method is deprecated. " \
+    "Please follow the Padrino setup guide in our docs for the new method: " \
+    "https://docs.appsignal.com/ruby/integrations/padrino.html"
+)
 
-        Padrino.before_load do
-          Appsignal.internal_logger.debug("Loading Padrino (#{Padrino::VERSION}) integration")
-
-          unless Appsignal.active?
-            root = Padrino.mounted_root
-            Appsignal.config = Appsignal::Config.new(root, Padrino.env)
-            Appsignal.start
-          end
-
-          next unless Appsignal.active?
-
-          Padrino.use ::Rack::Events, [Appsignal::Rack::EventHandler.new]
-          Padrino.use Appsignal::Rack::SinatraBaseInstrumentation,
-            :instrument_event_name => "process_action.padrino"
-        end
-      end
-    end
-  end
-end
-
-module Appsignal
-  module Integrations
-    # @api private
-    module PadrinoIntegration
-      def route!(base = settings, pass_block = nil)
-        return super if !Appsignal.active? || env["sinatra.static_file"]
-
-        begin
-          super
-        ensure
-          transaction = Appsignal::Transaction.current
-          transaction.set_action_if_nil(get_payload_action(request))
-        end
-      end
-
-      private
-
-      def get_payload_action(request)
-        # Short-circuit is there's no request object to obtain information from
-        return settings.name.to_s unless request
-
-        # Newer versions expose the action / controller on the request class.
-        # Newer versions also still expose a route_obj so we must prioritize the
-        # action/fullpath methods.
-        # The `request.action` and `request.controller` values are `nil` when a
-        # endpoint is not found, `""` if not specified by the user.
-        controller_name = request.controller if request.respond_to?(:controller)
-        action_name = request.action if request.respond_to?(:action)
-        action_name ||= ""
-
-        return "#{settings.name}:#{controller_name}##{action_name}" unless action_name.empty?
-
-        # Older versions of Padrino work with a route object
-        if request.respond_to?(:route_obj) && request.route_obj
-          return "#{settings.name}:#{request.route_obj.original_path}"
-        end
-
-        # Fall back to the application name if we haven't found an action name in
-        # any previous methods.
-        "#{settings.name}#unknown"
-      end
-    end
-  end
-end
-
-Appsignal::Integrations::PadrinoPlugin.init
+Appsignal.load(:padrino)
+Appsignal.start
diff --git a/lib/appsignal/integrations/sinatra.rb b/lib/appsignal/integrations/sinatra.rb
@@ -1,24 +1,13 @@
 # frozen_string_literal: true
 
 require "appsignal"
-require "appsignal/rack/sinatra_instrumentation"
 
-Appsignal.internal_logger.debug("Loading Sinatra (#{Sinatra::VERSION}) integration")
+Appsignal::Utils::StdoutAndLoggerMessage.warning(
+  "The 'require \"appsignal/integrations/sinatra\"' file require integration " \
+    "method is deprecated. " \
+    "Please follow the Sinatra setup guide in our docs for the new method: " \
+    "https://docs.appsignal.com/ruby/integrations/sinatra.html"
+)
 
-unless Appsignal.active?
-  app_settings = ::Sinatra::Application.settings
-  Appsignal.config = Appsignal::Config.new(
-    app_settings.root || Dir.pwd,
-    app_settings.environment
-  )
-
-  Appsignal.start
-end
-
-if Appsignal.active?
-  ::Sinatra::Base.use(
-    ::Rack::Events,
-    [Appsignal::Rack::EventHandler.new]
-  )
-  ::Sinatra::Base.use(Appsignal::Rack::SinatraBaseInstrumentation)
-end
+Appsignal.load(:sinatra)
+Appsignal.start