Merge pull request #1324 from appsignal/config-rb-file

Add config/appsignal.rb config file

.changesets/add-config-appsignal-rb-file-support.md

@@ -0,0 +1,32 @@
+---
+bump: minor
+type: add
+---
+
+Add `config/appsignal.rb` config file support. When a `config/appsignal.rb` file is present in the app, the Ruby gem will automatically load it when `Appsignal.start` is called.
+
+The `config/appsignal.rb` config file is a replacement for the `config/appsignal.yml` config file. When both files are present, only the `config/appsignal.rb` config file is loaded when the configuration file is automatically loaded by AppSignal  when the configuration file is automatically loaded by AppSignal.
+
+Example `config/appsignal.rb` config file:
+
+```ruby
+# config/appsignal.rb
+Appsignal.configure do |config|
+  config.name = "My app name"
+end
+```
+
+To configure different option values for environments in the `config/appsignal.rb` config file, use if-statements:
+
+```ruby
+# config/appsignal.rb
+Appsignal.configure do |config|
+  config.name = "My app name"
+  if config.env == "production"
+    config.ignore_actions << "My production action"
+  end
+  if config.env == "staging"
+    config.ignore_actions << "My staging action"
+  end
+end
+```

lib/appsignal.rb

@@ -103,16 +103,21 @@ def start # rubocop:disable Metrics/AbcSize
         return
       end
 
+      if config_file_context?
+        internal_logger.warn(
+          "Ignoring call to Appsignal.start in config file context."
+        )
+        return
+      end
+
       unless extension_loaded?
         internal_logger.info("Not starting AppSignal, extension is not loaded")
         return
       end
 
       internal_logger.debug("Loading AppSignal gem")
 
-      @config ||= Config.new(Config.determine_root_path, Config.determine_env)
-      @config.validate
-
+      _load_config!
       _start_logger
 
       if config.valid?
@@ -142,6 +147,41 @@ def start # rubocop:disable Metrics/AbcSize
       end
     end
 
+    # PRIVATE METHOD. DO NOT USE.
+    #
+    # @param env_var [String, NilClass] Used by diagnose CLI to pass through
+    #   the environment CLI option value.
+    # @api private
+    def _load_config!(env_param = nil)
+      context = Appsignal::Config::Context.new(
+        :env => Config.determine_env(env_param),
+        :root_path => Config.determine_root_path
+      )
+      # If there's a config/appsignal.rb file
+      if context.dsl_config_file?
+        if config
+          # When calling `Appsignal.configure` from an app, not the
+          # `config/appsignal.rb` file, with also a Ruby config file present.
+          message = "The `Appsignal.configure` helper is called from within an " \
+            "app while a `#{context.dsl_config_file}` file is present. " \
+            "The `config/appsignal.rb` file is ignored when the " \
+            "config is loaded with `Appsignal.configure` from within an app. " \
+            "We recommend moving all config to the `config/appsignal.rb` file " \
+            "or the `Appsignal.configure` helper in the app."
+          Appsignal::Utils::StdoutAndLoggerMessage.warning(message)
+        else
+          # Load it when no config is present
+          load_dsl_config_file(context.dsl_config_file, env_param)
+        end
+      else
+        # Load config if no config file was found and no config is present yet
+        # This will load the config/appsignal.yml file automatically
+        @config ||= Config.new(context.root_path, context.env)
+      end
+      # Validate the config, if present
+      config&.validate
+    end
+
     # Stop AppSignal's agent.
     #
     # Stops the AppSignal agent. Call this before the end of your program to
@@ -244,10 +284,28 @@ def configure(env_param = nil, root_path: nil)
       else
         @config = Config.new(
           root_path_param || Config.determine_root_path,
-          Config.determine_env(env_param)
+          Config.determine_env(env_param),
+          # If in the context of an `config/appsignal.rb` config file, do not
+          # load the `config/appsignal.yml` file.
+          # The `.rb` file is a replacement for the `.yml` file so it shouldn't
+          # load both.
+          :load_yaml_file => !config_file_context?
         )
       end
 
