From 932afa7700c4dd05250d386c07bace63573e6411 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Thu, 13 Feb 2025 11:58:52 -0500 Subject: [PATCH] Use new public 7.1 broadcast logging API We no longer need to wrap loggers as you can inspect all broadcasted loggers and automatically send the same log level or other method calls to each. We added a wrap method to wrap a logger with a broadcaster in: https://github.com/ManageIQ/manageiq-loggers/pull/63 See also: https://www.github.com/rails/rails/pull/48615 --- lib/vmdb/loggers.rb | 19 ++---------- spec/lib/vmdb/loggers_spec.rb | 55 +++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/lib/vmdb/loggers.rb b/lib/vmdb/loggers.rb index c85f3f25065..756e1d34609 100644 --- a/lib/vmdb/loggers.rb +++ b/lib/vmdb/loggers.rb @@ -86,23 +86,8 @@ def self.create_logger(log_file_name, logger_class = ManageIQ::Loggers::Base) log_file = ManageIQ.root.join("log", log_file) if log_file.try(:dirname).to_s == "." progname = log_file.try(:basename, ".*").to_s - 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)) - if logger.class.const_defined?(:FormatterMixin) - wrapped_logger.formatter.extend(logger.class.const_get(:FormatterMixin)) - end - - wrapped_logger.progname = progname - end + logger = logger_class.new(nil, :progname => progname) + logger.wrap(wrapped_logger).tap { |broadcaster| broadcaster.progname = progname } end private_class_method def self.configure_external_loggers diff --git a/spec/lib/vmdb/loggers_spec.rb b/spec/lib/vmdb/loggers_spec.rb index 53153d9ae28..e7705035566 100644 --- a/spec/lib/vmdb/loggers_spec.rb +++ b/spec/lib/vmdb/loggers_spec.rb @@ -32,7 +32,7 @@ def in_container_env(example) subject { described_class.create_logger(log_file) } - let(:container_log) { subject.try(:wrapped_logger) } + let(:container_log) { subject.try(:broadcasts).try(:last) } before do # Hide the container logger output to STDOUT @@ -65,7 +65,8 @@ def in_container_env(example) it "#logdev" do if container_log - expect(subject.logdev).to be_nil + expect(subject.broadcasts.first.logdev).to be_nil + expect(container_log.logdev).to be_a Logger::LogDevice else expect(subject.logdev).to be_a Logger::LogDevice end @@ -73,7 +74,12 @@ def in_container_env(example) describe "#datetime_format" do it "return nil" do - expect(subject.datetime_format).to be nil + if container_log + expect(subject.datetime_format.first).to be nil + expect(subject.datetime_format.last).to be nil + else + expect(subject.datetime_format).to be nil + end end it "does not raise an error" do @@ -91,15 +97,18 @@ def in_container_env(example) 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 - + if container_log + expect(subject.broadcasts.first).to receive(:add).with(1, nil, "test message").and_call_original + expect(subject.broadcasts.last).to receive(:add).with(1, nil, "test message").and_call_original + else + expect(subject).to receive(:add).with(1, nil, "test message").and_call_original + end 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(subject.broadcasts.first.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") @@ -139,8 +148,12 @@ 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 + if container_log + expect(subject.broadcasts.first).to receive(:add).with(1, nil, "test message").and_call_original + expect(subject.broadcasts.last).to receive(:add).with(1, nil, "test message").and_call_original + else + expect(subject).to receive(:add).with(1, nil, "test message").and_call_original + end subject.info("test message") @@ -154,8 +167,12 @@ 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 + if container_log + expect(subject.broadcasts.first).to receive(:add).with(1, nil, "test message").and_call_original + expect(subject.broadcasts.last).to receive(:add).with(1, nil, "test message").and_call_original + else + expect(subject).to receive(:add).with(1, nil, "test message").and_call_original + end subject.info("test message") @@ -169,9 +186,12 @@ 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 - + if container_log + expect(subject.broadcasts.first).to receive(:add).with(1, nil, "test message").and_call_original + expect(subject.broadcasts.last).to receive(:add).with(1, nil, "test message").and_call_original + else + expect(subject).to receive(:add).with(1, nil, "test message").and_call_original + end subject.info("test message") expect(File.read(log_file)).to include("test message") unless container_log @@ -212,12 +232,15 @@ 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 + container_log = log.broadcasts.last 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) + + described_class.apply_config_value({:level_foo => :debug}, log, :level_foo) + expect(log.level).to eq(Logger::DEBUG) + expect(container_log.level).to eq(Logger::DEBUG) end end end