diff --git a/.changesets/add-config-appsignal-rb-file-support.md b/.changesets/add-config-appsignal-rb-file-support.md new file mode 100644 index 00000000..51437463 --- /dev/null +++ b/.changesets/add-config-appsignal-rb-file-support.md @@ -0,0 +1,19 @@ +--- +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. + +Example `config/appsignal.rb` config file: + +```ruby +# config/appsignal.rb +Appsignal.configure do |config| + config.name = "My app name" +end +``` + +It is not possible to configure multiple environments in the `config/appsignal.rb` config file as it was possible in the YAML version. diff --git a/lib/appsignal.rb b/lib/appsignal.rb index dcf39887..9117f474 100644 --- a/lib/appsignal.rb +++ b/lib/appsignal.rb @@ -92,7 +92,7 @@ def testing? # # @return [void] # @since 0.7.0 - def start # rubocop:disable Metrics/AbcSize + def start # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity, Metrics/MethodLength, Metrics/CyclomaticComplexity if ENV.fetch("_APPSIGNAL_DIAGNOSE", false) internal_logger.info("Skipping start in diagnose context") return @@ -103,6 +103,13 @@ 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 @@ -110,12 +117,35 @@ def start # rubocop:disable Metrics/AbcSize internal_logger.debug("Loading AppSignal gem") - @config ||= Config.new(Config.determine_root_path, Config.determine_env) - @config.validate + context = Appsignal::Config::Context.new( + :env => Config.determine_env, + :root_path => Config.determine_root_path + ) + # If there's a config/appsignal.rb file + if context.dsl_config_file? + if config + # Do not load it if a config is already present + internal_logger.warn( + "Ignoring `#{context.dsl_config_file}` file because " \ + "`Appsignal.configure` has already been called." + ) + else + # Load it when no config is present + load_dsl_config_file(context.dsl_config_file) + # Disable AppSignal on config file error + config[:active] = false if config && config_file_error? + 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 _start_logger - if config.valid? + if config&.valid? if config.active? @started = true internal_logger.info "Starting AppSignal #{Appsignal::VERSION} " \ @@ -244,7 +274,12 @@ 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 @@ -397,6 +432,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 +448,41 @@ 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) + return if defined?(@dsl_config_file_loaded) + + ENV["_APPSIGNAL_CONFIG_FILE_CONTEXT"] = "true" + @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 + ENV.delete("_APPSIGNAL_CONFIG_FILE_CONTEXT") + 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 + + # Returns true if loading the `config/appsignal.rb` file resulted in an + # error. + def config_file_error? + defined?(@config_file_error) ? true : false + end + def start_internal_stdout_logger @internal_logger = Appsignal::Utils::IntegrationLogger.new($stdout) internal_logger.formatter = log_formatter("appsignal") diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb index e2a9cfe8..d3cf1b35 100644 --- a/lib/appsignal/config.rb +++ b/lib/appsignal/config.rb @@ -53,6 +53,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 +64,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 +236,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 +294,10 @@ 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) + end # Load config from environment variables @env_config = load_from_environment diff --git a/spec/lib/appsignal_spec.rb b/spec/lib/appsignal_spec.rb index 222a69c7..84fd742f 100644 --- a/spec/lib/appsignal_spec.rb +++ b/spec/lib/appsignal_spec.rb @@ -373,6 +373,201 @@ def on_load expect(Appsignal.config.env).to eq("env_env") end + + it "reads the config/appsignal.rb file if present" do + test_path = File.join(tmp_dir, "config_file_test_1") + FileUtils.mkdir_p(test_path) + Dir.chdir test_path do + config_contents = + <<~CONFIG + Appsignal.configure do |config| + config.active = false + config.name = "DSL app" + config.push_api_key = "config_file_push_api_key" + config.ignore_actions << "Test" + end + CONFIG + write_file(File.join(test_path, "config", "appsignal.rb"), config_contents) + end + + ENV["APPSIGNAL_APP_PATH"] = test_path + Appsignal.start + + expect(Appsignal.dsl_config_file_loaded?).to be(true) + expect(Appsignal.config.root_path).to eq(test_path) + expect(Appsignal.config[:active]).to be(false) + expect(Appsignal.config[:name]).to eq("DSL app") + expect(Appsignal.config[:push_api_key]).to eq("config_file_push_api_key") + expect(Appsignal.config[:ignore_actions]).to include("Test") + ensure + FileUtils.rm_rf(test_path) + end + + it "ignores calls to Appsignal.start from config/appsignal.rb" do + test_path = File.join(tmp_dir, "config_file_test_2") + FileUtils.mkdir_p(test_path) + Dir.chdir test_path do + config_contents = + <<~CONFIG + Appsignal.configure do |config| + config.active = false + config.name = "DSL app" + end + Appsignal.start + CONFIG + write_file(File.join(test_path, "config", "appsignal.rb"), config_contents) + end + + ENV["APPSIGNAL_APP_PATH"] = test_path + logs = capture_logs { Appsignal.start } + + expect(logs) + .to contains_log(:warn, "Ignoring call to Appsignal.start in config file context.") + expect(Appsignal.dsl_config_file_loaded?).to be(true) + expect(Appsignal.config.root_path).to eq(test_path) + expect(Appsignal.config[:active]).to be(false) + expect(Appsignal.config[:name]).to eq("DSL app") + ensure + FileUtils.rm_rf(test_path) + end + + it "only reads from config/appsignal.rb if it and config/appsignal.yml are present" do + test_path = File.join(tmp_dir, "config_file_test_3") + FileUtils.mkdir_p(test_path) + Dir.chdir test_path do + config_contents = + <<~CONFIG + Appsignal.configure(:test) do |config| + config.active = false + config.name = "DSL app" + config.push_api_key = "config_file_push_api_key" + end + CONFIG + write_file(File.join(test_path, "config", "appsignal.rb"), config_contents) + + yaml_contents = + <<~YAML + test: + active: true + name: "YAML app" + ignore_errors: ["YAML error"] + YAML + write_file(File.join(test_path, "config", "appsignal.yml"), yaml_contents) + end + + ENV["APPSIGNAL_APP_PATH"] = test_path + Appsignal.start + + expect(Appsignal.dsl_config_file_loaded?).to be(true) + expect(Appsignal.config.root_path).to eq(test_path) + expect(Appsignal.config[:active]).to be(false) + expect(Appsignal.config[:name]).to eq("DSL app") + expect(Appsignal.config[:push_api_key]).to eq("config_file_push_api_key") + expect(Appsignal.config[:ignore_errors]).to_not include("YAML error") + ensure + FileUtils.rm_rf(test_path) + end + + it "only reads from config/appsignal.rb even if it's empty" do + test_path = File.join(tmp_dir, "config_file_test_3") + FileUtils.mkdir_p(test_path) + Dir.chdir test_path do + config_contents = "# I am empty!" + write_file(File.join(test_path, "config", "appsignal.rb"), config_contents) + + yaml_contents = + <<~YAML + test: + active: true + name: "YAML app" + ignore_errors: ["YAML error"] + YAML + write_file(File.join(test_path, "config", "appsignal.yml"), yaml_contents) + end + + ENV["APPSIGNAL_APP_PATH"] = test_path + Appsignal.start + + expect(Appsignal.dsl_config_file_loaded?).to be(true) + # No Appsignal.configure was called, so it's misconfigured, but it + # shouldn't fall back on the YAML file. + expect(Appsignal.config).to be_nil + ensure + FileUtils.rm_rf(test_path) + end + + it "options set in config/appsignal.rb are leading" do + test_path = File.join(tmp_dir, "config_file_test_4") + FileUtils.mkdir_p(test_path) + Dir.chdir test_path do + config_contents = + <<~CONFIG + Appsignal.configure(:test) do |config| + config.active = true + config.name = "DSL app" + config.push_api_key = "config_file_push_api_key" + end + CONFIG + write_file(File.join(test_path, "config", "appsignal.rb"), config_contents) + end + + ENV["APPSIGNAL_APP_PATH"] = test_path + # These env vars should not be used as the config option values + ENV["APPSIGNAL_APP_ENV"] = "env_env" + ENV["APPSIGNAL_APP_NAME"] = "env_name" + ENV["APPSIGNAL_PUSH_API_KEY"] = "env_push_api_key" + Appsignal.start + + expect(Appsignal.dsl_config_file_loaded?).to be(true) + expect(Appsignal.config.root_path).to eq(test_path) + expect(Appsignal.config.env).to eq("test") + expect(Appsignal.config[:active]).to be(true) + expect(Appsignal.config[:name]).to eq("DSL app") + expect(Appsignal.config[:push_api_key]).to eq("config_file_push_api_key") + ensure + FileUtils.rm_rf(test_path) + end + + it "doesn't start if config/appsignal.rb raised an error" do + test_path = File.join(tmp_dir, "config_file_test_5") + FileUtils.mkdir_p(test_path) + Dir.chdir test_path do + config_contents = + <<~CONFIG + Appsignal.configure do |config| + config.active = true + config.name = "DSL app" + config.push_api_key = "config_file_push_api_key" + end + raise "uh oh" # Deliberatly crash + CONFIG + write_file(File.join(test_path, "config", "appsignal.rb"), config_contents) + end + + ENV["APPSIGNAL_APP_PATH"] = test_path + err_stream = std_stream + logs = + capture_std_streams(std_stream, err_stream) do + capture_logs do + Appsignal.start + end + end + + message = + "Not starting AppSignal because an error occurred while loading the " \ + "AppSignal config file.\n" \ + "File: \"#{File.join(test_path, "config/appsignal.rb")}\"\n" \ + "RuntimeError: uh oh\n" + expect(logs).to contains_log(:error, message) + expect(err_stream.read).to include("appsignal ERROR: #{message}") + expect(Appsignal.dsl_config_file_loaded?).to be(true) + expect(Appsignal.config.root_path).to eq(test_path) + expect(Appsignal.config[:active]).to be(false) # Disables the config on error + expect(Appsignal.config[:name]).to eq("DSL app") + expect(Appsignal.config[:push_api_key]).to eq("config_file_push_api_key") + ensure + FileUtils.rm_rf(test_path) + end end context "when config is loaded" do @@ -496,6 +691,45 @@ def on_start end end end + + it "doesn't load config/appsignal.rb if Appsignal.configure was called beforehand" do + Appsignal.configure do |config| + config.active = false + config.name = "DSL app" + config.push_api_key = "dsl_push_api_key" + end + + test_path = File.join(tmp_dir, "config_file_test_5") + FileUtils.mkdir_p(test_path) + config_file_path = File.join(test_path, "config", "appsignal.rb") + Dir.chdir test_path do + config_contents = + <<~CONFIG + Appsignal.configure do |config| + config.active = false + config.name = "DSL app" + config.push_api_key = "config_file_push_api_key" + end + CONFIG + write_file(config_file_path, config_contents) + end + + ENV["APPSIGNAL_APP_PATH"] = test_path + logs = capture_logs { Appsignal.start } + + expect(logs).to contains_log( + :warn, + "Ignoring `#{config_file_path}` file because " \ + "`Appsignal.configure` has already been called." + ) + expect(Appsignal.dsl_config_file_loaded?).to be(false) + expect(Appsignal.config.root_path).to eq(project_fixture_path) + expect(Appsignal.config[:active]).to be(false) + expect(Appsignal.config[:name]).to eq("DSL app") + expect(Appsignal.config[:push_api_key]).to eq("dsl_push_api_key") + ensure + FileUtils.rm_rf(test_path) + end end context "when already started" do diff --git a/spec/support/helpers/directory_helper.rb b/spec/support/helpers/directory_helper.rb index 109e8e8a..2380b2fc 100644 --- a/spec/support/helpers/directory_helper.rb +++ b/spec/support/helpers/directory_helper.rb @@ -17,6 +17,12 @@ def tmp_dir @tmp_dir ||= File.join(spec_dir, "tmp") end + def write_file(path, contents) + parent_dir = File.dirname(path) + FileUtils.mkdir_p(parent_dir) + File.write(path, contents) + end + def fixtures_dir @fixtures_dir ||= File.join(support_dir, "fixtures") end diff --git a/spec/support/testing.rb b/spec/support/testing.rb index 8fbb184e..3e980a64 100644 --- a/spec/support/testing.rb +++ b/spec/support/testing.rb @@ -24,6 +24,8 @@ def clear_config! # @api private def clear! + remove_instance_variable(:@dsl_config_file_loaded) if defined?(@dsl_config_file_loaded) + remove_instance_variable(:@config_file_error) if defined?(@config_file_error) Appsignal.internal_logger = nil clear_started!