michaeljmarshall opened a new pull request #11733:
URL: https://github.com/apache/pulsar/pull/11733


   The first of two PRs to address 
https://github.com/apache/pulsar/issues/11708.
   
   ### Motivation
   In order to solve #11708, I must first update the request logger we're using 
because the `Slf4jRequestLog` is deprecated. Here is the current javadoc:
   
   ```java
   /**
    * Implementation of NCSARequestLog where output is sent as a SLF4J INFO Log 
message on the named logger "org.eclipse.jetty.server.RequestLog"
    *
    * @deprecated use {@link CustomRequestLog} given format string {@link 
CustomRequestLog#EXTENDED_NCSA_FORMAT} with an {@link Slf4jRequestLogWriter}
    */
   ```
   
   We don't use the `EXTENDED_NCSA_FORMAT` exactly because we modify the 
`Slf4jRequestLog` with the following method calls:
   
   ```java
               Slf4jRequestLog requestLog = new Slf4jRequestLog();
               requestLog.setExtended(true);
               requestLog.setLogTimeZone(TimeZone.getDefault().getID());
               requestLog.setLogLatency(true);
   ```
   
   This PR replaces the `Slf4jRequestLog` with the `CustomRequestLog`. It then 
creates a custom log format that matches the configuration from the above code 
block.
   
   If we want to change the format at this time, please let me know.
   
   ### Modifications
   
   * Replace `Slf4jRequestLog` with the `CustomRequestLog`
   * Reduce code duplication by putting the logic to create the custom logger 
in a single class
   * Use a custom log format that matches the old format exactly 
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   This change is a trivial rework / code cleanup without any test coverage. I 
manually verified the change. Here is a log line from before the change and 
after the change:
   
   ```
   # before
   13:17:27.724 [pulsar-websocket-web-45-3] INFO  
org.eclipse.jetty.server.RequestLog - 127.0.0.1 - - [18/Aug/2021:13:17:27 
-0500] "GET 
/ws/consumer/persistent/my-property/use/my-ns/my-topic/my-sub?token=abce 
HTTP/1.1" 101 0 "-" "Jetty/9.4.43.v20210629" 698
   
   # after
   15:51:44.734 [pulsar-websocket-web-45-3] INFO  
org.eclipse.jetty.server.RequestLog - 127.0.0.1 - - [20/Aug/2021:15:51:43 
-0500] "GET 
/ws/consumer/persistent/my-property/use/my-ns/my-topic/my-sub?token=abce 
HTTP/1.1" 101 0 "-" "Jetty/9.4.43.v20210629" 846
   ```
   
   ### Does this pull request potentially affect one of the following parts:
   
   This is completely backwards compatible.
   
   ### Documentation
   
   #### For contributor
   
   For this PR, do we need to update docs?
   
   No, this is an internal change to comply with a dependency's class 
deprecation. The logs will be identical after this change.
   
   #### For committer
   
   For this PR, do we need to update docs?
   
   - If yes,
     
     - if you update docs in this PR, label this PR with the `doc` label.
     
     - if you plan to update docs later, label this PR with the `doc-required` 
label.
   
     - if you need help on updating docs, create a follow-up issue with the 
`doc-required` label.
     
   - If no, label this PR with the `no-need-doc` label and explain why.
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to