nodece commented on code in PR #25536:
URL: https://github.com/apache/pulsar/pull/25536#discussion_r3091365440


##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/PartitionedProducerConsumerTest.java:
##########
@@ -76,7 +75,7 @@ public void cleanupExecutor() throws Exception {
 
     @Test(timeOut = 30000)
     public void testRoundRobinProducer() throws Exception {
-        log.info("-- Starting {} test --", methodName);
+        log.info().attr("starting", methodName).log("-- Starting test");

Review Comment:
   **`"starting"` and `"exiting"` are adjectives/verbs from the message text, 
not names for `methodName` value. This repeats 24+ times in the file.**
   
   ```suggestion
           log.info().attr("testMethod", methodName).log("Starting test");
   ```
   Same for all `attr("exiting", methodName)` → `attr("testMethod", 
methodName)`.



##########
pulsar-broker/src/test/java/org/apache/pulsar/client/impl/MessageChunkingTest.java:
##########
@@ -627,7 +630,7 @@ public void testChunkSize() throws Exception {
                 .create();
         for (int size = 1; size <= maxMessageSize; size++) {
             final MessageId messageId = 
producer.send(createMessagePayload(size));
-            log.info("Send {} bytes to {}", size, messageId);
+            log.info().attr("size", size).attr("messageId", 
messageId).log("Sent bytes");

Review Comment:
   **`"Sent bytes"` lost the destination context from the original `"Send {} 
bytes to {}"` message.**
   
   Since `messageId` is already an attr, a clearer message would be:
   ```suggestion
               log.info().attr("size", size).attr("messageId", 
messageId).log("Sent bytes to messageId");
   ```



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