[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6037: Add list of allowed tables for emitting table level metrics

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

mcvsubbu commented on a change in pull request #6037:
URL: https://github.com/apache/incubator-pinot/pull/6037#discussion_r492959921



##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##########
@@ -279,7 +281,10 @@
     }
 
     public static final String DEFAULT_METRICS_PREFIX = "pinot.server.";
-    public static final boolean DEFAULT_METRICS_GLOBAL_ENABLED = false;
+    public static final String CONFIG_OF_ENABLE_TABLE_LEVEL_METRICS = "pinot.server.enableTableLevelMetrics";

Review comment:
       I think you are trying to keep the new configs you introduced in the same place. It is unfortunate that we don't have a BrokerConf (as yet), but let us keep the server-specific configs in the ServerConf file instead of moving that up to common constants. 

##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
##########
@@ -47,21 +53,36 @@
 
   private final Map<String, AtomicLong> _gaugeValues = new ConcurrentHashMap<String, AtomicLong>();
 
-  protected final boolean _global;
+  private final boolean _isTableLevelMetricsEnabled;

Review comment:
       thanks for changing this name 




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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org