Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #5678] Add logging exporter for metrics #6234

Merged
merged 5 commits into from
Mar 15, 2023

Conversation

lrybbs
Copy link
Contributor

@lrybbs lrybbs commented Mar 3, 2023

I have completed the issue #5678 @ShadowySpirits , now broker and proxy could export metrics in terminal.

@@ -376,12 +379,13 @@ public boolean isEnable() {
}
}

private MetricsExporterType metricsExporterType = MetricsExporterType.DISABLE;
private MetricsExporterType metricsExporterType = MetricsExporterType.LOGGER;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not change the default value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaron-ai Could you please approve running the ci? And we need to add a jul to sl4j bridge in aliyunmq/rocketmq-shaded-slf4j-api-bridge.

Copy link
Member

@aaron-ai aaron-ai Mar 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually there is an official bridge from SLF4j, @lrybbs just try it out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually there is an official bridge from SLF4j, @lrybbs just try it out.

You are right, but we should make it compatible with shaded sl4j. Can you give some guide for @lrybbs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since #5959 has been merge, the unshaded SLF4j is compatible with the shaded SLF4j we introduced now. This means that after using the jul-to-slf4j bridge, the logs can be automatically redirected to the shaded SLF4j.

Here is the related link you could refer to: https://stackoverflow.com/questions/9117030/jul-to-slf4j-bridge

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaron-ai Now, broker metrics and proxy metric could output to "rocketmqlogs" folder. Their names are "broker_metric.log" and "proxy_metric.log", respectively.

@aaron-ai aaron-ai requested a review from drpmma March 3, 2023 06:03
@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2023

Codecov Report

Attention: Patch coverage is 10.25641% with 35 lines in your changes missing coverage. Please review.

Project coverage is 43.21%. Comparing base (543c118) to head (2d90cfe).
Report is 736 commits behind head on develop.

Files with missing lines Patch % Lines
.../rocketmq/broker/metrics/BrokerMetricsManager.java 0.00% 14 Missing ⚠️
...he/rocketmq/proxy/metrics/ProxyMetricsManager.java 0.00% 14 Missing ⚠️
.../java/org/apache/rocketmq/common/BrokerConfig.java 42.85% 4 Missing ⚠️
.../org/apache/rocketmq/proxy/config/ProxyConfig.java 25.00% 3 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #6234      +/-   ##
=============================================
+ Coverage      43.18%   43.21%   +0.02%     
- Complexity      8851     8863      +12     
=============================================
  Files           1094     1094              
  Lines          77184    77230      +46     
  Branches       10070    10074       +4     
=============================================
+ Hits           33334    33374      +40     
- Misses         39684    39696      +12     
+ Partials        4166     4160       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Now, broker metrics and proxy metric could output to "rocketmqlogs" folder. Their names are "broker_metric.log" and "proxy_metric.log", respectively.
Copy link
Contributor

@drpmma drpmma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aaron-ai aaron-ai changed the title Add logging exporter for metrics [ISSUE #5678] Add logging exporter for metrics Mar 15, 2023
@aaron-ai aaron-ai merged commit 616885c into apache:develop Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants