ppkarwasz commented on PR #2419:
URL: https://github.com/apache/logging-log4j2/pull/2419#issuecomment-2024651863

   @rgoers,
   
   > @rocketraman I did look into that and what you are asking for just can't 
be implemented in a reasonable way without significant mods to the API and will 
likely impact performance. The problem is that Loggers are singletons …
   
   The fact that there is only one **logger** per **logger name** is not set in 
stone (unlike in Logback). 
[`LoggerRegistry`](https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/spi/LoggerRegistry)
 is a multi-key map with a `(String, MessageFactory)` key.
   
   Due to the architectural choices you did at the beginning, `Logger`s don't 
even need to form a hierarchy, only `LoggerConfig`s are hierarchical. Let's 
profit from that and remove the "singleton" restriction from 
[`LogManager#getLogger`](https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/LogManager.html#getLogger(java.lang.Class,org.apache.logging.log4j.message.MessageFactory)).
 It is not a breaking change for the users to allow them to use different 
`MessageFactory` instances for the same logger name.
   
   > … and the code where the LogEvent is constructed and the ContextMap is 
populated is pretty far down the stack.
   
   IMHO, the `ParameterizedMapMessage` class that you introduce is already half 
way to meet @rocketraman's expectations:
   
   * by using a special 
[`LogEventFactory`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/impl/LogEventFactory)
 the map data can be added to the context map and the `baseMessage` can be used 
as message. Something similar already happens with 
[`TimestampMessage`](https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/message/TimestampMessage)s
 that add their own timestamp to the `LogEvent`,
   * certainly the specification of 
[`ContextDataInjector#injectContextData`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/ContextDataInjector#injectContextData(java.util.List,org.apache.logging.log4j.util.StringMap))
 must change: currently it already receives the current context data of the 
event as `reusable` parameter, but most implementation ignore it.
   
   Another thing that I like about your approach is that is solves #1813: in 
SLF4J 2.x there is a [list of key/value 
pairs](https://www.slf4j.org/api/org/slf4j/event/LoggingEvent.html#getKeyValuePairs())
 that is **distinct** from the context map. Currently I am mixing the two sets 
in `log4j-slf4j2-impl`, which is not what users expect.
   
   > We defer creating LogEvents to avoid the performance overhead when they 
aren't going to be used. Messages, on the other hand, are constructed very 
early as they are fairly light-weight. So doing that in a per-instance Logger 
that wraps the singleton was fairly easy to do.
   
   Is there really such a difference between 
[`Message`](https://logging.apache.org/log4j/2.x/javadoc/log4j-api/org/apache/logging/log4j/message/Message)
 and 
[`LogEvent`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/LogEvent)?
 I am very happy that there is a net separation (unlike in Logback) between:
   
   * the data that the user **explicitly** specified in his code,
   * the data that the logging backend added,
   
   but an implementation could very well choose to use something like 
[`MutableLogEvent`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/impl/MutableLogEvent)
 all the way from the `Logger` to the `Appender`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to