[ISSUE #323] Support the expansion of metrics data statistics in RocketMQ-Spring. #324
Conversation
| @@ -176,9 +176,10 @@ | |||
| <exclude>.github/**</exclude> | |||
| <exclude>src/test/resources/certs/*</exclude> | |||
| <exclude>src/test/**/*.log</exclude> | |||
| <exclude>src/test/resources/META-INF/service/*</exclude> | |||
| <exclude>**/src/main/resources/META-INF/services/*</exclude> | |||
There was a problem hiding this comment.
why did you add prefix placeholder before the root directory /src?
There was a problem hiding this comment.
I tried it. When running mvn -B clean apache-rat:check and building RocketMQ Spring Boot, the rocketmq-spring-boot-samples project will also be checked. If I do not add the prefix placeholder, an unapproved license error will be reported. Like this: https://travis-ci.org/github/apache/rocketmq-spring/builds/748732956
...ume-demo/src/main/java/org/apache/rocketmq/samples/springboot/AtomicLongMetricExtension.java
Outdated
Show resolved
Hide resolved
| import org.apache.rocketmq.client.consumer.DefaultLitePullConsumer; | ||
| import org.apache.rocketmq.remoting.RPCHook; | ||
|
|
||
| public class DefaultLitePullConsumerWithTopic extends DefaultLitePullConsumer { |
There was a problem hiding this comment.
DefaultLitePullConsumerWithTopic is not a rational name here.
There was a problem hiding this comment.
Compared with DefaultLitePullConsumer, this class just has one more Topic attribute. So, I named this class DefaultLitePullConsumerWithTopic. Do you have any better suggestions for the naming of this class?
There was a problem hiding this comment.
Do not make a new class here.
There was a problem hiding this comment.
Where can this new class be maked? What's your suggestion?
rocketmq-spring-boot/src/main/java/org/apache/rocketmq/spring/core/RocketMQTemplate.java
Outdated
Show resolved
Hide resolved
rocketmq-spring-boot/src/main/java/org/apache/rocketmq/spring/metric/EConsumerMode.java
Outdated
Show resolved
Hide resolved
| * @param topic topic name | ||
| * @param count count of message | ||
| */ | ||
| void addProducerMessageCount(String topic, int count); |
There was a problem hiding this comment.
int or long should be consistent.
There was a problem hiding this comment.
The parameter type is always int, and long is not used.
| import java.util.List; | ||
| import java.util.ServiceLoader; | ||
|
|
||
| public class MetricExtensionProvider { |
There was a problem hiding this comment.
Do not make class by yourself. I strongly recommend you use a similar service loader mode in RocketMQ[1]. It has many backoff strategies and verified in the production environment.
There was a problem hiding this comment.
The ServiceProvider is in the broker module, RocketMQ-Spring has no dependency on the broker module, so it cannot be used directly.
There was a problem hiding this comment.
You could copy and make some improvements. Pls make a reference description when you do it .
There was a problem hiding this comment.
ServiceLoader is a simple Service Provder Framework provided since JDK 1.6. I think the reference description is this: https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html
|
|
||
| @Test | ||
| public void testAddProducerMessageCount() { | ||
| MetricExtensionProvider.addProducerMessageCount("topic1", 111); |
There was a problem hiding this comment.
It could be helpful for your code convincible if you could make more corner cases, such as have you simulated the count of failures?
There was a problem hiding this comment.
This unit test is mainly used to test whether the SPI of MetricExtensionProvider is running normally.The scenario where the counting fails you mentioned was considered by the developer when implementing the MetricExtension interface.
|
Do we consider the possibility of using aop to finish this feature? @RongtongJin Would you like to help to design here? |
What is the purpose of the change
Support the expansion of metrics data statistics in RocketMQ-Spring. This feature can enable:
Brief changelog
Created an interface called MetricExtension to record the number of messages produced or consumed. By obtaining the implementation of this interface through SPI, developers can easily write their own metrics data statistics method.
Verifying this change
Follow this checklist to help us incorporate your contribution quickly and easily. Notice,
it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.[ISSUE #123] 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.