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 <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>

Reply via email to