Skip to content

Commit

Permalink
Move config load logic to its own method
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
tombruijn committed Nov 5, 2024
1 parent f81248b commit 5c45e97
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 48 deletions.
109 changes: 62 additions & 47 deletions lib/appsignal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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} " \
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand Down
4 changes: 3 additions & 1 deletion spec/lib/appsignal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5c45e97

Please sign in to comment.