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