Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-26 Thread via GitHub


chia7712 merged PR #15596:
URL: https://github.com/apache/kafka/pull/15596


-- 
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: jira-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-26 Thread via GitHub


KevinZTW commented on code in PR #15596:
URL: https://github.com/apache/kafka/pull/15596#discussion_r1539428407


##
README.md:
##
@@ -54,9 +54,14 @@ Follow instructions in https://kafka.apache.org/quickstart
 ./gradlew clients:test --tests 
org.apache.kafka.clients.MetadataTest.testTimeToNextUpdate
 
 ### Running a particular unit/integration test with log4j output ###
-Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`
+By default, there will be only small number of logs output while testing. You 
can adjust it by changing the `log4j.properties` in the test project directory.
 
-./gradlew clients:test --tests RequestResponseTest
+For example, if you want to see more logs for clients project tests, you can 
modify the line in `clients/src/test/resources/log4j.properties` 

Review Comment:
   I haven't thought about this, thanks for the advise!



-- 
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: jira-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-26 Thread via GitHub


kirktrue commented on code in PR #15596:
URL: https://github.com/apache/kafka/pull/15596#discussion_r1539399970


##
README.md:
##
@@ -54,9 +54,14 @@ Follow instructions in https://kafka.apache.org/quickstart
 ./gradlew clients:test --tests 
org.apache.kafka.clients.MetadataTest.testTimeToNextUpdate
 
 ### Running a particular unit/integration test with log4j output ###
-Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`
+By default, there will be only small number of logs output while testing. You 
can adjust it by changing the `log4j.properties` in the test project directory.
 
-./gradlew clients:test --tests RequestResponseTest
+For example, if you want to see more logs for clients project tests, you can 
modify the line in `clients/src/test/resources/log4j.properties` 
+to `log4j.logger.org.apache.kafka=INFO` and then run:
+
+./gradlew cleanTest clients:test --tests NetworkClientTest   
+
+And you should see `INFO` level logs in the file under the 
`./clients/build/test-results/test` directory.

Review Comment:
   ```suggestion
   And you should see `INFO` level logs in the file under the 
`clients/build/test-results/test` directory.
   ```



##
README.md:
##
@@ -54,9 +54,14 @@ Follow instructions in https://kafka.apache.org/quickstart
 ./gradlew clients:test --tests 
org.apache.kafka.clients.MetadataTest.testTimeToNextUpdate
 
 ### Running a particular unit/integration test with log4j output ###
-Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`
+By default, there will be only small number of logs output while testing. You 
can adjust it by changing the `log4j.properties` in the test project directory.

Review Comment:
   ```suggestion
   By default, there will be only small number of logs output while testing. 
You can adjust it by changing the `log4j.properties` file in the module's 
`src/test/resources` directory.
   ```



-- 
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: jira-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-26 Thread via GitHub


chia7712 commented on code in PR #15596:
URL: https://github.com/apache/kafka/pull/15596#discussion_r1539297268


##
README.md:
##
@@ -54,9 +54,14 @@ Follow instructions in https://kafka.apache.org/quickstart
 ./gradlew clients:test --tests 
org.apache.kafka.clients.MetadataTest.testTimeToNextUpdate
 
 ### Running a particular unit/integration test with log4j output ###
-Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`
+By default, there will be only small number of logs output while testing. You 
can adjust it by changing the `log4j.properties` in the test project directory.
 
-./gradlew clients:test --tests RequestResponseTest
+For example, if you want to see more logs for clients project tests, you can 
modify the line in `clients/src/test/resources/log4j.properties` 

Review Comment:
   How adding the link: 
https://github.com/apache/kafka/blob/trunk/clients/src/test/resources/log4j.properties#L21



-- 
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: jira-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-26 Thread via GitHub


KevinZTW commented on code in PR #15596:
URL: https://github.com/apache/kafka/pull/15596#discussion_r1539145183


