From c518fa469981f1441710bd30a3083938cdc32167 Mon Sep 17 00:00:00 2001 From: Noemi <45180344+unflxw@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:48:09 +0100 Subject: [PATCH] Allow `#tagged` to be called without a block When `#tagged` is called without a block, it should return a new logger that applies the tags it is given to all log lines, instead of modifying the current logger. This commit implements that behaviour, in order to match the behaviour exposed by `ActiveSupport::TaggedLogging`. --- ...er-tagged--to-be-called-without-a-block.md | 11 ++ lib/appsignal/logger.rb | 29 ++++- spec/lib/appsignal/logger_spec.rb | 107 ++++++++++++++++++ 3 files changed, 142 insertions(+), 5 deletions(-) create mode 100644 .changesets/allow--logger-tagged--to-be-called-without-a-block.md diff --git a/.changesets/allow--logger-tagged--to-be-called-without-a-block.md b/.changesets/allow--logger-tagged--to-be-called-without-a-block.md new file mode 100644 index 00000000..5e9d79a4 --- /dev/null +++ b/.changesets/allow--logger-tagged--to-be-called-without-a-block.md @@ -0,0 +1,11 @@ +--- +bump: patch +type: fix +--- + +Allow `Appsignal::Logger#tagged` to be called without a block, in the same way as `ActiveSupport::TaggedLogging`: + +```ruby +Appsignal::Logger.new("rails").tagged("some tag").info("message") +# => logs "[some tag] message" +``` diff --git a/lib/appsignal/logger.rb b/lib/appsignal/logger.rb index 42364614..5ba59ddf 100644 --- a/lib/appsignal/logger.rb +++ b/lib/appsignal/logger.rb @@ -29,7 +29,7 @@ class Logger < ::Logger # @param format Format to use to parse log line attributes. # @param attributes Default attributes for all log lines. # @return [void] - def initialize(group, level: INFO, format: PLAINTEXT, attributes: {}) + def initialize(group, level: INFO, format: PLAINTEXT, attributes: {}, tags: []) raise TypeError, "group must be a string" unless group.is_a? String @group = group @@ -38,7 +38,7 @@ def initialize(group, level: INFO, format: PLAINTEXT, attributes: {}) @mutex = Mutex.new @default_attributes = attributes @appsignal_attributes = {} - @tags = [] + @tags = tags end # We support the various methods in the Ruby @@ -156,10 +156,19 @@ def tagged(*tags) # as separate arguments. Flatten the tags argument array to deal with them # indistinctly. tags = tags.flatten + + # If called without a block, return a new logger that always logs with the + # given set of tags. + return with_tags(tags) unless block_given? + + # If called with a block, modify the current logger to log with the given + # set of tags for the duration of the block. @tags.append(*tags) - yield self - ensure - @tags.pop(tags.length) + begin + yield self + ensure + @tags.pop(tags.length) + end end # Listen to ActiveSupport tagged logging tags set with `Rails.config.log_tags`. @@ -197,6 +206,16 @@ def silence(_severity = ERROR, &block) private + def with_tags(tags) + Logger.new( + @group, + :level => @level, + :format => @format, + :attributes => @default_attributes, + :tags => @tags + tags + ) + end + attr_reader :default_attributes, :appsignal_attributes def add_with_attributes(severity, message, group, attributes) diff --git a/spec/lib/appsignal/logger_spec.rb b/spec/lib/appsignal/logger_spec.rb index 2d12fa65..bce891de 100644 --- a/spec/lib/appsignal/logger_spec.rb +++ b/spec/lib/appsignal/logger_spec.rb @@ -122,6 +122,113 @@ Appsignal::Utils::Data.generate({}) ) end + + it "accepts tags in #tagged as an array" do + expect(Appsignal::Extension).to receive(:log) + .with( + "group", + 3, + 0, + a_string_starting_with("[My tag] [My other tag] Some message"), + Appsignal::Utils::Data.generate({}) + ) + + logger.tagged(["My tag", "My other tag"]) do + logger.info("Some message") + end + end + + # Calling `#tagged` without a block is not supported by + # `ActiveSupport::TaggedLogging` in Rails 6 and earlier. Only run this + # in builds where Rails is not present, or builds where Rails 7 or later + # is present. + if !DependencyHelper.rails_present? || DependencyHelper.rails7_present? + describe "when calling #tagged without a block" do + it "returns a new logger with the tags added" do + expect(Appsignal::Extension).to receive(:log) + .with( + "group", + 3, + 0, + a_string_starting_with("[My tag] [My other tag] Some message"), + Appsignal::Utils::Data.generate({}) + ) + + logger.tagged("My tag", "My other tag").info("Some message") + end + + it "does not modify the original logger" do + expect(Appsignal::Extension).to receive(:log) + .with( + "group", + 3, + 0, + a_string_starting_with("[My tag] [My other tag] Some message"), + Appsignal::Utils::Data.generate({}) + ) + + new_logger = logger.tagged("My tag", "My other tag") + new_logger.info("Some message") + + expect(Appsignal::Extension).to receive(:log) + .with( + "group", + 3, + 0, + a_string_starting_with("Some message"), + Appsignal::Utils::Data.generate({}) + ) + + logger.info("Some message") + end + + it "can be chained" do + expect(Appsignal::Extension).to receive(:log) + .with( + "group", + 3, + 0, + a_string_starting_with("[My tag] [My other tag] [My third tag] Some message"), + Appsignal::Utils::Data.generate({}) + ) + + logger.tagged("My tag", "My other tag").tagged("My third tag").info("Some message") + end + + it "can be chained before a block invocation" do + expect(Appsignal::Extension).to receive(:log) + .with( + "group", + 3, + 0, + a_string_starting_with("[My tag] [My other tag] [My third tag] Some message"), + Appsignal::Utils::Data.generate({}) + ) + + # We must explicitly use the logger passed to the block, + # as the logger returned from the first #tagged invocation + # is a new instance of the logger. + logger.tagged("My tag", "My other tag").tagged("My third tag") do |logger| + logger.info("Some message") + end + end + + it "can be chained after a block invocation" do + expect(Appsignal::Extension).to receive(:log) + .with( + "group", + 3, + 0, + a_string_starting_with("[My tag] [My other tag] [My third tag] Some message"), + Appsignal::Utils::Data.generate({}) + ) + + logger.tagged("My tag", "My other tag") do + logger.tagged("My third tag").info("Some message") + end + end + end + end end describe Appsignal::Logger do