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

Logger Tags don't get broadcasted to another Tagged Logger #43291

Closed
jvillarejo opened this issue Sep 22, 2021 · 4 comments
Closed

Logger Tags don't get broadcasted to another Tagged Logger #43291

jvillarejo opened this issue Sep 22, 2021 · 4 comments

Comments

@jvillarejo
Copy link
Contributor

I'm using ActiveSupport::Logger.broadcast to broadcast logging to multiple loggers.
For example STDOUT, File in log/ folder and another RemoteSysLogger

Also I'm using ActiveSupport::TaggedLogging to configure tags with middleware on the config file:

  config.log_tags = [:host, :request_id] 

But this log tags don't get broadcasted to the other logs.
I don't know if there something that I'm configuring wrong.

Steps to reproduce

Add on development.rb

  config.log_tags = [:host, :uuid]
  config.after_initialize do
    other_logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(Rails.root.join('log', "other.log")))
    Rails.logger.extend(ActiveSupport::Logger.broadcast(other_logger))
  end

Make a GET request to root_path '/'

Expected behavior

I expected that other.log file had request tags: [:host, :uuid]

# log/development.log
[localhost] [cfb7f921-0a7c-476a-8b0c-166963b87e4f] Started GET "/" for ::1 at 2021-09-22 15:44:50 -0300
# log/other.log
[localhost] [cfb7f921-0a7c-476a-8b0c-166963b87e4f] Started GET "/" for ::1 at 2021-09-22 15:44:50 -0300

Actual behavior

However other log didn't include tags

# log/development.log
[localhost] [cfb7f921-0a7c-476a-8b0c-166963b87e4f] Started GET "/" for ::1 at 2021-09-22 15:44:50 -0300
# log/other.log
Started GET "/" for ::1 at 2021-09-22 15:44:50 -0300

System configuration

Rails version: 6.1
Ruby version: 2.7

@jvillarejo
Copy link
Contributor Author

In order to make it work I apply a monkey patch on my application:

module TaggedBroadcast
  extend ActiveSupport::Concern

  class_methods do
    def broadcast(logger)
      super(logger).tap do |broadcast_logger_class|
        broadcast_logger_class.define_method(:tagged) do |*tags, &block|
          if logger.respond_to?(:tagged)
            logger.tagged(*tags) do 
              if defined?(super)
                super(*tags, &block)
              else
                block.call(self)
              end
            end
          else
            if defined?(super)
              super(*tags, &block)
            else
              block.call(self)
            end
          end
        end
      end
    end
  end
end

ActiveSupport::Logger.prepend(TaggedBroadcast)

I can submit a PR for this fix in ActiveSupport:Logger so it support tagged if the logger responds_to?(:tagged)
Or if you give me a better directions on how to solve it, I can do it too. 😄

@rafaelfranca
Copy link
Member

We have a test to make sure this works

@logger.extend(ActiveSupport::Logger.broadcast(broadcast_logger))
, and this test is passing.

I think you need to investigate why your setup is not behaving in the same way as the test.

@rafaelfranca
Copy link
Member

Ah, it seems it is only present in Rails 7. 70af536. I'll backport to 6-1-stable.

Fixed by b21394c

@jvillarejo
Copy link
Contributor Author

Cool, thanks!
Sorry I didn't search before posting 😿

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

No branches or pull requests

2 participants