Synchronize Log4jLogEvent-related changes from 2.x

[jvz]

This updates Log4jLogEvent and its builder class to work similarly to what is in 2.25.0. The previously added MementoLogEvent class can be removed as the builder can make memento copies better.

This ports changes introduced in #3171 which seems somewhat inspired by changes made in main previously. However, due to the changes in Log4jLogEvent, this removes the need for the added memento class.

[vy]

@ppkarwasz, you authored #3171, which this PR originates from. I'd appreciate it if you can help with the review.

[ppkarwasz]

I'll review this PR later this week, but please note this is a challenging port.

The changes introduced in #3171 primarily address backward compatibility. The concept of "mutability" in version 2.x has been quite confusing, and I believe we have an opportunity to improve clarity significantly in 3.x. Specifically:

• The methods ReusableMessage.memento() and LogEvent.toMemento() have similar names but return fundamentally different types (Message vs. LogEvent). This naming overlap can cause confusion, especially since classes like ReusableLogEvent implement both interfaces (ReusableMessage and LogEvent).

• Introducing a method such as freeze()/isFrozen() could be beneficial in scenarios where the log event has reached its final state and is guaranteed not to change further.

• Considering that approximately 99% of log messages utilize the standard three types (ObjectMessage, SimpleMessage, and ParameterizedMessage), it may be advantageous to streamline the logging pipeline by exclusively using either Log4jLogEvent or MutableLogEvent. These classes could implement both the Message (or ReusableMessage) and LogEvent interfaces, thereby simplifying the architecture.

These are preliminary thoughts, and since I haven't reviewed the changes thoroughly yet, some of these ideas might already be implemented.

[jvz]

• The methods ReusableMessage.memento() and LogEvent.toMemento() have similar names but return fundamentally different types (Message vs. LogEvent). This naming overlap can cause confusion, especially since classes like ReusableLogEvent implement both interfaces (ReusableMessage and LogEvent).

The naming overlap with similar but not equal method names is related to what you identified here (classes implementing both Message and LogEvent). Perhaps it would make more sense for the methods to be renamed to toMementoMessage() and toMementoEvent(). In case it wasn't clear by the names, I used the term "memento" here as a design pattern name for a snapshot of some object. As the old implementation made somewhat more explicit, it's the logical equivalent of running a data class through some form of round trip serialization/deserialization. While a memento object might be modifiable thanks to the type system, the point of a memento object is that such accidental manipulation doesn't affect the original object. In general, though, this problem would be easier to denote in the type system if Java supported immutable types like Kotlin does. 🤷🏼

• Introducing a method such as freeze()/isFrozen() could be beneficial in scenarios where the log event has reached its final state and is guaranteed not to change further.

That sounds a little bit of an internal detail considering these checks are largely relevant to when an object is returned a pool (which is how we got rid of the frozen flag in some reusable class after adding Recycler).

• Considering that approximately 99% of log messages utilize the standard three types (ObjectMessage, SimpleMessage, and ParameterizedMessage), it may be advantageous to streamline the logging pipeline by exclusively using either Log4jLogEvent or MutableLogEvent. These classes could implement both the Message (or ReusableMessage) and LogEvent interfaces, thereby simplifying the architecture.

I think with the changes you made such that Log4jLogEvent is effectively immutable while Log4jLogEvent.Builder is mutable sort of works toward that idea. While I haven't verified the feasibility of it yet, perhaps MutableLogEvent can be replaced by Log4jLogEvent.Builder.

These are preliminary thoughts, and since I haven't reviewed the changes thoroughly yet, some of these ideas might already be implemented.

I did remove an extra LogEvent implementation thanks to the changes from 2.x. Perhaps there is more simplification possible with MutableLogEvent.

[ppkarwasz]

Hi @jvz,

Thanks for syncing these changes from the 2.x branch. The PR looks faithful to those updates, but I think a few customizations could help it align better with the current 3.x codebase. Let me know if you’d prefer to address them here or in follow-up PRs.

1. toMemento() behavior
   In 3.x, the new toMemento() method is documented as removing parameter references from the original message, but as far as I can tell it just calls Message.memento() or Message.getFormattedMessage(). It is also effectively a synonym for toImmutable(), so the docs suggesting otherwise seem misleading.

2. Event factory differences
   Since DefaultLogEventFactory no longer exists in 3.x, all new log events are MutableLogEvent instances. Log4jLogEvent only appears as a snapshot, and Log4jLogEvent.newBuilder() is only used in tests (with one exception in LoggerConfig). We could consider removing it together with the public no-arg Builder constructor.

3. Builder initialization
   Given the above, Log4jLogEvent.Builder no longer needs Clock and ContextDataInjector: those responsibilities now live in ReusableLogEventFactory.

4. Timestamp handling
   The Log4jLogEvent.Builder has logic for initializing timestamps with TimestampMessage. That logic seems to be missing from ReusableLogEventFactory.

5. Lazy initialization
   With Java 17 as the baseline, I don’t see a strong reason for getThreadName() (and similar methods) to remain lazy.

6. Making Builder more flexible
   It could be useful to make Log4jLogEvent.Builder more universal by letting the caller decide:

   • Whether to call getSource() and possibly compute the location
   • Whether to call getFormattedMessage() on the copied message
   • Whether to initialize the context map
   • Whether to initialize the timestamp

If you agree with some of these, feel free to tackle them here or open follow-ups against main, whichever makes more sense.

[ppkarwasz]

Isn't toMemento() a synonym of toImmutable() now? The suggestion of calling toImmutable() before calling this method is misleading.

[ppkarwasz]

Nitpick: instead of a public constructor, we could have a newBuilder(LogEvent) factory method.