Re: Stack valued MDC
Hi Ralph, On Tue, 23 Aug 2022 at 08:14, Ralph Goers wrote: > I don’t see that as a problem at all. Log4jLogEvent’s constructor accepts a > List of properties. We use this to inject properties configured on Loggers > into the Context Map of the LogEvent. I see no reason why these key/value > pairs shouldn’t be treated the same way. Thanks, I didn't know about this feature. I didn't find a way of passing a list of properties from a logger to a Log4jLogEvent without extending the `Logger` interface, so I used a simple `CloseableThreadContext#putAll` to inject the values in the context map[1]. Regarding the "enhanced" MDC feature in SLF4J I am more and more convinced that SLF4J's `pushByKey` is the equivalent of `CloseableThreadContext#put` and `popByKey` is the equivalent of closing a `CloseableThreadContext.Instance`. Ceki's `MDC.putCloseable` does not restore the original value upon closure, which is probably the reason he "enhanced" the MDC interface. Anyway I think that either Pierrick's [2] or mine [3] attempt to implement the "enhanced" MDC should solve the problem. This would leave us with the question whether to implement `LoggingEventAware` or not and we fully support SLF4J 2.0. I am not entirely sure if we should allow users to create entire logging events. Piotr [1] https://github.com/apache/logging-log4j2/pull/1024 [2] https://github.com/apache/logging-log4j2/pull/1022 [3] https://github.com/apache/logging-log4j2/pull/1025
Re: Stack valued MDC
I don’t see that as a problem at all. Log4jLogEvent’s constructor accepts a List of properties. We use this to inject properties configured on Loggers into the Context Map of the LogEvent. I see no reason why these key/value pairs shouldn’t be treated the same way. Ralph > On Aug 22, 2022, at 10:51 PM, Piotr P. Karwasz > wrote: > > Hi everybody, > > Besides the enhanced NDC, there is another SLF4J 2.0 feature I don't > really know how to deal with: > > https://www.slf4j.org/api/org/slf4j/spi/LoggingEventBuilder.html#addKeyValue-java.lang.String-java.lang.Object- > > These key/value pairs in the `LoggingEventBuilder` seem to be separate > from the MDC, so we can not inject them into the ThreadContextMap. The > reference implementation of SLF4J adds them to its `LoggingEvent`, > which suggests that we might create a new `Slf4jMessage` and add them > there. The problem would be how to format such a message. > > What do you think? > > Piotr
Re: Stack valued MDC
Hi everybody, Besides the enhanced NDC, there is another SLF4J 2.0 feature I don't really know how to deal with: https://www.slf4j.org/api/org/slf4j/spi/LoggingEventBuilder.html#addKeyValue-java.lang.String-java.lang.Object- These key/value pairs in the `LoggingEventBuilder` seem to be separate from the MDC, so we can not inject them into the ThreadContextMap. The reference implementation of SLF4J adds them to its `LoggingEvent`, which suggests that we might create a new `Slf4jMessage` and add them there. The problem would be how to format such a message. What do you think? Piotr
Re: Stack valued MDC
I don’t think that’s the only sane option. Another sane option is to ignore it. There’s nothing that says Log4j has to implement any enhanced SLF4J APIs. He wants to be the lowest common denominator in logging APIs, then he can’t add APIs that aren’t supported in the major implementations. Just throw an UnsupportedOperationException. > On Aug 23, 2022, at 2:04, Ralph Goers wrote: > > It would be weird IMO to provide a PatternConverter for an NDC stack that is > supported by SLF4J but not Log4j. > > Unfortunately, I think the only sane thing to do is option 1. > > Ralph > >> On Aug 22, 2022, at 8:45 AM, Piotr P. Karwasz >> wrote: >> >> Hi all, >> >>> On Sat, 16 Apr 2022 at 00:17, Carter Kozak wrote: >>> I can understand how the stack-based mdc might be convenient and useful, >>> but I don't think it fits my use-cases. That said, I wonder if the API >>> could be improved in such a way that it could leverage the application >>> stack instead of maintaining its own -- this is an issue that I've >>> encountered in tracing implementations as well, where asymmetric >>> interactions to cause the application stack and internal stack to get out >>> of sync. Perhaps using something like putCloseable[1] would allow the data >>> needed to reset to be stored in the closeable without maintaining a >>> standalone stack (at the cost of the ability to support >>> getCopyOfDequeByKey[2]). >> >> Since Ceki announced the release of SLF4J 2.0.0, the topic is back. We >> need to decide whether: >> >> 1. we extend the Log4j2 API to support the "enhanced" NDC version of SLF4J2, >> 2. we use the default implementation provided by Ceki, >> 3. we hack around it (e.g. encoding a list of values to a JSON-like >> structure) like in https://github.com/apache/logging-log4j2/pull/820. >> >> I would use Ceki's implementation and provide a `PatternConverter` for >> those hordes of users that will use it. Alternatively we could inject >> the top values from Ceki's NDC into the usual ThreadContextMap. >> >> What do you think? >
Re: Stack valued MDC
It would be weird IMO to provide a PatternConverter for an NDC stack that is supported by SLF4J but not Log4j. Unfortunately, I think the only sane thing to do is option 1. Ralph > On Aug 22, 2022, at 8:45 AM, Piotr P. Karwasz wrote: > > Hi all, > > On Sat, 16 Apr 2022 at 00:17, Carter Kozak wrote: >> I can understand how the stack-based mdc might be convenient and useful, but >> I don't think it fits my use-cases. That said, I wonder if the API could be >> improved in such a way that it could leverage the application stack instead >> of maintaining its own -- this is an issue that I've encountered in tracing >> implementations as well, where asymmetric interactions to cause the >> application stack and internal stack to get out of sync. Perhaps using >> something like putCloseable[1] would allow the data needed to reset to be >> stored in the closeable without maintaining a standalone stack (at the cost >> of the ability to support getCopyOfDequeByKey[2]). > > Since Ceki announced the release of SLF4J 2.0.0, the topic is back. We > need to decide whether: > > 1. we extend the Log4j2 API to support the "enhanced" NDC version of SLF4J2, > 2. we use the default implementation provided by Ceki, > 3. we hack around it (e.g. encoding a list of values to a JSON-like > structure) like in https://github.com/apache/logging-log4j2/pull/820. > > I would use Ceki's implementation and provide a `PatternConverter` for > those hordes of users that will use it. Alternatively we could inject > the top values from Ceki's NDC into the usual ThreadContextMap. > > What do you think?
Re: Stack valued MDC
Hi all, On Sat, 16 Apr 2022 at 00:17, Carter Kozak wrote: > I can understand how the stack-based mdc might be convenient and useful, but > I don't think it fits my use-cases. That said, I wonder if the API could be > improved in such a way that it could leverage the application stack instead > of maintaining its own -- this is an issue that I've encountered in tracing > implementations as well, where asymmetric interactions to cause the > application stack and internal stack to get out of sync. Perhaps using > something like putCloseable[1] would allow the data needed to reset to be > stored in the closeable without maintaining a standalone stack (at the cost > of the ability to support getCopyOfDequeByKey[2]). Since Ceki announced the release of SLF4J 2.0.0, the topic is back. We need to decide whether: 1. we extend the Log4j2 API to support the "enhanced" NDC version of SLF4J2, 2. we use the default implementation provided by Ceki, 3. we hack around it (e.g. encoding a list of values to a JSON-like structure) like in https://github.com/apache/logging-log4j2/pull/820. I would use Ceki's implementation and provide a `PatternConverter` for those hordes of users that will use it. Alternatively we could inject the top values from Ceki's NDC into the usual ThreadContextMap. What do you think?
Re: Stack valued MDC
I can understand how the stack-based mdc might be convenient and useful, but I don't think it fits my use-cases. That said, I wonder if the API could be improved in such a way that it could leverage the application stack instead of maintaining its own -- this is an issue that I've encountered in tracing implementations as well, where asymmetric interactions to cause the application stack and internal stack to get out of sync. Perhaps using something like putCloseable[1] would allow the data needed to reset to be stored in the closeable without maintaining a standalone stack (at the cost of the ability to support getCopyOfDequeByKey[2]). I think there's a bug in the proposed PR where MDC consumers cannot distinguish between MDC.put values meant to take advantage of the string format and correctly-formatted MDC.pushByKey. I think we need agreement between put/push and get/pop. -ck 1. https://www.slf4j.org/api/org/slf4j/MDC.html#putCloseable-java.lang.String-java.lang.String- 2. https://www.slf4j.org/api/org/slf4j/MDC.html#getCopyOfDequeByKey-java.lang.String- On Fri, Apr 15, 2022, at 16:27, Piotr P. Karwasz wrote: > Hello everybody, > > In SLF4J-531 <https://jira.qos.ch/browse/SLF4J-531> Ceki added support for > an MDC containing stack values in SLF4J 2.0. Since some servlet containers > like Jetty are already distributing alpha versions of SLF4J 2.0, I started > implementing the new SLF4J 2.0 in the `slf4j-2.0` branch of the repository. > > Personally I am not convinced that introducing a second stack valued MDC > has many applications and I believe that Ralph shares my opinion. Therefore > I would propose to implement it using the currently available MDC (cf. PR > #820 <https://github.com/apache/logging-log4j2/pull/820>). What do you > think about it? > > Another idea might be to extend the current MDC to allow object values. > That is what is already available in JBoss Log Manager. > > Piotr
Stack valued MDC
Hello everybody, In SLF4J-531 <https://jira.qos.ch/browse/SLF4J-531> Ceki added support for an MDC containing stack values in SLF4J 2.0. Since some servlet containers like Jetty are already distributing alpha versions of SLF4J 2.0, I started implementing the new SLF4J 2.0 in the `slf4j-2.0` branch of the repository. Personally I am not convinced that introducing a second stack valued MDC has many applications and I believe that Ralph shares my opinion. Therefore I would propose to implement it using the currently available MDC (cf. PR #820 <https://github.com/apache/logging-log4j2/pull/820>). What do you think about it? Another idea might be to extend the current MDC to allow object values. That is what is already available in JBoss Log Manager. Piotr
