Re: Context data propagation and logger-bound context data

[Ralph Goers]

In theory that library could use the ContextData class I just added to get 
context data no matter how it got there. However, the Micrometer library’s 
setValue and getValue set and retrieve the full map, not values. That won’t 
play nice with a ScopedContext since once you set it you would immediately be 
leaving the context so what you set would be instantly forgotten. Under the 
covers of course it could push a new map on the queue but that would likely 
behave very differently than the way ScopedContext does - i.e. you are 
completely replacing maps not incrementally modifying them.

Ralph

> On Apr 15, 2024, at 10:07 AM, Piotr P. Karwasz  
> wrote:
> 
> Hi,
> 
> On Mon, 15 Apr 2024 at 18:44, Matt Sicker  > wrote:
>> Context Propagation support :: Micrometer Context Propagation
>> docs.micrometer.io
>> 
>>  Context 
>> Propagation support :: Micrometer Context Propagation 
>> 
>> docs.micrometer.io 
>> 
>> 
>> 
>> This is more in line with what I was thinking we should support. I’m not 
>> suggesting that we add Micrometer as a dependency to the API or Core, but we 
>> can replicate the minimal API here or figure out a smaller API for 
>> integration purposes.
> 
> I agree. To support `context-propagation` we don't need to add anything to 
> the API or SPI. To support it efficiently, we need the two methods from PR 
> [1].
> 
> Note that SLF4J is already supported by `context-propagation` (cf. [2]).
> 
> Piotr
> 
> [1] https://github.com/apache/logging-log4j2/pull/2442
> [2] https://github.com/micrometer-metrics/context-propagation/pull/194

Re: Context data propagation and logger-bound context data

[Piotr P. Karwasz]

Hi,

On Mon, 15 Apr 2024 at 18:44, Matt Sicker  wrote:

> Context Propagation support :: Micrometer Context Propagation
> 
> docs.micrometer.io
> 
> [image: favicon.ico]
> 
> 
>
> This is more in line with what I was thinking we should support. I’m not
> suggesting that we add Micrometer as a dependency to the API or Core, but
> we can replicate the minimal API here or figure out a smaller API for
> integration purposes.
>

I agree. To support `context-propagation` we don't need to add anything to
the API or SPI. To support it efficiently, we need the two methods from PR
[1].

Note that SLF4J is already supported by `context-propagation` (cf. [2]).

Piotr

[1] https://github.com/apache/logging-log4j2/pull/2442
[2] https://github.com/micrometer-metrics/context-propagation/pull/194

Re: Context data propagation and logger-bound context data

[Matt Sicker]

https://docs.micrometer.io/context-propagation/reference/

This is more in line with what I was thinking we should support. I’m not 
suggesting that we add Micrometer as a dependency to the API or Core, but we 
can replicate the minimal API here or figure out a smaller API for integration 
purposes.

Note that similar projects like the OpenTelemetry context propagation API is 
geared toward cross-process context propagation, not cross-thread or similar 
lower level primitives. The low level primitives belong in Log4j IMO.


> On Mar 27, 2024, at 16:56, Ralph Goers  wrote:
> 
> 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ı  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  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ı  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  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 

Re: Context data propagation and logger-bound context data

[Ralph Goers]

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ı  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  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ı  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  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 
 wrote:
> In the meantime, I provided
 https://github.com/apache/logging-log4j2/pull/2385 which I very loosely
 modeled after ScopedValues.

Re: Context data propagation and logger-bound context data

[Ralph Goers]

> On Mar 22, 2024, at 1:45 AM, Volkan Yazıcı  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.

I am working on this Volkan and with real code. It will be included in my PR 
branch shortly. You simply have to be patient.

Ralph

Re: Context data propagation and logger-bound context data

[Piotr P. Karwasz]

Hi Volkan,

On Fri, 22 Mar 2024 at 09:45, Volkan Yazıcı  wrote:

> *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.)
>

