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”? 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.
Ralph
> On Sep 16, 2016, at 11:43 AM, Gary Gregory <garydgreg...@gmail.com> wrote:
>
> Now I've dived into this part of the code and consider what this
> configuration means for my use case, I see that it works and that the new
> feature has merit on its own but... It feels to me like my specific use case
> is an edge case of this new routing appender feature: I will only ever have
> one route and that route is determined at start up time and will never
> change. So it feels rather a heavy hammer for my fly.
>
> What I think would be nicer is this:
>
> <Configuration status="WARN" name="RoutingTest">
> <Appenders>
> <Script name="AddAppender" language="JavaScript"><![CDATA[
> "OSNameFoo".search("Foo") > -1 ? "List2" : "List1";]]>
> </Script>
> <List name="List1" />
> <List name="List2" />
> </Appenders>
> <Loggers>
> <Root level="error">
> <AppenderRef ref="Routing" />
> </Root>
> </Loggers>
> </Configuration>
>
> The script AddAppender runs when Appenders is instantiated and picks which
> appender to add.
>
> I think this means that the Appenders plugin must have deferChildren=true.
> When created the Appender checks the name of the script, right now there is
> only "AddAppender" but you could imagine other names like "AddAppenders"
> (plural). If there is no script, the Appenders plugin converts the nodes into
> configurations, which gives us the same result as before this change, it's
> just that the convertion from nodes to configured items happens a little
> later. If there is a script, then it is run and the semantics are applied, in
> my case, pick the one Appender node and convert it to a configured appender.
>
> Thoughts?
>
> Gary
>
> On Tue, Sep 13, 2016 at 4:53 PM, Gary Gregory <garydgreg...@gmail.com
> <mailto:garydgreg...@gmail.com>> wrote:
> I committed a first cut, see comments in
> https://issues.apache.org/jira/browse/LOG4J2-1578
> <https://issues.apache.org/jira/browse/LOG4J2-1578>
>
> Gary
>
> On Mon, Sep 12, 2016 at 11:40 PM, Ralph Goers <ralph.go...@dslextreme.com
> <mailto:ralph.go...@dslextreme.com>> wrote:
> Yes, it returns the key. Remember, a Route can dynamically create an Appender
> so it isn’t required to be a reference. At the same time we can (and
> probably should) pass variables and/or a Map to the script that it can update
> in any way it wants for later usage by the Routing script. As is shown, it
> can also return null which leaves the default route in place. So it need not
> be strictly for returning the default route.
>
> Ralph
>
>> On Sep 12, 2016, at 10:30 PM, Gary Gregory <garydgreg...@gmail.com
>> <mailto:garydgreg...@gmail.com>> wrote:
>>
>> Wait a sec, the DefaultRouteScript should return the Route key, not the
>> Route ref. Right?
>>
>> Gary
>>
>> On Mon, Sep 12, 2016 at 9:53 PM, Gary Gregory <garydgreg...@gmail.com
>> <mailto:garydgreg...@gmail.com>> wrote:
>> "First, the init script changes the default route based on the OS."
>>
>> Maybe the tag should be called "DefaultRouteScript" since it's job is to
>> return the default route name?
>>
>> Gary
>>
>> On Mon, Sep 12, 2016 at 8:05 PM, Ralph Goers <ralph.go...@dslextreme.com
>> <mailto:ralph.go...@dslextreme.com>> wrote:
>> After reviewing what I wrote below and looking at the Routing Appender I
>> think the best thing to do is just to add script support to it. It already
>> has support for a default Route. The init script, if present, could override
>> which Route to use as I described below. Then we could add a script
>> attribute to the Routes plugin which could be used to select the Route
>> instead of only matching on the ThreadContext key.
>>
>> With that I think you would have everything you want, plus it could be used
>> as a more intelligent way to route to existing appenders.
>>
>> The configuration would then look like:
>>
>> <Appenders>
>> <Console name="STDOUT" target="SYSTEM_OUT">
>> <PatternLayout pattern="%m%n"/>
>> </Console>
>> <Flume name="AuditLogger" compress="true">
>> <Agent host="192.168.10.101" port="8800"/>
>> <Agent host="192.168.10.102" port="8800"/>
>> <RFC5424Layout enterpriseNumber="18060" includeMDC="true"
>> appName="MyApp"/>
>> </Flume>
>> <Routing name=?Routing?>
>> <InitScript name=?RoutingInit" language="groovy"><![CDATA[
>> if (System.getProperty(?os.name?).contains(?OS/390
>> <http://os.name/?).contains(?OS/390>") {
>> return “OS390";
>> }
>> return null;]]>
>> </InitScript>
>> <Routes>
>> <Script name="Router" language="groovy"><![CDATA[
>> if (logEvent.getMarker() != null &&
>> logEvent.getMarker().isInstanceOf("AUDIT")) {
>> return "AUDIT";
>> } else if (logEvent.getContextMap().containsKey("UserId")) {
>> return logEvent.getContextMap().get("UserId");
>> }
>> return "STDOUT";
>> ]]>
>> </Script>
>> <Route>
>> <OS390Appender name=“OS390-${mdc:UserId”/>
>> <RollingFile name="Rolling-${mdc:UserId}"
>> fileName="${mdc:UserId}.log"
>> filePattern="${mdc:UserId}.%i.log.gz">
>> <PatternLayout>
>> <pattern>%d %p %c{1.} [%t] %m%n</pattern>
>> </PatternLayout>
>> <SizeBasedTriggeringPolicy size="500" />
>> </RollingFile>
>> </Route>
>> <Route ref="AuditLogger" key="Audit"/>
>> <Route ref="STDOUT" key="STDOUT"/>
>> </Routes>
>> <IdlePurgePolicy timeToLive="15" timeUnit="minutes"/>
>> </Routing>
>> </Appenders>
>> First, the init script changes the default route based on the OS.
>> Second, notice that “Routes” has a new Script element and does not have a
>> pattern specified, so the script is determining the key instead of the
>> pattern.
>> Third, the real default route is now “STDOUT” since the actual default Route
>> is only referenced when a UserId is present in the thread context map.
>>
>> What would also be nice is if there was a way to have the returned value be
>> usable as a Lookup value in the default Appender definition, instead of
>> relying on the MDC as the code above does. I should be able to pick
>> something out of the message itself and use that as the key. That should be
>> doable but I am still pondering how I would implement that.
>>
>> Ralph
>>
>>
>>
>>> On Sep 12, 2016, at 6:06 PM, Ralph Goers <ralph.go...@dslextreme.com
>>> <mailto:ralph.go...@dslextreme.com>> wrote:
>>>
>>> I’ll try to describe it better but I’m not sure how good a job I’ll do if
>>> the dots aren’t clicking yet. Also, even though I might say to do it one
>>> way if I was coding I could very well change my mind as I implement it.
>>> That said:
>>> Create an Appender plugin named ScriptSelector or ScriptAppenderSelector.
>>> It needs the following parameters
>>> name, a String PluginAttribute
>>> default, a String PluginAttribute
>>> initScript or startupScript, an AbstractScript PluginElement
>>> script, an AbstractScript PluginElement
>>> appenderList, an AppenderList PluginElement.
>>> As always, the builder (or factory) creates an instance of the
>>> ScriptAppenderSelector.
>>> If there is an init script then the builder (or factory) executes it.
>>> If the returned value is not null then instantiate the Appender with that
>>> name using the configuration in the AppenderList.
>>> Whatever Appender the init script names should become the default Appender.
>>> If no init script is present or the init script returns null use the value
>>> of the default setting as the name of the default Appender to use.
>>> Create the default Appender and save it in the Map of created appenders
>>> wrapped by an AppenderControl.
>>> When the append method is called check for a script setting.
>>> If a script is found, run it.
>>> If it returns a value see if that appender is saved in the AppenderMap.
>>> If it is, call the appender and return.
>>> If it is not, locate the configuration for the appender, create it and add
>>> it to the AppenderMap. Then call it and return.
>>> If it returns null or no script is defined then call the default Appender
>>> and return.
>>> When the stop method is called call the stop method on each of the
>>> Appenders in the AppenderMap.
>>>
>>> Note that signatures for scripts are defined by the components that use
>>> them. In this case both the init script and script return the name of the
>>> appender to execute.
>>>
>>> Ralph
>>>
>>>
>>>> On Sep 12, 2016, at 12:54 PM, Gary Gregory <garydgreg...@gmail.com
>>>> <mailto:garydgreg...@gmail.com>> wrote:
>>>>
>>>> On Sun, Sep 11, 2016 at 12:47 PM, Ralph Goers <ralph.go...@dslextreme.com
>>>> <mailto:ralph.go...@dslextreme.com>> wrote:
>>>> Yes. The Appenders tag inside the ScriptSelector are the Appenders that
>>>> are to be created. But now that I think about it, we can’t use “Appenders”
>>>> for this. If you look at the RoutingAppender you will notice that
>>>> Appenders there are declared under a Route element. The Route plugin is
>>>> defined with deferChildren=true. This means that whatever is configured
>>>> under the Route will not be created during initial configuration. Instead
>>>> the Route keeps a reference to the Node and then configures the Appender
>>>> when it is required. So we would need a new plugin to wrap the Appenders
>>>> that are to be created.
>>>>
>>>> Can you please describe in more detail how this new plug fits in and what
>>>> it does? I can't quite connect the dots with the parallel of the routing
>>>> appender. I'm willing to implement this as I need the feature ASAP.
>>>>
>>>> Gary
>>>>
>>>>
>>>> Ralph
>>>>
>>>>> On Sep 11, 2016, at 11:10 AM, Gary Gregory <garydgreg...@gmail.com
>>>>> <mailto:garydgreg...@gmail.com>> wrote:
>>>>>
>>>>> Are the <Appenders> tags really meant to be nested?
>>>>>
>>>>> Gary
>>>>>
>>>>> On Sat, Sep 10, 2016 at 11:48 AM, Ralph Goers <ralph.go...@dslextreme.com
>>>>> <mailto:ralph.go...@dslextreme.com>> wrote:
>>>>> Oops. I forgot the closing CDATA tag in the script.
>>>>>
>>>>> Ralph
>>>>>
>>>>>> On Sep 10, 2016, at 11:43 AM, Ralph Goers <ralph.go...@dslextreme.com
>>>>>> <mailto:ralph.go...@dslextreme.com>> wrote:
>>>>>>
>>>>>> Interesting. OS/390. I worked on MVS, OS/370, z/OS, etc many moons ago
>>>>>> but haven’t worked on a mainframe since 2001.
>>>>>>
>>>>>> 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”><