[ROCKETMQ-367] Add logging component. #224
Conversation
| </parent> | ||
|
|
||
| <modelVersion>4.0.0</modelVersion> | ||
| <packaging>jar</packaging> |
There was a problem hiding this comment.
No need it in maven pom, that is default value
logging/pom.xml
Outdated
| </dependency> | ||
| <dependency> | ||
| <groupId>ch.qos.logback</groupId> | ||
| <artifactId>logback-core</artifactId> |
There was a problem hiding this comment.
logback-classic dependent core, so...
There was a problem hiding this comment.
This component is used in test cases for slf4j api test.
logging/pom.xml
Outdated
| <extension> | ||
| <groupId>kr.motd.maven</groupId> | ||
| <artifactId>os-maven-plugin</artifactId> | ||
| <version>1.4.0.Final</version> |
There was a problem hiding this comment.
why need it? I have not found similar netty's https OS dependency build task
| } | ||
|
|
||
| @Override | ||
| public boolean isTraceEnabled() { |
There was a problem hiding this comment.
trace and isXXXenable, Could we remove these non-practical methods in our log module?
There was a problem hiding this comment.
Ok,And this problem will be fix in the next commit.
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class InnerLoggerFactory extends InternalLoggerFactory { |
There was a problem hiding this comment.
what's different between inner and internal
There was a problem hiding this comment.
Internallogger is basic api used in RocketMQ modules,and InnerLogger is an impl of Internallogger
There was a problem hiding this comment.
Why we need an InnerLoggerFactory? It seems that Slf4jLoggerFactory as the default LoggerFactory is enough.
| return delimeterStartIndex >= 2 && messagePattern.charAt(delimeterStartIndex - 2) == 92; | ||
| } | ||
|
|
||
| private static void deeplyAppendParameter(StringBuilder sbuf, Object o, Map<Object[], Object> seenMap) { |
There was a problem hiding this comment.
Do we refer other exist implementation in Apache project or other projects, if that case, we must be cautious about our license and notice files :-)
There was a problem hiding this comment.
Thx for reminding. license and notice files are declare in package-info.java
|
well done, glad to see much polishes in the following commits |
|
BasicloggerTest should follow camel naming convention, aka, BasicLoggerTest |
| } | ||
| } | ||
|
|
||
| static boolean isEscapedDelimeter(String messagePattern, int delimeterStartIndex) { |
There was a problem hiding this comment.
typo: isEscapedDelimeter --> isEscapeDelimiter
There was a problem hiding this comment.
Thx,This will be fixed in the next commit.
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class InnerLoggerFactory extends InternalLoggerFactory { |
There was a problem hiding this comment.
Why we need an InnerLoggerFactory? It seems that Slf4jLoggerFactory as the default LoggerFactory is enough.
| @@ -0,0 +1,93 @@ | |||
| /* | |||
There was a problem hiding this comment.
It seems that , rocketmq's module including client,broker,namesrv, dose not adopt this InternalLoggerFactory.
When and how this new module is integrated with the other modules.
There was a problem hiding this comment.
This is Logging module,In the next PR,Slf4j api used in broker, common,remoting and other modules will be changed to InternalLoggerFactory
|
@dongeforever InnerLoggerFactory is used for client logging. As slf4j log configuration file may conflict with other logging system, InnerLoggerFactory is used to solve this probelm. |
….rocketmq.store.CommitLog#topicQueueTable in memory.
What is the purpose of the change
As logging is widely used in rocketmq such as in broker ,nameserver, client and slf4j is used as default logging module. In some cases, we want to use other logging system not slf4j, we have to merge log config file to use slf4j. In my opinion, a logging module is needed in this case.
Brief changelog
Verifying this change
This is a fully unit test driven PR.
Follow this checklist to help us incorporate your contribution quickly and easily:
[ROCKETMQ-XXX] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyleto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.