incubator-yoko-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan D. Cabrera" <l...@toolazydogs.com>
Subject Ready for a Vote? was: Logging
Date Sun, 21 May 2006 18:44:31 GMT
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Daniel Kulp wrote:
<blockquote cite="mid200605120314.52208.daniel.kulp@iona.com"
 type="cite">
  <pre wrap="">Lars,

  </pre>
  <blockquote type="cite">
    <pre wrap="">Daniel, sorry for not responding earlier (moved to a new house). First
of all, thanks for trying to find a consensus.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
Congrats on the new house.   That's always a huge undertaking and sucks up 
a LOT of time.



  </pre>
  <blockquote type="cite">
    <pre wrap="">My first reaction to the above suggestion was "If it was that simple,
then why are people still messing around with jakarta commons-logging,
slf4j and friends?" But I wanted to try it out and started to create a
prototype of a j.u.l.Logger subclass that delegates to a corresponding
log4j Logger. Here are my observations so far:

    * j.u.l.Logger.getLogger() hardcodes "new Logger()" for logger
      creation. We absolutely *must* use our LoggerFactory everywhere
or things will break. This means that there is a high probability for
errors, because the proposed design deviates from normal usage of the
API.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
Right.   That was part of the proposal.   The good thing is that it's very 
easy to get checkstyle to error out if Logger.getLogger(..) is ever 
called.   It's quite enforceable. 

  </pre>
  <blockquote type="cite">
    <pre wrap="">* I
couldn't find a solution that handles j.u.l specific Levels like
Level.CONFIG which do not exist in log4j. Same with user defined
levels.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
Well.   That would be a problem if we wrote our own logging API as well.   
If we wrote our own logging API, I would want at LEAST as many levels as 
j.u.l.    Making a "lowest common denominator" type API would be a big 
mistake.

  </pre>
  <blockquote type="cite">
    <pre wrap="">    * The combination of getLevel() and setLevel() is hard to
implement. I think they should delegate to the log4j logger, but then
setLevel() followed by getLevel() would not always return the same
result because FINER and FINEST are both mapped to the log4j TRACE
level.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
I wouldn't be too concerned about that one as long as getLevel returns the 
lowest level that would actually be logged.   In your example, return 
FINER.   

That said, Log4j supports custom Levels as well.   Thus, I would just 
define new Log4j levels to map those directly to. 

Out of curiosity: can I ask why any of our code would EVER call 
"setLevel(...)"?   Shouldn't configuring the log levels and such be part 
of the logging configuration by the user?    I don't think we should 
pretend to assume what the user wants and override it by calling 
setLevel(...).


  </pre>
  <blockquote type="cite">
    <pre wrap="">    * The implementation of addHandler() is unclear, probably that
      operation should not be supported. Delegating to log4j means that
      j.u.l Handlers will be ignored. This violates the API contract of
      j.u.l.Logger - logging messages are no longer forwarded to
      registered Handlers instead they are forwarded to log4j.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
As above, I'm not sure why any of our code would ever call 
get/setHandler(..).   That seems to again be a configuration thing.   In 
anycase, I THINK we can prevent that with a checkstyle rule as well.   
(This is a little harder as it's not a static method.)


  </pre>
  <blockquote type="cite">
    <pre wrap="">There are probably many more points in the same spirit. To summarize,
the design does not follow the substitution principle: the subclass is
not a replacement for j.u.l.Logger because many methods of j.u.l.Logger
do not make sense for a log4j backend. The only good news is that I
managed to get some log messages out of log4j.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
That's perfect.   That's the news I wanted to hear.


  </pre>
  <blockquote type="cite">
    <pre wrap="">To me, the code of the j.u.l.Logger subclass seems like a fragile hack.
Yes, I could probably bridge to log4j and get some yoko logging output,
but the code will continue to work only if the yoko dev team is
extremely disciplined and it is very clear which parts of j.u.l must
not be used.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
Or enforce it with checkstyle or accept that if you use some of the 
"advanced" things, they might not work the same (or at all) with a 
different backend.  


  </pre>
  <blockquote type="cite">
    <pre wrap="">I've also taken a look at j.u.l.Logging package itself. Let me say
it
politely: I'm still not convinced.

    * throwing() encourages an idiom very similar to the "log and
throw" style that is discouraged by our coding guidelines
    * Naming is horrible. Quick, what's the purpose of Logger.logrb()?
    </pre>
  </blockquote>
  <pre wrap=""><!---->
At least it PROVIDES an API for doing these things easily.   Log4J doesn't 
provide something equivalent to logrb.   

  </pre>
  <blockquote type="cite">
    <pre wrap="">I still think that using our own interface would be the best solution.
No external dependencies and maximum flexibility. I could live with
slf4j, but it seems that j.u.l is not a viable option for those of us
who need to integrate with log4j, even when using a LoggerFactory as
suggested above.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
Huh?   You just stated that the only thing that doesn't seem to work is 
the Handlers.    So how is it it not a viable option?   To me, you just 
proved that it IS a viable option.  



My summary is:

-1  to using 3rd party stuff

-1 to defining our own API that is a "lowest common denominator" type API.   
I STRONGLY don't believe in "lcd" approaches.   It's always the worst of 
everything.

-0 to defining our own API that is NOT a total "lcd".   Basically, take 
the j.u.l.Logger methods (ALL of them except the statics and the 
Handler/Filter stuff that our code shouldn't call anyway) and put them on 
an Interface (or abstract class).  Maybe even add a couple.  Keep using 
the j.u.l.Level object.   No need to define that.   This is -0.1 just 
because I think it's a waste of time as just using the Logger API 
directly works as you proved.   Then again, the time we've spent 
discussing this......

+1 to just using j.u.l directly

+0.5 to the proposal of j.u.l with the custom factory


  </pre>
</blockquote>
<br>
I apologize for my tardy reply.<br>
<br>
I think that we should use j.u.l directly.&nbsp; It's good enough to get the
job done.&nbsp; When people come to read our code I don't want them to get
side tracked by how clever we can be w/ logging technology.&nbsp; I want
them to focus on the actual CORBA architecture.<br>
<br>
Frankly, I cannot think of a situation, developmentally or
operationally, where j.u.l doesn't get the job done.&nbsp; This is not to
say that other loggers aren't better.&nbsp; <br>
<br>
With that said, I think that we are treading in that gray area called
personal preference and that no one can reasonably throw a technical
veto on any of the above solutions; though there is one exception of
_requiring_ 3rd party loggers, which I think we have a consensus on
that being a bad thing in light of our inclusion of Harmony.&nbsp; I think
that everyone has had their say and that we are ready for a vote.&nbsp; If
there are no objections by Wednesday, I'll start such a vote.<br>
<br>
<br>
Regards,<br>
Alan<br>
<br>
</body>
</html>

Mime
View raw message