avijayanhwx commented on a change in pull request #76:
URL: https://github.com/apache/incubator-ratis/pull/76#discussion_r420393046



##########
File path: 
ratis-metrics/src/main/java/org/apache/ratis/metrics/MetricsReporting.java
##########
@@ -34,17 +34,41 @@ private MetricsReporting() {
   }
 
   public static Consumer<RatisMetricRegistry> consoleReporter(TimeDuration 
rate) {
-    return ratisMetricRegistry -> 
ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
-        
.convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build()
-        .start(rate.getDuration(), rate.getUnit());
+    return ratisMetricRegistry -> {
+      ConsoleReporter reporter = 
ConsoleReporter.forRegistry(ratisMetricRegistry.getDropWizardMetricRegistry())
+          
.convertRatesTo(TimeUnit.SECONDS).convertDurationsTo(TimeUnit.MILLISECONDS).build();
+      reporter.start(rate.getDuration(), rate.getUnit());
+      ratisMetricRegistry.setConsoleReporter(reporter);
+    };
+  }
+
+  public static Consumer<RatisMetricRegistry> stopConsoleReporter() {

Review comment:
       @runzhiwang Thanks, but IMHO, given that the JMX and console reporters' 
implementation are present in Ratis code itself, we can expose an APIs that 
look like enableJmxReporter(), enableConsoleReporters(), and the current 
addReporterRegistration API can be used for external reporters like Prometheus. 
This keeps a client code simple, but provides scope for adding custom reporters.




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to