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..ccb8fe8af --- /dev/null +++ b/.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 +``` diff --git a/lib/appsignal.rb b/lib/appsignal.rb index dcf39887c..725e1f9a5 100644 --- a/lib/appsignal.rb +++ b/lib/appsignal.rb @@ -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,9 +117,7 @@ def start # rubocop:disable Metrics/AbcSize 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") 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 e2a9cfe85..3210de76e 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) @@ -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) 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/lib/appsignal_spec.rb b/spec/lib/appsignal_spec.rb index 222a69c78..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 @@ -373,6 +409,212 @@ 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 + 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) + 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[: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 + + 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 +738,50 @@ 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 + err_stream = std_stream + logs = + capture_logs do + capture_std_streams(std_stream, err_stream) do + Appsignal.start + end + end + + 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) + 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/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", 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!