Fix Ruby Logger add method signature

On the previous release, we added a new attribute to the `add` method of our Logger, which overwrites Ruby's base loggers. This made ActiveSupport Logger broadcast to fail as we called the function with 4 arguments internally after it was overwrittend by ActiveSupport, which expects 3 arguments. This commits addresses the problem by setting the attributes as an instance variable that is later taken inside the `add` method, so we can keep the signature untouched.
appsignal · Apr 4, 2023 · 5b7735a · 5b7735a
1 parent 0bcb0eb
commit 5b7735a
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 9 deletions.
diff --git a/.changesets/fix-logger-add-method-signature-error.md b/.changesets/fix-logger-add-method-signature-error.md
@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "fix"
+---
+
+Fix Logger add method signature error
diff --git a/lib/appsignal/logger.rb b/lib/appsignal/logger.rb
@@ -2,6 +2,7 @@
 
 require "logger"
 require "set"
+require "thread"
 
 module Appsignal
   # Logger that flushes logs to the AppSignal logging service
@@ -27,12 +28,13 @@ def initialize(group, level: INFO, format: PLAINTEXT)
       @group = group
       @level = level
       @format = format
+      @mutex = Mutex.new
     end
 
     # We support the various methods in the Ruby
     # logger class by supplying this method.
     # @api private
-    def add(severity, message = nil, group = nil, attributes = {})
+    def add(severity, message = nil, group = nil)
       severity ||= UNKNOWN
       return true if severity < level
       group = @group if group.nil?
@@ -52,7 +54,7 @@ def add(severity, message = nil, group = nil, attributes = {})
         SEVERITY_MAP.fetch(severity, 0),
         @format,
         message,
-        Appsignal::Utils::Data.generate(attributes)
+        Appsignal::Utils::Data.generate(appsignal_attributes)
       )
     end
     alias log add
@@ -65,7 +67,7 @@ def debug(message = nil, attributes = {})
       return if DEBUG < level
       message = yield if message.nil? && block_given?
       return if message.nil?
-      add(DEBUG, message, @group, attributes)
+      add_with_attributes(DEBUG, message, @group, attributes)
     end
 
     # Log an info level message
@@ -76,7 +78,7 @@ def info(message = nil, attributes = {})
       return if INFO < level
       message = yield if message.nil? && block_given?
       return if message.nil?
-      add(INFO, message, @group, attributes)
+      add_with_attributes(INFO, message, @group, attributes)
     end
 
     # Log a warn level message
@@ -87,7 +89,7 @@ def warn(message = nil, attributes = {})
       return if WARN < level
       message = yield if message.nil? && block_given?
       return if message.nil?
-      add(WARN, message, @group, attributes)
+      add_with_attributes(WARN, message, @group, attributes)
     end
 
     # Log an error level message
@@ -98,7 +100,7 @@ def error(message = nil, attributes = {})
       return if ERROR < level
       message = yield if message.nil? && block_given?
       return if message.nil?
-      add(ERROR, message, @group, attributes)
+      add_with_attributes(ERROR, message, @group, attributes)
     end
 
     # Log a fatal level message
@@ -109,7 +111,20 @@ def fatal(message = nil, attributes = {})
       return if FATAL < level
       message = yield if message.nil? && block_given?
       return if message.nil?
-      add(FATAL, message, @group, attributes)
+      add_with_attributes(FATAL, message, @group, attributes)
+    end
+
+    private
+
+    def add_with_attributes(severity, message, group, attributes)
+      Thread.current[:appsignal_logger_attributes] = attributes
+      add(severity, message, group)
+    ensure
+      Thread.current[:appsignal_logger_attributes] = nil
+    end
+
+    def appsignal_attributes
+      Thread.current.fetch(:appsignal_logger_attributes, {})
     end
   end
 end
diff --git a/spec/lib/appsignal/logger_spec.rb b/spec/lib/appsignal/logger_spec.rb
@@ -77,12 +77,12 @@
     describe "##{method[0]}" do
       it "should log with a message" do
         expect(Appsignal::Utils::Data).to receive(:generate)
-          .with({})
+          .with(:attribute => "value")
           .and_call_original
         expect(Appsignal::Extension).to receive(:log)
           .with("group", method[1], 0, "Log message", instance_of(Appsignal::Extension::Data))
 
-        logger.send(method[0], "Log message")
+        logger.send(method[0], "Log message", :attribute => "value")
       end
 
       it "should log with a block" do