I took a look and it looks OK to me.
> On Sep 19, 2016, at 12:02 AM, Gary Gregory <garydgreg...@gmail.com> wrote: > > Great, thanks Ralph. > > Gary > > On Sun, Sep 18, 2016 at 11:14 PM, Ralph Goers <ralph.go...@dslextreme.com > <mailto:ralph.go...@dslextreme.com>> wrote: > Yup to both. I’ll look at it tomorrow. > > Ralph > >> On Sep 18, 2016, at 10:31 PM, Gary Gregory <garydgreg...@gmail.com >> <mailto:garydgreg...@gmail.com>> wrote: >> >> Nevermind, I can just overwrite the name in the Node's attribute map... that >> works. I'd like a code review before or after merging to master. >> >> Gary >> >> On Sun, Sep 18, 2016 at 9:43 PM, Gary Gregory <garydgreg...@gmail.com >> <mailto:garydgreg...@gmail.com>> wrote: >> Hm, but how? org.apache.logging.log4j.core.appender.AbstractAppender.name >> <http://org.apache.logging.log4j.core.appender.abstractappender.name/> is >> final and there is no Appender.setName(String). Surely, we should not use >> reflection... >> >> Gary >> >> On Sun, Sep 18, 2016 at 9:34 PM, Ralph Goers <ralph.go...@dslextreme.com >> <mailto:ralph.go...@dslextreme.com>> wrote: >> I haven’t looked at your code but when you create the “real” appender you >> need to change its name to match the name of the selector so that >> AppenderRefs work. >> >> Ralph >> >>> On Sep 18, 2016, at 9:24 PM, Gary Gregory <garydgreg...@gmail.com >>> <mailto:garydgreg...@gmail.com>> wrote: >>> >>> I've implemented a first cut in the branch LOG4J2-1597 but I think I need >>> some help to connect the final dot (or two). >>> >>> When I run the new unit test >>> org.apache.logging.log4j.core.appender.ScriptSelectorAppenderTest, the >>> status logger shows: >>> >>> 2016-09-18 21:19:09,393 main ERROR Unable to locate appender "SelectIt" for >>> logger config "root" >>> 2016-09-18 21:19:09,465 main ERROR Unable to locate appender "SelectIt" for >>> logger config "root" >>> 2016-09-18 21:19:09,485 main ERROR Unable to locate appender "SelectIt" for >>> logger config "root" >>> 2016-09-18 21:19:09,505 main ERROR Unable to locate appender "SelectIt" for >>> logger config "root" >>> >>> Which initially makes sense: the appender created and returned by the >>> builder of "SelectIt" is really an appender named "List2". >>> >>> I tried to add a hack in >>> org.apache.logging.log4j.core.appender.ScriptSelector.Builder.build() to no >>> avail: >>> >>> // This feels like a hack and it does not work: >>> configuration.getAppenders().put(name, appender); >>> >>> Any thoughts? >>> >>> Gary >>> >>> On Sat, Sep 17, 2016 at 9:48 AM, Ralph Goers <ralph.go...@dslextreme.com >>> <mailto:ralph.go...@dslextreme.com>> wrote: >>> See inline >>> >>>> On Sep 16, 2016, at 10:31 PM, Gary Gregory <garydgreg...@gmail.com >>>> <mailto:garydgreg...@gmail.com>> wrote: >>>> >>>> On Fri, Sep 16, 2016 at 8:38 PM, Ralph Goers <ralph.go...@dslextreme.com >>>> <mailto:ralph.go...@dslextreme.com>> wrote: >>>> Gary, >>>> >>>> I have no problem with components that can be dumbed down to do simple >>>> things. I do have a problem with components that only do simple things >>>> because people will constantly asked to have them be enhanced. >>>> >>>> As for what you are proposing here, can I just say “No”? >>>> >>>> Sure! :-) You can say whatever you want! :-) >>>> >>>> Having the Appenders element deferred just smells to me and having an >>>> arbitrary script there just seems weird to me. Does it even have a >>>> contract or is it a free-for-all? How does it cause multiple appenders to >>>> be initialized? >>>> >>>> I think the RoutingAppender is a more appropriate solution. However, if >>>> you want to dumb it down a bit and turn it into an AppenderSelector I’d be >>>> ok with that. However, it would still be fairly similar to the >>>> RoutingAppender. >>>> >>>> OK, so going back to one of your eariler messages: >>>> >>>> ==copy start== >>>> >>>> This sort of sounds like you want an Appender Selector, which would be an >>>> Appender that uses a Selector to figure out which Appender to delegate to. >>>> This is a bit like the PatternSelector. I would imagine it would make >>>> sense to implement AppenderSelectors and LayoutSelectors. You probably >>>> would want to dynamically initialize the Appenders much like the >>>> RoutingAppender does. >>>> >>>> Maybe it would look like: >>>> >>>> <Appenders> >>>> <ScriptSelector name=“" default=“”> >>>> <Script language=“groovy”><![CDATA[ >>>> if (System.getProperty”os.name >>>> <http://os.name/>”).contains(“OS/390”)) then { >>>> return “Socket”; >>>> } else { >>>> return “File”; >>>> } >>>> </Script> >>>> <Appenders> >>>> <SocketAppender name=“Socket” …/> >>>> <FileAppender name=“File” …/> >>>> </Appenders> >>>> </ScriptSelector> >>>> </Appenders> >>>> >>>> The thing is that this script would run every time the Selector was >>>> accessed while it sounds like you would only want the script to run when >>>> the Selector is initialized. We could do that too but the script would >>>> need to be declared in a property that would only be used when the >>>> selector is initialized. I would want to support being able to do both. >>>> >>>> ==copy end== >>>> >>>> This is indeed like the RoutingAppender _except_ that the whole point is >>>> to do the script selection on start up. When you say that you'd want it >>>> both ways, on start up and on each log event; what would the configuration >>>> difference look like? >>>> >>>> But.. "Appender that uses a Selector to figure out which Appender to >>>> delegate to" ... that is _so_ much like a RoutingAppender as to be >>>> redundant, no? >>> >>> The difference is that a AppenderSelector can just implement the Builder or >>> Factory and invoke the script at that time to figure out which Appender to >>> create. It then returns that Appender. So while the AppenderSelector is >>> technically an Appender, it really is just an AppenderBuilder. The >>> RoutingAppender is a real Appender. >>> >>>> >>>> What I want is for the script to determine which appender to use (once), >>>> and instantiate that appender (once). There is no need for one appender to >>>> delegate to another appender. >>> >>> And that is what I just described. >>> >>>> >>>> The more general case is for the script to determine which appenders >>>> (plural) to use (once), and instantiate those appenders (plural) (once). >>>> There is no need for one appender to delegate to another appender list. I >>>> do not have a use case for this today, but I do for the one appender case. >>> >>> An AppenderSelector could only instantiate a single Appender, not a group. >>> If you wanted multiple appenders dynamically created this way you would >>> using multiple selectors. I’m not sure I see that as a drawback. >>> >>> >>>> >>>> My goal would be explained to a user like this: "This feature helps you >>>> build your configuration dynamically, all from the configuration file, to >>>> determine which appender(s) to configure. This is different from using a >>>> RoutingAppender which creates a level of indirection and decides what to >>>> do for each log event _at runtime_" Yes, this is a simpler explanation >>>> than also explaining the new role of scripts in the RoutingAppender but >>>> you get the idea. >>>> >>>> I am open different solutions that meet the goal of building the >>>> configuration dynamically, as if you'd done it in XML explicitly (or JSON) >>>> but does not end up with one appender delegating to another. >>>> >>>> Thoughts? >>>> >>>> Gary >>>> >>> >>> >>> >>> -- >>> E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | >>> ggreg...@apache.org <mailto:ggreg...@apache.org> >>> Java Persistence with Hibernate, Second Edition >>> <http://www.manning.com/bauer3/> >>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >>> Spring Batch in Action <http://www.manning.com/templier/> >>> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> >>> Home: http://garygregory.com/ <http://garygregory.com/> >>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> >> >> >> >> -- >> E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | >> ggreg...@apache.org <mailto:ggreg...@apache.org> >> Java Persistence with Hibernate, Second Edition >> <http://www.manning.com/bauer3/> >> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >> Spring Batch in Action <http://www.manning.com/templier/> >> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> >> Home: http://garygregory.com/ <http://garygregory.com/> >> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> >> >> >> -- >> E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | >> ggreg...@apache.org <mailto:ggreg...@apache.org> >> Java Persistence with Hibernate, Second Edition >> <http://www.manning.com/bauer3/> >> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> >> Spring Batch in Action <http://www.manning.com/templier/> >> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> >> Home: http://garygregory.com/ <http://garygregory.com/> >> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> > > > > -- > E-Mail: garydgreg...@gmail.com <mailto:garydgreg...@gmail.com> | > ggreg...@apache.org <mailto:ggreg...@apache.org> > Java Persistence with Hibernate, Second Edition > <http://www.manning.com/bauer3/> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> > Spring Batch in Action <http://www.manning.com/templier/> > Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> > Home: http://garygregory.com/ <http://garygregory.com/> > Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>