Re: [log4perl-devel] RFC: Log4perl Custom Filters Design Proposal

[<Msc...@ao...>]

In a message dated 2/22/03 9:05:04 AM Pacific Standard Time, ke...@go... 
writes:

> Some observations:

Kevin, you've made some excellent points, see details inline ...

> 1) Regarding log4j compatibility, if someone implements log4j-style
> filters it's just a matter of specifying a different implementation
> class in the config, right?  Hmm, except for the chaining thing, and
> that log4j filters return ACCEPT/DENY/NEUTRAL instead of true/false.  

The Log4j folks don't have a notion of defining filters in the Property-style 
config files, and probably won't have one in the forseeable future, so we're 
exploring unchartered waters. No matter how we're implementing it, it'll 
always be incompatible with Log4j. But, you're right, maybe some day we'll 
need a notion of a "chained" filter set. It can be accomplished by employing 
a strategy similar to the "Bool"ean filter: A "Chained" filter, taking a 
chain of filters. And the chain defines what it means (block/neutral/accept) 
if filter X returns true or false (note that the chain can base its decision 
on the same parameters as the filter has available, because the chain calls 
the filter in turn).

The current design is simple: An appender has only one filter attached 
(returning true or false), but it's up to this filter to be smart and use the 
services of other filters in creative ways.

> 
> Is there any way to differentiate?  Is it worth calling our filters
> "pfilters" or "bfilter" (b for boolean)?

We can make the distinction at the filter level, see above.

> 2) As I was writing those examples, I wondered if it's worthwhile
> setting up to allow defining a filter per-appender as well as global,
> like we do for the cspec's?   (see t/033UsrCspec.t).  Then you can use
> them close to where they're defined.

Interesting thought. With cspecs, I can clearly see the importance: You've 
got a very limited namespace (one-letter words) and local settings could 
pollute the global namespace quickly if they weren't separated.

However, the filter case is different, their namespace is virtually 
unlimited. Kind of like loggers reference appenders -- there's no localized 
namespace. And it's OK to define them close to where they're used:

        # Error appender
     log4perl.appender.AppError = Log::Dispatch::File
     log4perl.appender.AppError.filename = /tmp/app.err
     log4perl.appender.AppError.layout   = SimpleLayout
     log4perl.filter.MatchError                      = 
Log::Log4perl::Filter::LevelMatch
     log4perl.filter.MatchError.LevelToMatch = WARN
     log4perl.appender.AppError.Filter          = MatchError

What we could do, though (without too much effort, I hope) is shorten the 
definition by cutting out the filter name:

        # Error appender
     log4perl.appender.AppError = Log::Dispatch::File
     log4perl.appender.AppError.filename = /tmp/app.err
     log4perl.appender.AppError.layout   = SimpleLayout

     log4perl.appender.AppError.Filter          = 
Log::Log4perl::Filter::LevelMatch
     log4perl.appender.AppError.Filter.LevelToMatch = WARN

This way, there's no name pollution -- but on the other hand you're limited 
because filters like Bool(ean) expect named filters.

> 3) There seems to be a typo in the email, under "Attaching a filter to
> an appender" it says
>   log4perl.appender.MyAppender = MyFilter
> s/b
>   log4perl.appender.MyAppender.Filter = MyFilter
> right?

Good catch! Fixed in the docs now.

> 
> 4) Tiny point: do you really want to call it Bool.pm instead of
> Boolean.pm?  What's a Bool?

The name of the guy who invented it was "George Boole", so "Bool" doesn't 
make any sense, you're right :). I'll change it to Boolean (hope not too many 
people have read this week's recipe :). If you have, use "Boolean.pm" from 
now on! :)

> 
> 5) Why is "f" capitalized in "log4perl.appender.Screen.Filter" but not
> in "log4perl.filter.Match1"?  (I'm as guilty as anyone of inconistency
> here, but it's never to late to reform!)

Good point. It's "log4perl.filter" because it's "log4perl.appender". All 
that's lower-cased, by log4j standards. On the other hand, the situation with 
appender or other properties/attributes is less clear: Seems like they're 
uppercase if they're local to the current object and lowercase if they're 
property "keywords" (compare log4j.appender.X.threshold vs. 
log4j.appender.X.File). In log4perl, we haven't made that distinction so far, 
properties are typically lowercase for the perl appenders (Java appenders 
conform to log4j, though) -- mainly because Log::Dispatch has lowercase 
attributes. I was just concerned about an appender in the Log::Dispatch 
hierarchy accidently defining a "filter" attribute. If our "Filter" keyword 
is uppercase, it's less likely to clash (although still ambiguous), although, 
you're right about it being inconsistent.

> 
> 6) log4j's decide() method returns a constant ACCEPT/DENY/NEUTRAL, but
> our decide() returns a boolean.  Wouldn't it be better to name our
> method let_pass() or is_loggable() or something that implies how the
> boolean will be treated?  ("Hmm, I can't remember, does decide()==true
> mean decides to filter it out, or decides to log it?").  I think a hint
> to the developer might be appreciated some day.

Yep, makes sense. I'll put on my thinking cap ... :)

Thanks for the feedback, changes are checked in (BTW in case I haven't 
mentioned it, it's all implemented/documented already, check t/040Filter.t 
for more or less typical use cases).

-- Mike
Mike Schilli
log...@pe...