diff --git a/api/pom.xml b/api/pom.xml index 24d95e8..46bbdcb 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -59,9 +59,9 @@ 1.4.0 - log4j - log4j - 1.2.17 + org.slf4j + slf4j-api + 1.7.32 @@ -69,10 +69,28 @@ httpclient 4.5.13 + + org.junit.jupiter + junit-jupiter-api + 5.8.1 + test + + + org.junit.jupiter + junit-jupiter-engine + 5.8.1 + test + + + org.apache.logging.log4j + log4j-slf4j-impl + 2.14.1 + test + - 1.7 - 1.7 + 1.8 + 1.8 UTF-8 7.1.4.0 diff --git a/api/src/main/java/com/gpudb/GPUdbBase.java b/api/src/main/java/com/gpudb/GPUdbBase.java index e7c05f8..dbd18ca 100644 --- a/api/src/main/java/com/gpudb/GPUdbBase.java +++ b/api/src/main/java/com/gpudb/GPUdbBase.java @@ -50,7 +50,6 @@ import org.apache.http.entity.ByteArrayEntity; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.conn.socket.ConnectionSocketFactory; import org.apache.http.conn.socket.PlainConnectionSocketFactory; @@ -62,15 +61,10 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.ssl.SSLContextBuilder; import org.apache.http.util.EntityUtils; -import org.apache.log4j.Level; -import org.apache.log4j.Logger; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; import org.xerial.snappy.Snappy; - - /** * Base class for the GPUdb API that provides general functionality not specific * to any particular GPUdb request. This class is never instantiated directly; @@ -161,7 +155,6 @@ public static final class Options { private long intraClusterFailoverTimeout = DEFAULT_INTRA_CLUSTER_FAILOVER_TIMEOUT_MS; private int maxTotalConnections = DEFAULT_MAX_TOTAL_CONNECTIONS; private int maxConnectionsPerHost = DEFAULT_MAX_CONNECTIONS_PER_HOST; - private Level loggingLevel = GPUdbLogger.DEFAULT_LOGGING_LEVEL; /** * No-argument constructor needed for the copy constructor. @@ -194,7 +187,6 @@ public Options( Options other ) { this.intraClusterFailoverTimeout = other.intraClusterFailoverTimeout; this.maxTotalConnections = other.maxTotalConnections; this.maxConnectionsPerHost = other.maxConnectionsPerHost; - this.loggingLevel = other.loggingLevel; this.initialConnectionAttemptTimeout = other.initialConnectionAttemptTimeout; this.connectionInactivityValidationTimeout = other.connectionInactivityValidationTimeout; @@ -317,7 +309,7 @@ public boolean getDisableAutoDiscovery() { * @return the inter-cluster failover order * * @see #setHAFailoverOrder(HAFailoverOrder) - * @see GPUdbBase#HAFailoverOrder + * @see GPUdbBase#haFailoverOrder */ public HAFailoverOrder getHAFailoverOrder() { return this.haFailoverOrder; @@ -500,26 +492,6 @@ public long getIntraClusterFailoverTimeout() { return this.intraClusterFailoverTimeout; } - - /** - * Gets the logging level that will be used by the API. By default, - * logging is turned off; but if logging properties are provided - * by the user, those properties will be respected. If the user sets - * the logging level explicitly (and it is not the default log level), - * then the programmatically set level will be used instead of the - * one set in the properties file. - * - * @return the logging level - * - * @see #setLoggingLevel(String) - * @see #setLoggingLevel(Level) - */ - public Level getLoggingLevel() { - return this.loggingLevel; - } - - - /** * Sets the URL of the primary cluster to use amongst the HA clusters. * This cluster will always be used first. It can be part of the URLs @@ -743,7 +715,7 @@ public Options setDisableAutoDiscovery(boolean value) { * @return the current {@link Options} instance * * @see #getDisableAutoDiscovery() - * @see GPUdbBase#HAFailoverOrder + * @see GPUdbBase#haFailoverOrder */ public Options setHAFailoverOrder(HAFailoverOrder value) { this.haFailoverOrder = value; @@ -1043,83 +1015,6 @@ public Options setIntraClusterFailoverTimeout(long value) { this.intraClusterFailoverTimeout = value; return this; } - - - /** - * Sets the logging level that will be used by the API. - * Supported values: - * - * - * If `OFF` is given, and if logging properties are provided by the user - * (via log4j.properties or other files), those properties will be - * respected. If the user set logging level is not the default log - * level, i.e. `OFF`), then the programmatically set level will be used - * instead of the one set in the properties file. - * - * @return the current {@link Options} instance - * - * @see #getLoggingLevel() - */ - public Options setLoggingLevel(String value) throws GPUdbException { - - // Parse the level - Level level = Level.toLevel( value ); - - // Ensure a valid level was given - if ( (level == Level.DEBUG) - && !value.equalsIgnoreCase( "DEBUG" ) ) { - // The user didn't give debug, but Level returned it - // (which it does when it can't parse the given value) - String errorMsg = ( "Must provide a valid logging level " - + "(please see documentation); given: '" - + value + "'" ); - throw new GPUdbException( errorMsg ); - } - - this.loggingLevel = level; - return this; - } - - /** - * Sets the logging level that will be used by the API. - * Supported values: - * - * - * If `OFF` is given, and if logging properties are provided by the user - * (via log4j.properties or other files), those properties will be - * respected. If the user set logging level is not the default log - * level, i.e. `OFF`), then the programmatically set level will be used - * instead of the one set in the properties file. - * - * @return the current {@link Options} instance - * - * @see #getLoggingLevel() - */ - public Options setLoggingLevel(Level value) { - this.loggingLevel = value; - return this; - } - } // end class Options @@ -2085,10 +1980,6 @@ public String toString() { protected GPUdbBase(String url, Options options) throws GPUdbException { urlLock = new Object(); - // Initialize the logger before anything else. This MUST be donce - // before any logging happens! - GPUdbLogger.initializeLogger( options.getLoggingLevel() ); - if ( url == null ) { String errorMsg = ( "Must provide a non-null and non-empty " + "string for the URL; given null" ); @@ -2118,10 +2009,6 @@ protected GPUdbBase(String url, Options options) throws GPUdbException { protected GPUdbBase(URL url, Options options) throws GPUdbException { urlLock = new Object(); - // Initialize the logger before anything else. This MUST be donce - // before any logging happens! - GPUdbLogger.initializeLogger( options.getLoggingLevel() ); - if ( url == null ) { String errorMsg = "Must provide at least one URL; gave none!"; GPUdbLogger.error( errorMsg ); @@ -2136,10 +2023,6 @@ protected GPUdbBase(URL url, Options options) throws GPUdbException { protected GPUdbBase(List urls, Options options) throws GPUdbException { urlLock = new Object(); - // Initialize the logger before anything else. This MUST be donce - // before any logging happens! - GPUdbLogger.initializeLogger( options.getLoggingLevel() ); - if ( urls.isEmpty() ) { String errorMsg = "Must provide at least one URL; gave none!"; GPUdbLogger.error( errorMsg ); diff --git a/api/src/main/java/com/gpudb/GPUdbLogger.java b/api/src/main/java/com/gpudb/GPUdbLogger.java index e502df0..245b9df 100644 --- a/api/src/main/java/com/gpudb/GPUdbLogger.java +++ b/api/src/main/java/com/gpudb/GPUdbLogger.java @@ -1,161 +1,54 @@ package com.gpudb; -import org.apache.log4j.Appender; -import org.apache.log4j.BasicConfigurator; -import org.apache.log4j.ConsoleAppender; -import org.apache.log4j.Level; -import org.apache.log4j.Logger; -import org.apache.log4j.PatternLayout; -import java.util.Enumeration; - - +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class GPUdbLogger { - // The default logging level--off - protected static final Level DEFAULT_LOGGING_LEVEL = Level.OFF; - // The name of the logger for this API - protected static final String API_LOGGER_NAME = "Kinetica Java API"; + protected static final String API_LOGGER_NAME = "com.gpudb"; - // The loggers used for dependent libraries that might have obnxious + // The loggers used for dependent libraries that might have obnoxious // default log levels protected static final String DEP_LIB_APACHE_CLIENT_LOGGER = "org.apache.http"; - // Actual logger used for the API - private static Logger LOG = Logger.getLogger( API_LOGGER_NAME ); - - - /** - * Initializes the logger with the given logging level. It is - * **crucial** to call this method before using the logger. This method - * serves the following purpose: - * * If no log4j.properties or any other properties file is found - * in the class path of the application that uses this API, log4j - * emits warnings about not finding any appender. This method - * prevents that from happening. - * * Older versions of the API did not have logging, so suddenly having - * logging might throw off end user applications. So, by default, - * we turn off logging (controlled by the static final member above). - * * If no logging properties were provided by the end-user, then suppress - * the obnoxious debug logging that is turned on by default by the Apache - * HTTPClient library. - * * If the given log level is different from the default, set it explicitly. - */ - public static void initializeLogger( Level loggingLevel ) { - Logger rootLogger = Logger.getRootLogger(); - - // Check for appenders--if none found, then that means the - // end-user application has not supplied any logging properties - // (which indicates that they probably don't expect any logging). - Enumeration appenders = rootLogger.getAllAppenders(); - boolean isLoggerConfiguredByUser = appenders.hasMoreElements(); - - if ( !isLoggerConfiguredByUser ) { - // No appender is found; suppress log4j warnings by explicitly - // getting the logger for the API - LOG = Logger.getLogger( API_LOGGER_NAME ); - - // Configuring log4j helps towards suppressing the annoying log4j - // warnings - PatternLayout layout = new PatternLayout( "%d{yyyy-MM-dd HH:mm:ss} %-5p %m%n" ); - ConsoleAppender consoleAppender = new ConsoleAppender(); - consoleAppender.setLayout( layout ); - consoleAppender.activateOptions(); - BasicConfigurator.configure( consoleAppender ); - - // Set the API's log level - LOG.setLevel( loggingLevel ); - - // Set the Apache HTTPClient log leve as well - Logger.getLogger( "org.apache.http" ).setLevel( loggingLevel ); - } else { - // If the log level is different from the default, set it explicitly - if ( !loggingLevel.equals( DEFAULT_LOGGING_LEVEL ) ) { - LOG.setLevel( loggingLevel ); - LOG.warn( "Log properties set, but the log level is also " - + "programmatically set by the user ('" - + loggingLevel.toString() - + "'); using that one and ignoring the one in " - + "the properties."); - } - - // Some logging properties found; check if the libraries that this - // API uses have log levels defined in the properties. If not, we - // will turn them off (at least the obnoxious ones). For that, first - // look for such loggers in the user given properties. - boolean isApacheHttpClientLoggerFound = false; - Enumeration loggers = rootLogger.getHierarchy().getCurrentLoggers(); - while ( loggers.hasMoreElements() ) { - if ( loggers.nextElement() - .getName() - .equalsIgnoreCase( DEP_LIB_APACHE_CLIENT_LOGGER ) ) { - isApacheHttpClientLoggerFound = true; - } - } - - // Mute the obnoxious logs if not set by the user - if ( !isApacheHttpClientLoggerFound ) { - Logger.getLogger( DEP_LIB_APACHE_CLIENT_LOGGER ).setLevel( Level.OFF ); - } - } - } // end initializeLogger + private static final Logger LOG = LoggerFactory.getLogger(API_LOGGER_NAME); + public static boolean isInfoEnabled() { + return LOG.isInfoEnabled(); + } public static void info(String message) { - LOG.info( message ); + LOG.info(message); } public static void error(String message) { - LOG.error( message ); + LOG.error(message); } public static void warn(String message) { - LOG.warn( message ); + LOG.warn(message); } - public static void fatal(String message) { - LOG.fatal( message ); - } - - public static void debug(String message) { - LOG.debug( message ); + LOG.debug(message); } - public static void trace(String message) { - LOG.trace( message ); + LOG.trace(message); } - /** * Print extra information with the debug message. */ public static void debug_with_info(String message) { - if ( ( LOG.getEffectiveLevel() == Level.DEBUG ) - || ( LOG.getEffectiveLevel() == Level.TRACE ) - || ( LOG.getEffectiveLevel() == Level.ALL ) ) { - // Getting the line number is expensive, so only do this - // if the appropriate log level is chosen - StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); - + if (LOG.isTraceEnabled() || LOG.isDebugEnabled()) { // We want the calling method and class name and the line number - StackTraceElement callingPoint = stackTrace[ 2 ]; - - // Build the message - StringBuilder builder = new StringBuilder(); - builder.append( "[" ); - builder.append( callingPoint.toString() ); - builder.append( "] " ); - builder.append( message ); - - // Finally, log the debug message - LOG.debug( builder.toString() ); + StackTraceElement callingPoint = Thread.currentThread().getStackTrace()[2]; + debug("[" + callingPoint + "] " + message); } else { - // Nothing fancy to calculate if the log level is not debug - LOG.debug( message ); + debug(message); } } @@ -164,27 +57,12 @@ public static void debug_with_info(String message) { * Print extra information with the trace message. */ public static void trace_with_info(String message) { - if ( ( LOG.getEffectiveLevel() == Level.TRACE ) - || ( LOG.getEffectiveLevel() == Level.ALL ) ) { - // Getting the line number is expensive, so only do this - // if the appropriate log level is chosen - StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); - + if (LOG.isTraceEnabled()) { // We want the calling method and class name and the line number - StackTraceElement callingPoint = stackTrace[ 2 ]; - - // Build the message - StringBuilder builder = new StringBuilder(); - builder.append( "[" ); - builder.append( callingPoint.toString() ); - builder.append( "] " ); - builder.append( message ); - - // Finally, log the debug message - LOG.trace( builder.toString() ); + StackTraceElement callingPoint = Thread.currentThread().getStackTrace()[2]; + trace("[" + callingPoint + "] " + message); } else { - // Nothing fancy to calculate if the log level is not debug - LOG.trace( message ); + trace(message); } } diff --git a/api/src/test/java/com/gpudb/GPUdbLoggerTest.java b/api/src/test/java/com/gpudb/GPUdbLoggerTest.java new file mode 100644 index 0000000..c379a68 --- /dev/null +++ b/api/src/test/java/com/gpudb/GPUdbLoggerTest.java @@ -0,0 +1,48 @@ +package com.gpudb; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.*; + +class GPUdbLoggerTest { + + private static final Path TEST_LOG_FILE_PATH = Paths.get("build/logs/gpudb-api.log"); + + @BeforeEach + public void beforeEach() { + try { + Files.deleteIfExists(TEST_LOG_FILE_PATH); + } catch (IOException e) { + System.out.printf("Could not delete %s due to %s%n", TEST_LOG_FILE_PATH, e); + } + } + + @Test + public void testLogMessagesAreAppendedToLogFile() throws Exception { + GPUdbLogger.trace("Test log message"); + GPUdbLogger.debug("Test log message"); + GPUdbLogger.info("Test log message"); + GPUdbLogger.warn("Test log message"); + GPUdbLogger.error("Test log message"); + + // give some time for the logged messages to be flushed to the file + Thread.sleep(5000); + List traceMessages = Files.lines(TEST_LOG_FILE_PATH) + .filter(line -> line.contains("TRACE")) + .collect(Collectors.toList()); + assertTrue(traceMessages.isEmpty()); + List nonTraceMessages = Files.lines(TEST_LOG_FILE_PATH) + .filter(line -> !line.contains("TRACE")) + .collect(Collectors.toList()); + assertEquals(4, nonTraceMessages.size()); + } + +} \ No newline at end of file diff --git a/api/src/test/resources/log4j2-test.xml b/api/src/test/resources/log4j2-test.xml new file mode 100644 index 0000000..98497c3 --- /dev/null +++ b/api/src/test/resources/log4j2-test.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + \ No newline at end of file