Another potential option I just remembered is to adopt the plugin system as it exists to define a new elementType value. An example of something that has only half-way integrated this way are the various converter keys for pattern layout.
As for a tree structure, I mean that the plugin system is mostly structured around transforming a tree of nodes (originally XML data, but it's any configuration format) into the runtime graph of objects. As such, it's almost like Spring's XML minus the ability to reference the beans you instantiate other than in specific use cases like AppenderRef. I think this limitation has led to a bit of a mess over time with a weird in-between state where some objects might want to be configurable or extensible in ways that don't fit this original pattern. On the dependency-free side, that's the idea behind the rewrite I started. I think part of what lost momentum was trying to incorporate support for the existing plugin system syntax while including it into a broader and more consistent DI system. The goal of that DI system was to be as minimal as necessary for our use case so that we don't need any dependencies or unnecessary features. I had initially looked into how to potentially generate code from the annotation processor rather than probing objects at runtime via reflection, though I was still merging the models together before I could experiment with that idea. For some help, if you're familiar with how CDI works, perhaps you can help figure out a way to model the LoggerContext as a sort of scoped context similar to session-scope and request-scope. I believe this was one of the key integration points I was trying to reach before getting derailed. My previous work is still in this branch, though I haven't merged back from master in a while: https://github.com/apache/logging-log4j2/tree/mean-bean-machine In particular, the API and SPI structure I was working on is here: https://github.com/apache/logging-log4j2/tree/mean-bean-machine/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/api https://github.com/apache/logging-log4j2/tree/mean-bean-machine/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/spi And the reflection-based default implementation: https://github.com/apache/logging-log4j2/tree/mean-bean-machine/log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/defaults Note related to ServiceLoader that I haven't quite gotten to that point, but I'd like this to ideally only load bean metadata for things referenced in the config file so we can lazy-load classes to avoid long startup times. In fact, this entire bean system is detached from the rest of the codebase still as I was still working on defining the scoping rules for things. It's been a while since I was working on this code, but it's mostly based on extracting the core logic from Weld, OpenWebBeans, and Guice, following an API that's a sort of simplified hybrid of the two. On Fri, 29 Jan 2021 at 13:15, Volkan Yazıcı <volkan.yaz...@gmail.com> wrote: > > a) I understand the policy that core shouldn't have any dependencies. > b) Maybe `ServiceLoader`s? > c) I see your point and agree with that. But replacing `new > PluginManager(T.class).getPlugins()` with injection via `@PluginElement > Set<T> plugins` shouldn't be complicating the current system that much, I > guess. > > Classpath scanning is indeed a pain point regarding the start-up > performance and `PluginManager#getPlugins` is doing exactly that. > > By the way, an alternative approach for JsonTemplateLayout I had in my is > this: > > 1. Make `EventResolverFactories` and each individual `EventResolver` & > `EventResolverFactory` public. > 2. Add a > `JsonTemplateLayout.Builder#eventTemplateResolverFactoryMapProvider` field > of type `String` which is pointing to a method extending from > `Supplier<Map<String, TemplateResolverFactory<LogEvent, > EventResolverContext, ? extends TemplateResolver<LogEvent>>>` > 3. Set the default value of the `eventTemplateResolverFactoryMapProvider` > to `EventResolverFactories.RESOLVER_FACTORY_BY_NAME`. > > So that if need arises, users can introduce their own map providers. While > doing that, they can leverage the entire set of existing resolvers, or > inject custom ones in isolation. (Of course the actual necessary set of > plumbing that needs to be on our side is a little bit hairy than the 3 > bullets I have listed above.) > > On Fri, Jan 29, 2021 at 7:26 PM Ralph Goers <ralph.go...@dslextreme.com> > wrote: > > > My concern over this has always been that a) Log4j should not have any > > required dependencies (and in 3.0 should only require the java base > > module), b) The JDK itself doesn’t have dependency injection built in and > > c) creating a large dependency injection framework in Log4j itself seems > > like overkill. What was built to start with was the simplest thing I could > > do that would allow users to add their own components. The plugin system > > has been enhanced well beyond what I originally created but still isn’t > > overly complicated. We also need to avoid Class scanning during startup as > > much as possible as that has proven to be very slow. > > > > Ralph > > > > > On Jan 29, 2021, at 10:28 AM, Matt Sicker <boa...@gmail.com> wrote: > > > > > > That sounds like one of the main goals of the DI rewrite I started. To > > > give more context on the existing system works, the ability to inject > > > complex objects into plugins is primarily used for mapping nested > > > configurations. Basically, anything corresponding to an XML element > > > was injectable into other elements corresponding to the configuration > > > file structure. Not every component in Log4j is integrated into the > > > system yet, so there are some pain points like this in 2.x which can > > > be seen in some things being configurable via system properties while > > > others have custom plugin elements defined for users to define their > > > configuration inside. Some of these testability concerns were likely > > > missed because their underlying code was fast enough to not need > > > mocks. > > > > > > Now if you'd like to inject objects instantiated from some arbitrary > > > point similar to Spring/Guice/etc., that will require a larger > > > overhaul (likely by rebooting my mean-bean-machine branch at some > > > point), but if your needs are already structured in a tree, then > > > @PluginElement is the equivalent of @Inject, while @PluginFactory is > > > equivalent to @Produces. Depending on your use case, it could be > > > supportable with the existing plugin system, or it may require > > > revisiting the DI rewrite, especially if there's wider interest in > > > completing that. > > > > > > On Fri, 29 Jan 2021 at 06:56, Volkan Yazıcı <volkan.yaz...@gmail.com> > > wrote: > > >> > > >> Hello, > > >> > > >> I am doing some investigation on how to replace statically bound > > >> JsonTemplateLayout resolvers with plugins. For inspiration I was > > looking at > > >> PatternLayout and found the following: PatternLayout calls PatternParser > > >> ctor, which performs the following: > > >> > > >> final PluginManager manager = new PluginManager(converterKey); > > >> manager.collectPlugins(config == null ? null : > > config.getPluginPackages()); > > >> final Map<String, PluginType<?>> plugins = manager.getPlugins(); > > >> final Map<String, Class<PatternConverter>> converters = new > > >> LinkedHashMap<>(); > > >> > > >> for (final PluginType<?> type : plugins.values()) { > > >> try { > > >> @SuppressWarnings("unchecked") > > >> final Class<PatternConverter> clazz = (Class<PatternConverter>) > > >> type.getPluginClass(); > > >> if (filterClass != null && !filterClass.isAssignableFrom(clazz)) > > { > > >> continue; > > >> } > > >> > > >> > > >> This got me really puzzled. I was expecting matched plugins to be > > injected > > >> into the @PluginFactory ctor of PatternLayout as a parameter of type > > >> List<PatternConverter> like every other DI system out there. That is, > > >> > > >> @PluginFactory > > >> AwesomeLayout( > > >> @PluginConfiguration Configuration config, > > >> @PluginElement("converters") List<PatternConverter> converters) { > > >> // ... > > >> } > > >> > > >> > > >> I find usage of PluginManager::new an anti-pattern. For one, it is not > > >> possible to unit test Pattern{Layout,Parser} by injecting only an > > isolated > > >> set of converters. Second, it is not possible to mock PluginManager > > either. > > >> > > >> Is injection of plugins extending a certain interface/class as a > > >> @PluginFactory ctor parameter something we can introduce to the plugin > > >> system? > > >> > > >> Kind regards. > > > > > > > > >
