Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new java platform logger #1361

Open
wants to merge 2 commits into
base: 5.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions driver/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -409,4 +409,10 @@
<method>org.neo4j.driver.BaseSession session(java.lang.Class, org.neo4j.driver.SessionConfig)</method>
</difference>

<difference>
<className>org/neo4j/driver/Logging</className>
<differenceType>7012</differenceType>
<method>org.neo4j.driver.Logging javaPlatformLogging()</method>
</difference>

</differences>
2 changes: 1 addition & 1 deletion driver/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
requires io.netty.buffer;
requires io.netty.codec;
requires io.netty.resolver;
requires transitive java.logging;
requires static java.logging;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change triggers compilation failure, caused by the following warning: class java.util.logging.Level in module java.logging is not indirectly exported using requires transitive.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow interesting. I tried creating a reproducer locally but wasn't able to.

According to what I've seen online we can make it static and transitive, but I'm not 100% sure. In this case it's true, the logging class is exporting JUL to downstream users (using the logger in the public return types).

Reading through https://nipafx.dev/java-modules-implied-readability/ right now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still failing for the same reasons?

requires transitive org.reactivestreams;
requires static micrometer.core;
requires static org.graalvm.nativeimage.builder;
Expand Down
18 changes: 18 additions & 0 deletions driver/src/main/java/org/neo4j/driver/Logging.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.logging.Level;
import org.neo4j.driver.internal.logging.ConsoleLogging;
import org.neo4j.driver.internal.logging.JULogging;
import org.neo4j.driver.internal.logging.JavaPlatformLogging;
import org.neo4j.driver.internal.logging.Slf4jLogging;

/**
Expand All @@ -33,6 +34,9 @@
* <li>{@link #slf4j() SLF4J logging} - uses available SLF4J binding (Logback, Log4j, etc.) fails when no SLF4J implementation is available. Uses
* application's logging configuration from XML or other type of configuration file. This logging method is the preferred one and relies on the SLF4J
* implementation available in the classpath or modulepath.</li>
* <li>{@link #javaPlatformLogging() Java Logging API} - uses {@link java.lang.System.Logger} created via
* {@link java.lang.System#getLogger(String)}. This logging method is suitable when application
* uses the java system logger api for logging and provides the appropriate implementation (Logback, Log4j, etc.)</li>
* <li>{@link #javaUtilLogging(Level) Java Logging API (JUL)} - uses {@link java.util.logging.Logger} created via
* {@link java.util.logging.Logger#getLogger(String)}. Global java util logging configuration applies. This logging method is suitable when application
* uses JUL for logging and explicitly configures it.</li>
Expand Down Expand Up @@ -118,13 +122,27 @@ static Logging slf4j() {
return new Slf4jLogging();
}

/**
* Create logging implementation that uses {@link java.lang.System.Logger}.
*
* @return new logging implementation.
*/
static Logging javaPlatformLogging() {
return new JavaPlatformLogging();
}

