krishan1390 commented on code in PR #15387:
URL: https://github.com/apache/pinot/pull/15387#discussion_r2017899865


##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerMeter.java:
##########
@@ -18,195 +18,134 @@
  */
 package org.apache.pinot.common.metrics;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import org.apache.pinot.common.Utils;
-
+import org.apache.pinot.spi.exception.QueryErrorCode;
 
 /**
- * Enumeration containing all the metrics exposed by the Pinot broker.
+ * Class containing all the metrics exposed by the Pinot broker.

Review Comment:
   I couldn't figure out another way apart from above mentioned. tried creating 
a seperate class but AbstractMetrics restricts that. and didn't want to add 
that option of manual mistake if someone adds an error code but misses adding a 
metric. 
   
   Yea the class structure changes, but the usage of the class remains same. 
Clients still get to use same variables as earlier without any changes. and 
adding new metrics is also same effort as before by just creaing a new 
variable. 



-- 
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: [email protected]

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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to