+      # When calling `Appsignal.configure` from a Rails initializer and a YAML
+      # file is present. We will not load the YAML file in the future.
+      if !config_file_context? && config.yml_config_file?
+        message = "The `Appsignal.configure` helper is called while a " \
+          "`config/appsignal.yml` file is present. In future versions the " \
+          "`config/appsignal.yml` file will be ignored when loading the " \
+          "config. We recommend moving all config to the " \
+          "`config/appsignal.rb` file, or the `Appsignal.configure` helper " \
+          "in Rails initializer file, and remove the " \
+          "`config/appsignal.yml` file."
+        Appsignal::Utils::StdoutAndLoggerMessage.warning(message)
+      end
+
       config_dsl = Appsignal::Config::ConfigDSL.new(config)
       return unless block_given?
 
@@ -397,6 +455,11 @@ def active?
       config&.active? && extension_loaded?
     end
 
+    # @api private
+    def dsl_config_file_loaded?
+      defined?(@dsl_config_file_loaded) ? true : false
+    end
+
     private
 
     def params_match_loaded_config?(env_param, root_path_param)
@@ -408,6 +471,52 @@ def params_match_loaded_config?(env_param, root_path_param)
         (root_path_param.nil? || config.root_path == root_path_param)
     end
 
+    # Load the `config/appsignal.rb` config file, if present.
+    #
+    # If the config file has already been loaded once and it's trying to be
+    # loaded more than once, which should never happen, it will not do
+    # anything.
+    def load_dsl_config_file(path, env_param = nil)
+      return if defined?(@dsl_config_file_loaded)
+
+      begin
+        ENV["_APPSIGNAL_CONFIG_FILE_CONTEXT"] = "true"
+        ENV["_APPSIGNAL_CONFIG_FILE_ENV"] = env_param if env_param
+        @dsl_config_file_loaded = true
+        require path
+      rescue => error
+        @config_file_error = error
+        message = "Not starting AppSignal because an error occurred while " \
+          "loading the AppSignal config file.\n" \
+          "File: #{path.inspect}\n" \
+          "#{error.class.name}: #{error}"
+        Kernel.warn "appsignal ERROR: #{message}"
+        internal_logger.error "#{message}\n#{error.backtrace.join("\n")}"
+      ensure
+        unless Appsignal.config
+          # Ensure _a config object_ is present, even if something went wrong
+          # loading it or the file is empty. In this config file context, see
+          # the context env vars, it will intentionally not load the YAML file.
+          Appsignal.configure
+
+          # Disable if no config was loaded from the file but it is present
+          config[:active] = false
+        end
+
+        # Disable on config file error
+        config[:active] = false if defined?(@config_file_error)
+
+        ENV.delete("_APPSIGNAL_CONFIG_FILE_CONTEXT")
+        ENV.delete("_APPSIGNAL_CONFIG_FILE_ENV")
+      end
+    end
+
+    # Returns true if we're currently in the `config/appsignal.rb` file
+    # context.
+    def config_file_context?
+      ENV.fetch("_APPSIGNAL_CONFIG_FILE_CONTEXT", nil) == "true"
+    end
+
     def start_internal_stdout_logger
       @internal_logger = Appsignal::Utils::IntegrationLogger.new($stdout)
       internal_logger.formatter = log_formatter("appsignal")

lib/appsignal/cli/diagnose.rb

@@ -188,15 +188,16 @@ def puts_format(label, value, options = {})
         end
 
         def configure_appsignal(options)
+          env_option = options.fetch(:environment, nil)
           # Try and load the Rails app, if any.
           # This will configure AppSignal through the config file or an
           # initializer.
-          require_rails_app_if_present
+          require_rails_app_if_present(env_option)
 
-          # If no config was found by loading the app, load with the defaults.
-          Appsignal.configure(options.fetch(:environment, nil))
-          Appsignal.config.write_to_environment
+          # No config loaded yet, try loading as normal
+          Appsignal._load_config!(env_option) unless Appsignal.config
           Appsignal._start_logger
+          Appsignal.config.write_to_environment
           Appsignal.internal_logger.info("Starting AppSignal diagnose")
         end
 
@@ -631,9 +632,12 @@ def print_empty_line
           puts "\n"
         end
 
-        def require_rails_app_if_present
+        def require_rails_app_if_present(env_option)
           return unless rails_present?
 
