Add APIs to Log Checked Exceptions as Warning and Error (#9194)

* Add APIs to log checked exceptions as warning or errors * Enhanced tests, added '5' as a valid LogLevel environment configuration to support 'NOT_SET' * Rename new APIs to something better, add opens for logging package, added missing property reset * Additional logging changes * Update logging tests to mutate global configuration
AzureSDKAutomation · Mar 19, 2020 · dff992b · dff992b
1 parent aa9a879
commit dff992b
Show file tree

Hide file tree

Showing 8 changed files with 424 additions and 246 deletions.
diff --git a/pom.client.xml b/pom.client.xml
@@ -954,6 +954,7 @@
 
                 <!-- JUnit5 and Mockito use reflection to create objects. -->
                 --add-opens com.azure.core/com.azure.core=ALL-UNNAMED
+                --add-opens com.azure.core/com.azure.core.util.logging=ALL-UNNAMED
                 --add-opens com.azure.core/com.azure.core.util.polling=ALL-UNNAMED
                 --add-opens com.azure.core/com.azure.core.util.serializer=ALL-UNNAMED
                 --add-opens com.azure.core/com.azure.core.http=ALL-UNNAMED

diff --git a/sdk/core/azure-core-amqp/pom.xml b/sdk/core/azure-core-amqp/pom.xml
@@ -95,12 +95,6 @@
       <version>5.4.2</version> <!-- {x-version-update;org.junit.jupiter:junit-jupiter-params;external_dependency} -->
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.slf4j</groupId>
-      <artifactId>slf4j-simple</artifactId>
-      <version>1.7.25</version> <!-- {x-version-update;org.slf4j:slf4j-simple;external_dependency} -->
-      <scope>test</scope>
-    </dependency>
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>

diff --git a/sdk/core/azure-core-management/pom.xml b/sdk/core/azure-core-management/pom.xml
@@ -88,13 +88,6 @@
       <scope>test</scope>
     </dependency>
 
-    <dependency>
-      <groupId>org.slf4j</groupId>
-      <artifactId>slf4j-simple</artifactId>
-      <version>1.7.25</version> <!-- {x-version-update;org.slf4j:slf4j-simple;external_dependency} -->
-      <scope>test</scope>
-    </dependency>
-
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>

diff --git a/sdk/core/azure-core-test/pom.xml b/sdk/core/azure-core-test/pom.xml
@@ -65,11 +65,6 @@
       <version>5.4.2</version> <!-- {x-version-update;org.junit.jupiter:junit-jupiter-params;external_dependency} -->
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.slf4j</groupId>
-      <artifactId>slf4j-api</artifactId>
-      <version>1.7.28</version> <!-- {x-version-update;org.slf4j:slf4j-api;external_dependency} -->
-    </dependency>
     <dependency>
       <groupId>com.github.tomakehurst</groupId>
       <artifactId>wiremock-standalone</artifactId>

diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/ClientLogger.java b/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/ClientLogger.java
@@ -6,12 +6,13 @@
 import com.azure.core.implementation.logging.DefaultLogger;
 import com.azure.core.util.Configuration;
 import com.azure.core.util.CoreUtils;
-import java.util.Arrays;
-import java.util.Objects;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.helpers.NOPLogger;
 
+import java.util.Arrays;
+import java.util.Objects;
+
 /**
  * This is a fluent logger helper class that wraps a pluggable {@link Logger}.
  *
@@ -134,37 +135,76 @@ public void error(String format, Object... args) {
 
     /**
      * Logs the {@link RuntimeException} at the warning level and returns it to be thrown.
+     * <p>
+     * This API covers the cases where a runtime exception type needs to be thrown and logged. If a {@link Throwable} is
+     * being logged use {@link #logThowableAsWarning(Throwable)} instead.
      *
      * @param runtimeException RuntimeException to be logged and returned.
-     * @return The passed {@code RuntimeException}.
+     * @return The passed {@link RuntimeException}.
      * @throws NullPointerException If {@code runtimeException} is {@code null}.
      */
     public RuntimeException logExceptionAsWarning(RuntimeException runtimeException) {
         Objects.requireNonNull(runtimeException, "'runtimeException' cannot be null.");
+
+        return logThowableAsWarning(runtimeException);
+    }
+
+    /**
+     * Logs the {@link Throwable} at the warning level and returns it to be thrown.
+     * <p>
+     * This API covers the cases where a checked exception type needs to be thrown and logged. If a {@link
+     * RuntimeException} is being logged use {@link #logExceptionAsWarning(RuntimeException)} instead.
+     *
+     * @param throwable Throwable to be logged and returned.
+     * @param <T> Type of the Throwable being logged.
+     * @return The passed {@link Throwable}.
+     * @throws NullPointerException If {@code throwable} is {@code null}.
+     */
+    public <T extends Throwable> T logThowableAsWarning(T throwable) {
+        Objects.requireNonNull(throwable, "'throwable' cannot be null.");
         if (!logger.isWarnEnabled()) {
-            return runtimeException;
+            return throwable;
         }
 
-        performLogging(LogLevel.WARNING, true, runtimeException.getMessage(), runtimeException);
-        return runtimeException;
+        performLogging(LogLevel.WARNING, true, throwable.getMessage(), throwable);
+        return throwable;
     }
 
     /**
      * Logs the {@link RuntimeException} at the error level and returns it to be thrown.
+     * <p>
+     * This API covers the cases where a runtime exception type needs to be thrown and logged. If a {@link Throwable} is
+     * being logged use {@link #logThrowableAsError(Throwable)} instead.
      *
      * @param runtimeException RuntimeException to be logged and returned.
      * @return The passed {@code RuntimeException}.
      * @throws NullPointerException If {@code runtimeException} is {@code null}.
      */
     public RuntimeException logExceptionAsError(RuntimeException runtimeException) {
         Objects.requireNonNull(runtimeException, "'runtimeException' cannot be null.");
+
+        return logThrowableAsError(runtimeException);
+    }
+
+    /**
+     * Logs the {@link Throwable} at the error level and returns it to be thrown.
+     * <p>
+     * This API covers the cases where a checked exception type needs to be thrown and logged. If a {@link
+     * RuntimeException} is being logged use {@link #logExceptionAsError(RuntimeException)} instead.
+     *
+     * @param throwable Throwable to be logged and returned.
+     * @param <T> Type of the Throwable being logged.
+     * @return The passed {@link Throwable}.
+     * @throws NullPointerException If {@code throwable} is {@code null}.
+     */
+    public <T extends Throwable> T logThrowableAsError(T throwable) {
+        Objects.requireNonNull(throwable, "'throwable' cannot be null.");
         if (!logger.isErrorEnabled()) {
-            return runtimeException;
+            return throwable;
         }
 
-        performLogging(LogLevel.VERBOSE, true, runtimeException.getMessage(), runtimeException);
-
-        return runtimeException;
+        performLogging(LogLevel.ERROR, true, throwable.getMessage(), throwable);
+        return throwable;
     }
 
     /*

diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/LogLevel.java b/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/LogLevel.java
@@ -7,7 +7,7 @@
 import java.util.Locale;
 
 /**
- * Enum which represent logging levels used in Azure SDKs. 
+ * Enum which represent logging levels used in Azure SDKs.
  */
 public enum LogLevel {
     /**
@@ -33,7 +33,7 @@ public enum LogLevel {
     /**
      * Indicates that no log level is set.
      */
-    NOT_SET(5);
+    NOT_SET(5, "5");
 
     private final int numericValue;
     private final String[] allowedLogLevelVariables;
@@ -65,7 +65,7 @@ public int getLogLevel() {
      * Converts the passed log level string to the corresponding {@link LogLevel}.
      *
      * @param logLevelVal The log level value which needs to convert
-     * @return The LogLevel Enum if pass in the valid string. 
+     * @return The LogLevel Enum if pass in the valid string.
      * The valid strings for {@link LogLevel} are:
      * <ul>
      * <li>VERBOSE: "verbose", "debug"</li>

diff --git a/sdk/core/azure-core/src/test/java/com/azure/core/http/policy/HttpLoggingPolicyTests.java b/sdk/core/azure-core/src/test/java/com/azure/core/http/policy/HttpLoggingPolicyTests.java
@@ -15,6 +15,7 @@
 import com.azure.core.util.Context;
 import com.azure.core.util.CoreUtils;
 import com.azure.core.util.FluxUtil;
+import com.azure.core.util.logging.LogLevel;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -26,6 +27,7 @@
 import reactor.test.StepVerifier;
 
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.io.PrintStream;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -37,6 +39,7 @@
 import java.util.Set;
 import java.util.stream.Stream;
 
+import static com.azure.core.util.Configuration.PROPERTY_AZURE_LOG_LEVEL;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 
 /**
@@ -47,41 +50,37 @@ public class HttpLoggingPolicyTests {
     private static final Context CONTEXT = new Context("caller-method", HttpLoggingPolicyTests.class.getName());
 
     private String originalLogLevel;
-    private PrintStream originalErr;
+    private PrintStream originalSystemOut;
     private ByteArrayOutputStream logCaptureStream;
 
     @BeforeEach
     public void prepareForTest() {
         // Set the log level to information for the test.
-        originalLogLevel = System.getProperty(Configuration.PROPERTY_AZURE_LOG_LEVEL);
-        System.setProperty(Configuration.PROPERTY_AZURE_LOG_LEVEL, "2");
+        originalLogLevel = Configuration.getGlobalConfiguration().get(PROPERTY_AZURE_LOG_LEVEL);
+        Configuration.getGlobalConfiguration().put(PROPERTY_AZURE_LOG_LEVEL,
+            String.valueOf(LogLevel.INFORMATIONAL.getLogLevel()));
 
         /*
-         * Indicate to SLF4J to enable trace level logging for a logger named
-         * com.azure.core.util.logging.ClientLoggerTests. Trace is the maximum level of logging supported by the
-         * ClientLogger.
+         * DefaultLogger uses System.out to log. Inject a custom PrintStream to log into for the duration of the test to
+         * capture the log messages.
          */
-        System.setProperty("org.slf4j.simpleLogger.log.com.azure.core.util.logging.HttpLoggingPolicyTests", "trace");
-
-        // Override System.err as that is where SLF4J will log by default.
-        originalErr = System.out;
+        originalSystemOut = System.out;
         logCaptureStream = new ByteArrayOutputStream();
         System.setOut(new PrintStream(logCaptureStream));
     }
 
     @AfterEach
-    public void cleanupAfterTest() {
+    public void cleanupAfterTest() throws IOException {
         // Reset or clear the log level after the test completes.
         if (CoreUtils.isNullOrEmpty(originalLogLevel)) {
-            System.clearProperty(Configuration.PROPERTY_AZURE_LOG_LEVEL);
+            Configuration.getGlobalConfiguration().remove(PROPERTY_AZURE_LOG_LEVEL);
         } else {
-            System.setProperty(Configuration.PROPERTY_AZURE_LOG_LEVEL, originalLogLevel);
+            Configuration.getGlobalConfiguration().put(PROPERTY_AZURE_LOG_LEVEL, originalLogLevel);
         }
 
-        System.clearProperty("org.slf4j.simpleLogger.log.com.azure.core.util.logging.HttpLoggingPolicyTests");
-
         // Reset System.err to the original PrintStream.
-        System.setOut(originalErr);
+        System.setOut(originalSystemOut);
+        logCaptureStream.close();
     }
 
     /**