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/03/31 11:46:00 UTC
[jira] [Created] (QPID-8291) Inconsistent log level practices in
source code
Anuhan Torgonshar created QPID-8291:
---------------------------------------
Summary: 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
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 }
Look forward to your review and 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