From f81248bc9c557197a0dd123c6d5958f21c11af9d Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Thu, 31 Oct 2024 14:32:04 +0100 Subject: [PATCH 1/5] Add config/appsignal.rb config file Add `config/appsignal.rb` config file support. This is a replacement for the `config/appsignal.yml` config file. Load the `config/appsignal.rb` file automatically on `Appsignal.start`. For most apps, like Rails apps, this will work the same way as `config/appsignal.yml` does now. If `Appsignal.configure` is called manually in the app, like a Rails initializer, the `config/appsignal.rb` file is not loaded. Instead, we recommend moving the `Appsignal.configure` call to the `config/appsignal.rb` file. ## New behavior With this change the config is loaded as described in these scenarios: - Scenario 1: - `Appsignal.start` is called. - No config is loaded yet. - The `config/appsignal.rb` file is present. - The `config/appsignal.rb` is required and configures the gem. - Scenario 2: - `Appsignal.start` is called. - No config is loaded yet. - The `config/appsignal.rb` file is present. - The `config/appsignal.yml` file is present. - The `config/appsignal.rb` is required and configures the gem using the `Appsignal.configure` helper. - The `config/appsignal.yml` is not loaded. - Scenario 3: - `Appsignal.configure` is called. - The `config/appsignal.rb` file is present. - The `config/appsignal.rb` file is not loaded. - `Appsignal.start` is called. - Scenario 4: - `Appsignal.configure` is called. - The `config/appsignal.yml` file is present. - The `config/appsignal.yml` file is loaded. - `Appsignal.start` is called. This is more complex that I would have liked. In hindsight I shouldn't have made `Appsignal.configure` load the `config/appsignal.yml` file. That would have made it a bit more predictable. But I don't want `Appsignal.configure` in the `config/appsignal.rb` file to also load the `config/appsignal.yml` file, because then it just gets more confusing. In the next major version I want to remove `config/appsignal.yml` support, so this is the first step. ## App/root path I remember some of our users configured the Config class's `root_path` to specify where to load the config file from. If an app configures itself via a `config/appsignal.rb`, this makes it impossible to customize the root path. For that purpose I've added the `APPSIGNAL_APP_PATH` environment variable to customize the config path. Which I've also used to make it easier to test the different scenarios. --- Part of #1324 --- .../add-config-appsignal-rb-file-support.md | 19 ++ lib/appsignal.rb | 85 ++++++- lib/appsignal/config.rb | 33 ++- spec/lib/appsignal_spec.rb | 234 ++++++++++++++++++ spec/support/helpers/directory_helper.rb | 6 + spec/support/testing.rb | 2 + 6 files changed, 371 insertions(+), 8 deletions(-) create mode 100644 .changesets/add-config-appsignal-rb-file-support.md 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 000000000..514374630 --- /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 dcf39887c..9117f474c 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 e2a9cfe85..d3cf1b35d 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 222a69c78..84fd742fc 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 109e8e8ac..2380b2fcc 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 8fbb184e7..3e980a64c 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! From 5c45e9786307db8d2bae7364ad2fb74cd4cb0552 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Fri, 1 Nov 2024 17:28:53 +0100 Subject: [PATCH 2/5] Move config load logic to its own method Add a private method (that has to be publicly accessible within the gem itself), to load the config. This avoids duplication in the diagnose CLI when I update it to call this method as well. Make it more safe by replicating the existing behavior: it will never return a `nil` object for the Config once started. If it started with an error or the config file is empty, initialize the config but set `active` to `false`. --- lib/appsignal.rb | 109 +++++++++++++++++++++---------------- spec/lib/appsignal_spec.rb | 4 +- 2 files changed, 65 insertions(+), 48 deletions(-) diff --git a/lib/appsignal.rb b/lib/appsignal.rb index 9117f474c..fd6656cd0 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, Metrics/PerceivedComplexity, Metrics/MethodLength, Metrics/CyclomaticComplexity + def start # rubocop:disable Metrics/AbcSize if ENV.fetch("_APPSIGNAL_DIAGNOSE", false) internal_logger.info("Skipping start in diagnose context") return @@ -117,35 +117,10 @@ def start # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity, Metric internal_logger.debug("Loading AppSignal gem") - 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 - + _load_config! _start_logger - if config&.valid? + if config.valid? if config.active? @started = true internal_logger.info "Starting AppSignal #{Appsignal::VERSION} " \ @@ -172,6 +147,37 @@ def start # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity, Metric 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 + # 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) + 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 @@ -456,19 +462,34 @@ def params_match_loaded_config?(env_param, root_path_param) 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") + begin + 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 + 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") + end end # Returns true if we're currently in the `config/appsignal.rb` file @@ -477,12 +498,6 @@ 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/spec/lib/appsignal_spec.rb b/spec/lib/appsignal_spec.rb index 84fd742fc..a3a1e2afb 100644 --- a/spec/lib/appsignal_spec.rb +++ b/spec/lib/appsignal_spec.rb @@ -491,7 +491,9 @@ def on_load 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 + expect(Appsignal.config[:active]).to be(false) + expect(Appsignal.config[:name]).to be_nil + expect(Appsignal.config[:ignore_errors]).to be_empty ensure FileUtils.rm_rf(test_path) end From 59f8283b4c7aa16833cdfca2e5e0e6bc02ed237a Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Fri, 1 Nov 2024 17:32:25 +0100 Subject: [PATCH 3/5] Load config/appsignal.rb in diagnose CLI Update the diagnose CLI to call the new `Appsignal._load_config!` PRIVATE method to initialize the config. This way we have less duplication of how the config is initialized and we can be sure it works the same in the diagnose CLI as well. I've introduced a new PRIVATE `_APPSIGNAL_CONFIG_FILE_ENV` env var to set the environment, but only in the diagnose CLI context. It's one of the few ways we can pass the default environment as set by the CLI to whatever config object gets initialized in the config file. I didn't use the `APPSIGNAL_APP_ENV` environment variable so the config option doesn't show up in the report as being set by an env var or overwrite it when the user has set it. --- lib/appsignal.rb | 6 +- lib/appsignal/cli/diagnose.rb | 16 ++- lib/appsignal/config.rb | 1 + spec/lib/appsignal/cli/diagnose_spec.rb | 110 +++++++++++++++++- .../config/environment.rb | 5 - .../config/application.rb | 16 +++ .../config/appsignal.rb | 6 + .../config/environment.rb | 5 + .../log/.gitkeep | 0 spec/support/helpers/config_helpers.rb | 7 ++ 10 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/application.rb create mode 100644 spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/appsignal.rb create mode 100644 spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/environment.rb create mode 100644 spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/log/.gitkeep diff --git a/lib/appsignal.rb b/lib/appsignal.rb index fd6656cd0..cb2ebf913 100644 --- a/lib/appsignal.rb +++ b/lib/appsignal.rb @@ -167,7 +167,7 @@ def _load_config!(env_param = nil) ) else # Load it when no config is present - load_dsl_config_file(context.dsl_config_file) + 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 @@ -459,11 +459,12 @@ def params_match_loaded_config?(env_param, root_path_param) # 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) + 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 @@ -489,6 +490,7 @@ def load_dsl_config_file(path) config[:active] = false if defined?(@config_file_error) ENV.delete("_APPSIGNAL_CONFIG_FILE_CONTEXT") + ENV.delete("_APPSIGNAL_CONFIG_FILE_ENV") end end diff --git a/lib/appsignal/cli/diagnose.rb b/lib/appsignal/cli/diagnose.rb index a27812431..658a4be84 100644 --- a/lib/appsignal/cli/diagnose.rb +++ b/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? diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb index d3cf1b35d..e343804ce 100644 --- a/lib/appsignal/config.rb +++ b/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) diff --git a/spec/lib/appsignal/cli/diagnose_spec.rb b/spec/lib/appsignal/cli/diagnose_spec.rb index d2236c873..049516b23 100644 --- a/spec/lib/appsignal/cli/diagnose_spec.rb +++ b/spec/lib/appsignal/cli/diagnose_spec.rb @@ -916,8 +916,39 @@ def dont_accept_prompt_to_send_diagnostics_report end end + describe "with config/appsignal.rb" do + let(:root_path) { File.join(tmp_dir, "config_file_test_diagnose1") } + let(:app_name) { "DiagnoseDSLapp" } + let(:push_api_key) { "DiagnosePushAPIKey" } + let(:options) { { :environment => "production" } } + before do + FileUtils.mkdir_p(root_path) + config_contents = + <<~CONFIG + Appsignal.configure do |config| + config.name = "#{app_name}" + config.push_api_key = "#{push_api_key}" + end + CONFIG + write_file(File.join(root_path, "config", "appsignal.rb"), config_contents) + + ENV["APPSIGNAL_APP_PATH"] = root_path + ENV["APPSIGNAL_APP_NAME"] = "ENV app name" + run + end + + it "loads config/appsignal.rb" do + expect(output).to include( + " name: \"DiagnoseDSLapp\"\n" \ + " Sources:\n" \ + " env: \"ENV app name\"\n" \ + " dsl: \"DiagnoseDSLapp\"\n" + ) + end + end + if DependencyHelper.rails_present? - context "when is a Rails app" do + context "when is a Rails app with config/appsignal.yml config file" do let(:root_path) { rails_project_fixture_path } let(:app_name) { "TestApp" } let(:environment) { "test" } @@ -929,10 +960,11 @@ def dont_accept_prompt_to_send_diagnostics_report if defined?(MyApp) && MyApp::Application.initialized? Appsignal::Integrations::Railtie.load_default_config end - run_within_dir(root_path) end it "includes the Rails default config in the output and transmitted report" do + run_within_dir(root_path) + expect(output).to include( " name: \"TestApp\"\n" \ " Sources:\n" \ @@ -941,11 +973,10 @@ def dont_accept_prompt_to_send_diagnostics_report ) # Outputs values from the DSL expect(output).to include( - " ignore_actions: [\"Action from DSL\"]\n" \ + " ignore_actions: [\"Rails::HealthController#show\"]\n" \ " Sources:\n" \ " default: []\n" \ - " loaders: [\"Rails::HealthController#show\"]\n" \ - " dsl: [\"Action from DSL\"]\n" + " loaders: [\"Rails::HealthController#show\"]\n" ) expect(received_report["app"]["rails"]).to be(true) @@ -958,10 +989,77 @@ def dont_accept_prompt_to_send_diagnostics_report "ignore_actions" => ["Rails::HealthController#show"] } ) + end + + context "when there's a problem loading the app" do + before do + # A spot where we can mock an error raise + expect(Appsignal::Utils::RailsHelper).to receive(:environment_config_path) + .and_raise(ExampleStandardError, "error message", ["line 1", "line 2"]) + run_within_dir(root_path) + end + + it "includes a load error" do + expect(output).to include( + "ERROR: Error encountered while loading the Rails app\n" \ + "ExampleStandardError: error message" + ) + + expect(received_report["app"]["load_error"]) + .to eq("ExampleStandardError: error message\nline 1\nline 2") + end + end + end + + context "when is a Rails app with config/appsignal.rb config file" do + let(:root_path) { File.join(tmp_dir, "diagnose_test_app_#{SecureRandom.uuid}") } + let(:app_name) { "TestApp" } + let(:environment) { "test" } + # Set the environment option so it's more predictable which + # environment we should mock API requests for + let(:options) { { :environment => environment } } + before do + # Copy Rails project so we can require the same + # `config/appsignal.rb` file multiple times in one test suite + # from multiple locations. + FileUtils.cp_r(rails_project_with_config_rb_fixture_path, root_path) + + # Workaround to not being able to require the railtie file + # multiple times and triggering the Rails initialization process. + # This will be used whtn the MyApp app has already been loaded. + if defined?(MyApp) && MyApp::Application.initialized? + Appsignal::Integrations::Railtie.load_default_config + end + end + after { FileUtils.rm_rf(root_path) } + + it "includes the Rails default config in the output and transmitted report" do + run_within_dir(root_path) + + expect(output).to include( + " name: \"TestApp\"\n" \ + " Sources:\n" \ + " loaders: \"MyApp\"\n" \ + " dsl: \"TestApp\"\n" + ) + + expect(received_report["app"]["rails"]).to be(true) + expect(received_report["config"]["sources"]).to include( + "loaders" => { + "root_path" => root_path, + "env" => "test", + "log_path" => File.join(root_path, "log"), + "name" => "MyApp", + "ignore_actions" => ["Rails::HealthController#show"] + } + ) # Includes values from the DSL expect(received_report["config"]["sources"]).to include( "dsl" => { - "ignore_actions" => ["Action from DSL"] + "active" => true, + "name" => "TestApp", + "push_api_key" => "abc", + "enable_minutely_probes" => false } ) end diff --git a/spec/support/fixtures/projects/valid_with_rails_app/config/environment.rb b/spec/support/fixtures/projects/valid_with_rails_app/config/environment.rb index e8472b7aa..8f8b9ad4b 100644 --- a/spec/support/fixtures/projects/valid_with_rails_app/config/environment.rb +++ b/spec/support/fixtures/projects/valid_with_rails_app/config/environment.rb @@ -3,8 +3,3 @@ # Initialize the Rails application. MyApp::Application.initialize! - -# Asserted from the diagnose spec -Appsignal.configure do |config| - config.ignore_actions = ["Action from DSL"] -end diff --git a/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/application.rb b/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/application.rb new file mode 100644 index 000000000..8955293f1 --- /dev/null +++ b/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/application.rb @@ -0,0 +1,16 @@ +require "rails" + +module MyApp + class Application < Rails::Application + config.active_support.deprecation = proc { |message, stack| } + config.eager_load = false + + def self.initialize! + # Prevent errors about Rails being initialized more than once + return if defined?(@initialized) + + super + @initialized = true + end + end +end diff --git a/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/appsignal.rb b/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/appsignal.rb new file mode 100644 index 000000000..1623d491c --- /dev/null +++ b/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/appsignal.rb @@ -0,0 +1,6 @@ +Appsignal.configure do |config| + config.activate_if_environment(:production, :development, :test) + config.name = "TestApp" + config.push_api_key = "abc" + config.enable_minutely_probes = false +end diff --git a/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/environment.rb b/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/environment.rb new file mode 100644 index 000000000..8f8b9ad4b --- /dev/null +++ b/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/config/environment.rb @@ -0,0 +1,5 @@ +# Load the Rails application. +require_relative "application" + +# Initialize the Rails application. +MyApp::Application.initialize! diff --git a/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/log/.gitkeep b/spec/support/fixtures/projects/valid_with_rails_app_with_config_rb/log/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/spec/support/helpers/config_helpers.rb b/spec/support/helpers/config_helpers.rb index 98b66ba6d..48504e895 100644 --- a/spec/support/helpers/config_helpers.rb +++ b/spec/support/helpers/config_helpers.rb @@ -13,6 +13,13 @@ def rails_project_fixture_path end module_function :rails_project_fixture_path + def rails_project_with_config_rb_fixture_path + File.expand_path( + File.join(File.dirname(__FILE__), "../fixtures/projects/valid_with_rails_app_with_config_rb") + ) + end + module_function :rails_project_fixture_path + def build_config( root_path: project_fixture_path, env: "production", From 48d16c7ab4dbc35ac3d56ecf042ca165d609bb64 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Mon, 11 Nov 2024 17:29:40 +0100 Subject: [PATCH 4/5] Improve changeset copy for appsignal.rb file These are suggestions from the review and my own improvements. --- .../add-config-appsignal-rb-file-support.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/.changesets/add-config-appsignal-rb-file-support.md b/.changesets/add-config-appsignal-rb-file-support.md index 514374630..ccb8fe8af 100644 --- a/.changesets/add-config-appsignal-rb-file-support.md +++ b/.changesets/add-config-appsignal-rb-file-support.md @@ -5,7 +5,7 @@ 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. +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: @@ -16,4 +16,17 @@ Appsignal.configure do |config| end ``` -It is not possible to configure multiple environments in the `config/appsignal.rb` config file as it was possible in the YAML version. +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 +``` From cdbec613ec43bd469f70cf637665b0f16451e964 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Mon, 11 Nov 2024 17:33:26 +0100 Subject: [PATCH 5/5] Add warnings for config file behaviors Add warnings to stderr and the log file when some config combination is used that may be expected to work, but does not. This is in preparation of the YAML file removal and to encourage apps to use the `config/appsignal.rb` config file and/or `Appsignal.configure` helper. --- lib/appsignal.rb | 27 ++++++++-- lib/appsignal/config.rb | 19 ++++++- spec/lib/appsignal_spec.rb | 104 +++++++++++++++++++++++++++---------- 3 files changed, 117 insertions(+), 33 deletions(-) diff --git a/lib/appsignal.rb b/lib/appsignal.rb index cb2ebf913..725e1f9a5 100644 --- a/lib/appsignal.rb +++ b/lib/appsignal.rb @@ -160,11 +160,15 @@ def _load_config!(env_param = nil) # 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." - ) + # 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) @@ -289,6 +293,19 @@ def configure(env_param = nil, root_path: nil) ) 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? diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb index e343804ce..3210de76e 100644 --- a/lib/appsignal/config.rb +++ b/lib/appsignal/config.rb @@ -298,6 +298,16 @@ def load_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 @@ -463,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 @@ -486,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) diff --git a/spec/lib/appsignal_spec.rb b/spec/lib/appsignal_spec.rb index a3a1e2afb..857b7be5a 100644 --- a/spec/lib/appsignal_spec.rb +++ b/spec/lib/appsignal_spec.rb @@ -5,9 +5,18 @@ let(:transaction) { http_request_transaction } describe ".configure" do - context "when active" do + let(:root_path) { tmp_dir } + before do + log_dir = File.join(root_path, "log") + FileUtils.mkdir_p(log_dir) + end + + context "when started" do it "doesn't update the config" do - start_agent + start_agent( + :root_path => root_path, + :options => { :active => true, :push_api_key => "dummy" } + ) Appsignal::Testing.store[:config_called] = false expect do Appsignal.configure do |_config| @@ -18,7 +27,10 @@ end it "logs a warning" do - start_agent + start_agent( + :root_path => tmp_dir, + :options => { :active => true, :push_api_key => "dummy" } + ) logs = capture_logs do Appsignal.configure do |_config| @@ -34,7 +46,7 @@ context "with config but not started" do it "reuses the already loaded config if no env arg is given" do - Appsignal.configure(:my_env, :root_path => project_fixture_path) do |config| + Appsignal.configure(:my_env, :root_path => root_path) do |config| config.ignore_actions = ["My action"] end @@ -56,7 +68,7 @@ end it "reuses the already loaded config if the env is the same" do - Appsignal.configure(:my_env, :root_path => project_fixture_path) do |config| + Appsignal.configure(:my_env, :root_path => root_path) do |config| config.ignore_actions = ["My action"] end @@ -76,7 +88,7 @@ end it "loads a new config if the env is not the same" do - Appsignal.configure(:my_env, :root_path => project_fixture_path) do |config| + Appsignal.configure(:my_env, :root_path => root_path) do |config| config.name = "Some name" config.push_api_key = "Some key" config.ignore_actions = ["My action"] @@ -104,7 +116,7 @@ config.ignore_actions = ["My action"] end - Appsignal.configure(:my_env, :root_path => project_fixture_path) do |config| + Appsignal.configure(:my_env, :root_path => root_path) do |config| expect(config.ignore_actions).to be_empty config.active = true config.name = "My app" @@ -162,21 +174,41 @@ end it "uses the given root path to read the config file" do - Appsignal.configure(:test, :root_path => project_fixture_path) + err_stream = std_stream + logs = + capture_logs do + capture_std_streams(std_stream, err_stream) do + Appsignal.configure(:test, :root_path => project_fixture_path) + end + end Appsignal.start + message = "The `Appsignal.configure` helper is called while a `config/appsignal.yml` " \ + "file is present." + expect(logs).to contains_log(:warn, message) + expect(err_stream.read).to include("appsignal WARNING: #{message}") expect(Appsignal.config.env).to eq("test") expect(Appsignal.config[:push_api_key]).to eq("abc") # Ensure it loads from the config file in the given path expect(Appsignal.config.file_config).to_not be_empty end - it "loads the config without a block being given" do - Dir.chdir project_fixture_path do - Appsignal.configure(:test) - end + it "loads the config from the YAML file" do + err_stream = std_stream + logs = + capture_logs do + capture_std_streams(std_stream, err_stream) do + Dir.chdir project_fixture_path do + Appsignal.configure(:test) + end + end + end Appsignal.start + message = "The `Appsignal.configure` helper is called while a `config/appsignal.yml` " \ + "file is present." + expect(logs).to contains_log(:warn, message) + expect(err_stream.read).to include("appsignal WARNING: #{message}") expect(Appsignal.config.env).to eq("test") expect(Appsignal.config[:push_api_key]).to eq("abc") # Ensure it loads from the config file in the current working directory @@ -202,14 +234,6 @@ end end - it "loads the config from the YAML file" do - Dir.chdir project_fixture_path do - Appsignal.configure(:test) do |config| - expect(config.name).to eq("TestApp") - end - end - end - it "recognizes valid config" do Appsignal.configure(:my_env) do |config| config.push_api_key = "key" @@ -243,6 +267,18 @@ expect(Appsignal.config.env).to eq("env_env") end + it "reads config options from the environment" do + ENV["APPSIGNAL_APP_ENV"] = "env_env" + ENV["APPSIGNAL_APP_NAME"] = "AppNameFromEnv" + Appsignal.configure do |config| + expect(config.env).to eq("env_env") + expect(config.name).to eq("AppNameFromEnv") + end + + expect(Appsignal.config.env).to eq("env_env") + expect(Appsignal.config[:name]).to eq("AppNameFromEnv") + end + it "reads the environment from a loader default" do clear_integration_env_vars! define_loader(:loader_env) do @@ -456,8 +492,17 @@ def on_load end ENV["APPSIGNAL_APP_PATH"] = test_path - Appsignal.start + err_stream = std_stream + logs = + capture_logs do + capture_std_streams(std_stream, err_stream) do + Appsignal.start + end + end + warning_message = "Both a Ruby and YAML configuration file are found." + expect(logs).to contains_log(:warn, warning_message) + expect(err_stream.read).to include("appsignal WARNING: #{warning_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) @@ -717,13 +762,18 @@ def on_start end ENV["APPSIGNAL_APP_PATH"] = test_path - logs = capture_logs { Appsignal.start } + err_stream = std_stream + logs = + capture_logs do + capture_std_streams(std_stream, err_stream) do + Appsignal.start + end + end - expect(logs).to contains_log( - :warn, - "Ignoring `#{config_file_path}` file because " \ - "`Appsignal.configure` has already been called." - ) + message = "The `Appsignal.configure` helper is called from within an " \ + "app while a `#{config_file_path}` file is present." + expect(logs).to contains_log(:warn, message) + expect(err_stream.read).to include("appsignal WARNING: #{message}") 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)