RFR: JDK-8046565: Platform Logger API and Service (original) (raw)
Mandy Chung mandy.chung at oracle.com
Wed Oct 14 05:21:41 UTC 2015
- Previous message: RFR: JDK-8046565: Platform Logging API and Service
- Next message: RFR: JDK-8046565: Platform Logger API and Service
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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
- a LoggerFinder provider is instantiated
- LoggerFinder::getLoggerFinder() is called
- 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 extends AbstractLoggerWrapper 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
- Previous message: RFR: JDK-8046565: Platform Logging API and Service
- Next message: RFR: JDK-8046565: Platform Logger API and Service
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]