> On May 19, 2022, at 12:15 AM, Volkan Yazıcı <vol...@yazi.ci> wrote:
>
> In the last couple of weeks, I have been interrogating Matt on why and how
> of the 3.x plugin infra. This inevitably led to some code archaeology. I
> will try to share my take out of this exercise.
>
> 1.x required users to type the fully-qualified class names of components
> (appenders, layouts, etc.) in either configuration files or properties:
>
> log4j.appender.A1=org.apache.log4j.ConsoleAppender
> log4j.appender.A1.layout=org.apache.log4j.PatternLayout
>
> Plugins were first introduced in 2.x to address this concern:
It wasn’t a concern. It was an annoyance.
> providing a
> mechanism to alias components. Later on it also got used to glue the rest
> of the configuration components together. Consequently maintainers started
> exercising this throughout the entire code base, whenever something needed
> to be indirectly injected somewhere.
Only where we thought it made sense. Some things use system properties,
some things use ServiceLoader.
>
> That was a great epiphany and I am in love with the plugins! It feels like
> Log4j is composed of simple beans beautifully practicing
> separation-of-concerns while running on a marvellous Spring-like
> fully-fledged dependency injection (DI) framework... almost... sort of...
> The reality is sadly a little bit different than that. In particular, I see
> two major issues:
>
> 1. Missing `@ConditionalOn*` support
I am certain I added the ability to do this in 3.0. I added constraint checking
at the class level where previously it was only available on parameters. That
said, we currently only have RequiredClass and RequiredProperty validators.
> 2. Static access to DI
>
> I guess Matt is already working on issue #1. He is trying to make sure
> `@Required` et al. annotations are executed the same way at every injection
> site.
>
> What do I mean with the static access to DI? In a `@Bean`-annotated method
> of a Spring application, do you create your own `ApplicationContext`
> instance and access other beans from that? Do you statically access
> `ApplicationContext.getBean("foo")`? Certainly not! Though these two
> examples are what we exactly do in Log4j. We create single-use
> `PluginManager` instances and use it to collect plugins. We call
> `DI.createInjector().getInstance("foo")`. What we should be rather doing is
> to inject the `ApplicationContext` and/or the beans we are interested in,
> that is, in Log4j terms, inject the `Injector` and/or the plugins we are
> interested in.
>
> Thoughts?
What you are suggesting is that the injector should just be another bean
that can be injected. I have no problem with that.
I should mention that I asked Matt to look into an issue I have with the Spring
support. Spring needs access to its Environment. We currently save that in
the LoggerContex. However, the SpringPropertySource has a circularity problem.
PropertySources are created before anything else in Log4j, including the
Injector.
The Spring support currently uses an EnvironmentHolder singleton. I’d really
like
to do a PR for Spring to move the Spring Boot code there but I am not
comfortable
doing it with the EnvironmentHolder.
What happens how is that the EnvironmentHolder checks to see if Log4j has
initialized.
If it hasn’t it returns null. If it has then it accesses the LoggerContext to
get the Environment.
This just feels clunky. What I would prefer is to have the PropertySource be
injected
with the Environment when it becomes available. In essence, deferred injection.
Ralph
