Replace LowLevelLogUtil with a more sophisticated bootstrap logger

[jvz]

This was something I experimented with in the spi-updates branch. Essentially, the strategy used by the low-level logging util should be an interface that begins with a simple implementation that enqueues status data elements until this reference is replaced with a StatusLogger-backed version once StatusLogger has been initialized. This would drain the accumulated status data to the logger.

Such an API would look something like this (abbreviated):

@NullMarked
public interface BootstrapLogger {
    void log(
            final Level level,
            @Nullable final Marker marker,
            final String message,
            @Nullable final Throwable exception);
}

This could potentially be changed to log a StatusData object or a newly defined object for bootstrap messages. In any case, this API should be internal to log4j-api (i.e., not exported in the module) as it's only relevant for classes involved in bootstrapping.

[vy]

@jvz, do we really need this sophistication? Both LowLevelLogUtil and StatusLogger are supposed to help Log4j components to log. That is, they are the logging framework of the logging framework – the Log4j. It is not expected to be used by anything but Log4j. Right? (Please correct me if I am wrong.) Why can't we simply have a StatusLogger such that

• It doesn't extend the logger API of Log4j
• It provides a public static void log(Level level, String message) method
• It provides a public static void log(Level level, String format, Object... args) method supporting Throwable in the last argument
• It provides a public static void isLevelEnabled(Level level) method
• It is only activated by a system property log4j.statusLogger.level=(debug|info|warn|error)
• It only buffers logged messages iff instructed so by a system property log4j.statusLogger.bufferLength=<positiveInteger>
• Buffering is exposed via public static Collection<StatusMessage> buffer(), where StatusMessage is composed of
  • Instant instant
  • Level level
  • String message
  • Throwable error
• It is destination is configured by a system property log4j.statusLogger.target=(stdout|stderr)

Note the similarity to Spring's DeferredLog.

I have a feeling this single-class-file design will not only greatly simplify the code, but will also serve all existing use cases. This will render the status attribute in the configuration files redundant.

@jvz, WDYT?

[jvz]

Hmm, that's an interesting proposal! I can try your idea out. The main purpose of my proposal is to make it easier to configure logging there as LowLevelLogUtil is option-less.

[jvz]

Reading what you said a little more closely, are you thinking we can replace StatusLogger entirely with this sort of simplification? If so, I also like that idea.

[rgoers]

1. There is really no good reason NOT to use Log4J-api for status logging. Most of the work is handled by AbstractLogger and the logging implementation only has to implement a couple of methods. This is actually one of the first decisions I made when I started the work.
2. LowLevelLogger is only used while processing PropertiesUtil. Your proposal will face the same problem as you require a property before setting up logging.

[vy]

1. There is really no good reason NOT to use Log4J-api for status logging. Most of the work is handled by AbstractLogger and the logging implementation only has to implement a couple of methods. This is actually one of the first decisions I made when I started the work.

There are several good reasons: It is bloated, difficult to implement, and extending from AbstractLogger is only making things worse due to various irrelevant requirements: MessageFactory, FlowMessageFactory, Recycler, etc. I cannot care less about any of these details when using StatusLogger. All I need is a glorified printf().

StatusLogger is already pretty thick. We even went further and enhanced it with LowLevelLogUtil. I am not able to see how this complexity is needed given the simple goal a low level logger needs to serve.

LowLevelLogger is only used while processing PropertiesUtil. Your proposal will face the same problem as you require a property before setting up logging.

It won't, because I propose a self-contained StatusLogger class that doesn't depend on any Log4j class and access system properties via System.getProperty().

Let me make it clear: the StatusLogger I propose will only contain the members/features I described above and nothing else. It won't use Log4j PropertiesUtil, Clock, Message, etc.

I would like to submit a PR for this if there are no objections.

[jvz]

Works for me! Since we're using Java 17, you can even make StatusMessage a record class.

[jvz]

Update: I'm going to update StatusLogger to work without PropertiesUtil such that we can remove LowLevelLogUtil. Due to our plans related to separating the API from 2.x for use in 3.x, I'll only make this change in the 2.x branch.

[jvz]

Superseded by #2221