search

ops4j / org.ops4j.pax.logging

The OSGi Logging framework implementation. Supports SLF4J,LOG4J,JCL etc.
https://ops4j1.jira.com/wiki/spaces/paxlogging/overview
Apache License 2.0
46 stars 79 forks source link

GH-550: Upgrade to log4j2 2.23.1 #551

Closed ihrasko closed 2 months ago

ihrasko commented 2 months ago

Upgrade log4j2 from 2.22.1 to 2.23.1.

jbonofre commented 2 months ago

Did you check class updates in api bundle ? That's actually the most important to check for an update.

grgrzybek commented 2 months ago

True - unfortunately we have to shade some classes. See for example 54f2d59f5bc34b9eec6f0cff3a37eadf725bf1bc. When I update Log4j2, I check out related https://github.com/apache/logging-log4j2/ branch and compare (like git diff rel/2.22.1..rel/2.23.1 -- path/to/what/we/shade/in/pax-logging to check if we need to pick up some changes.

I can do this next week.

grgrzybek commented 2 months ago

Yikes, there's a lot of changes in StatusLogger...:

$ git lg rel/2.22.1..rel/2.23.1 -- log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
* (2024-03-06 09:34:20 +0100) a4a8e99a7c Fix `StatusLogger` to correctly read `log4j2.StatusLogger.properties` (#2354) <Volkan Yazıcı>
* (2024-03-01 14:49:28 +0100) c5420410df Fix `StatusLogger` log level filtering when debug mode is enabled (#2337, #2338) <Volkan Yazıcı>
* (2024-02-27 21:59:44 +0100) af045db99e Fix `StatusLogger` time-zone issues and stack overflow (#2322) <Volkan Yazıcı>
* (2024-02-12 20:33:42 +0100) 7a7555663c Improve `StatusLogger` javadoc <Volkan Yazıcı>
* (2024-02-12 20:02:48 +0100) e87064f22d Reset the fallback listener on `StatusLogger#reset()` (#2280) <Volkan Yazıcı>
* (2024-02-08 13:29:25 +0100) ce1a3a432e Make `StatusLogger` self-contained and testable (#2249) <Volkan Yazıcı>
grgrzybek commented 2 months ago

No no no no ;) If we just shade without changing, we wouldn't have to do it at all ;) See for example changes made in StatusLogger after upgrading to 2.11.2: https://github.com/ops4j/org.ops4j.pax.logging/commit/02bcfcac18f3ee56ffba462b9960a53037669663#diff-ff765668251d1511bc1870ffade3dfe4105cb9bc9b1fd8dc57da6e1cfbcc61df

You seem to have simply copied the sources and removed Pax Logging specific changes. Did you run the tests? (maybe they work? :)

ihrasko commented 2 months ago

Thanks for your help. I will try to make it better next time.

grgrzybek commented 2 months ago

No worries - it's not something that can be done without spending a lot of time in Pax Logging ;) Thanks for the PR anyway. Mind that Pax Logging's first commit was made almost 20 years ago and the amount of technical debt here is enormous. When I first actively joined at 1.10 time to make 1.11 release, I've increased number of integration tests from around 2 to more than 100 - it was a huge undertaking... ;)