Skip to content

Commit

Permalink
Fix MulticastLogger DEBUG mode
Browse files Browse the repository at this point in the history
When passing the config/settings data to the `.apply_config_value`
method of Vmdb::Loggers, if the value was set to `DEBUG` the level
wouldn't be changed in the lower level logger to reflect that.

This is because the MulticastLogger is instantiated and defaulted at
DEBUG, but the logger it casts to (normally an instance of VMDBLogger)
would have it's level defaulted to `info`.  When `.apply_config_value`
is then called, it would compare the old value to the new one, and see
it as the same, even though it was different under the covers.

This means when a user attempts to do a `$log.debug "my message"`, the
message would then not be applied.

The fix here effectively makes sure we are defaulting the lower level
logger to the same log level as the parent, or the MulticastLogger.
When `.apply_config_value` is then eventually called, if the setting is
set to `DEBUG`, nothing will change, but the level will be set properly
for the lower level logger.  Anything level set higher than DEBUG will
function as it has.

* * *

There was also some additional tests added to the new test file,
`spec/lib/vmdb/loggers_spec.rb`, that covered some of the quirks of log
levels in a container config.  Since `$container_log` is set to use the
`lib/vmdb/loggers/container_logger.rb`, this means it's `#level=` method
is set to override the default and always set itself to "DEBUG".  This
is a bit confusing at first glace if you are trying to grok what is
happening in `lib/vmdb/loggers.rb`, as it can seem like the whole thing
wouldn't work as expected, and the level for the $container_log would be
overwritten with each log type.
  • Loading branch information
NickLaMuro committed Feb 13, 2018
1 parent 04aea3e commit a7ed2cb
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
5 changes: 4 additions & 1 deletion lib/vmdb/loggers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ def self.create_loggers
private_class_method :create_loggers

def self.create_multicast_logger(log_file_path, logger_class = VMDBLogger)
MulticastLogger.new(logger_class.new(log_file_path)).tap do |l|
logger_instance = logger_class.new(log_file_path).tap do |logger|
logger.level = Logger::DEBUG
end
MulticastLogger.new(logger_instance).tap do |l|
l.loggers << $container_log if ENV["CONTAINER"]
end
end
Expand Down
54 changes: 54 additions & 0 deletions spec/lib/vmdb/loggers_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
describe Vmdb::Loggers do
let(:log_file) { Rails.root.join("log", "foo.log").to_s }

def in_container_env(example)
old_env = ENV.delete('CONTAINER')
ENV['CONTAINER'] = 'true'
example.run
ensure
# ENV['x'] = nil deletes the key because ENV accepts only string values
ENV['CONTAINER'] = old_env
end

describe "#create_multicast_logger (private)" do
it "defaults the lower level loggers to `DEBUG`" do
log = described_class.send(:create_multicast_logger, log_file)

expect(log.loggers.first.level).to eq(Logger::DEBUG)
expect(log.loggers.to_a.size).to eq(1)
end

context "in a container environment" do
around { |example| in_container_env(example) }

it "sets logger_instance and $container_log to debug" do
log = described_class.send(:create_multicast_logger, log_file)

expect(log.loggers.first.level).to eq(Logger::DEBUG)
expect(log.loggers.to_a.last.level).to eq(Logger::DEBUG)
end
end
end

describe "#apply_config_value (private)" do
it "will update the main lower level logger instance" do
log = described_class.send(:create_multicast_logger, log_file)
described_class.send(:apply_config_value, {:foo => :info}, log, :foo)

expect(log.loggers.first.level).to eq(Logger::INFO)
expect(log.loggers.to_a.size).to eq(1)
end

context "in a container environment" do
around { |example| in_container_env(example) }

it "will always keep $container_log as DEBUG" do
log = described_class.send(:create_multicast_logger, log_file)
described_class.send(:apply_config_value, {:foo => :info}, log, :foo)

expect(log.loggers.first.level).to eq(Logger::INFO)
expect(log.loggers.to_a.last.level).to eq(Logger::DEBUG)
end
end
end
end

0 comments on commit a7ed2cb

Please sign in to comment.