z3d1k commented on code in PR #139:
URL:
https://github.com/apache/flink-connector-aws/pull/139#discussion_r1604612066
##########
flink-connector-aws/flink-connector-aws-kinesis-firehose/src/main/java/org/apache/flink/connector/firehose/sink/KinesisFirehoseSinkWriter.java:
##########
@@ -220,9 +220,8 @@ private void handleFullyFailedRequest(
}
LOG.warn(
- "KDF Sink failed to write and will retry {} entries to KDF
first request was {}",
+ "KDF Sink failed to write and will retry {} entries to KDF",
Review Comment:
Logging the first entry in warn log here has several issues:
- records can be very big, bloating logs
- records are in serialised format (i.e. byte data), which has questionable
value.
If this data is required, this can be logged separately at debug level to
control log granularity
##########
flink-connector-aws/flink-connector-aws-kinesis-firehose/src/main/java/org/apache/flink/connector/firehose/sink/KinesisFirehoseSinkWriter.java:
##########
@@ -220,9 +220,8 @@ private void handleFullyFailedRequest(
}
LOG.warn(
- "KDF Sink failed to write and will retry {} entries to KDF
first request was {}",
+ "KDF Sink failed to write and will retry {} entries to KDF",
Review Comment:
Logging the first entry in warn log here has several issues:
- records can be very big, bloating logs
- records are in serialised format (i.e. byte data), which has questionable
value.
If this data is required, this can be logged separately at debug level to
control log granularity
--
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]
