You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/10/26 10:50:49 UTC
[GitHub] [rocketmq] ShadowySpirits opened a new pull request, #5407: [ISSUE #5354] Implement proxy metrics framework
ShadowySpirits opened a new pull request, #5407:
URL: https://github.com/apache/rocketmq/pull/5407
Please take a look. @lollipopjin @MatrixHB
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter commented on pull request #5407: [ISSUE #5354] [RIP-46] Implement proxy metrics framework
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #5407:
URL: https://github.com/apache/rocketmq/pull/5407#issuecomment-1293209677
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/5407?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#5407](https://codecov.io/gh/apache/rocketmq/pull/5407?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9036c9) into [develop](https://codecov.io/gh/apache/rocketmq/commit/a7c1d49454ce405fc651f95d036571d91bd8c9e0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a7c1d49) will **decrease** coverage by `0.08%`.
> The diff coverage is `20.37%`.
```diff
@@ Coverage Diff @@
## develop #5407 +/- ##
=============================================
- Coverage 43.18% 43.09% -0.09%
- Complexity 8009 8024 +15
=============================================
Files 1016 1020 +4
Lines 71535 71861 +326
Branches 9467 9517 +50
=============================================
+ Hits 30891 30971 +80
- Misses 36751 36966 +215
- Partials 3893 3924 +31
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/5407?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/rocketmq/broker/BrokerController.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvQnJva2VyQ29udHJvbGxlci5qYXZh) | `43.76% <0.00%> (-2.27%)` | :arrow_down: |
| [...rocketmq/broker/metrics/BrokerMetricsConstant.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbWV0cmljcy9Ccm9rZXJNZXRyaWNzQ29uc3RhbnQuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...e/rocketmq/proxy/metrics/ProxyMetricsConstant.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Byb3h5L21ldHJpY3MvUHJveHlNZXRyaWNzQ29uc3RhbnQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../rocketmq/broker/metrics/BrokerMetricsManager.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbWV0cmljcy9Ccm9rZXJNZXRyaWNzTWFuYWdlci5qYXZh) | `13.26% <7.14%> (-0.14%)` | :arrow_down: |
| [.../java/org/apache/rocketmq/common/BrokerConfig.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQnJva2VyQ29uZmlnLmphdmE=) | `28.23% <17.64%> (-0.06%)` | :arrow_down: |
| [...he/rocketmq/proxy/metrics/ProxyMetricsManager.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Byb3h5L21ldHJpY3MvUHJveHlNZXRyaWNzTWFuYWdlci5qYXZh) | `18.51% <18.51%> (ø)` | |
| [.../org/apache/rocketmq/proxy/config/ProxyConfig.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Byb3h5L2NvbmZpZy9Qcm94eUNvbmZpZy5qYXZh) | `47.15% <28.30%> (-2.59%)` | :arrow_down: |
| [...n/java/org/apache/rocketmq/proxy/ProxyStartup.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJveHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Byb3h5L1Byb3h5U3RhcnR1cC5qYXZh) | `48.43% <100.00%> (+1.23%)` | :arrow_up: |
| [...va/org/apache/rocketmq/store/FlushDiskWatcher.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0ZsdXNoRGlza1dhdGNoZXIuamF2YQ==) | `81.25% <0.00%> (-9.38%)` | :arrow_down: |
| [...org/apache/rocketmq/common/stats/StatsItemSet.java](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNJdGVtU2V0LmphdmE=) | `41.79% <0.00%> (-8.96%)` | :arrow_down: |
| ... and [26 more](https://codecov.io/gh/apache/rocketmq/pull/5407/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] lizhanhui merged pull request #5407: [ISSUE #5354] [RIP-46] Implement proxy metrics framework
Posted by GitBox <gi...@apache.org>.
lizhanhui merged PR #5407:
URL: https://github.com/apache/rocketmq/pull/5407
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] lizhanhui commented on a diff in pull request #5407: [ISSUE #5354] [RIP-46] Implement proxy metrics framework
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on code in PR #5407:
URL: https://github.com/apache/rocketmq/pull/5407#discussion_r1006539426
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java:
##########
@@ -108,26 +113,26 @@ private boolean checkConfig() {
private void init() {
if (!checkConfig()) {
- LOGGER.error("check broker metrics config failed, will not export metrics");
+ LOGGER.error("check metrics config failed, will not export metrics");
return;
}
- String labels = brokerConfig.getBrokerMetricsLabel();
+ String labels = brokerConfig.getMetricsLabel();
if (StringUtils.isNotBlank(labels)) {
List<String> kvPairs = Splitter.on(',').omitEmptyStrings().splitToList(labels);
for (String item : kvPairs) {
String[] split = item.split(":");
if (split.length != 2) {
- LOGGER.warn("brokerMetricsLabel is not valid: {}", brokerConfig.getBrokerMetricsLabel());
+ LOGGER.warn("metricsLabel is not valid: {}", brokerConfig.getMetricsLabel());
continue;
}
LABEL_MAP.put(split[0], split[1]);
}
}
- if (brokerConfig.isBrokerMetricsPreferDelta()) {
+ if (brokerConfig.isMetricsPreferDelta()) {
Review Comment:
> I want to keep all metrics related config prefixed with "metrics". Do you have any better suggestions?
How about isMetricInDelta?
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] lizhanhui commented on a diff in pull request #5407: [ISSUE #5354] [RIP-46] Implement proxy metrics framework
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on code in PR #5407:
URL: https://github.com/apache/rocketmq/pull/5407#discussion_r1006505619
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java:
##########
@@ -108,26 +113,26 @@ private boolean checkConfig() {
private void init() {
if (!checkConfig()) {
- LOGGER.error("check broker metrics config failed, will not export metrics");
+ LOGGER.error("check metrics config failed, will not export metrics");
return;
}
- String labels = brokerConfig.getBrokerMetricsLabel();
+ String labels = brokerConfig.getMetricsLabel();
if (StringUtils.isNotBlank(labels)) {
List<String> kvPairs = Splitter.on(',').omitEmptyStrings().splitToList(labels);
for (String item : kvPairs) {
String[] split = item.split(":");
if (split.length != 2) {
- LOGGER.warn("brokerMetricsLabel is not valid: {}", brokerConfig.getBrokerMetricsLabel());
+ LOGGER.warn("metricsLabel is not valid: {}", brokerConfig.getMetricsLabel());
continue;
}
LABEL_MAP.put(split[0], split[1]);
}
}
- if (brokerConfig.isBrokerMetricsPreferDelta()) {
+ if (brokerConfig.isMetricsPreferDelta()) {
Review Comment:
isMetricsPreferDelta --> isDeltaMetricPerferred
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ShadowySpirits commented on a diff in pull request #5407: [ISSUE #5354] [RIP-46] Implement proxy metrics framework
Posted by GitBox <gi...@apache.org>.
ShadowySpirits commented on code in PR #5407:
URL: https://github.com/apache/rocketmq/pull/5407#discussion_r1006511417
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java:
##########
@@ -108,26 +113,26 @@ private boolean checkConfig() {
private void init() {
if (!checkConfig()) {
- LOGGER.error("check broker metrics config failed, will not export metrics");
+ LOGGER.error("check metrics config failed, will not export metrics");
return;
}
- String labels = brokerConfig.getBrokerMetricsLabel();
+ String labels = brokerConfig.getMetricsLabel();
if (StringUtils.isNotBlank(labels)) {
List<String> kvPairs = Splitter.on(',').omitEmptyStrings().splitToList(labels);
for (String item : kvPairs) {
String[] split = item.split(":");
if (split.length != 2) {
- LOGGER.warn("brokerMetricsLabel is not valid: {}", brokerConfig.getBrokerMetricsLabel());
+ LOGGER.warn("metricsLabel is not valid: {}", brokerConfig.getMetricsLabel());
continue;
}
LABEL_MAP.put(split[0], split[1]);
}
}
- if (brokerConfig.isBrokerMetricsPreferDelta()) {
+ if (brokerConfig.isMetricsPreferDelta()) {
Review Comment:
I want to keep all metrics related config prefixed with "metrics". Do you have any better suggestions?
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ShadowySpirits commented on a diff in pull request #5407: [ISSUE #5354] [RIP-46] Implement proxy metrics framework
Posted by GitBox <gi...@apache.org>.
ShadowySpirits commented on code in PR #5407:
URL: https://github.com/apache/rocketmq/pull/5407#discussion_r1006557591
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java:
##########
@@ -108,26 +113,26 @@ private boolean checkConfig() {
private void init() {
if (!checkConfig()) {
- LOGGER.error("check broker metrics config failed, will not export metrics");
+ LOGGER.error("check metrics config failed, will not export metrics");
return;
}
- String labels = brokerConfig.getBrokerMetricsLabel();
+ String labels = brokerConfig.getMetricsLabel();
if (StringUtils.isNotBlank(labels)) {
List<String> kvPairs = Splitter.on(',').omitEmptyStrings().splitToList(labels);
for (String item : kvPairs) {
String[] split = item.split(":");
if (split.length != 2) {
- LOGGER.warn("brokerMetricsLabel is not valid: {}", brokerConfig.getBrokerMetricsLabel());
+ LOGGER.warn("metricsLabel is not valid: {}", brokerConfig.getMetricsLabel());
continue;
}
LABEL_MAP.put(split[0], split[1]);
}
}
- if (brokerConfig.isBrokerMetricsPreferDelta()) {
+ if (brokerConfig.isMetricsPreferDelta()) {
Review Comment:
> How about isMetricInDelta?
I have done this modification.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org