mimaison commented on code in PR #12046:
URL: https://github.com/apache/kafka/pull/12046#discussion_r942316745
##########
server-common/src/main/java/org/apache/kafka/server/metrics/KafkaYammerMetrics.java:
##########
@@ -53,16 +72,21 @@ public static MetricsRegistry defaultRegistry() {
}
private final MetricsRegistry metricsRegistry = new MetricsRegistry();
- private final FilteringJmxReporter jmxReporter = new
FilteringJmxReporter(metricsRegistry,
- metricName -> true);
+ private FilteringJmxReporter jmxReporter;
private KafkaYammerMetrics() {
- jmxReporter.start();
- Exit.addShutdownHook("kafka-jmx-shutdown-hook", jmxReporter::shutdown);
}
@Override
+ @SuppressWarnings("deprecation")
public void configure(Map<String, ?> configs) {
+ AbstractConfig config = new AbstractConfig(CONFIG_DEF, configs);
+ List<String> reporters =
config.getList(CommonClientConfigs.METRIC_REPORTER_CLASSES_CONFIG);
+ if
(config.getBoolean(CommonClientConfigs.AUTO_INCLUDE_JMX_REPORTER_CONFIG) ||
reporters.stream().anyMatch(r -> JmxReporter.class.getName().equals(r))) {
Review Comment:
It's not trivial to consolidate all call sites as they are slightly
different.
This instance and the one in `DynamicBrokerConfig` use `Map` objects for
configs. In `DynamicBrokerConfig`, reporters are in the Scala `Buffer` instead
of Java `List`. And finally depending on the file, the reporters are either
class names or class instances.
--
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
