I’ve got a much simpler refactor for API initialization which avoids introducing new SPIs (i.e., it re-uses the Provider service as-is). I’ll file it as a PR later today. It can likely be improved further (mostly related to the PropertyEnvironment init and injection), but it’s an improvement at least for centralizing most of the startup logic into a single class (basically a rewrite of ProviderUtil). One major change I was able to make in this, though, was simplifying the OSGi bundle activators quite a bit. The API activator mainly needs to acquire an initialization lock and set up a bundle event listener to wait for other bundles to be installed. Once a bundle is installed that actually has a Log4j Provider to load from it, it can register its providers via ServiceRegistry (rather than directly invoking ProviderUtil) and then unlock the initialization lock. Using ServiceLoader::load in the bundle change listener seems to work well enough (only disadvantage to this approach is that it requires giving the Log4j bundles some high level permissions in order to acquire different bundles’ ClassLoader or BundleWiring objects, but that assumes the user is using both OSGi and a SecurityManager at the same time which just sounds dreadful), and it can also remove a bit of custom OSGi service registration or lookup logic.
If we go as far as providing a full lifecycle for, say, LoggerContextFactory (and not just the LoggerContext objects which already have a lifecycle), then it could potentially be shutdown from the bundle activator as well, though that might already rely on the ShutdownCallbackRegistry. > On Nov 7, 2022, at 12:19 AM, Ralph Goers <ralph.go...@dslextreme.com> wrote: > > Note that PropertiesUtil will need some work in master to implement > https://cwiki.apache.org/confluence/display/LOGGING/Properties+Enhancement > <https://cwiki.apache.org/confluence/display/LOGGING/Properties+Enhancement>. > > Ralph > >> On Nov 6, 2022, at 6:37 PM, Matt Sicker <m...@musigma.org> wrote: >> >> On the LowLevelLogUtil front, I’ve updated that to use StatusLogger now once >> StatusLogger has initialized. The code could probably be backported to 2.x. >> >> As for how PropertiesUtil caches values, simplifications there would be >> great! >> — >> Matt Sicker >> >>> On Nov 6, 2022, at 03:37, Piotr P. Karwasz <piotr.karw...@gmail.com> wrote: >>> >>> Hi Matt, >>> >>> On Sat, 5 Nov 2022 at 22:32, Matt Sicker <m...@musigma.org> wrote: >>>> After an attempt at soloing the idea, I’ve reverted my changes and am >>>> instead documenting the current system along with adding some proposals in >>>> the following Confluence doc [0]. I’ve analyzed the initialization >>>> ordering of the API (without going into detail of how log4j-core >>>> initializes as this differs fairly significantly in 2.x and 3.x at this >>>> point). Right now, some of the main differences between 2.x and master is >>>> that most of the documented static fields from initialization are >>>> currently refactored into Lazy<T> static fields, though that only defers >>>> initialization until slightly later. >>> >>> Thanks for analyzing and writing down this quite complex initialization >>> process. >>> >>> My main concerns with the way this works now are: >>> >>> 1. Logging of errors in the pre-StatusLogger phase is inconsistent. >>> Some classes use `LowLevelLogUtil`, some ignore errors, while the >>> usage of StatusLogger I introduced in `ServiceLoaderUtil` is brittle: >>> it requires to call `loadServices` with `verbose = false` to disable >>> the usage of `StatusLogger`. I think that the ideal solution would be >>> to remove enough static initialization from `StatusLogger` and >>> `AbstractLogger` (i.e. move it to constructors) so that >>> LowLevelLogUtil can create a `StatusLogger` with a hardcoded >>> configuration. Later in the initialization process we can replace it >>> with the real `StatusLogger`. >>> >>> 2. There are still a couple of singleton classes that cache the result >>> of `PropertiesUtil.getProperty` in private static fields. I think >>> these calls can be easily moved to the constructor of the class. E.g. >>> `StatusLogger` could easily have a constructor that does not require >>> `PropertiesUtil` at all. >>> >>> 3. The way `PropertiesUtil` caches values could be simplified (cf. >>> discussion in https://issues.apache.org/jira/browse/LOG4J2-3621): it >>> caches all values from environment variables and Java system >>> properties at startup. Most of the time none of those values configure >>> Log4j2. On the other hand missing values are not cached, so most of >>> the time a lookup for `log4j2.configurationFile` will find no value in >>> the cache, perform a lookup in all available property sources and >>> return `null`. >>> >>> Most of these changes can also be done in `release-2.x` without >>> breaking public and protected members. >>> >>> Piotr >> >
