Suggestion: Change Loggable logger initialization from lazy to eager

[keithrbennett]

Environment

• Ruby version 2.7.6
• Semantic Logger version 4.11.0.

---

The SemanticLogger::Loggable module provides an instance logger method that lazily initializes a @semantic_logger instance variable. If the instance is frozen the first time logger is called, then an error like this is raised:

...semantic_logger-4.11.0/lib/semantic_logger/loggable.rb:55:in `logger': 
can't modify frozen C: #<C:0x00000001100935b0> (FrozenError)

One solution to this is to call logger somewhere in your constructor. That way the instance variable will be initialized before the object can be frozen. The following script can be used to illustrate this:

#!/usr/bin/env ruby

require 'semantic_logger'

class C
  include SemanticLogger::Loggable

  def initialize
    # Enabling the `logger` line below will cause the logger instance variable to be initialized
    # here in the constructor, before `freeze` can be called on the object.
    # logger  
  end

  def foo
    logger.error 'error'
  end
end

c = C.new
c.freeze
c.foo

# If the logger has not been initialized before freezing, the following error will be raised:
#   semantic_logger-4.11.0/lib/semantic_logger/loggable.rb:55:in `logger': 
#   can't modify frozen C: #<C:0x00000001100935b0> (FrozenError)

It would be even nicer if there were an option to turn off lazy initialization, e.g.:

SemanticLogger.lazy_initialization_enabled = false

This would eliminate the need for the workaround in the constructor. This workaround would need to be in any class whose instances could potentially be frozen, which really means any class at all. As freezing objects becomes more prevalent, this will become more of an issue.

Alternatively, the nonlazy approach could be used by default, or possibly even be the only option.

[keithrbennett]

Thinking more about this, I understand the intention of minimizing memory use with the lazy initialization, but in many cases, I think it is premature optimization (not of speed but of memory usage). If I'm not mistaken, these loggers are very lightweight objects, and in many cases the number of classes including Loggable is not in an order of magnitude that would be significant with regard to memory usage. Given that it's also possible the number of classes would be high enough to care, I think the best solution is to provide an option.

As for the default setting, my personal opinion is that the risk of FrozenErrors is more important than the memory saving provided by the lazy initialization option, so that disabling lazy initialization would be the better default. Just my opinion...

[keithrbennett]

I have posted a PR that I would think would be a good fix for this, at #239.

[keithrbennett]

I have closed my PR at #239, as it is was an incomplete solution.

[keithrbennett]

After thinking more about this, I think the best solution would be to just change the lazy initialization to eager initialization, rather than to make it an option. This would be much simpler than making it optional. Is the lazy initialization important to anyone?

I have therefore changed the title of this issue from "Add install_eagerly method to Loggable" to "Change Loggable logger initialization from lazy to eager".

@reidmorrison, could you let me know what you think about this?

[reidmorrison]

The SemanticLogger::Loggable mix-in is designed to be lazy loaded. Recommend creating your own mix-in, or just add the logging methods directly to your class in the same way you would have to when using the built-in ruby Logger.