[GitHub] [kafka] mimaison commented on a change in pull request #10807: KAFKA-12797: Log the evictor of fetch sessions

[GitBox <gi...@apache.org>]

mimaison commented on a change in pull request #10807:
URL: https://github.com/apache/kafka/pull/10807#discussion_r683367337



##########
File path: core/src/main/scala/kafka/server/KafkaApis.scala
##########
@@ -692,6 +692,7 @@ class KafkaApis(val requestChannel: RequestChannel,
       fetchRequest.version,
       fetchRequest.metadata,
       fetchRequest.isFromFollower,
+      s"clientId=${request.context.clientId}, principal=${request.context.principal}",

Review comment:
       It's not ideal that we have to build the string even if we don't use it.
   
   In practice, this extra logging is useful if there's a malicious user forcing sessions to roll or if a user is using a broken client (like Sarama 1.26.0). So I wonder if we really need the clientId. While it's nice to have, it's a user-controlled field so this could be problematic for large values. WDYT?




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org