use jul logger by default (#66)

roc-streaming · Apr 8, 2023 · 98135b9 · 98135b9
1 parent c389c3e
commit 98135b9
Show file tree

Hide file tree

Showing 8 changed files with 127 additions and 33 deletions.
diff --git a/roc_jni/src/main/export/org_rocstreaming_roctoolkit_Logger.h b/roc_jni/src/main/export/org_rocstreaming_roctoolkit_Logger.h
diff --git a/roc_jni/src/main/impl/logger.c b/roc_jni/src/main/impl/logger.c
@@ -172,7 +172,7 @@ JNIEXPORT void JNICALL Java_org_rocstreaming_roctoolkit_Logger_setLevel(
     }
 }
 
-JNIEXPORT void JNICALL Java_org_rocstreaming_roctoolkit_Logger_setCallback(
+JNIEXPORT void JNICALL Java_org_rocstreaming_roctoolkit_Logger_setCallbackNative(
     JNIEnv* env, jclass clazz, jobject jhandler) {
     jclass handlerClass = NULL;
     jmethodID handlerMethod = NULL;

diff --git a/src/main/java/org/rocstreaming/roctoolkit/Logger.java b/src/main/java/org/rocstreaming/roctoolkit/Logger.java
@@ -1,8 +1,38 @@
 package org.rocstreaming.roctoolkit;
 
+import java.util.logging.Level;
+
 public class Logger {
+
+    private static final java.util.logging.Logger LOGGER = java.util.logging.Logger.getLogger(Logger.class.getName());
+
+    private static final LogHandler DEFAULT_HANDLER = (level, component, message) -> {
+        Level julLevel = mapLogLevel(level);
+        if (LOGGER.isLoggable(julLevel)) {
+            LOGGER.logp(julLevel, component, "", message);
+        }
+    };
+
     static {
         RocLibrary.loadLibrary();
+        setCallbackNative(DEFAULT_HANDLER);
+        // Jvm could be terminated before roclib, so we need to clear callback to avoid crash
+        Runtime.getRuntime().addShutdownHook(new Thread(() -> setCallbackNative(null)));
+    }
+
+    private static java.util.logging.Level mapLogLevel(LogLevel level) {
+        switch (level) {
+            case NONE:
+                return java.util.logging.Level.OFF;
+            case INFO:
+                return java.util.logging.Level.INFO;
+            case DEBUG:
+                return java.util.logging.Level.FINE;
+            case TRACE:
+                return java.util.logging.Level.FINER;
+            default:
+                return java.util.logging.Level.SEVERE;
+        }
     }
 
     /**
@@ -19,7 +49,7 @@ public class Logger {
      * Set log handler.
      * <p>
      * If <code>handler</code> is not null, messages are passed to the handler. Otherwise,
-     * messages are printed to stderr. By default the log handler is set to null.
+     * messages are printed by jul logger.
      * <p>
      * It's guaranteed that the previously set handler, if any, will not be used after this
      * function returns.
@@ -28,7 +58,22 @@ public class Logger {
      *
      * @param handler the log handler to set
      */
-    public native static void setCallback(LogHandler handler);
+    public static void setCallback(LogHandler handler) {
+        if (handler == null) {
+            setCallbackNative(DEFAULT_HANDLER);
+        } else {
+            LogHandler wrapper = (level, component, message) -> {
+                try {
+                    handler.log(level, component, message);
+                } catch (Throwable e) {
+                    LOGGER.log(Level.SEVERE, "Logger failed to log message", e);
+                }
+            };
+            setCallbackNative(wrapper);
+        }
+    }
+
+    private native static void setCallbackNative(LogHandler handler);
 
     private Logger() {
     }

diff --git a/src/test/java/org/rocstreaming/roctoolkit/ContextTest.java b/src/test/java/org/rocstreaming/roctoolkit/ContextTest.java
@@ -10,7 +10,7 @@ public class ContextTest {
 
     @BeforeAll
     public static void beforeAll() {
-        Logger.setLevel(LogLevel.ERROR);
+        Logger.setLevel(LogLevel.INFO);
     }
 
     @Test

diff --git a/src/test/java/org/rocstreaming/roctoolkit/LoggerTest.java b/src/test/java/org/rocstreaming/roctoolkit/LoggerTest.java
@@ -1,46 +1,79 @@
 package org.rocstreaming.roctoolkit;
 
 import org.awaitility.Duration;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.Collections;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.stream.Stream;
 
 import static org.awaitility.Awaitility.await;
 import static org.junit.jupiter.api.Assertions.*;
 
 public class LoggerTest {
 
-    @Test
-    public void TestValidLoggerSetLevel() {
+    @BeforeEach
+    public void beforeEach() {
+        Logger.setLevel(LogLevel.INFO);
+    }
+
+    private static Stream<Arguments> TestValidLoggerSetLevelProvider() {
+        return Stream.of(
+                Arguments.of(LogLevel.NONE, false, false),
+                Arguments.of(LogLevel.ERROR, true, false),
+                Arguments.of(LogLevel.INFO, true, true),
+                Arguments.of(LogLevel.DEBUG, true, true),
+                Arguments.of(LogLevel.TRACE, true, true)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("TestValidLoggerSetLevelProvider")
+    public void TestValidLoggerSetLevel(LogLevel level, boolean expectError, boolean expectInfo) {
+        Map<LogLevel, Integer> msgCount = new ConcurrentHashMap<>();
+        LogHandler handler = (lvl, component, message) -> msgCount.compute(lvl, (k, v) -> v == null ? 1 : v + 1);
+        Logger.setCallback(handler);
+
         assertDoesNotThrow(() -> {
-            Logger.setLevel(LogLevel.TRACE);
-            Logger.setLevel(LogLevel.DEBUG);
-            Logger.setLevel(LogLevel.INFO);
-            Logger.setLevel(LogLevel.ERROR);
-            Logger.setLevel(LogLevel.NONE);
+            Logger.setLevel(level);
+            try {
+                // trigger error logs
+                new Endpoint("invalid");
+            } catch (Exception ignored) {
+            }
+            // trigger info logs
+            //noinspection EmptyTryBlock
+            try (Context ignored = new Context()) {
+            }
         });
+        await().atMost(Duration.FIVE_MINUTES)
+                .untilAsserted(() -> {
+                    assertEquals(expectError, msgCount.containsKey(LogLevel.ERROR));
+                    assertEquals(expectInfo, msgCount.containsKey(LogLevel.INFO));
+                });
+        Logger.setCallback(null);
     }
 
     @Test
     public void TestInvalidLoggerSetLevel() {
-        assertThrows(IllegalArgumentException.class, () -> {
-            Logger.setLevel(null);
-        });
+        assertThrows(IllegalArgumentException.class, () -> Logger.setLevel(null));
     }
 
     @Test
     public void TestLoggerSetNullCallback() {
-        assertDoesNotThrow(() -> {
-            Logger.setCallback(null);
-        });
+        assertDoesNotThrow(() -> Logger.setCallback(null));
     }
 
     @Test
     public void TestValidLoggerSetCallback() throws Exception {
         Set<String> logs = Collections.newSetFromMap(new ConcurrentHashMap<>());
-        Logger.setLevel(LogLevel.INFO);
         Logger.setCallback((level, component, message) ->
                 logs.add(String.format("[level=\"%s\", component=\"%s\"]: %s", level, component, message)));
 
@@ -52,16 +85,32 @@ public void TestValidLoggerSetCallback() throws Exception {
         String logClose = "[level=\"INFO\", component=\"libroc\"]: roc_context_close";
         await().atMost(Duration.FIVE_MINUTES)
                 .until(() -> {
-                        boolean hasLogOpen = false, hasLogClose = false;
-                        for (String log : logs) {
-                            if (log.startsWith(logOpen)) {
-                                hasLogOpen = true;
-                            }
-                            if (log.startsWith(logClose)) {
-                                hasLogClose = true;
-                            }
+                    boolean hasLogOpen = false, hasLogClose = false;
+                    for (String log : logs) {
+                        if (log.startsWith(logOpen)) {
+                            hasLogOpen = true;
                         }
-                        return hasLogOpen && hasLogClose;
-                    });
+                        if (log.startsWith(logClose)) {
+                            hasLogClose = true;
+                        }
+                    }
+                    return hasLogOpen && hasLogClose;
+                });
+    }
+
+    @Test
+    public void TestInvalidLoggerNotThrows() throws InterruptedException {
+        CountDownLatch latch = new CountDownLatch(1);
+        assertDoesNotThrow(() -> {
+            Logger.setCallback((level, component, message) -> {
+                latch.countDown();
+                throw new RuntimeException("Fails to log");
+            });
+            //noinspection EmptyTryBlock
+            try (Context ignored = new Context()) {
+            }
+        });
+        latch.await();
+        Logger.setCallback(null);
     }
 }
diff --git a/src/test/java/org/rocstreaming/roctoolkit/ReceiverTest.java b/src/test/java/org/rocstreaming/roctoolkit/ReceiverTest.java
@@ -22,7 +22,7 @@ public class ReceiverTest {
 
     @BeforeAll
     public static void beforeAll() {
-        Logger.setLevel(LogLevel.ERROR);
+        Logger.setLevel(LogLevel.INFO);
     }
 
     @BeforeEach

diff --git a/src/test/java/org/rocstreaming/roctoolkit/SenderReceiverTest.java b/src/test/java/org/rocstreaming/roctoolkit/SenderReceiverTest.java
@@ -34,7 +34,7 @@ public SenderReceiverTest() {
 
     @BeforeAll
     public static void beforeAll() {
-        Logger.setLevel(LogLevel.ERROR);
+        Logger.setLevel(LogLevel.INFO);
     }
 
     @Test

diff --git a/src/test/java/org/rocstreaming/roctoolkit/SenderTest.java b/src/test/java/org/rocstreaming/roctoolkit/SenderTest.java
@@ -38,7 +38,7 @@ private void gensine(float[] samples) {
 
     @BeforeAll
     public static void beforeAll() {
-        Logger.setLevel(LogLevel.ERROR);
+        Logger.setLevel(LogLevel.INFO);
     }
 
     @BeforeEach