From aac66f2acde15e78cf0678db2c68ad31e41102e4 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Thu, 5 Dec 2024 16:25:22 -0500 Subject: [PATCH] remove JUL wrapper for now. refactor getLogger(). Add tests for LoggingUtils. --- gax-java/gax/pom.xml | 14 + .../google/api/gax/logging/LoggingUtils.java | 414 ++---------------- .../internal/SystemEnvironmentProvider.java | 50 +++ .../api/gax/logging/LoggingUtilsTest.java | 130 ++++++ 4 files changed, 224 insertions(+), 384 deletions(-) create mode 100644 gax-java/gax/src/main/java/com/google/api/gax/rpc/internal/SystemEnvironmentProvider.java create mode 100644 gax-java/gax/src/test/java/com/google/api/gax/logging/LoggingUtilsTest.java diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index 5ada305a25..2f86586d82 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -75,6 +75,20 @@ slf4j-api 2.0.16 + + + ch.qos.logback + logback-classic + + 1.3.14 + test + + + ch.qos.logback + logback-core + 1.3.14 + test + diff --git a/gax-java/gax/src/main/java/com/google/api/gax/logging/LoggingUtils.java b/gax-java/gax/src/main/java/com/google/api/gax/logging/LoggingUtils.java index 436a9e35c8..6eba4a3430 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/logging/LoggingUtils.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/logging/LoggingUtils.java @@ -31,86 +31,47 @@ package com.google.api.gax.logging; import com.google.api.core.InternalApi; -import com.google.gson.JsonObject; +import com.google.api.gax.rpc.internal.EnvironmentProvider; +import com.google.api.gax.rpc.internal.SystemEnvironmentProvider; import java.util.Map; -import java.util.logging.Level; -import java.util.logging.LogRecord; import org.slf4j.ILoggerFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; -import org.slf4j.Marker; -import org.slf4j.helpers.FormattingTuple; -import org.slf4j.helpers.MessageFormatter; -import org.slf4j.helpers.NOPLogger; @InternalApi public class LoggingUtils { - private static final java.util.logging.Logger LOGGER = - java.util.logging.Logger.getLogger(LoggingUtils.class.getName()); + private static EnvironmentProvider environmentProvider = SystemEnvironmentProvider.getInstance(); + private static final Logger NO_OP_LOGGER = org.slf4j.helpers.NOPLogger.NOP_LOGGER; + private static boolean loggingEnabled = isLoggingEnabled(); + static final String GOOGLE_SDK_JAVA_LOGGING = "GOOGLE_SDK_JAVA_LOGGING"; + // expose this setter for testing purposes + static void setEnvironmentProvider(EnvironmentProvider provider) { + environmentProvider = provider; + // Recalculate LOGGING_ENABLED after setting the new provider + loggingEnabled = isLoggingEnabled(); + } private LoggingUtils() {} public static Logger getLogger(Class clazz) { - if (!isLoggingEnabled()) { - // use SLF4j's NOP logger regardless of bindings - return LoggerFactory.getLogger(NOPLogger.class); - } - - ILoggerFactory loggerFactory = LoggerFactory.getILoggerFactory(); - if (loggerFactory != null && !(loggerFactory instanceof org.slf4j.helpers.NOPLoggerFactory)) { - // Use SLF4j binding when present - return LoggerFactory.getLogger(clazz); - } - // No SLF4j binding found, use JUL as fallback - Logger logger = new JulWrapperLogger(clazz.getName()); - logger.info("No SLF4J providers were found, fall back to JUL."); - return logger; - } - - public static boolean isLoggingEnabled() { - String enableLogging = System.getenv("GOOGLE_SDK_JAVA_LOGGING"); - LOGGER.info("GOOGLE_SDK_JAVA_LOGGING=" + enableLogging); // log for debug now, remove it. - return "true".equalsIgnoreCase(enableLogging); - } - - public static JsonObject mergeJsonObject(JsonObject jsonObject1, JsonObject jsonObject2) { - JsonObject mergedObject = jsonObject1.deepCopy(); - jsonObject2.entrySet().forEach(entry -> mergedObject.add(entry.getKey(), entry.getValue())); - return mergedObject; + return getLogger(clazz, new DefaultLoggerFactoryProvider()); } - public static Level mapToJulLevel(org.slf4j.event.Level slf4jLevel) { - switch (slf4jLevel) { - case ERROR: - return Level.SEVERE; - case WARN: - return Level.WARNING; - case INFO: - return Level.INFO; - case DEBUG: - return Level.FINE; - case TRACE: - return Level.FINEST; - default: - return Level.INFO; + // constructor with LoggerFactoryProvider to make testing easier + static Logger getLogger(Class clazz, LoggerFactoryProvider factoryProvider) { + if (loggingEnabled) { + return factoryProvider.getLoggerFactory().getLogger(clazz.getName()); + } else { + // use SLF4j's NOP logger regardless of bindings + return NO_OP_LOGGER; } } public static void logWithMDC( Logger logger, org.slf4j.event.Level level, Map contextMap, String message) { - - if (logger instanceof JulWrapperLogger) { - // Simulate MDC behavior for JUL - LogRecord record = new LogRecord(mapToJulLevel(level), message); - // Add context map to the LogRecord - record.setParameters(new Object[] {contextMap}); - ((JulWrapperLogger) logger).getJulLogger().log(record); - return; - } contextMap.forEach(MDC::put); - switch (level) { case TRACE: logger.trace(message); @@ -131,337 +92,22 @@ public static void logWithMDC( logger.info(message); // Default to INFO level } - MDC.clear(); } - // JulWrapperLogger implementation - static class JulWrapperLogger implements Logger { - - private final java.util.logging.Logger julLogger; - - public JulWrapperLogger(String name) { - this.julLogger = java.util.logging.Logger.getLogger(name); - } - - public java.util.logging.Logger getJulLogger() { - return julLogger; - } - - @Override - public String getName() { - return julLogger.getName(); - } - - @Override - public boolean isTraceEnabled() { - return julLogger.isLoggable(java.util.logging.Level.FINEST); - } - - @Override - public void trace(String msg) { - julLogger.log(java.util.logging.Level.FINEST, msg); - } - - @Override - public void trace(String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void trace(String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void trace(String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void trace(String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isTraceEnabled(Marker marker) { - return false; - } - - @Override - public void trace(Marker marker, String s) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void trace(Marker marker, String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void trace(Marker marker, String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void trace(Marker marker, String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void trace(Marker marker, String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isDebugEnabled() { - return julLogger.isLoggable(Level.FINE); - } - - @Override - public void debug(String msg) { - - if (isDebugEnabled()) { - julLogger.log(java.util.logging.Level.FINE, msg); - } - } - - @Override - public void debug(String format, Object arg) { - if (isDebugEnabled()) { - FormattingTuple ft = MessageFormatter.format(format, arg); - julLogger.log(Level.FINE, ft.getMessage()); - } - } - - @Override - public void debug(String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void debug(String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void debug(String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isDebugEnabled(Marker marker) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void debug(Marker marker, String s) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void debug(Marker marker, String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void debug(Marker marker, String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void debug(Marker marker, String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void debug(Marker marker, String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isInfoEnabled() { - return julLogger.isLoggable(Level.INFO); - } - - @Override - public void info(String msg) { - if (isInfoEnabled()) { - julLogger.log(java.util.logging.Level.INFO, msg); - } - } - - @Override - public void info(String format, Object arg) { - if (isInfoEnabled()) { - FormattingTuple ft = MessageFormatter.format(format, arg); - julLogger.log(java.util.logging.Level.INFO, ft.getMessage()); - } - } - - @Override - public void info(String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void info(String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void info(String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isInfoEnabled(Marker marker) { - return true; - } - - @Override - public void info(Marker marker, String s) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void info(Marker marker, String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void info(Marker marker, String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void info(Marker marker, String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void info(Marker marker, String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isWarnEnabled() { - return true; - } - - @Override - public void warn(String msg) { - julLogger.log(Level.WARNING, msg); - } - - @Override - public void warn(String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void warn(String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void warn(String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void warn(String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isWarnEnabled(Marker marker) { - return false; - } - - @Override - public void warn(Marker marker, String s) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void warn(Marker marker, String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void warn(Marker marker, String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void warn(Marker marker, String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void warn(Marker marker, String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isErrorEnabled() { - return false; - } - - @Override - public void error(String s) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void error(String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void error(String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void error(String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void error(String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public boolean isErrorEnabled(Marker marker) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void error(Marker marker, String s) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void error(Marker marker, String s, Object o) { - throw new UnsupportedOperationException("This method is not supported."); - } - - @Override - public void error(Marker marker, String s, Object o, Object o1) { - throw new UnsupportedOperationException("This method is not supported."); - } + static boolean isLoggingEnabled() { + String enableLogging = environmentProvider.getenv(GOOGLE_SDK_JAVA_LOGGING); + return "true".equalsIgnoreCase(enableLogging); + } - @Override - public void error(Marker marker, String s, Object... objects) { - throw new UnsupportedOperationException("This method is not supported."); - } + interface LoggerFactoryProvider { + ILoggerFactory getLoggerFactory(); + } + static class DefaultLoggerFactoryProvider implements LoggerFactoryProvider { @Override - public void error(Marker marker, String s, Throwable throwable) { - throw new UnsupportedOperationException("This method is not supported."); + public ILoggerFactory getLoggerFactory() { + return LoggerFactory.getILoggerFactory(); } } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/internal/SystemEnvironmentProvider.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/internal/SystemEnvironmentProvider.java new file mode 100644 index 0000000000..29d45ba3c5 --- /dev/null +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/internal/SystemEnvironmentProvider.java @@ -0,0 +1,50 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.api.gax.rpc.internal; + +import com.google.api.core.InternalApi; + +/** Represents the default system environment provider. */ +@InternalApi +public class SystemEnvironmentProvider implements EnvironmentProvider { + static final SystemEnvironmentProvider INSTANCE = new SystemEnvironmentProvider(); + + private SystemEnvironmentProvider() {} + + @Override + public String getenv(String name) { + return System.getenv(name); + } + + public static SystemEnvironmentProvider getInstance() { + return INSTANCE; + } +} diff --git a/gax-java/gax/src/test/java/com/google/api/gax/logging/LoggingUtilsTest.java b/gax-java/gax/src/test/java/com/google/api/gax/logging/LoggingUtilsTest.java new file mode 100644 index 0000000000..d07a00439b --- /dev/null +++ b/gax-java/gax/src/test/java/com/google/api/gax/logging/LoggingUtilsTest.java @@ -0,0 +1,130 @@ +package com.google.api.gax.logging; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.encoder.PatternLayoutEncoder; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.ConsoleAppender; +import com.google.api.gax.logging.LoggingUtils.LoggerFactoryProvider; +import com.google.api.gax.rpc.internal.EnvironmentProvider; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.mockito.Mockito; +import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.helpers.NOPLogger; + +public class LoggingUtilsTest { + + private EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); + + @BeforeEach + public void setup() { + EnvironmentProvider envProvider = Mockito.mock(EnvironmentProvider.class); + + // need to setup a ConsoleAppender and attach to root logger because TestAppender + // does not correctly capture MDC info + LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); + + PatternLayoutEncoder patternLayoutEncoder = new PatternLayoutEncoder(); + patternLayoutEncoder.setPattern("%-4relative [%thread] %-5level %logger{35} - %msg%n"); + patternLayoutEncoder.setContext(loggerContext); + + patternLayoutEncoder.start(); + + ConsoleAppender consoleAppender = new ConsoleAppender<>(); + consoleAppender.setEncoder(patternLayoutEncoder); + + consoleAppender.setContext(loggerContext); + consoleAppender.setName("CONSOLE"); + + consoleAppender.start(); + + ch.qos.logback.classic.Logger rootLogger = loggerContext.getLogger(Logger.ROOT_LOGGER_NAME); + rootLogger.addAppender(consoleAppender); + } + + @AfterEach + public void tearDown() { + LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); + loggerContext.getLogger(Logger.ROOT_LOGGER_NAME).detachAppender("CONSOLE"); + } + + @org.junit.Test + public void testGetLogger_loggingEnabled_slf4jBindingPresent() { + Mockito.when(envProvider.getenv(LoggingUtils.GOOGLE_SDK_JAVA_LOGGING)).thenReturn("true"); + LoggingUtils.setEnvironmentProvider(envProvider); + Logger logger = LoggingUtils.getLogger(LoggingUtilsTest.class); + assertTrue(logger instanceof org.slf4j.Logger); + assertNotEquals(logger.getClass(), NOPLogger.class); + } + + @org.junit.Test + public void testGetLogger_loggingDisabled() { + Mockito.when(envProvider.getenv(LoggingUtils.GOOGLE_SDK_JAVA_LOGGING)).thenReturn("false"); + LoggingUtils.setEnvironmentProvider(envProvider); + + Logger logger = LoggingUtils.getLogger(LoggingUtilsTest.class); + assertEquals(NOPLogger.class, logger.getClass()); + } + + @org.junit.Test + public void testGetLogger_loggingEnabled_noBinding() { + Mockito.when(envProvider.getenv(LoggingUtils.GOOGLE_SDK_JAVA_LOGGING)).thenReturn("true"); + LoggingUtils.setEnvironmentProvider(envProvider); + // Create a mock LoggerFactoryProvider + LoggerFactoryProvider mockLoggerFactoryProvider = mock(LoggerFactoryProvider.class); + ILoggerFactory mockLoggerFactory = mock(ILoggerFactory.class); + when(mockLoggerFactoryProvider.getLoggerFactory()).thenReturn(mockLoggerFactory); + when(mockLoggerFactory.getLogger(anyString())) + .thenReturn(org.slf4j.helpers.NOPLogger.NOP_LOGGER); + + // Use the mock LoggerFactoryProvider in getLogger() + Logger logger = LoggingUtils.getLogger(LoggingUtilsTest.class, mockLoggerFactoryProvider); + + // Assert that the returned logger is a NOPLogger + assertTrue(logger instanceof org.slf4j.helpers.NOPLogger); + } + + @org.junit.Test + public void testIsLoggingEnabled_true() { + Mockito.when(envProvider.getenv(LoggingUtils.GOOGLE_SDK_JAVA_LOGGING)).thenReturn("true"); + LoggingUtils.setEnvironmentProvider(envProvider); + assertTrue(LoggingUtils.isLoggingEnabled()); + Mockito.when(envProvider.getenv(LoggingUtils.GOOGLE_SDK_JAVA_LOGGING)).thenReturn("TRUE"); + LoggingUtils.setEnvironmentProvider(envProvider); + assertTrue(LoggingUtils.isLoggingEnabled()); + Mockito.when(envProvider.getenv(LoggingUtils.GOOGLE_SDK_JAVA_LOGGING)).thenReturn("True"); + LoggingUtils.setEnvironmentProvider(envProvider); + assertTrue(LoggingUtils.isLoggingEnabled()); + } + + @org.junit.Test + public void testIsLoggingEnabled_defaultToFalse() { + LoggingUtils.setEnvironmentProvider(envProvider); + assertFalse(LoggingUtils.isLoggingEnabled()); + } + + // @Test + // public void testLogWithMDC_slf4jLogger() { + // TestAppender.clearEvents(); + // Map contextMap = new HashMap<>(); + // contextMap.put("key", "value"); + // LoggingUtils.logWithMDC(LOGGER, org.slf4j.event.Level.DEBUG, contextMap, "test message"); + // + // assertEquals(1, TestAppender.events.size()); + // assertEquals("test message", TestAppender.events.get(0).getFormattedMessage()); + // + // // Verify MDC content + // ILoggingEvent event = TestAppender.events.get(0); + // assertEquals("value", event.getMDCPropertyMap().get("key")); + // } +}