kirktrue commented on code in PR #17993:
URL: https://github.com/apache/kafka/pull/17993#discussion_r1884689959
##########
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java:
##########
@@ -117,8 +117,10 @@ public AbstractConfig(ConfigDef definition, Map<?, ?>
originals, Map<String, ?>
this.values.putAll(configUpdates);
definition.parse(this.values);
this.definition = definition;
- if (doLog)
- logAll();
+ if (doLog) {
+ Map<String, Object> loggingConfig =
clearUnsupportedConfigsForLogging(this.values);
+ logAll(loggingConfig);
+ }
Review Comment:
Is it possible to invoke `clearUnsupportedConfigsForLogging()` inside
`logAll()` and thus the method signature can remain as is?
##########
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java:
##########
@@ -355,13 +357,20 @@ public Map<String, Object>
valuesWithPrefixAllOrNothing(String prefix) {
return nonInternalConfigs;
}
- private void logAll() {
+ /**
+ * Won't do any filter in the abstract config, but can be overridden in
subclasses.
+ */
+ protected Map<String, Object>
clearUnsupportedConfigsForLogging(Map<String, Object> values) {
+ return new TreeMap<>(values);
+ }
+
+ private void logAll(Map<String, Object> values) {
StringBuilder b = new StringBuilder();
b.append(getClass().getSimpleName());
b.append(" values: ");
b.append(Utils.NL);
Review Comment:
That way we can invoke this from inside `logAll()`, keeping the logging bits
closer together.
```suggestion
Map<String, Object> valuesToLog -
clearUnsupportedConfigsForLogging(values);
```
--
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
