Re: [log4j] The great plugin migration - can we donate some plugins to more appropriate maintainers?
I think the appender API has been stable since 2.0. I do have a backlog item about adding an async API for appenders, but that would be additive. — Matt Sicker > On Jun 7, 2022, at 19:03, Robert Middleton wrote: > > It seems to me that this would only work if a) there exists a stable > API/ABI for log4j and b) sufficient demand for the appenders exists. > If a stable API/ABI doesn't exist, then the new maintainers would be > on the hook for any changes that log4j made, which seems unreasonable > to me. > > -Robert Middleton > >> On Tue, Jun 7, 2022 at 2:40 PM Piotr P. Karwasz >> wrote: >> >> Hi, >> >>> On Tue, 7 Jun 2022 at 20:17, Matt Sicker wrote: >>> * JDBC/JPA appenders -> ? (Eclipse Jakarta perhaps?) >> >> I would try EclipseLink, since Hibernate made their choice a long time ago. >> >>> * JMS appender (could go to ActiveMQ or Jakarta) >> >> An in-house collaboration with ActiveMQ seems more realistic. >> >>> * SMTP appender -> makybe Jakarta? >> >> Jakarta Mail has a stable API, we can keep this one. >> >>> For any plugins we're trying to move, if the upstream projects don't >>> want them, then those modules get EOL'd and removed for 3.x. >> >> I would settle for a list of maintainers from those projects that are >> willing to help, whenever they decide on breaking an API or to advise >> which version should be supported and which one can be dropped. >> >> Regarding `log4j-appserver`, the Jetty part is obsolete (does not work >> with Jetty 10) and the Tomcat part could be migrated to Tomcat. It's >> only one class, but there is also an old PR of mine that I closed in >> order not to pollu^wenrich Log4j2 with new external dependencies. >> >> Piotr
Re: [log4j] The great plugin migration - can we donate some plugins to more appropriate maintainers?
It seems to me that this would only work if a) there exists a stable API/ABI for log4j and b) sufficient demand for the appenders exists. If a stable API/ABI doesn't exist, then the new maintainers would be on the hook for any changes that log4j made, which seems unreasonable to me. -Robert Middleton On Tue, Jun 7, 2022 at 2:40 PM Piotr P. Karwasz wrote: > > Hi, > > On Tue, 7 Jun 2022 at 20:17, Matt Sicker wrote: > > * JDBC/JPA appenders -> ? (Eclipse Jakarta perhaps?) > > I would try EclipseLink, since Hibernate made their choice a long time ago. > > > * JMS appender (could go to ActiveMQ or Jakarta) > > An in-house collaboration with ActiveMQ seems more realistic. > > > * SMTP appender -> makybe Jakarta? > > Jakarta Mail has a stable API, we can keep this one. > > > For any plugins we're trying to move, if the upstream projects don't > > want them, then those modules get EOL'd and removed for 3.x. > > I would settle for a list of maintainers from those projects that are > willing to help, whenever they decide on breaking an API or to advise > which version should be supported and which one can be dropped. > > Regarding `log4j-appserver`, the Jetty part is obsolete (does not work > with Jetty 10) and the Tomcat part could be migrated to Tomcat. It's > only one class, but there is also an old PR of mine that I closed in > order not to pollu^wenrich Log4j2 with new external dependencies. > > Piotr
Re: LogManager.getContext(true)
Hi all, On Tue, 7 Jun 2022 at 20:22, Matt Sicker wrote: > For the record, I like Piotr's idea on repurposing this parameter to > allow for better caching of LoggerContext in the anchor spot. This > should help with some test parallelization efforts we're working on. I believe this can help in more than just test parallelisation. Most frameworks I have worked with have some kind of thread bound context, e.g. Spring's `RequestContextHolder` or Vaadin's `UI.getCurrent()` and similar. These are usually propagated during async calls by the framework or the developer that makes the call. If you can create a custom context selector that uses the external thread bound context to retrieve the appropriate `LoggerContext`, async calls will go smoothly. However that is only true for the async calls that the framework/developer is aware of. Log4j2 should take care of propagating the `LoggerContext` along its internal async calls. I believe that enforcing a "get thread bound context or usual selector logic" policy on context selectors could solve this problem. As far as I can see the implications of this change would be: 1. `BasicContextSelector` called with `currentContext = false` will always give a common logger context. This is in line with what we describe in [1], while the threadlocal behavior is only documented in [2]. I think this can be considered an internal change. 2. `log4j-web` + `BasicContextSelector` could break, since `log4j-web` sets the current context and `LogManager.getContext(false)` retrieves it anyway. Piotr [1] https://logging.apache.org/log4j/2.x/manual/logsep.html [2] https://logging.apache.org/log4j/2.x/manual/extending.html
Re: LogManager.getContext(true)
I would think we would want to use some sort of JunitContextSelector and modify PropertiesUtil so that JUnit can provide the SystemProperties. You do not want to point the external context to JUnit’s ExtensionContext. Instead, you should add it as a key to the externalMap. Ralph > On Jun 7, 2022, at 12:27 PM, Piotr P. Karwasz wrote: > > Hi Volcan, > > On Tue, 7 Jun 2022 at 21:04, Volkan Yazıcı wrote: >> Piotr, thanks so much for sparing time for test parallelization. Is there >> anything else we can do rather than thread-local `LoggerContext`s? Can't we >> configure a `ContextSelector` for tests such that it will attach the >> `LoggerContext`s to the lifetime of a test? > > From my perspective the problem is not the `LoggerContext` itself that > can be injected as a parameter in a JUnit5 test, but the > `PropertiesUtil` calls used by many Log4j2 components. > > The `PropertiesUtil` object in release 2.x is global, so I need to use > `ThreadLocal`s to replace the system properties source that is usually > used. The implementation of `TestPropertySource` I provided in [1] > partially works, but it fails if the test creates new threads and > calls `PropertiesUtil` from there. For example > HttpThreadContextMapFilterTest[2] will check if JNDI is allowed in a > new thread. > > The most proper solution I could think of is to bind the > `LoggerContext` to the thread and enhance all Log4j2 executors to > propagate the current context. If I set the `LoggerContext`'s external > context to JUnit5 `ExtensionContext`, I can easily implement a > properties source that is restricted to the currently running test, no > matter how many thread the test uses. > > Piotr > > [1] https://github.com/apache/logging-log4j2/pull/916 > [2] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-core/src/test/java/org/apache/logging/log4j/core/filter/HttpThreadContextMapFilterTest.java
Re: LogManager.getContext(true)
Hi Volcan, On Tue, 7 Jun 2022 at 21:04, Volkan Yazıcı wrote: > Piotr, thanks so much for sparing time for test parallelization. Is there > anything else we can do rather than thread-local `LoggerContext`s? Can't we > configure a `ContextSelector` for tests such that it will attach the > `LoggerContext`s to the lifetime of a test? >From my perspective the problem is not the `LoggerContext` itself that can be injected as a parameter in a JUnit5 test, but the `PropertiesUtil` calls used by many Log4j2 components. The `PropertiesUtil` object in release 2.x is global, so I need to use `ThreadLocal`s to replace the system properties source that is usually used. The implementation of `TestPropertySource` I provided in [1] partially works, but it fails if the test creates new threads and calls `PropertiesUtil` from there. For example HttpThreadContextMapFilterTest[2] will check if JNDI is allowed in a new thread. The most proper solution I could think of is to bind the `LoggerContext` to the thread and enhance all Log4j2 executors to propagate the current context. If I set the `LoggerContext`'s external context to JUnit5 `ExtensionContext`, I can easily implement a properties source that is restricted to the currently running test, no matter how many thread the test uses. Piotr [1] https://github.com/apache/logging-log4j2/pull/916 [2] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-core/src/test/java/org/apache/logging/log4j/core/filter/HttpThreadContextMapFilterTest.java
Re: LogManager.getContext(true)
I have the impression that the purpose of `currentContext` is already ambiguous and it is not clearly implemented, if not already bloated. I am not sure repurposing this would be a good idea. This said, I am far from being an expert in that area. Piotr, thanks so much for sparing time for test parallelization. Is there anything else we can do rather than thread-local `LoggerContext`s? Can't we configure a `ContextSelector` for tests such that it will attach the `LoggerContext`s to the lifetime of a test? On Mon, Jun 6, 2022 at 8:36 AM Piotr P. Karwasz wrote: > Hi all, > > While working on test parallelisation, I was looking for the existent > mechanisms to bind a logger context to a thread. That is easily > provided by `ContextAnchor.THREAD_CONTEXT` which can be accessed by > `LogManager.getContext()/LogManager.getContext(true)`. > > However I am not sure what is the exact semantics of the > `LogManager.getContext(boolean)` call: > > * the BasicContextSelector always returns a thread-bound context, > even if the parameter is `false`, > * the ClassLoaderContextSelector only does it if called with `true` > and if no thread bound context is found it returns a default context > (which is almost certainly not what the caller wants). > > Would it be correct to modify the behavior of these selectors to: > > 1. if called with `currentContext == false` apply the selector's > logic. I believe that the `BasicContextSelector` should always return > the same context in this case, > 2. if called with `currentContext == true` return a thread bound > context and fall back to `currentContext == false` if no context is > present. This would require a modification for the > `ClassLoaderContextSelector`. > > Piotr >
Re: [log4j] The great plugin migration - can we donate some plugins to more appropriate maintainers?
Hi, On Tue, 7 Jun 2022 at 20:17, Matt Sicker wrote: > * JDBC/JPA appenders -> ? (Eclipse Jakarta perhaps?) I would try EclipseLink, since Hibernate made their choice a long time ago. > * JMS appender (could go to ActiveMQ or Jakarta) An in-house collaboration with ActiveMQ seems more realistic. > * SMTP appender -> makybe Jakarta? Jakarta Mail has a stable API, we can keep this one. > For any plugins we're trying to move, if the upstream projects don't > want them, then those modules get EOL'd and removed for 3.x. I would settle for a list of maintainers from those projects that are willing to help, whenever they decide on breaking an API or to advise which version should be supported and which one can be dropped. Regarding `log4j-appserver`, the Jetty part is obsolete (does not work with Jetty 10) and the Tomcat part could be migrated to Tomcat. It's only one class, but there is also an old PR of mine that I closed in order not to pollu^wenrich Log4j2 with new external dependencies. Piotr
Re: LogManager.getContext(true)
For the record, I like Piotr's idea on repurposing this parameter to allow for better caching of LoggerContext in the anchor spot. This should help with some test parallelization efforts we're working on. On Mon, Jun 6, 2022 at 1:36 AM Piotr P. Karwasz wrote: > > Hi all, > > While working on test parallelisation, I was looking for the existent > mechanisms to bind a logger context to a thread. That is easily > provided by `ContextAnchor.THREAD_CONTEXT` which can be accessed by > `LogManager.getContext()/LogManager.getContext(true)`. > > However I am not sure what is the exact semantics of the > `LogManager.getContext(boolean)` call: > > * the BasicContextSelector always returns a thread-bound context, > even if the parameter is `false`, > * the ClassLoaderContextSelector only does it if called with `true` > and if no thread bound context is found it returns a default context > (which is almost certainly not what the caller wants). > > Would it be correct to modify the behavior of these selectors to: > > 1. if called with `currentContext == false` apply the selector's > logic. I believe that the `BasicContextSelector` should always return > the same context in this case, > 2. if called with `currentContext == true` return a thread bound > context and fall back to `currentContext == false` if no context is > present. This would require a modification for the > `ClassLoaderContextSelector`. > > Piotr
[log4j] The great plugin migration - can we donate some plugins to more appropriate maintainers?
Hey all, I was wondering if you'd all be on board for 3.x to try and move some of our 3rd party integration plugins to their respective projects rather than maintaining them here. For example, all the networked appender plugins and other plugins that typically require a third party client library or driver would be more appropriately located with their respective dependencies. A concrete example would be moving the Flume appender to Apache Flume once they're finished with 1.10.0 which is under review right now. While we'd keep releasing this appender in 2.x, come 3.x, we'd be able to point to the official Flume plugin instead. The same idea could apply to numerous plugins such as: * Cassandra appender -> Apache Cassandra * Flume appender -> Apache Flume * JDBC/JPA appenders -> ? (Eclipse Jakarta perhaps?) * JMS appender (could go to ActiveMQ or Jakarta) * Kafka appender -> Apache Kafka * MongoDB appenders -> MongoDB? * CouchDB appenders -> Apache CouchDB * SMTP appender -> makybe Jakarta? * ZeroMQ appender -> ZeroMQ project? * Spring integration stuff -> Spring project * Liquibase binding -> Liquibase I imagine it makes some sense to continue publishing our -web and -appserver modules as they're more specific to hooking log4j-core into those things, but I'm open to other ideas there. For any plugins we're trying to move, if the upstream projects don't want them, then those modules get EOL'd and removed for 3.x.
