xiangfu0 commented on code in PR #20112:
URL: https://github.com/apache/pinot/pull/20112#discussion_r4434217122
##########
pinot-plugins/pinot-stream-ingestion/pinot-kafka-base/src/main/java/org/apache/pinot/plugin/stream/kafka/KafkaConfigUtils.java:
##########
@@ -48,15 +59,92 @@ private KafkaConfigUtils() {
public static Properties filterAndValidateKafkaProperties(Properties
properties, Set<String> validConfigNames) {
Properties filteredProperties = new Properties();
for (String key : properties.stringPropertyNames()) {
- if (validConfigNames.contains(key) ||
key.equals(AbstractConfig.CONFIG_PROVIDERS_CONFIG)
+ if (validConfigNames.contains(key) ||
(key.equals(AUTO_INCLUDE_JMX_REPORTER_CONFIG)
+ &&
validConfigNames.contains(CommonClientConfigs.METRIC_REPORTER_CLASSES_CONFIG))
+ || key.equals(AbstractConfig.CONFIG_PROVIDERS_CONFIG)
|| key.startsWith(CONFIG_PROVIDERS_PREFIX)) {
filteredProperties.put(key, properties.get(key));
}
}
validateConfigProviderReferences(filteredProperties);
+ normalizeLegacyProperties(filteredProperties);
return filteredProperties;
}
+ private static void normalizeLegacyProperties(Properties properties) {
+ boolean hasLegacyJmxSetting =
properties.containsKey(AUTO_INCLUDE_JMX_REPORTER_CONFIG);
Review Comment:
MINOR: The translation only runs when `auto.include.jmx.reporter` is set
explicitly. In Kafka 3 that flag defaulted to `true`. So a table that sets only
a custom `metric.reporters` used to get `[Custom, JmxReporter]` on
kafka-clients 3.9.2, and now gets `[Custom]` on 4.2.0 (verified locally). JMX
consumer metrics such as records-lag then disappear without any warning.
Suggest treating an absent flag as `true` whenever `metric.reporters` is
explicitly configured. That would also change the `{null, custom}` test case.
AI-generated review via IntentLab ยท Model: Claude Opus 5.5 (claude-opus-5-5).
--
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]