You can also Google "log4j javadoc" now!
Apparently our 301 HTTP redirects are working and Google does not index
`/log4j/log4j-2.3.4` any more, but the redirected resource.

Piotr

Re: Context data propagation and logger-bound context data

[Volkan Yazıcı]

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  wrote:

> LogManager - log4j-api 3.0.0-alpha1 javadoc
> 
> javadoc.io
> 
> [image: fb2db6ea7688d54ae047109e0d71e3a0-favicon-32.png]
> 
> 
>
> 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ı  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  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 
> wrote:
>
> In the meantime, I 

Re: Context data propagation and logger-bound context data

[Piotr P. Karwasz]

Hi Ralph,

On Fri, 22 Mar 2024 at 03:08, Ralph Goers  wrote:
> > On Mar 21, 2024, at 3:19 PM, Piotr P. Karwasz  
> > wrote:
> >
> >
> > PS: In the `main` branch I plan to replace the default
> > ReusableMessageFactory with ReusableLogEventFactory:
> >
> > * this change should have a minimal impact on memory (events are not
> > much larger than messages),
> > * it can skip a data transfer between ReusableMessage and MutableLogEvent,
> > * it allows the message factory to collect data that is usually
> > reserved for events.
>
> I have to question this. I don’t understand the concept of replacing a 
> MessageFactory with a LogEventFactory. They are completely different things. 
> What does this even mean?

We already have two `LogEvent` implementations `MutableLogEvent` (used
by `ReusableLogEventFactory`) and `RingBufferLogEvent` that also
implement `ReusableMessage`. I plan to merge the two
`ReusableMessageFactory` and `ReusableLogEventFactory` classes into a
single entity that will produce objects of a single type:
`MutableLogEvent`.

Currently the default Log4j Core 3.x configuration has 4 object pools:
one for `ReusableSimpleMessage`, one for `ReusableObjectMessage`, one
for `ReusableParameterizedMessage` and one for `MutableLogEvent`.
After the changes only one pool will remain.

Piotr

Re: Context data propagation and logger-bound context data

[Raman Gupta]

On Thu, Mar 21, 2024 at 9:58 PM Ralph Goers 
wrote:

