A weird Rails logger Pokémon

I love codebases for the surprises they offer. Some pieces of code jump at you like rare Pokémons and you gotta figure out what kind of a species it is and what it can do.

Gotta catch'em all!

In one project that I'm contributing to, I've recently encountered a Rails logger that I found weird. It's a special logger that is supposed to be used for data migrations and log to a specific log file (migrations.log).

class DataMigrations::Logger < ActiveSupport::Logger
  LOG_PATH = Rails.root.join("log/data_migrations.log")

  class << self
    delegate :debug, :info, :warn, :error, :fatal, :unknown, to: :logger

    private

    def logger
      @logger ||= ActiveSupport::TaggedLogging.new(
        ActiveSupport::Logger.new(LOG_PATH)
      )
    end
  end
end

The logger is designed to be used like this:

class DataMigrations::VideoThumbnailsToRecording
  def self.run
    DataMigrations::Logger.info("Start data migration")
    # ... more code ...
  end
end

So far so good.

After working with this logger a couple of times, I encountered some problems with it. For one, it doesn't offer an intuitive way of usage. For two, it was hard to test. But most importantly, I find that it was broken.

Not intuitive

I'm used to the Rails logger in Rails, it's easy:

Rails.logger.info("Log message")
# Or...
Rails.logger.tagged(log_id).warn("Almost failed 😬")

So maybe something like this would work too?

Migrations.logger.error("Better go home today, it doesn't make any sense at this point...")

Hard to test

As mentioned in my logger tests, I find value to test my loggers because I see them as part of the app's behavior and I want to see them working properly alongside the feature.

To recap real quick: In order to test your loggers, you need a logger instance so you can shove expected messages into it:

test "logger example"
  logger = Rails.logger
  expected_messages = ["message 1", "message 2"]
  
  logger.expects(:info).times(expected_messages.count).with do |actual|  
    actual =~ expected_messages.shift
  end
  
  DataMigrations::VideoThumbnailsToRecording.run
end

That's not possible here as easily because you can't get to the actual logger instance like this (it's private). You need to hack your way in by accessing the private logger method:

test "logger example"
  logger = DataMigrations::Logger.send(:logger)
  expected_messages = ["message 1", "message 2"]
  
  logger.expects(:info).times(expected_messages.count).with do |actual|  
    actual =~ expected_messages.shift
  end
  
  DataMigrations::VideoThumbnailsToRecording.run
end

All of this might be easy to grasp now, but it might not be as obvious when you work with a logger like this for the first time, trying to understand what and how it is doing things...

Broken

I first wrote an ugly custom method inside of DataMigrations::VideoThumbnailsToRecording so that my messages are prefixed with a log_id to look like this:

"[ji32j32] Start logging"
"[ji32j32] Log message 2"

One of the code reviewers mentioned that this is not the right responsibility for the data migration class and that it rather should go into DataMigrations::Logger itself...

So getting back into the actual logger code, I noticed that our logger is a subclass of ActiveSupport::TaggedLogging. Let's see what it can do:

https://api.rubyonrails.org/classes/ActiveSupport/TaggedLogging.html

# TL;DR
logger = ActiveSupport::TaggedLogging.new(Logger.new(STDOUT))
logger.tagged("BCX").info "Stuff"                 # Logs "[BCX] Stuff"
logger.tagged("BCX", "Jason").info "Stuff"        # Logs "[BCX] [Jason] Stuff"
logger.tagged("BCX").tagged("Jason").info "Stuff" # Logs "[BCX] [Jason] Stuff"

Basically, the exact functionality is already there.

But doing this didn't really work, exploding with a NoMethodError:

DataMigrations::Logger.tagged(log_id).info("Log message")

Because of the restrictive design of the logger:

# DataMigrations::Logger
  class << self
    delegate :debug, :info, :warn, :error, :fatal, :unknown, to: :logger

So the meat of the super ActiveSupport::TaggedLogging, the .tagged method, is not technically working because it's not added to delegate to the private .logger method.

Adding :tagged to the list of delegate args solves the issue:

# DataMigrations::Logger
  class << self
    # Also adding :tagged to open up the interface a bit more:
    delegate :tagged, :debug, :info, :warn, :error, :fatal, :unknown, to: :logger

🤷
But it does not solve the mystery in my head of why you would do that. Maybe the originator actually had something smart in mind there (unfortunately he has left the project, so there's no one to ask). Why would you restrict the base classes' interface to a set of hand-picked methods, disallowing others you may not know about or that may be added in future versions of the base class? And finally, am I the weirdo or the Pokélogger?