[GitHub] [kafka] anatasiavela commented on a change in pull request #9526: KAFKA-10525: Emit JSONs with new auto-generated schema

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

anatasiavela commented on a change in pull request #9526:
URL: https://github.com/apache/kafka/pull/9526#discussion_r541240558



##########
File path: generator/src/main/java/org/apache/kafka/message/JsonConverterGenerator.java
##########
@@ -377,11 +387,24 @@ private void generateVariableLengthTargetToJson(Target target, Versions versions
                 headerGenerator.addImport(MessageGenerator.ARRAYS_CLASS);
                 buffer.printf("%s;%n", target.assignmentStatement(
                     String.format("new BinaryNode(Arrays.copyOf(%s, %s.length))",
-                        target.sourceVariable(), target.sourceVariable())));
+                            target.sourceVariable(), target.sourceVariable())));
             }
         } else if (target.field().type().isRecords()) {
             headerGenerator.addImport(MessageGenerator.BINARY_NODE_CLASS);
+            headerGenerator.addImport(MessageGenerator.INT_NODE_CLASS);
+            // KIP-673: When logging requests/responses, we do not serialize the record, instead we
+            // output its sizeInBytes, because outputting the bytes is not very useful and can be
+            // quite expensive. Otherwise, we will serialize the record.
+            buffer.printf("if (_serializeRecords) {%n");
+            buffer.incrementIndent();
             buffer.printf("%s;%n", target.assignmentStatement("new BinaryNode(new byte[]{})"));
+            buffer.decrementIndent();
+            buffer.printf("} else {%n");
+            buffer.incrementIndent();
+            buffer.printf("%s;%n", target.assignmentStatement(

Review 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.




----------------------------------------------------------------
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:
users@infra.apache.org