Major Description
Hi,
I found there are inconsistent log level practices in the Qpid project, and we suspect some of them should be fixed.
We select 4 problematic practices to report.
The detail code as well as the modification suggestions are shown below.
***************** Report1 ********************************* the problematic snippet: ============ ReplicatedEnvironmentFacade.java =================== file path: qpid-java-6.1.7\bdbstore\src\main\java\org\apache\qpid\server\store\berkeleydb\replication\ReplicatedEnvironmentFacade.java logging statement line: 916 modification suggestion: change log level to WARN 890 try 891 { 892 return future.get(timeout, TimeUnit.SECONDS); 893 } 894 catch (InterruptedException e) 895 { 896 Thread.currentThread().interrupt(); 897 } 898 catch (ExecutionException e) 899 { 900 Throwable cause = e.getCause(); 901 if (cause instanceof Error) 902 { 903 throw (Error) cause; 904 } 905 else if (cause instanceof RuntimeException) 906 { 907 throw (RuntimeException) cause; 908 } 909 else 910 { 911 throw new ConnectionScopedRuntimeException("Unexpected exception while " + action, e); 912 } 913 } 914 catch (TimeoutException e) 915 { 916 LOGGER.info("{} on {} timed out after {} seconds", action, _prettyGroupNodeName, timeout); 917 } the similar snippet: ============ SelectorThread.java =============================== file path: qpid-java-6.1.7\broker-core\src\main\java\org\apache\qpid\server\transport\SelectorThread.java logging statement line: 464 449 try 450 { 451 result.get(ACCEPT_CANCELATION_TIMEOUT, TimeUnit.MILLISECONDS); 452 } 453 catch (InterruptedException e) 454 { 455 LOGGER.warn("Cancellation of accepting socket was interrupted"); 456 Thread.currentThread().interrupt(); 457 } 458 catch (ExecutionException e) 459 { 460 LOGGER.warn("Cancellation of accepting socket failed", e.getCause()); 461 } 462 catch (TimeoutException e) 463 { 464 LOGGER.warn("Cancellation of accepting socket timed out"); 465 } ============ BDBHAVirtualHostNodeImpl.java ==================== file path: qpid-java-6.1.7\bdbstore\src\main\java\org\apache\qpid\server\virtualhostnode\berkeleydb\BDBHAVirtualHostNodeImpl.java logging statement line: 912 906 try 907 { 908 future.get(MUTATE_JE_TIMEOUT_MS, TimeUnit.MILLISECONDS); 909 } 910 catch (TimeoutException e) 911 { 912 LOGGER.warn(timeoutLogMessage); 913 } ***************** Report2 ********************************* the problematic snippet: ============ AbstractJDBCPreferenceStore.java =================== file path: qpid-java-6.1.7\broker-core\src\main\java\org\apache\qpid\server\store\preferences\AbstractJDBCPreferenceStore.java logging statement line: 334 modification suggestion: change log level to ERROR 325 try 326 { 327 if (connection != null) 328 { 329 connection.close(); 330 } 331 } 332 catch (SQLException e) 333 { 334 getLogger().warn("Failed to close JDBC connection", e); 335 } the similar snippets: ============ JdbcUtils.java ================================== file path: qpid-java-6.1.7\broker-core\src\main\java\org\apache\qpid\server\store\JdbcUtils.java logging statement line: 42 36 try 37 { 38 conn.close(); 39 } 40 catch (SQLException e) 41 { 42 logger.error("Problem closing connection", e); 43 } ***************** Report3 ********************************* the problematic snippet: ============ DojoHelper.java ================================= file path: qpid-java-6.1.7\broker-plugins\management-http\src\main\java\org\apache\qpid\server\management\plugin\DojoHelper.java logging statement line: 75 modification suggestion: change log level to ERROR 69 try 70 { 71 propertyStream.close(); 72 } 73 catch (IOException e) 74 { 75 _logger.warn("Exception closing InputStream for " + VERSION_FILE + " resource:", e); 76 } the similar snippet: ============ CallbackHandlerRegistry.java ======================= file path: qpid-java-6.1.7\client\src\main\java\org\apache\qpid\client\security\CallbackHandlerRegistry.java logging statement line: 112 105 try 106 { 107 is.close(); 108 109 } 110 catch (IOException e) 111 { 112 _logger.error("Unable to close properties stream: " + e, e); 113 } ============ DynamicSaslRegistrar.java ========================= file path: qpid-java-6.1.7\client\src\main\java\org\apache\qpid\client\security\DynamicSaslRegistrar.java logging statement line: 143 136 try 137 { 138 is.close(); 139 140 } 141 catch (IOException e) 142 { 143 _logger.error("Unable to close properties stream: " + e, e); 144 } ***************** Report4 ********************************* the problematic snippet: ============ SSLUtil.java ==================================== file path: qpid-java-6.1.7\common\src\main\java\org\apache\qpid\transport\network\security\ssl\SSLUtil.java logging statement line: 231 modification suggestion: change log level to ERROR 206 try 207 { 208 LdapName ln = new LdapName(dn); 209 for(Rdn rdn : ln.getRdns()) 210 { 211 if("CN".equalsIgnoreCase(rdn.getType())) 212 { 213 cnStr = rdn.getValue().toString(); 214 } 215 else if("DC".equalsIgnoreCase(rdn.getType())) 216 { 217 if(dcStr == null) 218 { 219 dcStr = rdn.getValue().toString(); 220 } 221 else 222 { 223 dcStr = rdn.getValue().toString() + '.' + dcStr; 224 } 225 } 226 } 227 return cnStr == null || cnStr.length()==0 ? "" : dcStr == null ? cnStr : cnStr + '@' + dcStr; 228 } 229 catch (InvalidNameException e) 230 { 231 LOGGER.warn("Invalid name: '{}'", dn); 232 return ""; 233 } the similar snippet: ============ NonJavaTrustStoreImpl.java ========================== file path: qpid-java-6.1.7\broker-core\src\main\java\org\apache\qpid\server\security\NonJavaTrustStoreImpl.java logging statement line: 163 147 try 148 { 149 LdapName ldapDN = new LdapName(dn); 150 name = dn; 151 for (Rdn rdn : ldapDN.getRdns()) 152 { 153 if (rdn.getType().equalsIgnoreCase("CN")) 154 { 155 name = String.valueOf(rdn.getValue()); 156 break; 157 } 158 } 159 160 } 161 catch (InvalidNameException e) 162 { 163 LOGGER.error("Error getting subject name from certificate"); 164 name = null; 165 } ============ NonJavaKeyStoreImpl.java ========================= file path: qpid-java-6.1.7\broker-core\src\main\java\org\apache\qpid\server\security\NonJavaKeyStoreImpl.java logging statement line: 132 115 try 116 { 117 String dn = _certificate.getSubjectX500Principal().getName(); 118 LdapName ldapDN = new LdapName(dn); 119 String name = dn; 120 for (Rdn rdn : ldapDN.getRdns()) 121 { 122 if (rdn.getType().equalsIgnoreCase("CN")) 123 { 124 name = String.valueOf(rdn.getValue()); 125 break; 126 } 127 } 128 return name; 129 } 130 catch (InvalidNameException e) 131 { 132 LOGGER.error("Error getting subject name from certificate"); 133 return null; 134 }
We will highly appreciate your feedback!