You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "Anuhan Torgonshar (JIRA)" <ji...@apache.org> on 2019/04/01 01:02:00 UTC
[jira] [Commented] (QPID-8291) Inconsistent log level practices in
source code
[ https://issues.apache.org/jira/browse/QPID-8291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16806317#comment-16806317 ]
Anuhan Torgonshar commented on QPID-8291:
-----------------------------------------
[~orudyy], thank you very much for your prompt response and feedback. Should I seperate this report to several Jira issues?
> Inconsistent log level practices in source code
> -----------------------------------------------
>
> Key: QPID-8291
> URL: https://issues.apache.org/jira/browse/QPID-8291
> Project: Qpid
> Issue Type: Improvement
> Components: Broker-J
> Affects Versions: qpid-java-6.1.7
> Reporter: Anuhan Torgonshar
> Priority: Major
> Labels: easyfix
>
> 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.
> {code:java}
> ***************** 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 }
> {code}
> We will highly appreciate your feedback!
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org