From d08a1cec705db71999afa9c4af36e244a7b6483a Mon Sep 17 00:00:00 2001 From: Noemi <45180344+unflxw@users.noreply.github.com> Date: Thu, 19 Dec 2024 12:34:34 +0100 Subject: [PATCH] Implement `#silence` logger functionality Our current implementation of the `#silence` method does not actually silence anything. If we are implementing the method, we should actually implement its functionality. --- ...ix---silence--implementation-for-logger.md | 6 +++ lib/appsignal/logger.rb | 11 +++--- spec/lib/appsignal/logger_spec.rb | 37 +++++++++++++++++++ 3 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 .changesets/fix---silence--implementation-for-logger.md diff --git a/.changesets/fix---silence--implementation-for-logger.md b/.changesets/fix---silence--implementation-for-logger.md new file mode 100644 index 00000000..7b5118c1 --- /dev/null +++ b/.changesets/fix---silence--implementation-for-logger.md @@ -0,0 +1,6 @@ +--- +bump: patch +type: fix +--- + +Fix `#silence` implementation for `Appsignal::Logger`. diff --git a/lib/appsignal/logger.rb b/lib/appsignal/logger.rb index 095da369..12f36579 100644 --- a/lib/appsignal/logger.rb +++ b/lib/appsignal/logger.rb @@ -192,16 +192,17 @@ def clear_tags! # When using ActiveSupport::TaggedLogging without the broadcast feature, # the passed logger is required to respond to the `silence` method. - # In our case it behaves as the broadcast feature of the Rails logger, but - # we don't have to check if the parent logger has the `silence` method defined - # as our logger directly inherits from Ruby base logger. # - # Links: + # Reference links: # # - https://github.com/rails/rails/blob/e11ebc04cfbe41c06cdfb70ee5a9fdbbd98bb263/activesupport/lib/active_support/logger.rb#L60-L76 # - https://github.com/rails/rails/blob/e11ebc04cfbe41c06cdfb70ee5a9fdbbd98bb263/activesupport/lib/active_support/logger_silence.rb - def silence(_severity = ERROR, &block) + def silence(severity = ERROR, &block) + previous_level = @level + @level = severity block.call(self) + ensure + @level = previous_level end private diff --git a/spec/lib/appsignal/logger_spec.rb b/spec/lib/appsignal/logger_spec.rb index bce891de..7f22fe61 100644 --- a/spec/lib/appsignal/logger_spec.rb +++ b/spec/lib/appsignal/logger_spec.rb @@ -336,6 +336,43 @@ expect(num).to eq(2) expect(Appsignal::Extension).not_to receive(:log) end + + it "silences the logger up to, but not including, the given level" do + # Expect not to receive info + expect(Appsignal::Extension).not_to receive(:log) + .with("group", 3, 0, "Log message", instance_of(Appsignal::Extension::Data)) + + # Expect to receive warn + expect(Appsignal::Extension).to receive(:log) + .with("group", 5, 0, "Log message", instance_of(Appsignal::Extension::Data)) + + logger.silence(::Logger::WARN) do + logger.info("Log message") + logger.warn("Log message") + end + end + + it "silences the logger to error level by default" do + # Expect not to receive debug, info or warn + [2, 3, 5].each do |severity| + expect(Appsignal::Extension).not_to receive(:log) + .with("group", severity, 0, "Log message", instance_of(Appsignal::Extension::Data)) + end + + # Expect to receive error and fatal + [6, 7].each do |severity| + expect(Appsignal::Extension).to receive(:log) + .with("group", severity, 0, "Log message", instance_of(Appsignal::Extension::Data)) + end + + logger.silence do + logger.debug("Log message") + logger.info("Log message") + logger.warn("Log message") + logger.error("Log message") + logger.fatal("Log message") + end + end end [