Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Drop broadcast logger #22798

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[WIP] Drop broadcast logger #22798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e722858
Refactor create_logger to remove ||
agrare Nov 28, 2023
9f8ddcd
Extract log_file path building to common method
agrare Nov 28, 2023
6ed92ad
Add a log_path_from_file helper method
agrare Nov 28, 2023
86b127a
Set the logger progname in the main methods
agrare Nov 28, 2023
b8aabc6
Drop the raw_create_* logger methods
agrare Nov 28, 2023
811ea67
Return the raw logger unless logger_class is present
agrare Nov 28, 2023
515eefe
Remove wrapped_logger hack
agrare Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 30 additions & 38 deletions lib/vmdb/loggers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ def self.apply_config(config)
Vmdb::Plugins.each { |p| p.try(:apply_logger_config, config) }
end

def self.create_logger(log_file_name, logger_class = ManageIQ::Loggers::Base)
create_container_logger(log_file_name, logger_class) ||
create_journald_logger(log_file_name, logger_class) ||
def self.create_logger(log_file_name, logger_class = nil)
if MiqEnvironment::Command.is_container?
create_container_logger(log_file_name, logger_class)
elsif MiqEnvironment::Command.supports_systemd?
create_journald_logger(log_file_name, logger_class)
else
create_file_logger(log_file_name, logger_class)
end
end

private_class_method def self.create_loggers
Expand All @@ -45,63 +49,51 @@ def self.create_logger(log_file_name, logger_class = ManageIQ::Loggers::Base)
end

private_class_method def self.create_file_logger(log_file, logger_class)
log_file = Pathname.new(log_file) if log_file.kind_of?(String)
log_file = ManageIQ.root.join("log", log_file) if log_file.try(:dirname).to_s == "."
progname = log_file.try(:basename, ".*").to_s
log_file = log_path_from_file(log_file)
progname = progname_from_file(log_file)

logger_class ||= ManageIQ::Loggers::Base
logger_class.new(log_file, :progname => progname)
end

private_class_method def self.create_container_logger(log_file_name, logger_class)
return nil unless (logger = create_raw_container_logger)
require "manageiq/loggers/container"
logger = ManageIQ::Loggers::Container.new

create_wrapper_logger(log_file_name, logger_class, logger)
end
progname = progname_from_file(log_file_name)
logger.progname = progname

private_class_method def self.create_raw_container_logger
return unless ENV["CONTAINER"]
return logger if logger_class.nil?

require "manageiq/loggers/container"
ManageIQ::Loggers::Container.new
create_wrapper_logger(progname, logger_class, logger)
end

private_class_method def self.create_journald_logger(log_file_name, logger_class)
return nil unless (logger = create_raw_journald_logger)
require "manageiq/loggers/journald"
logger = ManageIQ::Loggers::Journald.new

create_wrapper_logger(log_file_name, logger_class, logger)
end
progname = progname_from_file(log_file_name)
logger.progname = progname

private_class_method def self.create_raw_journald_logger
return unless MiqEnvironment::Command.supports_systemd?
return logger if logger_class.nil?

require "manageiq/loggers/journald"
ManageIQ::Loggers::Journald.new
rescue LoadError
nil
create_wrapper_logger(progname, logger_class, logger)
end

private_class_method def self.progname_from_file(log_file_name)
File.basename(log_file_name, ".*")
private_class_method def self.log_path_from_file(log_file)
log_file = Pathname.new(log_file) if log_file.kind_of?(String)
log_file = ManageIQ.root.join("log", log_file) if log_file.try(:dirname).to_s == "."
log_file
end

private_class_method def self.create_wrapper_logger(log_file, logger_class, wrapped_logger)
private_class_method def self.progname_from_file(log_file_name)
log_file = Pathname.new(log_file) if log_file.kind_of?(String)
log_file = ManageIQ.root.join("log", log_file) if log_file.try(:dirname).to_s == "."
progname = log_file.try(:basename, ".*").to_s
log_file.try(:basename, ".*").to_s
end

private_class_method def self.create_wrapper_logger(progname, logger_class, wrapped_logger)
logger_class.new(nil, :progname => progname).tap do |logger|
# HACK: In order to access the wrapped logger in test, we inject it as an instance var.
if Rails.env.test?
logger.instance_variable_set(:@wrapped_logger, wrapped_logger)

