RFR: JDK-8046565: Platform Logging API and Service (original) (raw)

Daniel Fuchs daniel.fuchs at oracle.com
Tue Oct 13 12:14:19 UTC 2015

Hi Mandy,

Thanks a lot for your feedback!

On 13/10/15 04:50, Mandy Chung wrote:

Hi Daniel,

On Oct 9, 2015, at 12:53 PM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:

JEP: http://openjdk.java.net/jeps/264 https://bugs.openjdk.java.net/browse/JDK-8046565 specdiff: http://cr.openjdk.java.net/~dfuchs/8046565/proto.01/specdiff-api/overview-summary.html webrev: http://cr.openjdk.java.net/~dfuchs/8046565/proto.01/webrev.01/ This is very good work. I’m glad to see this work that enables the dependency on java.logging be eliminated. The sun.util.logging.PlatformLogger was a stop-gap approach. There are many new tests that I will need time to review them all.

I tried to split the tests into tests that only test the public API, and tests that are implementation dependent: those also access the internals or test the internal artifacts that the implementation is using. The split may not be obvious at the first glance - but tests under 'internal' or 'jdk' subdirs are implementation specific. The other tests should not use internal APIs. I have tried to convey the intent of the test in the @summary tag, and those tests that are implementation specific say so in their summary.

I’m going to pass you my comments in several batches. This is the first batch.

LogManager.java 1938 if (caller.getClassLoader() == null) { 1939 return LogManager.getLogManager().demandSystemLogger(name, 1940 Logger.SYSTEMLOGGERRBNAME, caller); This only considers the classes loaded by the boot loader as system classes. We have deprivileged several modules to be loaded by the ext class loader such as JAX-WS, JAXB, CORBA etc. Loggers created by modules loaded by boot and ext class loader should be system.

Right. I agree. But I also think this is orthogonal to this JEP implementation. Would you agree to handle that in a separate JBS issue (probably more 'Bug' than 'RFE')?

In the absence of java.logging, the default provider implementation will be used to print the log messages to System.err. This is useful since most JDK log messages are for debugging. I expect that a component may want to turn on debug messages without requiring the presence of java.logging.

Is there any mechanism to change the default level of the default simple console loggers? It may be useful; otherwise it would need to run on an image with java.logging module. Maybe something to add in the future.

There isn't at this time. We could add for instance a global system property that would allow to define the 'default level' for all SimpleConsoleLogger. Something like '-Djdk.loggers.level.default=DEBUG' which would have an effect similar to setting the root logger level .level=FINE in the default configuration. The question then is whether that property would only be used when java.logging is not present, or whether the LogManager should be modified to take this into account too - and whether it would have precedence over what is in the configuration (default or not)...

Possibly something to handle in a second time. Should I add a subtask to JDK-8046565 - to log an RFE about that after the initial implementation gets in?

System.Logger 964 * Unless - incomplete sentence?

Thanks for catching that. I'd been considering adding some kind of global blanket statement for the throwing of NullPointerException, then decided against it but forgot I had started writing the sentence... I will remove this line.

1697 * @implSpec 1698 * Instances returned by this method route messages to loggers 1699 * obtained by calling {@link LoggerFinder#getLogger(java.lang.String, java.lang.Class) 1700 * LoggerFinder.getLogger(name, ca

Is this intended to be implementation specification but not API spec?

It's intended to be an API spec that the messages should be routed to loggers obtained from LoggerFinder.getLogger(name, caller).

Whether the logger returned is directly the logger obtained, or a wrapper, and at which point exactly the 'real' logger will be created is implementation dependent. I think we need to have a bit of freedom here to deal with bootstrap issues according to the needs of the modules in the JDK.

RuntimePermission(“LoggerFinder) is required in the provider constructor. - is it time to define a ServiceProviderPermission for provider constructor security permission check? Rather than overloading RuntimePermission

If there was a ServiceProviderPermission I would use it, but I don't think this JEP should introduce it. We could log another RFE for that.

public static final RuntimePermission LOGGERFINDERPERMISSION = new RuntimePermission("loggerFinder”); - is there any need to define this constant? Suggest this be implementation-details.

This was mostly temporary convenience for me - until I'm sure I won't get further feedback that the name should be changed. Avoids copy/paste typo mistakes and spending time debugging it. If you think it would be better to not define such a constant, then I agree to remove it in the final API. Should I had a subtask to the JEP to get this removed, so that it's not forgotten?

private LoggerFinderLoader() { throw new InternalError("LoggerFinderLoader cannot be instantiated"); } - is throwing InternalError necessary? No one else except its enclosing class can construct it.

It clearly indicates that it should not be instantiated. I like it :-)

JdkLoggingProvider::LOGGINGCONTROLPERMISSION - I think this field is unused.

Thanks. Removed.

sun/management/ManagementFactoryHelper.java - Is LoggingMXBeanSupport intended to get java.management to remove its dependency on java.logging?

172 public interface LoggingMXBean 173 extends PlatformLoggingMXBean, java.util.logging.LoggingMXBean { 174 } This static dependency will still keep the dependency on java.logging.

Yes. I have been scratching my head trying to figure out how to remove it. If at some point we change java.management to use System.Logger instead of java.util.logging.Logger, then we will still need to specify that java.management requires java.logging - because of this. Maybe we can find a clever way to deprecate this and use ServiceLoader to load the LogginMXBean implementation. That would be preferable to the reflection tricks I had to add here, but it might not be a trivial task.

However the code I added to guard the uses of PlatformLoggingImpl and ManagemetFactoryHelper.LoggingMXBean should make it possible to use java.management without java.logging - in the hypothesis where java.management would be migrated to use System.Logger.

Though of course I haven't tested it. Anyway - that's something for later.

best regards,

-- daniel

Mandy

More information about the core-libs-dev mailing list