[
https://issues.apache.org/jira/browse/QPID-8291?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anuhan Torgonshar updated QPID-8291:
------------------------------------
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.
{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!
was:
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.
> 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: [email protected]
For additional commands, e-mail: [email protected]
