[GitHub] [tomcat] zisding commented on pull request #514: Registry gives inaccurate log message saying "Creating MBeanServer ... " after already created the MBeanServer

[GitBox <gi...@apache.org>]

zisding commented on PR #514:
URL: https://github.com/apache/tomcat/pull/514#issuecomment-1133097834

   Hi Mark, @markt-asf 
   
   Thanks for merging the PR. Really hope my contribution can make Tomcat more perfect, even if it is incremental. 
   
   Based on the feedback, I further reviewed some source code and logging messages and found the following  inaccurate expression of the relation between the logging statement and its corresponding action in the source code. Meanwhile, I also proposed some suggestions. Could you please help me figure out whether they are appropriate or not? I can make a PR later.
   
   1.  Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/session/DataSourceStore.java#L537 
   Suggestion: Change `Removing` to `Removed` in the localstrings,  `dataSourceStore.removing=Removing Session [{0}] at database [{1}]`
   Reason: as the logging statement is placed at the end of the function `remove()` indicting the function has been successfully executed.
   2. Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/valves/RequestFilterValve.java/#L383 
   Suggestion: Change `Denied` to `Denying` in the localstrings, `requestFilterValve.deny=Denied request for [{0}] based on property [{1}]`
   Reason:  the logging statement is placed before the target function `denyRequest()` indicting the function will be executed.
   3. Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/realm/JAASRealm.java/#L569 
   Suggestion: Change `Adding` to `Added` in the localstrings, `jaasRealm.rolePrincipalAdd=Adding role Principal [{0}] to this user Principal''s roles`,
   Reason:  the logging statement is placed after the target function `add()` indicting the status of `add` is completed.
   4. Logging statement: https://github.com/apache/tomcat/blob/b78d415754b8d9c3160f84c111a993533568e8df/java/org/apache/catalina/ha/session/DeltaManager.java/#L1385
   Suggestion: Change `Sent` to `Sending` in the localstrings, `deltaManager.createMessage.allSessionData=Manager [{0}] sent all session data.`,  
   Reason:  the logging statement is placed before the target function `send()` indicting the status of `send` will be executed or in progress.
   5. Logging statement: https://github.com/apache/tomcat/blob/0f2e084d1583cfa4c00cec1958ec30113fb4f41e/java/org/apache/coyote/AbstractProtocol.java#L392
   Suggestion: Change `Removed` to `Removing` in the localstrings, `abstractProtocol.waitingProcessor.remove=Removed processor [{0}] from waiting processors`,  
   Reason:  the logging statement is placed before the target function `remove()` indicting the status of `remove` will be executed.
   6. Logging statement: https://github.com/apache/tomcat/blob/0f2e084d1583cfa4c00cec1958ec30113fb4f41e/java/org/apache/coyote/http2/Stream.java#L655
   Suggestion: Change `sent ` to `sending` in the localstrings, `stream.reset.send=Connection [{0}], Stream [{1}], Reset sent due to [{2}]`,  
   Reason:  the logging statement is placed before the target function `sendReset()` indicting the status of `sendReset ` will be executed.
   7. Logging statement: https://github.com/apache/tomcat/blob/0f2e084d1583cfa4c00cec1958ec30113fb4f41e/java/org/apache/coyote/http2/Stream.java/#L686
   Suggestion: Change `has been recycled ` to `will be recycled ` in the localstrings, `stream.recycle=Connection [{0}], Stream [{1}] has been recycled`,  
   Reason:  the logging statement is placed before the target source code indicting the status of `recycle ` will be executed or in progress.
   
   Thanks for reviewing.


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org