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.

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

ChildLogger does not define #add #112

Closed
kalmenius opened this issue Dec 30, 2020 · 3 comments · Fixed by #113
Closed

ChildLogger does not define #add #112

kalmenius opened this issue Dec 30, 2020 · 3 comments · Fixed by #113

Comments

@kalmenius
Copy link

Because Ougai::ChildLogger does not subclass Ruby's stdlib Logger, I have run into issues when passing a "child" Ougai logger into a library which calls #add(...) on the passed logger.

A good example is in Bunny: https://github.com/ruby-amqp/bunny/blob/master/lib/bunny/transport.rb#L374

Here is a quick spec I prepared which illustrates the issue:

require 'json'
require 'ougai'

describe '#add should work' do
  let(:io) { StringIO.new }
  let(:logger) { Ougai::Logger.new(io) }

  it 'for normal loggers' do
    logger.add(Logger::INFO, 'foo')
    expect(JSON.parse(io.string)['msg']).to eq 'foo'
  end

  it 'for child loggers' do
    logger.child({ baz: 'qux' }).add(Logger::INFO, 'bar')
    JSON.parse(io.string).tap do |logged|
      expect(logged['msg']).to eq 'bar'
      expect(logged['baz']).to eq 'qux'
    end
  end
end

I think I have an idea for a clean fix if that would be helpful, but I wanted to see if this was intentional or not first.

Thanks very much! This is a great gem. 😃

@kalmenius kalmenius changed the title ChildLogger does not define 'add' ChildLogger does not define #add Dec 30, 2020
@kalmenius
Copy link
Author

kalmenius commented Dec 30, 2020

Below is the rough idea I had for a fix -- it has the added benefit of also introducing support for #log(...) to Ougai loggers.

Please let me know if this makes sense for this gem. If so, I'm happy to work on a branch which also includes some new specs.

diff --git a/lib/ougai/logger.rb b/lib/ougai/logger.rb
index 91632fa..c6f88cb 100644
--- a/lib/ougai/logger.rb
+++ b/lib/ougai/logger.rb
@@ -38,9 +38,9 @@ module Ougai
     # @param logger [Logger] The logger receiving broadcast logs.
     def self.broadcast(logger)
       Module.new do |mdl|
-        define_method(:log) do |*args|
-          logger.log(*args)
-          super(*args)
+        define_method(:log) do |*args, &block|
+          logger.log(*args, &block)
+          super(*args, &block)
         end
 
         define_method(:level=) do |level|
@@ -111,7 +111,7 @@ module Ougai
       hooks.each do |hook|
         return false if hook.call(data) == false
       end
-      add(severity, data)
+      method(:add).super_method.call(severity, data)
     end
 
     def to_item(args)
diff --git a/lib/ougai/logging.rb b/lib/ougai/logging.rb
index 0e65636..a93ccb8 100644
--- a/lib/ougai/logging.rb
+++ b/lib/ougai/logging.rb
@@ -28,7 +28,7 @@ module Ougai
     # @return [Boolean] true
     # @see Logging#debug
     def trace(message = nil, ex = nil, data = nil, &block)
-      log(TRACE, message, ex, data, block)
+      log(TRACE, message, ex, data, &block)
     end
 
     # Log any one or more of a message, an exception and structured data as DEBUG.
@@ -39,43 +39,42 @@ module Ougai
     # @yieldreturn [String|Exception|Object|Array] Any one or more of former parameters
     # @return [Boolean] true
     def debug(message = nil, ex = nil, data = nil, &block)
-      log(DEBUG, message, ex, data, block)
+      log(DEBUG, message, ex, data, &block)
     end
 
     # Log any one or more of a message, an exception and structured data as INFO.
     # @return [Boolean] true
     # @see Logging#debug
     def info(message = nil, ex = nil, data = nil, &block)
-      log(INFO, message, ex, data, block)
+      log(INFO, message, ex, data, &block)
     end
 
     # Log any one or more of a message, an exception and structured data as WARN.
     # @return [Boolean] true
     # @see Logging#debug
     def warn(message = nil, ex = nil, data = nil, &block)
-      log(WARN, message, ex, data, block)
+      log(WARN, message, ex, data, &block)
     end
 
     # Log any one or more of a message, an exception and structured data as ERROR.
     # @return [Boolean] true
     # @see Logging#debug
     def error(message = nil, ex = nil, data = nil, &block)
-      log(ERROR, message, ex, data, block)
+      log(ERROR, message, ex, data, &block)
     end
 
     # Log any one or more of a message, an exception and structured data as FATAL.
     # @return [Boolean] true
     # @see Logging#debug
     def fatal(message = nil, ex = nil, data = nil, &block)
-      log(FATAL, message, ex, data, block)
+      log(FATAL, message, ex, data, &block)
     end
 
     # Log any one or more of a message, an exception and structured data as UNKNOWN.
     # @return [Boolean] true
     # @see Logging#debug
     def unknown(message = nil, ex = nil, data = nil, &block)
-      args = block ? yield : [message, ex, data]
-      append(UNKNOWN, args)
+      log(UNKNOWN, message, ex, data, &block)
     end
 
     # Whether the current severity level allows for logging TRACE.
@@ -107,11 +106,11 @@ module Ougai
       end
     end
 
-    # @private
-    def log(severity, message, ex, data, block)
+    public
+    def add(severity, *args)
       return true if level > severity
-      args = block ? block.call : [message, ex, data]
-      append(severity, args)
+      append(severity, block_given? ? yield : args)
     end
+    alias log add
   end
 end

@tilfin
Copy link
Owner

tilfin commented Jan 10, 2021

released v1.9.0 for this.

@kalmenius
Copy link
Author

Thanks @tilfin! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants