logging-log4net-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ron Grabowski <rongrabow...@yahoo.com>
Subject RE: Feedback for log4net.Contrib.AspNetPatternConverters
Date Sun, 11 Sep 2005 18:35:06 GMT
I don't know why I didn't think of this sooner...if you have groups of
related converters that you'd like to register the amount of additional
xml nodes makes for some ugly xml:

<layout type="log4net.Layout.PatternLayout">
 <converter>
  <name value="aspnet-response" />
  <type
value="log4netContrib.AspNetPatternConverters.ResponsePatternConverter,
log4net.Contrib.AspNetPatternConverters" />
 </converter>
 <converter>
  <name value="aspnet-response-cookies" />
  <type
value="log4netContrib.AspNetPatternConverters.ResponsePatternConverter+Cookies+Value,
log4net.Contrib.AspNetPatternConverters" />
 </converter>
 <conversionPattern value="%p %d %aspnet-response{ContentType}
%aspnet-response-cookies-value{MyCookie} - %m%n" />
</layout>

A cleaner alternative is to group related converters into a layout:

public class AspNetPatternLayout : PatternLayout
{
 public override void ActivateOptions()
 {
  AddConverter("aspnet-cache", typeof(AspNetCachePatternConverter));
  AddConverter("aspnet-context",
typeof(AspNetContextPatternConverter));
  AddConverter("aspnet-request",
typeof(AspNetRequestPatternConverter));
  AddConverter("aspnet-session",
typeof(AspNetSessionPatternConverter));
  // snip
  ActivateOptions();
 }

The confusing xml is now reduced to:

 <layout
  type="log4netContrib.Layout.AspNetPatternLayout, log4netContrib"
  value="%aspnet-request{WidgetId} %aspnet-context{ProductId}" />

--- Nicko Cadell <nicko@neoworks.com> wrote:

> Ron,
> 
> This does look like a very nice idea and does provide additional
> functionality that log4net does not have out of the box. 
> 
> I think that it makes sense to be able to register new converters
> into
> the pattern layout on a per-repository basis. I will need to think
> about
> where that would be stored, but I think that it would make the
> registration of this kind of large pack of additional patterns much
> simpler. E.g. it could be done with a single <plugin type="xxx" />
> line
> in the config file.
> 
> 
> > Should there be try/catch blocks around the code or is 
> > checking for null values good enough?
> 
> That depends. Checking for null may be good enough if no other
> exceptions can be thrown. If you don't know it is better to catch any
> exception and output an error into the output; otherwise the log
> message
> will not be output, or it will be corrupt.
> 
> 
> > I'm not sure how to handle access to certain properties like 
> > the Count property of the Cache object. If I use this 
> > conversion pattern:
> > 
> >  <conversionPattern value="%cache{Count} - %m%n" />
> > 
> > I'm unable to determine which of the following the user wants
> > displayed:
> > 
> >  Cache.Count
> >  Cache["Count"]
> 
> I think that to be consistent the %cache{Count} must mean
> Cache["Count"]. You can always have another %cache-count pattern to
> get
> the property value.
> 
> 
> > Is there a more appropriate naming convention for my classes? 
> > The design for the inner classes is based off of 
> > FileAppender's inner file locking classes.
> 
> The inner classes in the appenders are not exactly best practice
> according to the MS design guidelines. The guidelines say that inner
> classes should not be used as a way of grouping related classes
> together
> - which is sort of what that are used for in the appenders. The
> guidelines recommend using nested namespaces to achieve this.
> 
> 
> > 
> > I'd like to hear some feedback and have an chance to make 
> > somemore minor changes to the code before I make it availabe 
> > for download.
> > 
> > Comments, questions, cool, not cool, don't care? 
> 
> Definitely cool.
> 
> Nicko
> 
> 
> > 
> > - Ron
> > 
> > 
> 


Mime
View raw message