+          # Set the environment given as an option to the diagnose CLI so the
+          # Rails app uses it when loaded.
+          ENV["_APPSIGNAL_CONFIG_FILE_ENV"] = env_option
           # Mark app as Rails app
           data[:app][:rails] = true
           # Manually require the railtie, because it wasn't loaded when the CLI
@@ -649,6 +653,8 @@ def require_rails_app_if_present
           puts error.backtrace
           data[:app][:load_error] =
             "#{error.class}: #{error.message}\n#{error.backtrace.join("\n")}"
+        ensure
+          ENV.delete("_APPSIGNAL_CONFIG_FILE_ENV")
         end
 
         def rails_present?

lib/appsignal/config.rb

@@ -35,6 +35,7 @@ def self.add_loader_defaults(name, env: nil, root_path: nil, **options)
     def self.determine_env(initial_env = nil)
       [
         initial_env,
+        ENV.fetch("_APPSIGNAL_CONFIG_FILE_ENV", nil), # PRIVATE ENV var used by the diagnose CLI
         ENV.fetch("APPSIGNAL_APP_ENV", nil),
         ENV.fetch("RAILS_ENV", nil),
         ENV.fetch("RACK_ENV", nil)
@@ -53,6 +54,9 @@ def self.determine_env(initial_env = nil)
     # Determine which root path AppSignal should initialize with.
     # @api private
     def self.determine_root_path
+      app_path_env_var = ENV.fetch("APPSIGNAL_APP_PATH", nil)
+      return app_path_env_var if app_path_env_var
+
       loader_defaults.reverse.each do |loader_defaults|
         root_path = loader_defaults[:root_path]
         return root_path if root_path
@@ -61,6 +65,26 @@ def self.determine_root_path
       Dir.pwd
     end
 
+    # @api private
+    class Context
+      DSL_FILENAME = "config/appsignal.rb"
+
+      attr_reader :env, :root_path
+
+      def initialize(env: nil, root_path: nil)
+        @env = env
+        @root_path = root_path
+      end
+
+      def dsl_config_file
+        File.join(root_path, DSL_FILENAME)
+      end
+
+      def dsl_config_file?
+        File.exist?(dsl_config_file)
+      end
+    end
+
     # @api private
     DEFAULT_CONFIG = {
       :activejob_report_errors => "all",
@@ -213,8 +237,10 @@ def self.determine_root_path
     #   How to integrate AppSignal manually
     def initialize(
       root_path,
-      env
+      env,
+      load_yaml_file: true
     )
+      @load_yaml_file = load_yaml_file
       @root_path = root_path.to_s
       @config_file_error = false
       @config_file = config_file
@@ -269,8 +295,20 @@ def load_config
       @initial_config[:env] = @env
 
       # Load the config file if it exists
-      @file_config = load_from_disk || {}
-      merge(file_config)
+      if @load_yaml_file
+        @file_config = load_from_disk || {}
+        merge(file_config)
+      elsif yml_config_file?
+        # When in a `config/appsignal.rb` file and it detects a
+        # `config/appsignal.yml` file.
+        # Only logged and printed on `Appsignal.start`.
+        message = "Both a Ruby and YAML configuration file are found. " \
+          "The `config/appsignal.yml` file is ignored when the " \
+          "config is loaded from `config/appsignal.rb`. Move all config to " \
+          "the `config/appsignal.rb` file and remove the " \
+          "`config/appsignal.yml` file."
+        Appsignal::Utils::StdoutAndLoggerMessage.warning(message)
+      end
 
       # Load config from environment variables
       @env_config = load_from_environment
@@ -435,6 +473,13 @@ def freeze
       config_hash.transform_values(&:freeze)
     end
 
+    # @api private
+    def yml_config_file?
+      return false unless config_file
+
+      File.exist?(config_file)
+    end
+
     private
 
     def logger
@@ -458,7 +503,7 @@ def detect_from_system
     end
 
     def load_from_disk
-      return if !config_file || !File.exist?(config_file)
+      return unless yml_config_file?
 
       read_options = YAML::VERSION >= "4.0.0" ? { :aliases => true } : {}
       configurations = YAML.load(ERB.new(File.read(config_file)).result, **read_options)