Skip to content

Commit

Permalink
Remove logger from config class
Browse files Browse the repository at this point in the history
Directly access the `Appsignal.internal_logger` method instead of the
Config class having its own logger instance that could be different from
the configured logger.
  • Loading branch information
tombruijn committed Sep 26, 2024
1 parent 089d032 commit e671a35
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 20 deletions.
3 changes: 1 addition & 2 deletions lib/appsignal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ def configure(env = nil, root_path: nil)
else
@config = Config.new(
root_path || Config.determine_root_path,
Config.determine_env(env),
Appsignal.internal_logger
Config.determine_env(env)
)
end

Expand Down
17 changes: 7 additions & 10 deletions lib/appsignal/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,13 @@ def self.determine_root_path
attr_accessor :root_path, :env, :config_hash
attr_reader :system_config, :loaders_config, :initial_config, :file_config,
:env_config, :override_config, :dsl_config
# @api private
attr_accessor :logger

# Initialize a new configuration object for AppSignal.
#
# @param root_path [String] Root path of the app.
# @param env [String] The environment to load when AppSignal is started. It
# will look for an environment with this name in the `config/appsignal.yml`
# config file.
# @param logger [Logger] The logger to use for the AppSignal gem. This is
# used by the configuration class only. Default:
# {Appsignal.internal_logger}. See also {Appsignal.start}.
#
# @api private
# @see https://docs.appsignal.com/ruby/configuration/
Expand All @@ -230,13 +225,11 @@ def self.determine_root_path
# How to integrate AppSignal manually
def initialize(
root_path,
env,
logger = Appsignal.internal_logger
env
)
@root_path = root_path
@config_file_error = false
@config_file = config_file
@logger = logger
@valid = false

@env = env.to_s
Expand Down Expand Up @@ -426,7 +419,7 @@ def validate
push_api_key = config_hash[:push_api_key] || ""
if push_api_key.strip.empty?
@valid = false
@logger.error "Push API key not set after loading config"
logger.error "Push API key not set after loading config"
else
@valid = true
end
Expand All @@ -445,6 +438,10 @@ def freeze

private

def logger
Appsignal.internal_logger
end

def config_file
@config_file ||=
root_path.nil? ? nil : File.join(root_path, "config", "appsignal.yml")
Expand Down Expand Up @@ -546,7 +543,7 @@ def determine_overrides

def merge(new_config)
new_config.each do |key, value|
@logger.debug("Config key '#{key}' is being overwritten") unless config_hash[key].nil?
logger.debug("Config key '#{key}' is being overwritten") unless config_hash[key].nil?
config_hash[key] = value
end
end
Expand Down
3 changes: 1 addition & 2 deletions lib/appsignal/integrations/capistrano/appsignal.cap
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ namespace :appsignal do

appsignal_config = Appsignal::Config.new(
Dir.pwd,
appsignal_env,
Appsignal.internal_logger
appsignal_env
).tap do |c|
c.merge_dsl_options(fetch(:appsignal_config, {}))
c.validate
Expand Down
3 changes: 1 addition & 2 deletions lib/appsignal/integrations/capistrano/capistrano_2_tasks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ def self.tasks(config)

appsignal_config = Appsignal::Config.new(
ENV.fetch("PWD", nil),
env,
Appsignal.internal_logger
env
).tap do |c|
c.merge_dsl_options(fetch(:appsignal_config, {}))
c.validate
Expand Down
6 changes: 2 additions & 4 deletions spec/support/helpers/config_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@ def rails_project_fixture_path
def build_config(
root_path: project_fixture_path,
env: "production",
options: {},
logger: Appsignal.internal_logger
options: {}
)
Appsignal::Config.new(
root_path,
env,
logger
env
).tap do |c|
c.merge_dsl_options(options) if options.any?
c.validate
Expand Down

0 comments on commit e671a35

Please sign in to comment.