Yunyung commented on code in PR #19068:
URL: https://github.com/apache/kafka/pull/19068#discussion_r2021264688


##########
clients/src/main/java/org/apache/kafka/server/quota/ClientQuotaCallback.java:
##########
@@ -24,6 +24,13 @@
 
 /**
  * Quota callback interface for brokers and controllers that enables 
customization of client quota computation.
+ * Implement {@link org.apache.kafka.common.metrics.Monitorable} to enable the 
callback to register metrics. 
+ * The following tags are automatically added to all metrics registered: 
+ * <ul>
+ *     <li><code>config</code> set to 
<code>clientQuotaCallback.class</code></li>
+ *     <li><code>class</code> set to the ClientQuotaCallback class name</li>
+ *     <li><code>role</code> set to broker/ controller, which indicates the 
role of the server</li>

Review Comment:
   Weird space here.
   broker/ controller -> broker/controller



##########
clients/clients-integration-tests/src/test/java/org/apache/kafka/server/quota/CustomQuotaCallbackTest.java:
##########
@@ -121,4 +154,22 @@ public void configure(Map<String, ?> configs) {
         }
 
     }
+
+    public static class MonitorableCustomQuotaCallback extends 
CustomQuotaCallback implements Monitorable {
+
+        public static MetricName metricName = null;
+        public static String name = "client quota callback";
+        public static String description = "client quota callback";

Review Comment:
   nit: I think making name and description different val would be better for 
distinct.



##########
core/src/main/scala/kafka/server/ClientQuotaManager.scala:
##########
@@ -137,23 +138,30 @@ object ClientQuotaManager {
  * @param quotaType Quota type of this quota manager
  * @param time @Time object to use
  * @param threadNamePrefix The thread prefix to use
- * @param clientQuotaCallback An optional @ClientQuotaCallback
+ * @param clientQuotaCallbackPlugin An optional @ClientQuotaCallback and 
+ *                                  warp it in a {@link 
org.apache.kafka.common.internals.Plugin}

Review Comment:
   wrap



##########
clients/clients-integration-tests/src/test/java/org/apache/kafka/server/quota/CustomQuotaCallbackTest.java:
##########
@@ -121,4 +154,22 @@ public void configure(Map<String, ?> configs) {
         }
 
     }
+
+    public static class MonitorableCustomQuotaCallback extends 
CustomQuotaCallback implements Monitorable {
+
+        public static MetricName metricName = null;
+        public static String name = "client quota callback";
+        public static String description = "client quota callback";
+
+        @Override
+        public void withPluginMetrics(PluginMetrics metrics) {
+            metricName = metrics.metricName(name, description, Map.of());
+            metrics.addMetric(metricName, (Gauge<Integer>) (config, now) -> 1);
+        }
+
+        @Override
+        public boolean updateClusterMetadata(Cluster cluster) {

Review Comment:
   Unused method.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to