##
README.md:
##
@@ -56,7 +56,11 @@ Follow instructions in https://kafka.apache.org/quickstart
 ### Running a particular unit/integration test with log4j output ###
 Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`

Review Comment:
   Thanks for reviewing my PR!
   Agree with you that the original phrase is a bit unclear and misleading. I 
think the version you suggested is much clearer and easy to understand, let me 
revise base on that!



-- 
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: jira-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-25 Thread via GitHub


showuon commented on code in PR #15596:
URL: https://github.com/apache/kafka/pull/15596#discussion_r1538537571


##
README.md:
##
@@ -56,7 +56,11 @@ Follow instructions in https://kafka.apache.org/quickstart
 ### Running a particular unit/integration test with log4j output ###
 Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`

Review Comment:
   I think there are many other `log4j.properties` needed to be modified if you 
are running tests for that project, not just these 2 files. For example, when 
you are running tests in `TopicCommandTest`, you should update: 
`tools/src/test/resources/log4j.properties`, instead of the files listed here. 
I'm thinking we should rephrase it to a more general way. Maybe:
   
   By default, there will be only small number of logs output while testing. 
You can adjust it by changing the `log4j.properties` in the test project 
directory. 
   For example, if you want to see more logs for clients project tests, you can 
modify the line in `clients/src/test/resources/log4j.properties` to 
`log4j.logger.org.apache.kafka=INFO` and then run: 
   
   
   WDYT?




-- 
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: jira-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-25 Thread via GitHub


KevinZTW commented on code in PR #15596:
URL: https://github.com/apache/kafka/pull/15596#discussion_r1537700914


##
README.md:
##
@@ -56,7 +56,11 @@ Follow instructions in https://kafka.apache.org/quickstart
 ### Running a particular unit/integration test with log4j output ###
 Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`
 
-./gradlew clients:test --tests RequestResponseTest
+For example, you can modify the line in 
`clients/src/test/resources/log4j.properties` to 
`log4j.logger.org.apache.kafka=INFO` and then run:
+
+./gradlew cleanTest clients:test --tests NetworkClientTest --info   

Review Comment:
   I see, I add the `--info`  is because I want the user could also see logs on 
the console directly agree it do print out lots of info though...



-- 
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: jira-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-25 Thread via GitHub


chia7712 commented on code in PR #15596:
URL: https://github.com/apache/kafka/pull/15596#discussion_r1537677312


##
README.md:
##
@@ -56,7 +56,11 @@ Follow instructions in https://kafka.apache.org/quickstart
 ### Running a particular unit/integration test with log4j output ###
 Change the log4j setting in either 
`clients/src/test/resources/log4j.properties` or 
`core/src/test/resources/log4j.properties`
 
-./gradlew clients:test --tests RequestResponseTest
+For example, you can modify the line in 
`clients/src/test/resources/log4j.properties` to 
`log4j.logger.org.apache.kafka=INFO` and then run:
+
+./gradlew cleanTest clients:test --tests NetworkClientTest --info   

Review Comment:
   `--info` will output gradle-related logs. That is unnecessary since all we 
want to observe is the kafka-related logs.



-- 
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: jira-unsubscr...@kafka.apache.org

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



[PR] KAFKA-16416: Use NetworkClientTest to replace RequestResponseTest to be the example of log4j output [kafka]

2024-03-25 Thread via GitHub


KevinZTW opened a new pull request, #15596:
URL: https://github.com/apache/kafka/pull/15596

   jira url: https://issues.apache.org/jira/browse/KAFKA-16416
   
   For the part to introduce how to show log4j's info when executing unit test 
in README, it seems that current example 
   `RequestResponseTest` only have `WARN` and `TRACE` level info.
   
   I switch to another test case, which has `INFO`, `DEBUG`, `TRACE` and `WARN` 
so it's easier to observe the log level changes.  Also add `cleanTest` and 
`--info` to ensure the unit test do execute each time and the info directly 
shows on the console
   
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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: jira-unsubscr...@kafka.apache.org

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