KAFKA-10525: Emit JSONs with new auto-generated schema#9526
KAFKA-10525: Emit JSONs with new auto-generated schema#9526dajac merged 22 commits intoapache:trunkfrom
Conversation
There was a problem hiding this comment.
@anatasiavela Thanks for the PR. Overall, the PR looks good. I have left some comments/questions.
anatasiavela
force-pushed
the
KAFKA-10525
branch
from
f93a245 to
b47a9b1
Compare
dajac
left a comment
dajac
left a comment
There was a problem hiding this comment.
@anatasiavela Thanks for the update. I have left few more comments.
dajac
left a comment
dajac
left a comment
There was a problem hiding this comment.
@anatasiavela Thanks for the update! It looks pretty good now. There are few things to fix and to improve but I think that we are on the right track wrt. the overall approach. I have left some more comments.
dajac
left a comment
dajac
left a comment
There was a problem hiding this comment.
@anatasiavela Thanks for the update. The PR looks good overall. I have left minor suggestions and one question.
|
@anatasiavela Both PRs have been merged so we can proceed with this one. There is something that we must consider that I was not aware of: https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java#L235. When the ProduceRequest is processed in the KafkaApis layer, its internal data is set to null to free up the memory. That means that we won't have it to log the request. We need to take this into account. |
anatasiavela
force-pushed
the
KAFKA-10525
branch
from
bc7c61d to
db4cbab
Compare
Kafka’s request and response traces currently output in a format that is JSON-like and are not easily parsable. There is a new auto-generated schema for each request type that supports outputting JSON payloads for request and response payloads. These can be adapted to provide structured request tracing.
dajac
left a comment
dajac
left a comment
There was a problem hiding this comment.
@anatasiavela Thanks for the updates. I think that we are very close. I have left few small comments and suggestions. Could you also rebase the PR please?
There was a problem hiding this comment.
I wonder if we should name the field %sSizeInBytes. I just looked at the result and having "records":83 in the request log is not super clear to me.
There was a problem hiding this comment.
I do see the issue of it not being super clear, but I don't think we can change the field name from here. Doing %sSizeInBytes would just add the name at the end of the line which would result in a compilation error. Unless you mean to change the field name in the .json file, but it would change the name for both the serialize and non-serialize case.
There was a problem hiding this comment.
Would something like the following work?
buffer.printf("_node.set(\"%sSizeInBytes\", new IntNode(%s.sizeInBytes()));%n",
target.field().camelCaseName(),
target.sourceVariable());
9f176ff to
97072bb
Compare
dajac
left a comment
dajac
left a comment
There was a problem hiding this comment.
@anatasiavela Thanks for the updates. The PR looks pretty good. I have left few more minor comments.
There was a problem hiding this comment.
Would something like the following work?
buffer.printf("_node.set(\"%sSizeInBytes\", new IntNode(%s.sizeInBytes()));%n",
target.field().camelCaseName(),
target.sourceVariable());
dajac
left a comment
dajac
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your contribution, @anatasiavela!
…keys * apache-github/trunk: KAFKA-10776: Add version attribute in RequestsPerSec metrics documentation (apache#9661) KAFKA-10854: fix flaky testConnectionRatePerIp test (apache#9752) KAFKA-10525: Emit JSONs with new auto-generated schema (KIP-673) (apache#9526)
4 participants
Kafka’s request and response traces currently output in a format that is JSON-like and are not easily parsable. There is a new auto-generated schema for each request type that supports outputting JSON payloads for request and response payloads. These can be adapted to provide structured request tracing.
Includes tests that iterate through all the request types and ensure we handle all of them in RequestConvertToJson.
KIP-673