/**
* Create logging implementation that uses {@link java.util.logging}.
*
* @param level the log level.
* @return new logging implementation.
* @throws IllegalStateException if java.logging is not available.
*/
static Logging javaUtilLogging(Level level) {
IllegalStateException unavailabilityError = JULogging.checkAvailability();
if (unavailabilityError != null) {
throw unavailabilityError;
}
return new JULogging(level);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,15 @@ public JULogging(Level loggingLevel) {
public Logger getLog(String name) {
return new JULogger(name, loggingLevel);
}

public static IllegalStateException checkAvailability() {
try {
Class.forName("java.util.logging.Logger");
return null;
} catch (Throwable error) {
return new IllegalStateException(
"java.util.logging is not available. Please add the java.logging module.",
error);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal.logging;

import org.neo4j.driver.Logger;

import java.lang.System.Logger.Level;
import java.util.Objects;

public class JavaPlatformLogger implements Logger {
private final java.lang.System.Logger delegate;

public JavaPlatformLogger(System.Logger delegate) {
this.delegate = Objects.requireNonNull(delegate);
}

@Override
public void error(String message, Throwable cause) {
if (delegate.isLoggable(Level.ERROR)) {
delegate.log(Level.ERROR, message, cause);
}
}

@Override
public void info(String format, Object... params) {
if (delegate.isLoggable(Level.INFO)) {
delegate.log(Level.INFO, String.format(format, params));
}
}

@Override
public void warn(String format, Object... params) {
if (delegate.isLoggable(Level.WARNING)) {
delegate.log(Level.WARNING, String.format(format, params));
}
}

@Override
public void warn(String message, Throwable cause) {
if (delegate.isLoggable(Level.WARNING)) {
delegate.log(Level.WARNING, message, cause);
}
}

@Override
public void debug(String format, Object... params) {
if (isDebugEnabled()) {
delegate.log(Level.DEBUG, String.format(format, params));
}
}

@Override
public void debug(String message, Throwable throwable) {
if (isDebugEnabled()) {
delegate.log(Level.DEBUG, message, throwable);
}
}

@Override
public void trace(String format, Object... params) {
if (isTraceEnabled()) {
delegate.log(Level.TRACE, String.format(format, params));
}
}

@Override
public boolean isTraceEnabled() {
return delegate.isLoggable(Level.TRACE);
}

@Override
public boolean isDebugEnabled() {
return delegate.isLoggable(Level.DEBUG);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal.logging;

import org.neo4j.driver.Logger;
import org.neo4j.driver.Logging;

import java.io.Serializable;

/**
* Internal implementation of the java platform logging module.
* <b>This class should not be used directly.</b> Please use {@link Logging#javaPlatformLogging()} factory method instead.
*
* @see Logging#javaPlatformLogging()
*/
public class JavaPlatformLogging implements Logging, Serializable {
private static final long serialVersionUID = -1145576859241657833L;

public JavaPlatformLogging() {
}

@Override
public Logger getLog(String name) {
return new JavaPlatformLogger(System.getLogger(name));
}
}
3 changes: 2 additions & 1 deletion driver/src/test/java/org/neo4j/driver/ConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.neo4j.driver.internal.logging.DevNullLogging;
import org.neo4j.driver.internal.logging.JULogging;
import org.neo4j.driver.internal.logging.Slf4jLogging;
import org.neo4j.driver.internal.logging.JavaPlatformLogging;
import org.neo4j.driver.net.ServerAddressResolver;
import org.neo4j.driver.testutil.TestUtil;

Expand Down Expand Up @@ -486,7 +487,7 @@ void shouldSerializeSerializableLogging() throws IOException, ClassNotFoundExcep
}

@ParameterizedTest
@ValueSource(classes = {DevNullLogging.class, JULogging.class, ConsoleLogging.class, Slf4jLogging.class})
@ValueSource(classes = {DevNullLogging.class, JULogging.class, ConsoleLogging.class, Slf4jLogging.class, JavaPlatformLogging.class})
void officialLoggingProvidersShouldBeSerializable(Class<? extends Logging> loggingClass) {
assertTrue(Serializable.class.isAssignableFrom(loggingClass));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal.logging;

import org.junit.jupiter.api.Test;
import org.neo4j.driver.Logger;

import java.util.logging.Level;

import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.junit.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertNull;

class JULoggingTest {
@Test
void shouldCreateLoggers() {
JULogging logging = new JULogging(Level.ALL);

Logger logger = logging.getLog("My Log");

assertThat(logger, instanceOf(JULogger.class));
}

@Test
void shouldCheckIfAvailable() {
assertNull(JULogging.checkAvailability());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* This file is part of Neo4j.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.driver.internal.logging;

import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.lang.System.Logger;
import java.lang.System.Logger.Level;

class JavaPlatformLoggerTest {
private final Logger logger = Mockito.mock(Logger.class);
private final JavaPlatformLogger javaPlatformLogger = new JavaPlatformLogger(logger);

@Test
void shouldLogErrorWithMessageAndThrowable() {
Mockito.when(logger.isLoggable(Level.ERROR)).thenReturn(true);
String message = "Hello";
IllegalArgumentException error = new IllegalArgumentException("World");

javaPlatformLogger.error(message, error);

Mockito.verify(logger).log(Level.ERROR, message, error);
}

@Test
void shouldLogInfoWithMessageAndParams() {
Mockito.when(logger.isLoggable(Level.INFO)).thenReturn(true);
String message = "One %s, two %s, three %s";
Object[] params = {"111", "222", "333"};

javaPlatformLogger.info(message, params);

Mockito.verify(logger).log(Level.INFO, "One 111, two 222, three 333");
}

@Test
void shouldLogWarnWithMessageAndParams() {
Mockito.when(logger.isLoggable(Level.WARNING)).thenReturn(true);
String message = "C for %s, d for %s";
Object[] params = {"cat", "dog"};

javaPlatformLogger.warn(message, params);

Mockito.verify(logger).log(Level.WARNING, "C for cat, d for dog");
}

@Test
void shouldLogWarnWithMessageAndThrowable() {
Mockito.when(logger.isLoggable(Level.WARNING)).thenReturn(true);
String message = "Hello";
RuntimeException error = new RuntimeException("World");

javaPlatformLogger.warn(message, error);

Mockito.verify(logger).log(Level.WARNING, message, error);
}

@Test
void shouldLogDebugWithMessageAndParams() {
Mockito.when(logger.isLoggable(Level.DEBUG)).thenReturn(true);
String message = "Hello%s%s!";
Object[] params = {" ", "World"};

javaPlatformLogger.debug(message, params);

Mockito.verify(logger).log(Level.DEBUG, "Hello World!");
}

@Test
void shouldLogTraceWithMessageAndParams() {
Mockito.when(logger.isLoggable(Level.TRACE)).thenReturn(true);
String message = "I'll be %s!";
Object[] params = {"back"};

javaPlatformLogger.trace(message, params);

Mockito.verify(logger).log(Level.TRACE, "I'll be back!");
}

@Test
void shouldCheckIfDebugIsEnabled() {
Mockito.when(logger.isLoggable(Level.DEBUG)).thenReturn(false);
assertFalse(javaPlatformLogger.isDebugEnabled());

Mockito.when(logger.isLoggable(Level.DEBUG)).thenReturn(true);
assertTrue(javaPlatformLogger.isDebugEnabled());
}

@Test
void shouldCheckIfTraceIsEnabled() {
Mockito.when(logger.isLoggable(Level.TRACE)).thenReturn(false);
assertFalse(javaPlatformLogger.isTraceEnabled());

Mockito.when(logger.isLoggable(Level.TRACE)).thenReturn(true);
assertTrue(javaPlatformLogger.isTraceEnabled());
}
}
Loading