RFR: JDK-8046565: Platform Logger API and Service

Mandy Chung mandy.chung at oracle.com
Wed Oct 14 05:21:41 UTC 2015


> On Oct 8, 2015, at 5:26 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> webrev:
> http://cr.openjdk.java.net/~dfuchs/8046565/proto.01/webrev.01/

System.Logger

Several log methods throws NPE if the level is legible and the parameter is null.  E.g.
* @throws NullPointerException if {@code level} is {@code null}, or if the level
*         is loggable and {@code msgSupplier} is {@code null}.

Should it throw NPE if the input parameter is null regardless of whether the level is loggable? 

RuntimePermission(“loggerFinder”) is required when 
1. a LoggerFinder provider is instantiated
2. LoggerFinder::getLoggerFinder() is called
3. LoggerFinder::getLogger is called

This is very specific to finding system logger (more than just the provider construction check).  It does seem worth defining a specific Permission type for it e.g. LoggerPermission.    Since LoggerFinder::getLogger requires permission check, does LoggerFinder::getLoggerFinder() need the explicit permission check?   We will need to consult with the security experts.

LazyLoggers::getLazyLogger(String name, Class<?> caller)
- does it need permission check? it’s currently commented out

public class LoggerWrapper<L extends Logger> extends AbstractLoggerWrapper<L>
    implements Logger, PlatformLoggerBridge

- AbstractLoggerWrapper implements Logger, PlatformLoggerBridge, ConfigurableLoggerBridgem.  Is "implements Logger, PlatformLoggerBridge” needed at all?

Are all *Logger and *LoggerWrapper types implementing Logger, PlatformLoggerBridge, ConfigurableLoggerBridge?  I might be missing it - I don’t see any non-configurable logger type implements only Logger, PlatformLoggerBridge.

SimpleConsoleLogger.Formatting
static final String FORMAT_PROP_KEY =
            "java.util.logging.SimpleFormatter.format";
- is this used when java.logging is not present?  It’s for the simple console logger formatting.  If so, the property name should be renamed to idk.logger.xxx?

Can you explain the difference of sun.util.logging and sun.util.logger package? I think the reason to have two different packages is to make sure that sun.util.logger not used by other modules.  What other issue to put the classes in sun.util.logging - the existing package?  I don’t have any issue to create a new package but the name is hard to differentiate.   Maybe rename sun.util.logger to jdk.internal.logger if not in sun.util.logging?

Similarly, sun.util.logging.internal.JdkLoggingProvider and sun.util.logger.JdkLoggerProvider - the package names are too close and they are in two different module.  Probably good to rename the classname - what about 
s/JdkLoggerProvider/SystemLoggerProvider/
s/JdkLoggingProvider/LoggingProvider/

I would have assumed that sun.util.logger.JdkLoggerProvider should be LoggerFinder.  Is there any reason why it can’t extend LoggerFinder?   I think that would be cleaner and getJdkLogger is not needed.  Maybe DefaultLoggerFinder can be simplified.

Have you considered moving out LoggerFinderLoader to a separate class?  This change adds many lines in System.java.

PlatformLogger - main reason to retain it is to minimize changes.  I wonder if changing it to an interface would help or not.  I’m still studying the sun.util.logger.* classes.

jdk/test/java/lang/System/Logger/custom/AccessSystemLogger.java
jdk/test/java/lang/System/Logger/default/AccessSystemLogger.java
jdk/test/java/lang/System/LoggerFinder/public/BaseLoggerFinderTest/AccessSystemLogger.java
- they seem to be duplicated code to setup boot directory.  Worth putting them in logging test library?
- what does the directory name "public" mean?  maybe just take that directory out?

jdk/test/java/lang/System/LoggerFinder/internal/backend/SystemClassLoader.java
- copyright header

That’s all for today.

Mandy





More information about the core-libs-dev mailing list