def logger.wrapped_logger
@wrapped_logger
end
end

logger.extend(ActiveSupport::Logger.broadcast(wrapped_logger))

wrapped_logger.progname = progname
end
end

Expand Down
57 changes: 7 additions & 50 deletions spec/lib/vmdb/loggers_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ def in_container_env(example)

subject { described_class.create_logger(log_file) }

let(:container_log) { subject.try(:wrapped_logger) }

before do
# Hide the container logger output to STDOUT
allow(container_log.logdev).to receive(:write) if container_log
end

it "responds to #<<" do
expect(subject).to respond_to(:<<)
end
Expand All @@ -64,11 +57,7 @@ def in_container_env(example)
end

it "#logdev" do
if container_log
expect(subject.logdev).to be_nil
else
expect(subject.logdev).to be_a Logger::LogDevice
end
expect(subject.logdev).to be_a Logger::LogDevice
end

describe "#datetime_format" do
Expand All @@ -90,20 +79,8 @@ def in_container_env(example)
subject.level = old_level
end

it "forwards to the other loggers" do
expect(subject).to receive(:add).with(1, nil, "test message").and_call_original
expect(container_log).to receive(:add).with(1, nil, "test message").and_call_original if container_log

subject.info("test message")
end

it "only forwards the message if the severity is correct" do
if container_log
expect(subject.logdev).to be_nil
expect(container_log.logdev).not_to receive(:write).with("test message")
else
expect(subject.logdev).not_to receive(:write).with("test message")
end
expect(subject.logdev).not_to receive(:write).with("test message")

subject.debug("test message")
end
Expand All @@ -128,8 +105,7 @@ def in_container_env(example)

context "#<<" do
it "forwards to the other loggers" do
expect(subject).to receive(:<<).with("test message").and_call_original
expect(container_log).to receive(:<<).with("test message").and_call_original if container_log
expect(subject).to receive(:<<).with("test message")#.and_call_original

subject << "test message"
end
Expand All @@ -139,12 +115,9 @@ def in_container_env(example)
let(:log_file) { StringIO.new }

it "logs correctly" do
expect(subject).to receive(:add).with(1, nil, "test message").and_call_original
expect(container_log).to receive(:add).with(1, nil, "test message").and_call_original if container_log
expect(subject).to receive(:add).with(1, nil, "test message")

subject.info("test message")

expect(log_file.string).to include("test message") unless container_log
end
end

Expand All @@ -154,12 +127,9 @@ def in_container_env(example)
after { log_file.delete if log_file.exist? }

it "logs correctly" do
expect(subject).to receive(:add).with(1, nil, "test message").and_call_original
expect(container_log).to receive(:add).with(1, nil, "test message").and_call_original if container_log
expect(subject).to receive(:add).with(1, nil, "test message")

subject.info("test message")

expect(log_file.read).to include("test message") unless container_log
end
end

Expand All @@ -169,31 +139,20 @@ def in_container_env(example)
after { File.delete(log_file) if File.exist?(log_file) }

it "logs correctly" do
expect(subject).to receive(:add).with(1, nil, "test message").and_call_original
expect(container_log).to receive(:add).with(1, nil, "test message").and_call_original if container_log
expect(subject).to receive(:add).with(1, nil, "test message")

subject.info("test message")

expect(File.read(log_file)).to include("test message") unless container_log
end
end
end

context "in a non-container environment" do
it "does not have a container logger" do
expect(container_log).to be_nil
end

include_examples "has basic logging functionality"
end

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

it "has a container logger" do
expect(container_log).to_not be_nil
end

include_examples "has basic logging functionality"
end
end
Expand All @@ -212,12 +171,10 @@ def in_container_env(example)

it "will honor the log level in the container logger" do
log = described_class.create_logger(log_file_name)
container_log = log.wrapped_logger

described_class.apply_config_value({:level_foo => :error}, log, :level_foo)

expect(log.level).to eq(Logger::ERROR)
expect(container_log.level).to eq(Logger::ERROR)
expect(log.level).to eq(Logger::ERROR)
end
end
end
Expand Down