Volkan,

No more hand waving. Please see 
https://github.com/apache/logging-log4j2/pull/2419.

I should note that while implementing the classes I added to support this makes 
it easier I did not have to make any changes to the logging internals to make 
this work.

Ralph

> On Mar 22, 2024, at 1:45 AM, Volkan Yazıcı <vol...@yazi.ci> wrote:
> 
> No, it is not the same thing Matt. Let me be as explicit as I can:
> 
> var logger0 = getLogger();  // MDC: {}
> var logger1 = logger0.withContextData("x", 1);  // MDC: {x: 1}
> var logger2 = logger1.withContextData("y", 2);  // MDC: {x: 1, y: 2}
> 
> This is the functionality being requested. Whoever claims this can be done 
> using a `MessageFactory`, they need to share a working [pseudo] code, instead 
> of hand waving. So far, nobody responded to this. Piotr, speculated on a 
> non-existing `Logger#withMessageFactory(MessageFactory)`, but there is not 
> one single working example shared. Hence, unless you can prove me wrong with 
> a working practical[1] example, the requested feature is currently known to 
> be not practically possible in Log4j.
> 
> [1] Implementing `logger.withContextData("x", 1)` with 50 LoC Java code using 
> the existing Log4j feature set is not a "practical example".
> 
> P.S. For Log4j 3 API Javadocs, you can browse to 
> https://logging.apache.org/log4j/3.x and search for "Javadoc" in the menu. 
> (Obviously, same works for Log4j 2 too.)
> 
> On Thu, Mar 21, 2024 at 6:10 PM Matt Sicker <m...@musigma.org> wrote:
> LogManager - log4j-api 3.0.0-alpha1 javadoc
> javadoc.io
> 
> Pass your custom MessageFactory here. It’s an optional argument when creating 
> the Logger.
> 
> Also, I’m not sure where to even find the current javadocs for the API. 
> javadoc.io only seems to have this alpha release.
> 
> 
>> On Mar 21, 2024, at 04:34, Volkan Yazıcı <vol...@yazi.ci> wrote:
>> 
>> Ralph, could you show how those two users can use a `MessageFactory` to
>> create `Logger`s with predefined additional context data?
>> 
>> On Thu, Mar 21, 2024 at 7:25 AM Ralph Goers <rgo...@apache.org> wrote:
>> 
>>> Unfortunately this is another message I somehow didn't get in my inbox.
>>> Replying to it via lists.a.o is not a great experience but is the best I
>>> can do.
>>> 
>>> On 2024/03/20 13:51:56 Volkan Yazıcı wrote:
>>>> I agree with the way Piotr dissects the problem. I think `ScopedContext`,
>>>> even though it has its own merits, doesn't address the problem reported
>>> by
>>>> users. They simply want a new logger associated with some additional
>>>> context data.
>>> 
>>> Two users do.  I have personally been asked for something like
>>> ScopedContext several times.
>>> As I replied to Piotr, we already solved the problem of adding data to
>>> Loggers. That is what MessageFactories are intended for.
>>> 
>>>> 
>>>> *[See my comments below.]*
>>>> 
>>>> On Mon, Mar 18, 2024 at 10:40 AM Piotr P. Karwasz <
>>> piotr.karw...@gmail.com>
>>>> wrote:
>>>> 
>>>>> * we can create a `Logger` wrapper "bound" to context data as Mikko
>>>>> does. This wrapper will take care of setting the `ThreadContext`
>>>>> before the logger call and restore it after it.
>>>> 
>>>> Creating a wrapper `Logger` can work without needing to deal with
>>>> `ThreadContext`. I can think of two different ways to carry this out:
>>>> 
>>>>   1. Currently, `AbstractLogger` only creates `Message`s. We can rework
>>> it
>>>>   to create `LogEvent`s too. Once `AbstractLogger` gets its hand on a
>>>>   `LogEvent`, it can enrich its context data as it wishes.
>>>>   2. We can extend `ContextDataInjector` with a new `void
>>>>   injectContextData(Logger logger, StringMap target)` method, provide a
>>>>   `ContextDataInjector` implementation that branches on `logger
>>> instanceof
>>>>   ContextDataProvider`, and call `ContextDataInjector` with the
>>> associated
>>>>   `Logger` in `LogEventFactory`.
>>> 
>>> We can do lots of things, most of which I wouldn't recommend. As to yours:
>>> 1. Logger/AbstractLogger got very complex with Async, Garbage Free,
>>> Reliablity Strategies, etc. Trying to move creating the LogEvent sooner is
>>> likely to be a major PITA and could seriously impact performance. While we
>>> could add a context map to AbstractLogger we would have to pass that on the
>>> logging calls to LoggerConfig and deal with all that that means - remember,
>>> a LoggerConfig can be handling multiple Loggers.
>>> 2. I don't recommend extending ContextDataInjector. That proved difficult
>>> to work with which is why we now recommend using ContextDataProviders. You
>>> really can only have one ContextDataInjector. Also, please note that
>>> ContextDataInjector is called while constructing the LogEvent. The LogEvent
>>> isn't passed the Logger, only the LoggerName. Looking up the Logger to do
>>> this is yet another way to slow down logging.
>>> 
>>>> 
>>>> On Tue, Mar 19, 2024 at 7:45 AM Ralph Goers <ralph.go...@dslextreme.com>
>>>> wrote:
>>>>> In the meantime, I provided
>>>> https://github.com/apache/logging-log4j2/pull/2385 which I very loosely
>>>> modeled after ScopedValues.
>>>> 
>>>> The fact that `ScopedContext` tries to imitate `ScopedValue` using
>>>> `ThreadLocal`s is extremely confusing (from a user's pov) and risky
>>>> liability (from a maintainer's pov). I guess you wanted to implement *a*
>>>> `ScopedValue` without using *the* `ScopedValue` to be compatible with
>>> Java
>>>> 8. If so, that really sounds like the `o.a.l.log4j.util.Supplier`
>>> downward
>>>> spiral. We can rather have an *internal* `Log4jScopedValue` interface and
>>>> provide Java 8 (using `InheritableThreadLocal`) and Java 21+ (using
>>>> `ScopedValue`) compatible solutions in an MRJ (Multi-Release JAR).
>>> 
>>> I am NOT trying to make ScopedContext compatible with ScopedValue. I am
>>> trying to make it conceptually close enough to ScopedValue that users will
>>> understand what it is doing.
>>> We can argue about naming if you want. Gary has already expressed his
>>> opinion.
>>>> 
>>>> We can integrate `ScopedContext` to the `LogEventFactory` by providing a
>>>> specialized `ContextDataInjector` plugin – assuming `LogEventFactory`
>>>> employs all available `ContextDataInjector` plugins.
>>> 
>>> ScopedContext is integrated with a ContextDataProvider, which is the
>>> supported way to do this. Again, you cannot have more than one
>>> ContextDataInjector so providing "specialized versions" is a pipe dream.
>>> You will simply have to enhance the one we already have.
>>> ContextDataInjector is NOT a plugin.
>>> 
>>>> 
>>>> I find the current ceremony also too long:
>>>> `ScopedContext.getCurrent().where("key1", "value1").run(...)`. I would
>>>> rather aim for `ScopedContext.run(key, value, runnable)` and similar
>>>> `ScopedContext.op(..., runnable)` interaction.
>>> 
>>> Those are going to be provided as well.
>>> 
>>> Ralph
>>> 
> 

Reply via email to