Moving log4j to logback [1/n] #107
Conversation
|
Just for the record there is only flaky C test and it's been fixed in master https://issues.apache.org/jira/browse/ZOOKEEPER-4479. We have to cherry pick this commit to prevent our efforts of rerunning CI. |
| </appender--> | ||
|
|
||
| <!-- | ||
| zk audit logging |
There was a problem hiding this comment.
Yup we need this. For now i kept it in comments and next few PRs should have correct audit and console logging enabled.
| @@ -322,8 +312,7 @@ private static void verifyLog(String expectedLog, String log) { | |||
| String searchString = " - "; | |||
| int logStartIndex = log.indexOf(searchString); | |||
| String auditLog = log.substring(logStartIndex + searchString.length()); | |||
| assertEquals(expectedLog, auditLog); | |||
|
|
|||
| Assert.assertTrue(auditLog.endsWith(expectedLog)); | |||
There was a problem hiding this comment.
Just curious, why with log4j it's "equals", and with logback it's "endWith"? Does logback add something in the front?
There was a problem hiding this comment.
yeah logback logging format is little different hence we had to check with endsWith. I verified logback had all existing things from log4j in logs but just few extra things like classname in detail or few formatting changes.
zookeeper-server/src/test/java/org/apache/zookeeper/audit/Slf4JAuditLoggerTest.java
Show resolved
Hide resolved
| @@ -16,7 +16,6 @@ REM limitations under the License. | |||
|
|
|||
| set ZOOCFGDIR=%~dp0%..\conf | |||
| set ZOO_LOG_DIR=%~dp0%..\logs | |||
| set ZOO_LOG4J_PROP=INFO,CONSOLE | |||
There was a problem hiding this comment.
is there equivalent to set log-levels to o/p mapping in logback?
There was a problem hiding this comment.
yup that's correct. And that can be done using xml file.
| @@ -198,6 +205,9 @@ public void testConnectionEvents() throws Exception { | |||
| */ | |||
| @Test(timeout = 90000) | |||
| public void testSessionEstablishment() throws Exception { | |||
| qu.enableLocalSession(true); | |||
There was a problem hiding this comment.
this is not related to logback change? any info as to why we had to add this?
There was a problem hiding this comment.
Looks like few APIs have been changed due to which this code has to be added to pass this UT
* Moving log4j to logback Co-authored-by: Rahul Rane <rrane@linkedin.com>
Description
Currently zk application and audit logs are rotated based on size and not on time/duration. For GCN debugging sometimes it's hard to debug things when logs are rolled over and cleaned up because current policy. Currently log4j do not allow to have rollover policy for size as well as duration based rotation. Even upstream community is moved to logback after discussing pros and cons about log4j 2.x and logback apache#1793.
This PR is based out of PR apache#1793. We havn't pulled latest changes from upstream zookeeper so i reverted few changes and kept it compatible with our current zk implementation.
Tests
CI and e2e pipeline
Verified locally running zk server and connecting via some internal tools and performing crwd ops.
[INFO] Hosts: ['127.0.0.1:2181'] (CONNECTED [127.0.0.1:2181]) /> ls TestCluster ephe1 folder nomapping test3 test4 test5 test6 test7 testwithacl testwithacl1 zookeeper (CONNECTED [127.0.0.1:2181]) /> create test1 lskd (CONNECTED [127.0.0.1:2181]) /> get test1 lskd (CONNECTED [127.0.0.1:2181]) /> set test1 ksdj (CONNECTED [127.0.0.1:2181]) /> rm test1 (CONNECTED [127.0.0.1:2181]) />
Changes that Break Backward Compatibility (Optional)
NA
Documentation (Optional)
In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)