[ISSUE #323] Support the expansion of metrics data statistics in RocketMQ-Spring. by heihaozi · Pull Request #324 · apache/rocketmq-spring

heihaozi · 2020-12-09T12:25:22Z

What is the purpose of the change

Support the expansion of metrics data statistics in RocketMQ-Spring. This feature can enable:

Provide standard and diverse metrics representation(e.g. percentile, gauge, histogram).
Standardization of monitoring API and transport server API, and adapt to open-source monitoring components (e.g. Prometheus).
Integration with open-source visualization solution(e.g. Grafana).

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.

Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist.
Run mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

vongosling · 2020-12-10T02:18:53Z

pom.xml

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


why did you add prefix placeholder before the root directory /src?

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

vongosling · 2020-12-10T02:27:06Z

...ing-boot/src/main/java/org/apache/rocketmq/spring/core/DefaultLitePullConsumerWithTopic.java

+import org.apache.rocketmq.client.consumer.DefaultLitePullConsumer;
+import org.apache.rocketmq.remoting.RPCHook;
+
+public class DefaultLitePullConsumerWithTopic extends DefaultLitePullConsumer {


DefaultLitePullConsumerWithTopic is not a rational name here.

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?

Do not make a new class here.

Where can this new class be maked? What's your suggestion?

rocketmq-spring-boot/src/main/java/org/apache/rocketmq/spring/core/RocketMQTemplate.java

rocketmq-spring-boot/src/main/java/org/apache/rocketmq/spring/metric/EConsumerMode.java

vongosling · 2020-12-10T02:33:44Z

rocketmq-spring-boot/src/main/java/org/apache/rocketmq/spring/metric/MetricExtension.java

+     * @param topic topic name
+     * @param count count of message
+     */
+    void addProducerMessageCount(String topic, int count);


int or long should be consistent.

The parameter type is always int, and long is not used.

vongosling · 2020-12-10T02:36:16Z

...tmq-spring-boot/src/main/java/org/apache/rocketmq/spring/metric/MetricExtensionProvider.java

+import java.util.List;
+import java.util.ServiceLoader;
+
+public class MetricExtensionProvider {


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.

[1] https://github.com/apache/rocketmq/blob/master/broker/src/main/java/org/apache/rocketmq/broker/util/ServiceProvider.java

The ServiceProvider is in the broker module, RocketMQ-Spring has no dependency on the broker module, so it cannot be used directly.

You could copy and make some improvements. Pls make a reference description when you do it .

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

vongosling · 2020-12-10T02:38:57Z

rocketmq-spring-boot/src/test/java/org/apache/rocketmq/spring/metric/MetricExtensionTest.java

+
+    @Test
+    public void testAddProducerMessageCount() {
+        MetricExtensionProvider.addProducerMessageCount("topic1", 111);


It could be helpful for your code convincible if you could make more corner cases, such as have you simulated the count of failures?

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.

vongosling · 2020-12-11T11:02:57Z

Do we consider the possibility of using aop to finish this feature? @RongtongJin Would you like to help to design here?

vongosling linked an issue Dec 10, 2020 that may be closed by this pull request

Support the expansion of metrics data statistics in RocketMQ-Spring. #323

Open

vongosling added this to the 2.1.2 milestone Dec 10, 2020

vongosling reviewed Dec 10, 2020

View reviewed changes

heihaozi requested a review from vongosling December 11, 2020 08:04

RongtongJin added the discuss Discuss whether to support label Jan 2, 2021

RongtongJin removed this from the 2.1.2 milestone Jan 2, 2021

heihaozi added 7 commits June 21, 2021 11:34

Support the expansion of metrics data statistics in RocketMQ-Spring.

1c5669b

Add Apache License headers.

8b070b6

Fix the checkstyle error.

b52eaa8

Add exclusion rules for checkstyle.

958f1ba

Optimize code structure in MetricExtensionProvider.

0a435b2

Fix the checkstyle error.

efc25a7

Encapsulate EConsumerMode to the inner enum.

6c7a2de

Conversation

heihaozi commented Dec 9, 2020

What is the purpose of the change

Brief changelog

Verifying this change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vongosling Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vongosling commented Dec 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vongosling Dec 10, 2020 •

edited

Loading