[GitHub] [ratis] adoroszlai commented on a change in pull request #481: RATIS-1381. Remove MetricsRegistries warning for creating without adding reporters

[GitBox <gi...@apache.org>]

adoroszlai commented on a change in pull request #481:
URL: https://github.com/apache/ratis/pull/481#discussion_r649927041



##########
File path: ratis-metrics/src/main/java/org/apache/ratis/metrics/RatisMetrics.java
##########
@@ -31,9 +31,16 @@
   protected RatisMetricRegistry registry;
 
   protected static RatisMetricRegistry create(MetricRegistryInfo info) {
+    return create(info, true);
+  }
+
+  protected static RatisMetricRegistry create(MetricRegistryInfo info,
+      boolean shouldDebugLog) {
     Optional<RatisMetricRegistry> metricRegistry = MetricRegistries.global().get(info);
     return metricRegistry.orElseGet(() -> {
-      LOG.info("Creating Metrics Registry : {}", info.getName());
+      if (shouldDebugLog) {

Review comment:
       Why not `LOG.isDebugEnabled()`?

##########
File path: ratis-metrics/src/main/java/org/apache/ratis/metrics/impl/MetricRegistriesImpl.java
##########
@@ -61,11 +61,6 @@ public MetricRegistriesImpl(MetricRegistryFactory factory) {
   @Override
   public RatisMetricRegistry create(MetricRegistryInfo info) {
     return registries.put(info, () -> {
-      if (reporterRegistrations.isEmpty()) {
-        LOG.warn(
-            "First MetricRegistry has been created without registering reporters. You may need to call" +
-                " MetricRegistries.global().addReporterRegistration(...) before.");
-      }

Review comment:
       Doesn't the warning point to a real problem?  Are client message metrics being reported?




-- 
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:
users@infra.apache.org