From 5ff0bd808448e3cd8e028f3efa85adf8d613c569 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Fri, 1 Nov 2024 17:32:25 +0100 Subject: [PATCH] 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 the app starts. 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. --- 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 9a4e0f9c7..1c473191c 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",