>
> > On Mar 21, 2024, at 2:48 PM, Raman Gupta  wrote:
> >
> > On Thu, Mar 21, 2024 at 5:30 AM Piotr P. Karwasz <
> piotr.karw...@gmail.com>
> > wrote:
> >
> >> Hi Ralph,
> >>
> >> On Thu, 21 Mar 2024 at 07:03, Ralph Goers 
> >> wrote:
>  1. Raman and Mikko would like to bind context data to an object
>  implementing the `Logger` interface or more generally to a service
>  object (e.g. resource manager, DAO and variants),
> >>>
> >>> Yes, I’ve seen that. Personally, I am not much of a fan of this use
> case
> >> as it is pretty easy to add the data you want to a single class. That
> said,
> >> we already offer a solution. Allowing a MessageFactory to be provided
> on a
> >> Logger was done for exactly this reason.
> >>>
> >>> For example, a User could configure a custom MessageFactory that
> >> provides an extension of MapMessage that causes the message to include
> data
> >> from the class or resource. Going to the extreme of trying to shoehorn
> in
> >> Context data as well simply isn’t necessary.
> >>
> >> Great point! This effectively solves Raman's and Mikko's problem. We
> >> just need to introduce a new sub-interface of `Message` and require
> >> all implementations to support it.
> >>
> >>
> > I'm not sure it does. I differentiate between context and message. When I
> > want the data from the resource in the context, it's because the data is
> > contextual, not part of the message.
> >
> > This approach seems to be not much better than making sure all log
> messages
> > in a class have a consistent string format so it can be grepped.
>
> No, it is nothing like that. LogEvents effectively support 2 kinds of
> Maps; those associated with the ThreadContext (or injected into the
> LogEvents contextMap, or key/value pairs associated with MapMessages. In
> the PatternLayout these are distinguished by %X and %K. If you know the
> keys you want you can use a pattern like:
>
> %d %p %C{1.} [%t] {requestId=%X{requestId}
> customerNumber=%X{customerNumber} userName=%K{userName} %{userType} - %m%n
>
> Thus making the ThreadContext key/values and Message key/values virtually
> indistinguishable.


When one controls the layout with something like PatternLayout, yes.
However in structured logging use cases (e.g. to Google Cloud Logging) the
context and message maps are shown under separate "context" and "message"
keys. As they should be because they are different things.

We have context and we have a message -- I don't think it should be
considered best practice to conflate these.


> The only issue at the moment is the result of %m on a MapMessage isn’t
> really what you would like. I am looking into addressing that as I have run
> across this before in things I have tried to do.
>
> Ralph

Re: Context data propagation and logger-bound context data

[Ralph Goers]

> On Mar 21, 2024, at 3:19 PM, Piotr P. Karwasz  wrote:
> 
> 
> PS: In the `main` branch I plan to replace the default
> ReusableMessageFactory with ReusableLogEventFactory:
> 
> * this change should have a minimal impact on memory (events are not
> much larger than messages),
> * it can skip a data transfer between ReusableMessage and MutableLogEvent,
> * it allows the message factory to collect data that is usually
> reserved for events.

I have to question this. I don’t understand the concept of replacing a 
MessageFactory with a LogEventFactory. They are completely different things. 
What does this even mean?

Ralph

Re: Context data propagation and logger-bound context data

[Ralph Goers]

> On Mar 21, 2024, at 2:48 PM, Raman Gupta  wrote:
> 
> On Thu, Mar 21, 2024 at 5:30 AM Piotr P. Karwasz 
> wrote:
> 
>> Hi Ralph,
>> 
>> On Thu, 21 Mar 2024 at 07:03, Ralph Goers 
>> wrote:
 1. Raman and Mikko would like to bind context data to an object
 implementing the `Logger` interface or more generally to a service
 object (e.g. resource manager, DAO and variants),
>>> 
>>> Yes, I’ve seen that. Personally, I am not much of a fan of this use case
>> as it is pretty easy to add the data you want to a single class. That said,
>> we already offer a solution. Allowing a MessageFactory to be provided on a
>> Logger was done for exactly this reason.
>>> 
>>> For example, a User could configure a custom MessageFactory that
>> provides an extension of MapMessage that causes the message to include data
>> from the class or resource. Going to the extreme of trying to shoehorn in
>> Context data as well simply isn’t necessary.
>> 
>> Great point! This effectively solves Raman's and Mikko's problem. We
>> just need to introduce a new sub-interface of `Message` and require
>> all implementations to support it.
>> 
>> 
> I'm not sure it does. I differentiate between context and message. When I
> want the data from the resource in the context, it's because the data is
> contextual, not part of the message.
> 
> This approach seems to be not much better than making sure all log messages
> in a class have a consistent string format so it can be grepped.

No, it is nothing like that. LogEvents effectively support 2 kinds of Maps; 
those associated with the ThreadContext (or injected into the LogEvents 
contextMap, or key/value pairs associated with MapMessages. In the 
PatternLayout these are distinguished by %X and %K. If you know the keys you 
want you can use a pattern like:

%d %p %C{1.} [%t] {requestId=%X{requestId} customerNumber=%X{customerNumber} 
userName=%K{userName} %{userType} - %m%n

Thus making the ThreadContext key/values and Message key/values virtually 
indistinguishable. 

The only issue at the moment is the result of %m on a MapMessage isn’t really 
what you would like. I am looking into addressing that as I have run across 
this before in things I have tried to do.

Ralph

Re: Context data propagation and logger-bound context data

[Raman Gupta]

On Thu, Mar 21, 2024 at 6:19 PM Piotr P. Karwasz 
wrote:

> Hi Raman,
>
> On Thu, 21 Mar 2024 at 22:51, Raman Gupta  wrote:
> > > > For example, a User could configure a custom MessageFactory that
> > > provides an extension of MapMessage that causes the message to include
> data
> > > from the class or resource. Going to the extreme of trying to shoehorn
> in
> > > Context data as well simply isn’t necessary.
> > >
> > > Great point! This effectively solves Raman's and Mikko's problem. We
> > > just need to introduce a new sub-interface of `Message` and require
> > > all implementations to support it.
> > >
> > >
> > I'm not sure it does. I differentiate between context and message. When I
> > want the data from the resource in the context, it's because the data is
> > contextual, not part of the message.
> >
> > This approach seems to be not much better than making sure all log
> messages
> > in a class have a consistent string format so it can be grepped.
>
> I was thinking about a code like this:
>
> private static final Logger mainLogger = LogManager.getLogger();
>
> private final Logger logger;
>
> public ResourceManager() {
> Map contextData = Map.of(...);
> logger = mainLogger.withMessageFactory(new
> ContextualMessageFactory(contextData));
> }
>
> The new `Logger#withMessageFactory` method will give you a logger with
> the same name as `mainLogger`, but with a different message factory.
> The message factory will create LogEvents initialized with the given
> context, which will be further complemented by the context data
> injected by ContextDataInjector. Is this the effect you are looking
> for?
>

Yes, your description sounds like what I'm looking for, but I am confused
with the name `withMessageFactory`. As I understand it, the
`MessageFactory` interface allows creating Messages, not LogEvents with
context.

Thanks,
Raman


> Piotr
>
> PS: In the `main` branch I plan to replace the default
> ReusableMessageFactory with ReusableLogEventFactory:
>
> * this change should have a minimal impact on memory (events are not
> much larger than messages),
> * it can skip a data transfer between ReusableMessage and MutableLogEvent,
> * it allows the message factory to collect data that is usually
> reserved for events.
>

Re: Context data propagation and logger-bound context data

[Piotr P. Karwasz]

Hi Raman,

On Thu, 21 Mar 2024 at 22:51, Raman Gupta  wrote:
> > > For example, a User could configure a custom MessageFactory that
> > provides an extension of MapMessage that causes the message to include data
> > from the class or resource. Going to the extreme of trying to shoehorn in
> > Context data as well simply isn’t necessary.
> >
> > Great point! This effectively solves Raman's and Mikko's problem. We
> > just need to introduce a new sub-interface of `Message` and require
> > all implementations to support it.
> >
> >
> I'm not sure it does. I differentiate between context and message. When I
> want the data from the resource in the context, it's because the data is
> contextual, not part of the message.
>
> This approach seems to be not much better than making sure all log messages
> in a class have a consistent string format so it can be grepped.

I was thinking about a code like this:

private static final Logger mainLogger = LogManager.getLogger();

private final Logger logger;

public ResourceManager() {
Map contextData = Map.of(...);
logger = mainLogger.withMessageFactory(new
ContextualMessageFactory(contextData));
}

The new `Logger#withMessageFactory` method will give you a logger with
the same name as `mainLogger`, but with a different message factory.
The message factory will create LogEvents initialized with the given
context, which will be further complemented by the context data
injected by ContextDataInjector. Is this the effect you are looking
for?

Piotr

PS: In the `main` branch I plan to replace the default
ReusableMessageFactory with ReusableLogEventFactory:

* this change should have a minimal impact on memory (events are not
much larger than messages),
* it can skip a data transfer between ReusableMessage and MutableLogEvent,
* it allows the message factory to collect data that is usually
reserved for events.

Re: Context data propagation and logger-bound context data

[Raman Gupta]

On Thu, Mar 21, 2024 at 5:30 AM Piotr P. Karwasz 
wrote:

> Hi Ralph,
>
> On Thu, 21 Mar 2024 at 07:03, Ralph Goers 
> wrote:
> > > 1. Raman and Mikko would like to bind context data to an object
> > > implementing the `Logger` interface or more generally to a service
> > > object (e.g. resource manager, DAO and variants),
> >
> > Yes, I’ve seen that. Personally, I am not much of a fan of this use case
> as it is pretty easy to add the data you want to a single class. That said,
> we already offer a solution. Allowing a MessageFactory to be provided on a
> Logger was done for exactly this reason.
> >
> > For example, a User could configure a custom MessageFactory that
> provides an extension of MapMessage that causes the message to include data
> from the class or resource. Going to the extreme of trying to shoehorn in
> Context data as well simply isn’t necessary.
>
> Great point! This effectively solves Raman's and Mikko's problem. We
> just need to introduce a new sub-interface of `Message` and require
> all implementations to support it.
>
>
I'm not sure it does. I differentiate between context and message. When I
want the data from the resource in the context, it's because the data is
contextual, not part of the message.

This approach seems to be not much better than making sure all log messages
in a class have a consistent string format so it can be grepped.



> To be sure we don't cherry-pick each element of a log event with a
> different interface (like we did with `TimeStampMessage`), let's use a
> single interface to supply all the additional data the user might want
> to have. Let's throw in it Ceki's `KeyValuePair`s[1]
>
> Piotr
>
> [1] https://www.slf4j.org/api/org/slf4j/event/LoggingEvent.html
>

Re: Context data propagation and logger-bound context data

[Matt Sicker]

https://www.javadoc.io/doc/org.apache.logging.log4j/log4j-api/3.0.0-alpha1/org.apache.logging.log4j/org/apache/logging/log4j/LogManager.html

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ı  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  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 
>>> 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 

Re: Context data propagation and logger-bound context data

[Volkan Yazıcı]

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  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 
> > 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
>

Re: Context data propagation and logger-bound context data

[Piotr P. Karwasz]

Hi Ralph,

On Thu, 21 Mar 2024 at 07:03, Ralph Goers  wrote:
> > 1. Raman and Mikko would like to bind context data to an object
> > implementing the `Logger` interface or more generally to a service
> > object (e.g. resource manager, DAO and variants),
>
> Yes, I’ve seen that. Personally, I am not much of a fan of this use case as 
> it is pretty easy to add the data you want to a single class. That said, we 
> already offer a solution. Allowing a MessageFactory to be provided on a 
> Logger was done for exactly this reason.
>
> For example, a User could configure a custom MessageFactory that provides an 
> extension of MapMessage that causes the message to include data from the 
> class or resource. Going to the extreme of trying to shoehorn in Context data 
> as well simply isn’t necessary.

Great point! This effectively solves Raman's and Mikko's problem. We
just need to introduce a new sub-interface of `Message` and require
all implementations to support it.

To be sure we don't cherry-pick each element of a log event with a
different interface (like we did with `TimeStampMessage`), let's use a
single interface to supply all the additional data the user might want
to have. Let's throw in it Ceki's `KeyValuePair`s[1]

Piotr

[1] https://www.slf4j.org/api/org/slf4j/event/LoggingEvent.html

Re: Context data propagation and logger-bound context data

[Piotr P. Karwasz]

Hi Ralph,

On Wed, 20 Mar 2024 at 17:11, Ralph Goers  wrote:
>
> 1. I am experimenting with adding Thread support to ScopedContext as we 
> speak. It should be straightforward.
>
> I am not sure we ever want to deal with ScopedValue. ScopedContext provides 
> the same functionality plus integration with logging.

>From what I read about ScopedValue I am also less and less convinced
we want to use it. It can be used this way:

ScopedValue.where(KEY1, "value1").where(KEY2, "value2").run(() ->
doSomething());

Normally the values are not propagated to other "threads" or anything
of this sort. Sure, `doSomething` can run as virtual thread on
multiple Java `Thread`s (and each of them can run on multiple CPU
cores), but as soon as `doSomething` calls `new Thread` or calls
`ExecutorService.submit`, the context is lost.

There is a **special** executor (`StructuredTaskScope`) that will
automatically copy the values bound to a `ScopedValue` before jumping
threads, but there are specialized executors that already do that.

Why don't we implement the same functionality with a **minimal** API extension?

CloseableThreadContext.put("key1", "value1").put("key2",
"value2").run(() -> doSomething());

Piotr

Re: Context data propagation and logger-bound context data

[Ralph Goers]

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 
> 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 
> 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

Re: Context data propagation and logger-bound context data

[Ralph Goers]

> On Mar 18, 2024, at 2:39 AM, Piotr P. Karwasz  wrote:
> 
> Hi,
> 
> Given the ongoing context data propagation in three Github
> issues/PRs/discussions (cf. [1], [2] and [3]). I think we should move
> the debate to the mailing list, which guarantees the maximal reachout.
> 
> == Problem summary
> 
> We have two different problems regarding context data in log files:
> 
> 1. Raman and Mikko would like to bind context data to an object
> implementing the `Logger` interface or more generally to a service
> object (e.g. resource manager, DAO and variants),

Yes, I’ve seen that. Personally, I am not much of a fan of this use case as it 
is pretty easy to add the data you want to a single class. That said, we 
already offer a solution. Allowing a MessageFactory to be provided on a Logger 
was done for exactly this reason.

For example, a User could configure a custom MessageFactory that provides an 
extension of MapMessage that causes the message to include data from the class 
or resource. Going to the extreme of trying to shoehorn in Context data as well 
simply isn’t necessary.


> 2. Other users would like to bind context data to a processing
> pipeline, regardless of which threads it uses.
> 

ScopedContext covers this use case and is the one I find far more compelling. 
That is why I am adding support for passing data to Threads to my PR. I will 
let you know when it is there.

Ralph

Re: Context data propagation and logger-bound context data

[Ralph Goers]

1. I am experimenting with adding Thread support to ScopedContext as we speak. 
It should be straightforward. 

I am not sure we ever want to deal with ScopedValue. ScopedContext provides the 
same functionality plus integration with logging.

Ralph

> On Mar 20, 2024, at 4:22 AM, Piotr P. Karwasz  wrote:
> 
> Hi Ralph,
> 
> On Tue, 19 Mar 2024 at 07:45, Ralph Goers  wrote:
>> 2. The links you provide describe the problem of passing contexts to threads 
>> magnificently. Unfortunately, they solve it by requiring you NOT to use 
>> standard JDK tooling for managing threads. I am not prepared to be dependent 
>> on any specific framework other than the JDK.
>> ...
>> 3. As I stated, I am not prepared to provide a solution that is dependent on 
>> another framework. However that framework is welcome to integrate with us.
> 
> I would propose to meet those libraries half-way:
> 
> * we extend our SPI to provide a fast method to copy the contents of
> the ThreadLocal used by ThreadContextMap, restore it on another thread
> or clear it,
> * the `context-propagation` library integrates our newly added SPI.
> 
>> 2. As I noted, adding a wrapper around a Logger doesn’t accomplish anything, 
>> at least in terms of getting data into LogEvents.
> 
> The wrapper could do something like:
> 
> public void info(String message) {
>if (logger.isEnabled(null, Level.INFO, message)) {
>try (CloseableThreadContext.Instance ignored =
> CloseableThreadContext.putAll(context)) {
>logger.info(message);
>}
>}
> }
> 
> using only Log4j API. Log4j Core could do better of course.
> 
>> SLF4J has recently introduced 
>> https://www.slf4j.org/apidocs-2.1.0-alpha0/org.slf4j/org/slf4j/MDCAmbit.html 
>> which to me looks like a horrible way to add a “scope-based” context as it 
>> relies on the user to perform cleanup.
> 
> This is a horrible workaround of the way "try-with-resources" work,
> when "catch" clauses are also present. Since the resource is closed
> **before** the "catch" code is executed, the new context keys are no
> longer available in the "catch" clauses.
> 
>> Java 21 introduces ScopedValues - 
>> https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/ScopedValue.html.
>>  It does allow ScopedValues to be passed to child threads but I haven’t 
>> experimented with it enough yet to know how it really works.
> 
> In the future we will certainly switch to this, but for now I would
> keep all usages of `ScopedValue` in an `o.a.l.l.experimental` package,
> so that users know that we can remove it any time we want.
> 
> Piotr

Re: Context data propagation and logger-bound context data

[Volkan Yazıcı]

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.

*[See my comments below.]*

On Mon, Mar 18, 2024 at 10:40 AM Piotr P. Karwasz 
wrote:
> == Possible solutions
>
> As far as I know there are currently the following approaches we can
> take for problem 1:
>
> * we can add a list of `` elements in the configuration of a
> ``, which will add the given keys to all loggers using that
> configuration. This is something we can do for very static data, e.g.
> we can add to each log statement the name of the library that issued
> it.

I don't like this approach. This is simply a hack, an abuse of a component
designed for totally something else.

> * 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`.

On Tue, Mar 19, 2024 at 7:45 AM Ralph Goers 
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).

We can integrate `ScopedContext` to the `LogEventFactory` by providing a
specialized `ContextDataInjector` plugin – assuming `LogEventFactory`
employs all available `ContextDataInjector` plugins.

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.

I will also drop some other remarks to the PR.

Re: Context data propagation and logger-bound context data

[Piotr P. Karwasz]

Hi Ralph,

On Tue, 19 Mar 2024 at 07:45, Ralph Goers  wrote:
> 2. The links you provide describe the problem of passing contexts to threads 
> magnificently. Unfortunately, they solve it by requiring you NOT to use 
> standard JDK tooling for managing threads. I am not prepared to be dependent 
> on any specific framework other than the JDK.
> ...
> 3. As I stated, I am not prepared to provide a solution that is dependent on 
> another framework. However that framework is welcome to integrate with us.

I would propose to meet those libraries half-way:

* we extend our SPI to provide a fast method to copy the contents of
the ThreadLocal used by ThreadContextMap, restore it on another thread
or clear it,
* the `context-propagation` library integrates our newly added SPI.

> 2. As I noted, adding a wrapper around a Logger doesn’t accomplish anything, 
> at least in terms of getting data into LogEvents.

The wrapper could do something like:

public void info(String message) {
if (logger.isEnabled(null, Level.INFO, message)) {
try (CloseableThreadContext.Instance ignored =
CloseableThreadContext.putAll(context)) {
logger.info(message);
}
}
}

using only Log4j API. Log4j Core could do better of course.

> SLF4J has recently introduced 
> https://www.slf4j.org/apidocs-2.1.0-alpha0/org.slf4j/org/slf4j/MDCAmbit.html 
> which to me looks like a horrible way to add a “scope-based” context as it 
> relies on the user to perform cleanup.

This is a horrible workaround of the way "try-with-resources" work,
when "catch" clauses are also present. Since the resource is closed
**before** the "catch" code is executed, the new context keys are no
longer available in the "catch" clauses.

> Java 21 introduces ScopedValues - 
> https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/ScopedValue.html.
>  It does allow ScopedValues to be passed to child threads but I haven’t 
> experimented with it enough yet to know how it really works.

In the future we will certainly switch to this, but for now I would
keep all usages of `ScopedValue` in an `o.a.l.l.experimental` package,
so that users know that we can remove it any time we want.

Piotr

Re: Context data propagation and logger-bound context data

[Ralph Goers]

First, lets consider the two usages you highlighted

1. In theory it is possible to add properties to a Logger. In practice, we only 
allow this from the LoggerConfig. While the values of those properties can 
include Lookup references this isn’t going to help much since there is no way 
for the Lookup to reference any specific object instance. While the user could 
override Logger they would also have to override AsyncLogger for a complete 
solution, but even doing that isn’t going to help much since nothing in 
Logger/AbstractLogger creates a LogEvent so does not provide any place to add 
properties associated with the instance. Creating a context at the class level 
is certainly level but the only way to locate it is via a ThreadLocal. This 
introduces problems of having objects tied to classes and ClassLoaders and 
means Thread termination may hang if everything isn’t recycled correctly.  In 
short, the solution for this is complex.

2. The links you provide describe the problem of passing contexts to threads 
magnificently. Unfortunately, they solve it by requiring you NOT to use 
standard JDK tooling for managing threads. I am not prepared to be dependent on 
any specific framework other than the JDK. 

Specifics about your solutions:

1. Again, adding Property elements to a Logger doesn’t really help for the use 
cases requested. We support this now and if it was sufficient we wouldn’t have 
these requests.
2. As I noted, adding a wrapper around a Logger doesn’t accomplish anything, at 
least in terms of getting data into LogEvents.
3. As I stated, I am not prepared to provide a solution that is dependent on 
another framework. However that framework is welcome to integrate with us.

Other items of note:
SLF4J has recently introduced 
https://www.slf4j.org/apidocs-2.1.0-alpha0/org.slf4j/org/slf4j/MDCAmbit.html 
which to me looks like a horrible way to add a “scope-based” context as it 
relies on the user to perform cleanup.
Java 21 introduces ScopedValues - 
https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/ScopedValue.html.
 It does allow ScopedValues to be passed to child threads but I haven’t 
experimented with it enough yet to know how it really works.

In the meantime, I provided https://github.com/apache/logging-log4j2/pull/2385 
which I very loosely modeled after ScopedValues. ScopedContext allows you to do:

public class SomeClass {

private static final Logger LOGGER = 
LogManager.getLogger(SomeClass.class);

public void someMethod(String arg1, Request request) {
ScopedContext.INITIAL_CONTEXT
.where(“param”, “arg1”)
.where(“loginId”, request.getLoginid())
.run(() -> {
LOGGER.info (“This will 
include param and loginId in the LogEvent’s ThreadContext Map”);
});
} 

Nesting of ScopedContext’s is allowed. They can be used in place of 
ScoedVallues but also allow them to included in the LogEvent. A ScopedContext 
can also be saved and passed to another thread. Because ScopedContexts are 
immutable there is no risk in them being shared across threads. It simply 
requires the ScopedContext’s run method be called on the child thread to 
register it and ensure it is cleaned up when no longer needed.

Ralph

> On Mar 18, 2024, at 2:39 AM, Piotr P. Karwasz  wrote:
> 
> Hi,
> 
> Given the ongoing context data propagation in three Github
> issues/PRs/discussions (cf. [1], [2] and [3]). I think we should move
> the debate to the mailing list, which guarantees the maximal reachout.
> 
> == Problem summary
> 
> We have two different problems regarding context data in log files:
> 
> 1. Raman and Mikko would like to bind context data to an object
> implementing the `Logger` interface or more generally to a service
> object (e.g. resource manager, DAO and variants),
> 2. Other users would like to bind context data to a processing
> pipeline, regardless of which threads it uses.
> 
> == Possible solutions
> 
> As far as I know there are currently the following approaches we can
> take for problem 1:
> 
> * we can add a list of `` elements in the configuration of a
> ``, which will add the given keys to all loggers using that
> configuration. This is something we can do for very static data, e.g.
> we can add to each log statement the name of the library that issued
> it.
> * 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.
> 
> For problem 2, the traditional solution was provided by the
> inheritability of `ThreadLocal`s, but this approach only works if the
> current thread creates new ones, it does not work with thread pools.
> 
> Now there seems to be another emerging solution provided by the
> `context-propagation` [9] library and explained by Dariusz Jędrzejczyk
> in his blog