Jens Elkner <[email protected]> writes:

> On Mon, Jun 20, 2011 at 12:07:09PM +0000, [email protected] wrote:
>> https://defect.opensolaris.org/bz/show_bug.cgi?id=18585
>> 
>> Knut Anders Hatlen <[email protected]> changed:
>> 
>>            What    |Removed                     |Added
>> ----------------------------------------------------------------------------
>>              Status|ACCEPTED                    |CLOSED
>>          Resolution|                            |FIXED
>>    Target Milestone|---                         |next
>> 
>> --- Comment #1 from Knut Anders Hatlen <[email protected]> 2011-06-20 
>> 12:07:08 UTC ---
>> changeset:   1258:b889068b5d93
>
> Hmmm, well, actually the intention was to silent logs, i.e. print 
> stacktraces only, when debugging for this class is enabled. So the 
> "correct" way would be:
>
>       if (null != thrown &&
>               Logger.getLogger(record.getLoggerName()).getLevel().intValue() >
>                       Level.CONFIG.intValue())
>       {
>               ...
>       }
>
> I call that, from the back through the breast ;-), since the method
> which calls "format(...)" already knows, what gets accomplished "brute
> force like" with Logger.getLogger...

You're probably right. I only wanted to fix the obvious bug with the
wrong operator being used to compare the levels.

> Anyway:
>
> To get Logging correct, in general the pattern one should always use is:
> class Foo ... {
>       static final Logger log = Logger.getLogger(Foo.class.getName());
>
>       try {
>               ...
>       } catch (*Exception e) {
>               log.warn(e.getLocalizedMessage();
>               if (log.isDebugEnabled()) {
>                       log.debug("$hint-e-g-methodName", e);
>               }
>       }
> }
>
> So yes, "Logger logger = OpenGrokLogger.getLogger(); log..." is plain
> wrong. In addition, OpenGrokLogger hard codes pretty everything, which
> makes logging almost useless.

Well, "plain wrong" is a bit too strong, isn't it? :)

The API says this about the "name" parameter to Logger.getLogger():

Parameters:
    name - A name for the logger. This should be a dot-separated name
    and should normally be based on the package name or class name of
    the subsystem, such as java.net or javax.swing

OpenGrokLogger calls it with "org.opensolaris.opengrok", which satisfies
both the requirement that it's dot-separated and the suggestion that
it's based on the package name.

I'm not so familiar with the logging API. What would switching to using
class names buy us? The ability for users to do more fine-tuning of what
to log? If so, it sounds like a good idea, provided that it doesn't at
the same time make it more a lot more difficult to set up for those who
don't need the fine-tuning.

> IMHO, we should switch over to use slf4j (see slf4j.org) and than
> everybody can use the logging framework he like and make use of its
> features. Also this way, OpenGrokLogger and *Formatter can be phased out
> (i.e. removed) completely and actually _should be_.

I tend to be reluctant to add third-party dependencies, especially when
similar functionality is provided by the Java platform. Couldn't those
who need to plug OpenGrok into another logging framework do that with a
bridge instead of us using a third-party API? For example this one:
http://www.slf4j.org/legacy.html#jul-to-slf4j

> Would that be a problem: Nope. Tomcats already use slf4j, glassfish comes
> with an adapter and for standalone usage, slf4j comes with adapters
> (i.e. implementation wrappers) for jul, apach.commons.logging, log4j etc.

The indexer also uses logging, and runs outside of Tomcat and GlassFish,
so we would need to bundle slf4j with OpenGrok, wouldn't we?

> Finally, I would ship opengrok with a native implementation named logback
> (see http://logback.qos.ch/): here one is able to get all the functionallity
> OpenGrokLogger is trying to deliver, but in a much more flexible way
> just by supplying an appropriate logback.xml file ... 
>
> What do you think?

I think it would make sense to change the Logger.getLogger() calls as
you suggested. I would prefer to stick to java.util.logging for now and
not switch to a third-party library.

-- 
Knut Anders
_______________________________________________
opengrok-dev mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/opengrok-dev

Reply via email to