valdar commented on a change in pull request #193:
URL:
https://github.com/apache/camel-kafka-connector/pull/193#discussion_r420762557
##########
File path:
core/src/test/java/org/apache/camel/kafkaconnector/CamelSourceTaskTest.java
##########
@@ -179,7 +182,7 @@ public void testSourcePollingTimeout() throws
InterruptedException {
CamelSourceTask camelSourceTask = new CamelSourceTask();
camelSourceTask.start(props);
- long sleepTime = 30L;
+ long sleepTime = 11L;
Review comment:
If that is the case I think it should be addressed in a separate PR
after opening an issue.
##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -122,7 +122,14 @@ public void put(Collection<SinkRecord> sinkRecords) {
}
exchange.getMessage().setHeaders(headers);
exchange.getMessage().setBody(record.value());
- LOG.debug("Sending {} to {}", exchange, LOCAL_URL);
+
+ if (LOG.isDebugEnabled()) {
Review comment:
I seem to remember that in modern logging frameworks this is done
already by using the logging placeholders syntax (i.e.`LOG.debug("... {}",
yourValue);`) so it should be useless and just pollute the code, but I might be
wrong.
##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -122,7 +122,14 @@ public void put(Collection<SinkRecord> sinkRecords) {
}
exchange.getMessage().setHeaders(headers);
exchange.getMessage().setBody(record.value());
- LOG.debug("Sending {} to {}", exchange, LOCAL_URL);
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Sending exchange {} to {}",
exchange.getExchangeId(), LOCAL_URL);
+ }
+ if
(config.getBoolean(CamelSinkConnectorConfig.CAMEL_SINK_LOG_PAYLOAD_CONF)) {
+ LOG.info(TaskHelper.getExchangePayload(exchange));
Review comment:
As @davsclaus pointed out in a comment, sensitive data can be in other
places than the body and is not clear from the option name that, if enabled,
you log at info level (I would have left it to debug and harmonized somehow
with the previuse logging statement in order not to duplicate logging entries).
##########
File path:
core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkConnectorConfig.java
##########
@@ -38,10 +38,16 @@
public static final String CAMEL_SINK_URL_DOC = "The camel url to
configure the destination. If this is set " + CAMEL_SINK_COMPONENT_CONF
+ " and all the properties starting with " +
CamelSinkTask.getCamelSinkEndpointConfigPrefix() + ".<" +
CAMEL_SINK_COMPONENT_CONF + " value> are ignored.";
+ // do not log record's payload by default, as it may contain sensitive
information
+ public static final Boolean CAMEL_SINK_LOG_PAYLOAD_DEFAULT = false;
+ public static final String CAMEL_SINK_LOG_PAYLOAD_CONF =
"camel.source.logPayload";
Review comment:
This I think should be `"camel.sink.logPayload"` ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
