I am OK with ScriptAppenderSelector so I'll change it to that. Gary
On Mon, Sep 19, 2016 at 3:23 PM, Ralph Goers <[email protected]> wrote: > The 3 obvious choices: ScriptSelectorAppender, ScriptAppenderSelector, or > AppenderScriptSelector. I actually prefer the second as it is an > AppenderSelector that uses a Script (or rather, a Selector of an Appender > that uses a Script). The first isn’t too bad as it is an Appender that is a > ScriptSelector (or an Appender that acts as a Selector using a Script). > The third choice is just terrible. I suppose with 3 words there are other > choices as well but they probably suck too. > > Ralph > > > On Sep 19, 2016, at 2:42 PM, Gary Gregory <[email protected]> wrote: > > ScriptSelectorAppender? > > On Sep 19, 2016 2:29 PM, "Ralph Goers" <[email protected]> wrote: > >> Actually, I do have one minor issue. It will cause confusion calling it >> ScriptSelector. I am imagining the next step will be to create a >> LayoutSelector that uses Scripts. I can also imagine a Selector for >> AppenderRefs that uses Scripts. So calling it ScriptSelector is a bit >> ambiguous, unless it is going to support creating any of these (which would >> be hard since it extends AbstractAppender). >> >> Ralph >> >> On Sep 19, 2016, at 12:02 AM, Gary Gregory <[email protected]> >> wrote: >> >> Great, thanks Ralph. >> >> Gary >> >> On Sun, Sep 18, 2016 at 11:14 PM, Ralph Goers <[email protected] >> > wrote: >> >>> Yup to both. I’ll look at it tomorrow. >>> >>> Ralph >>> >>> On Sep 18, 2016, at 10:31 PM, Gary Gregory <[email protected]> >>> 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 <[email protected]> >>> 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 < >>>> [email protected]> 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 <[email protected]> >>>>> 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 < >>>>> [email protected]> wrote: >>>>> >>>>>> See inline >>>>>> >>>>>> On Sep 16, 2016, at 10:31 PM, Gary Gregory <[email protected]> >>>>>> wrote: >>>>>> >>>>>> On Fri, Sep 16, 2016 at 8:38 PM, Ralph Goers <ralph.goers@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”).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: [email protected] | [email protected] >>>>> 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 >>>>> Home: http://garygregory.com/ >>>>> Tweet! http://twitter.com/GaryGregory >>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> E-Mail: [email protected] | [email protected] >>>> 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 >>>> Home: http://garygregory.com/ >>>> Tweet! http://twitter.com/GaryGregory >>>> >>> >>> >>> >>> -- >>> E-Mail: [email protected] | [email protected] >>> 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 >>> Home: http://garygregory.com/ >>> Tweet! http://twitter.com/GaryGregory >>> >>> >>> >> >> >> -- >> E-Mail: [email protected] | [email protected] >> 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 >> Home: http://garygregory.com/ >> Tweet! http://twitter.com/GaryGregory >> >> >> > -- E-Mail: [email protected] | [email protected] 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 Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
