On Fri, Sep 16, 2016 at 8:38 PM, Ralph Goers <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”).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?
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.
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.
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
>
> 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>
> wrote:
>
>> I committed a first cut, see comments in https://issues.apache.org/j
>> ira/browse/LOG4J2-1578
>>
>> Gary
>>
>> On Mon, Sep 12, 2016 at 11:40 PM, Ralph Goers <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>
>>> 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>
>>> 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> 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>
>>>>> 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:
>>>>>
>>>>> 1. 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.
>>>>> 2. 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.
>>>>> 3. 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.
>>>>> 4. 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>
>>>>> wrote:
>>>>>
>>>>> On Sun, Sep 11, 2016 at 12:47 PM, Ralph Goers <ralph.goers@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>
>>>>>> 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> 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> 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”><