RFR: 8145686: SimpleConsoleLogger and LogRecord should take advantage of StackWalker to skip classes implementing System.Logger

[Mandy Chung]

> On Dec 17, 2015, at 11:23 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> On 12/17/15 7:02 PM, Mandy Chung wrote:
>> 
>>> On Dec 17, 2015, at 8:42 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>>> 
>>> Hi,
>>> 
>>> Please find below a small enhancement for the method that infers
>>> the calling class name and method name in  SimpleConsoleLogger
>>> and LogRecord.
>>> 
>>> The idea is that since these methods now use the new StackWalker API
>>> they should take advantage of that in order to skip classes that
>>> implement System.Logger. This should make it possible for components
>>> that need it to easily wrap a System.Logger (and even a j.u.l.Logger).
>>> 
>>> 8145686: SimpleConsoleLogger and LogRecord should take advantage
>>>         of StackWalker to skip classes implementing System.Logger
>>> https://bugs.openjdk.java.net/browse/JDK-8145686
>>> 
>>> webrev:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8145686/webrev.00/
>> 
>> What is the difference between skipLoggingFrame and isLoggerImplFrame?
> 
> isLoggingImplFrame stops when it finds the concrete logger implementation. It will unstack all the LogRecord, Handler, Filter
> implementations etc... (which could be custom application classes)
> 
> skipLoggingFrame will skip everything above the first method found
> on the concrete Logger implementation until it reaches something that
> looks like a calling frame. In particular it should skip other
> methods of the logger implementation (e.g. log() calls log(LogRecord),
> default interface methods provided by the logging framework,
> reflection/method handle frames etc...
> In particular it should skip any Logger wrapper classes.
> 

It’d be helpful to add some comment in the skipLoggingFrame method.  Maybe even better to change this method to isFilteredFrame??  Reflection frames and lambda forms should already be skipped by StackWalker.  The remaining to skip are method handles, doPrivileged, other logging internal (do you have the case that the logging internal are called via reflection and is that why they are included here)?  Looks like it’s good to check if this list should be cleaned up.

Should Formatting::skipLoggingFrame to return true if System.Logger.class.isAssignableFrom(t.getDeclaringClass()) so that that check doesn’t need to be duplicated in both SimpleConsoleLogger and LogRecord?

 422                 if (cname.startsWith("java.lang.System$Logger"))       return true;

This line can be removed.

> For instance, RMI has a logger that wraps a j.u.l.Logger so
> ideally that should be skipped too...
> JMX remote has a ClassLogger that should also be skipped.
> etc…


That might be the reason why skipLoggingFrame filters sun.rmi.runtime.Log?  Is this necessary?  This white list needs to be maintained manually which is fragile.  


186                 @Override public StackWalker run() {

- Nit: separate @Override into a separate line

Mandy