steveloughran commented on a change in pull request #2321:
URL: https://github.com/apache/hadoop/pull/2321#discussion_r493762992



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
##########
@@ -636,18 +636,19 @@ public static void closeAll() throws IOException {
   public static void closeAllForUGI(UserGroupInformation ugi)
   throws IOException {
     if (LOGGER.isDebugEnabled()) {
-      debugLogFileSystemClose("closeAllForUGI", "UGI: " + ugi.toString());
+      debugLogFileSystemClose("closeAllForUGI", "UGI: " + ugi);
     }
     CACHE.closeAll(ugi);
   }
 
   private static void debugLogFileSystemClose(String methodName, String 
additionalInfo) {
     StackTraceElement callingMethod = new 
Throwable().fillInStackTrace().getStackTrace()[2];
-    LOGGER.debug(
-        "FileSystem." + methodName + "() called by method: "
-            + callingMethod.getClassName() + "." + 
callingMethod.getMethodName()
-            + "(" + callingMethod.getFileName() + ":" + 
callingMethod.getLineNumber() + "); "
-            + (additionalInfo != null ? additionalInfo : ""));
+    LOGGER.debug("FileSystem.{}() called by method: {}.{}({}:{}); {}", 
methodName, callingMethod.getClassName(),
+            callingMethod.getMethodName(), callingMethod.getFileName(), 
callingMethod.getLineNumber(), additionalInfo);
+    if (LOGGER.isTraceEnabled()) {
+      LOGGER.trace("FileSystem.{}() full stack trace:", methodName, new 
Throwable().fillInStackTrace());

Review comment:
       just use the throwable created on Line 645

##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
##########
@@ -636,18 +636,19 @@ public static void closeAll() throws IOException {
   public static void closeAllForUGI(UserGroupInformation ugi)
   throws IOException {
     if (LOGGER.isDebugEnabled()) {
-      debugLogFileSystemClose("closeAllForUGI", "UGI: " + ugi.toString());
+      debugLogFileSystemClose("closeAllForUGI", "UGI: " + ugi);
     }
     CACHE.closeAll(ugi);
   }
 
   private static void debugLogFileSystemClose(String methodName, String 
additionalInfo) {
     StackTraceElement callingMethod = new 
Throwable().fillInStackTrace().getStackTrace()[2];
-    LOGGER.debug(
-        "FileSystem." + methodName + "() called by method: "
-            + callingMethod.getClassName() + "." + 
callingMethod.getMethodName()
-            + "(" + callingMethod.getFileName() + ":" + 
callingMethod.getLineNumber() + "); "
-            + (additionalInfo != null ? additionalInfo : ""));
+    LOGGER.debug("FileSystem.{}() called by method: {}.{}({}:{}); {}", 
methodName, callingMethod.getClassName(),

Review comment:
       * include URI of filesystem
   * pass in callingMethod and let it's toString() include all the information; 
no need to replicate -especially as toString handles file/line unknown.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to