You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dajac (via GitHub)" <gi...@apache.org> on 2025/01/24 16:22:58 UTC

[PR] (WIP) KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

dajac opened a new pull request, #18695:
URL: https://github.com/apache/kafka/pull/18695

   WIP - This is an attempt to refactor the ApiMessageFormatter to follow what we have done in https://github.com/apache/kafka/pull/18688. The main difference with the current implementation is that unknown versions of values are still deserialized on a best effort basis. The result depends on the schema. This makes me wonder whether we should strengthen the version validation in the CoordinatorRecordSerdes.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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


Re: [PR] (WIP) KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on code in PR #18695:
URL: https://github.com/apache/kafka/pull/18695#discussion_r1929843433


##########
tools/src/main/java/org/apache/kafka/tools/consumer/TransactionLogMessageFormatter.java:
##########
@@ -16,45 +16,36 @@
  */
 package org.apache.kafka.tools.consumer;
 
-import org.apache.kafka.common.errors.UnsupportedVersionException;
-import org.apache.kafka.common.protocol.ByteBufferAccessor;
-import org.apache.kafka.coordinator.transaction.generated.CoordinatorRecordType;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogKey;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogKeyJsonConverter;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogValue;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogValueJsonConverter;
+import org.apache.kafka.common.protocol.ApiMessage;
+import org.apache.kafka.coordinator.common.runtime.CoordinatorRecord;
+import org.apache.kafka.coordinator.common.runtime.CoordinatorRecordSerde;
+import org.apache.kafka.coordinator.transaction.TransactionCoordinatorRecordSerde;
+import org.apache.kafka.coordinator.transaction.generated.CoordinatorRecordJsonConverters;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.node.NullNode;
-import com.fasterxml.jackson.databind.node.TextNode;
 
 import java.nio.ByteBuffer;
 
-public class TransactionLogMessageFormatter extends ApiMessageFormatter {
+public class TransactionLogMessageFormatter extends CoordinatorRecordMessageFormatter {
+    private CoordinatorRecordSerde serde = new TransactionCoordinatorRecordSerde();
+
+    @Override
+    protected CoordinatorRecord deserialize(ByteBuffer key, ByteBuffer value) {
+        return serde.deserialize(key, value);
+    }
+
+    @Override
+    protected boolean shouldPrint(short recordType) {
+        return true;

Review Comment:
   I have a question, could you help me understand why this is `true` instead of `CoordinatorRecordType. TRANSACTION_LOG.id() == recordType`?



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


Re: [PR] KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18695:
URL: https://github.com/apache/kafka/pull/18695#discussion_r2013554650


##########
tools/src/test/java/org/apache/kafka/tools/consumer/CoordinatorRecordMessageFormatterTest.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.tools.consumer;
+
+import org.apache.kafka.clients.consumer.ConsumerRecord;
+import org.apache.kafka.common.MessageFormatter;
+import org.apache.kafka.common.header.internals.RecordHeaders;
+import org.apache.kafka.common.record.TimestampType;
+
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import static java.util.Collections.emptyMap;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)

Review Comment:
   It is used to allow using a non-static method in `@MethodSource`.



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


Re: [PR] (WIP) KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #18695:
URL: https://github.com/apache/kafka/pull/18695#issuecomment-2612918164

   @chia7712 What do you think about doing something like this?


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


Re: [PR] (WIP) KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #18695:
URL: https://github.com/apache/kafka/pull/18695#issuecomment-2747976021

   @chia7712 I finally had the time to update this one. Could you take another look when you get a chance?


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


Re: [PR] KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #18695:
URL: https://github.com/apache/kafka/pull/18695#issuecomment-2753506292

   @chia7712 Thanks for your comments. I just addressed them.


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


Re: [PR] KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac merged PR #18695:
URL: https://github.com/apache/kafka/pull/18695


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


Re: [PR] KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18695:
URL: https://github.com/apache/kafka/pull/18695#discussion_r2013553668


##########
tools/src/main/java/org/apache/kafka/tools/consumer/CoordinatorRecordMessageFormatter.java:
##########
@@ -31,44 +34,50 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-public abstract class ApiMessageFormatter implements MessageFormatter {
-
+public abstract class CoordinatorRecordMessageFormatter implements MessageFormatter {
     private static final String TYPE = "type";
     private static final String VERSION = "version";
     private static final String DATA = "data";
     private static final String KEY = "key";
     private static final String VALUE = "value";
-    static final String UNKNOWN = "unknown";
+
+    private final CoordinatorRecordSerde serde;
+
+    public CoordinatorRecordMessageFormatter(CoordinatorRecordSerde serde) {
+        this.serde = serde;
+    }
 
     @Override
     public void writeTo(ConsumerRecord<byte[], byte[]> consumerRecord, PrintStream output) {
+        if (Objects.isNull(consumerRecord.key())) return;
+
         ObjectNode json = new ObjectNode(JsonNodeFactory.instance);
+        try {
+            CoordinatorRecord record = serde.deserialize(
+                ByteBuffer.wrap(consumerRecord.key()),
+                consumerRecord.value() != null ? ByteBuffer.wrap(consumerRecord.value()) : null
+            );
+
+            if (!isRecordTypeAllowed(record.key().apiKey())) return;
 
-        byte[] key = consumerRecord.key();
-        if (Objects.nonNull(key)) {
-            short keyVersion = ByteBuffer.wrap(key).getShort();
-            JsonNode dataNode = readToKeyJson(ByteBuffer.wrap(key));
+            json
+                .putObject(KEY)
+                .put(TYPE, record.key().apiKey())

Review Comment:
   That's right. We will it in https://github.com/apache/kafka/pull/18510.



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


Re: [PR] (WIP) KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #18695:
URL: https://github.com/apache/kafka/pull/18695#issuecomment-2624051971

   > > This makes me wonder whether we should strengthen the version validation in the CoordinatorRecordSerdes.
   > 
   > If the version of the value record bumps in the future, formatters that lack version checks may print incorrect data. In such cases, throwing an accurate exception would be more appropriate.
   
   @chia7712 I agree. I opened https://github.com/apache/kafka/pull/18749 to address it.


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


Re: [PR] (WIP) KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18695:
URL: https://github.com/apache/kafka/pull/18695#discussion_r1929570741


##########
tools/src/main/java/org/apache/kafka/tools/consumer/OffsetsMessageFormatter.java:
##########
@@ -16,50 +16,41 @@
  */
 package org.apache.kafka.tools.consumer;
 
-import org.apache.kafka.common.errors.UnsupportedVersionException;
-import org.apache.kafka.common.protocol.ByteBufferAccessor;
+import org.apache.kafka.common.protocol.ApiMessage;
+import org.apache.kafka.coordinator.common.runtime.CoordinatorRecord;
+import org.apache.kafka.coordinator.common.runtime.CoordinatorRecordSerde;
+import org.apache.kafka.coordinator.group.GroupCoordinatorRecordSerde;
+import org.apache.kafka.coordinator.group.generated.CoordinatorRecordJsonConverters;
 import org.apache.kafka.coordinator.group.generated.CoordinatorRecordType;
-import org.apache.kafka.coordinator.group.generated.OffsetCommitKey;
-import org.apache.kafka.coordinator.group.generated.OffsetCommitKeyJsonConverter;
-import org.apache.kafka.coordinator.group.generated.OffsetCommitValue;
-import org.apache.kafka.coordinator.group.generated.OffsetCommitValueJsonConverter;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.node.NullNode;
-import com.fasterxml.jackson.databind.node.TextNode;
 
 import java.nio.ByteBuffer;
 
 /**
  * Formatter for use with tools such as console consumer: Consumer should also set exclude.internal.topics to false.
  */
-public class OffsetsMessageFormatter extends ApiMessageFormatter {
+public class OffsetsMessageFormatter extends CoordinatorRecordMessageFormatter {
+    private CoordinatorRecordSerde serde = new GroupCoordinatorRecordSerde();

Review Comment:
   Maybe we can move `serde` to `CoordinatorRecordMessageFormatter` like `CoordinatorRecordMessageParser`? 



##########
tools/src/main/java/org/apache/kafka/tools/consumer/CoordinatorRecordMessageFormatter.java:
##########
@@ -31,44 +34,44 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-public abstract class ApiMessageFormatter implements MessageFormatter {
+public abstract class CoordinatorRecordMessageFormatter implements MessageFormatter {
 
     private static final String TYPE = "type";
     private static final String VERSION = "version";
     private static final String DATA = "data";
     private static final String KEY = "key";
     private static final String VALUE = "value";
-    static final String UNKNOWN = "unknown";
 
     @Override
     public void writeTo(ConsumerRecord<byte[], byte[]> consumerRecord, PrintStream output) {
+        if (Objects.isNull(consumerRecord.key())) return;
+
         ObjectNode json = new ObjectNode(JsonNodeFactory.instance);
+        try {
+            CoordinatorRecord record = deserialize(
+                consumerRecord.key() != null ? ByteBuffer.wrap(consumerRecord.key()) : null,

Review Comment:
   line#47 has checked the null, so this check is redundant.



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


Re: [PR] KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18695:
URL: https://github.com/apache/kafka/pull/18695#discussion_r2011905649


##########
tools/src/test/java/org/apache/kafka/tools/consumer/CoordinatorRecordMessageFormatterTest.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.tools.consumer;
+
+import org.apache.kafka.clients.consumer.ConsumerRecord;
+import org.apache.kafka.common.MessageFormatter;
+import org.apache.kafka.common.header.internals.RecordHeaders;
+import org.apache.kafka.common.record.TimestampType;
+
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import static java.util.Collections.emptyMap;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+public abstract class CoordinatorRecordMessageFormatterTest {
+    private static final String TOPIC = "TOPIC";
+
+    protected abstract CoordinatorRecordMessageFormatter formatter();
+    protected abstract Stream<Arguments> parameters();
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    public void testMessageFormatter(byte[] keyBuffer, byte[] valueBuffer, String expectedOutput) {

Review Comment:
   Could you please add unit test for `UnknownRecordTypeException`?



##########
tools/src/test/java/org/apache/kafka/tools/consumer/CoordinatorRecordMessageFormatterTest.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.tools.consumer;
+
+import org.apache.kafka.clients.consumer.ConsumerRecord;
+import org.apache.kafka.common.MessageFormatter;
+import org.apache.kafka.common.header.internals.RecordHeaders;
+import org.apache.kafka.common.record.TimestampType;
+
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import static java.util.Collections.emptyMap;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)

Review Comment:
   Is it used to speedup those tests?



##########
tools/src/main/java/org/apache/kafka/tools/consumer/CoordinatorRecordMessageFormatter.java:
##########
@@ -31,44 +34,50 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-public abstract class ApiMessageFormatter implements MessageFormatter {
-
+public abstract class CoordinatorRecordMessageFormatter implements MessageFormatter {
     private static final String TYPE = "type";
     private static final String VERSION = "version";
     private static final String DATA = "data";
     private static final String KEY = "key";
     private static final String VALUE = "value";
-    static final String UNKNOWN = "unknown";
+
+    private final CoordinatorRecordSerde serde;
+
+    public CoordinatorRecordMessageFormatter(CoordinatorRecordSerde serde) {
+        this.serde = serde;
+    }
 
     @Override
     public void writeTo(ConsumerRecord<byte[], byte[]> consumerRecord, PrintStream output) {
+        if (Objects.isNull(consumerRecord.key())) return;
+
         ObjectNode json = new ObjectNode(JsonNodeFactory.instance);
+        try {
+            CoordinatorRecord record = serde.deserialize(
+                ByteBuffer.wrap(consumerRecord.key()),
+                consumerRecord.value() != null ? ByteBuffer.wrap(consumerRecord.value()) : null
+            );
+
+            if (!isRecordTypeAllowed(record.key().apiKey())) return;
 
-        byte[] key = consumerRecord.key();
-        if (Objects.nonNull(key)) {
-            short keyVersion = ByteBuffer.wrap(key).getShort();
-            JsonNode dataNode = readToKeyJson(ByteBuffer.wrap(key));
+            json
+                .putObject(KEY)
+                .put(TYPE, record.key().apiKey())

Review Comment:
   `ShareGroupStateMessageFormatter` use "version" rather than "type", but I guess we fix it in the follow-up by refactoring `ShareGroupStateMessageFormatter`?



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


Re: [PR] (WIP) KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on code in PR #18695:
URL: https://github.com/apache/kafka/pull/18695#discussion_r1929845481


##########
tools/src/main/java/org/apache/kafka/tools/consumer/TransactionLogMessageFormatter.java:
##########
@@ -16,45 +16,36 @@
  */
 package org.apache.kafka.tools.consumer;
 
-import org.apache.kafka.common.errors.UnsupportedVersionException;
-import org.apache.kafka.common.protocol.ByteBufferAccessor;
-import org.apache.kafka.coordinator.transaction.generated.CoordinatorRecordType;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogKey;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogKeyJsonConverter;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogValue;
-import org.apache.kafka.coordinator.transaction.generated.TransactionLogValueJsonConverter;
+import org.apache.kafka.common.protocol.ApiMessage;
+import org.apache.kafka.coordinator.common.runtime.CoordinatorRecord;
+import org.apache.kafka.coordinator.common.runtime.CoordinatorRecordSerde;
+import org.apache.kafka.coordinator.transaction.TransactionCoordinatorRecordSerde;
+import org.apache.kafka.coordinator.transaction.generated.CoordinatorRecordJsonConverters;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.node.NullNode;
-import com.fasterxml.jackson.databind.node.TextNode;
 
 import java.nio.ByteBuffer;
 
-public class TransactionLogMessageFormatter extends ApiMessageFormatter {
+public class TransactionLogMessageFormatter extends CoordinatorRecordMessageFormatter {
+    private CoordinatorRecordSerde serde = new TransactionCoordinatorRecordSerde();
+
+    @Override
+    protected CoordinatorRecord deserialize(ByteBuffer key, ByteBuffer value) {
+        return serde.deserialize(key, value);
+    }
+
+    @Override
+    protected boolean shouldPrint(short recordType) {
+        return true;

Review Comment:
   I think the reason is that there is only one element, TRANSACTION_LOG, in CoordinatorType, so there is no need to add extra checking here.



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


Re: [PR] KAFKA-18616; Refactor Tools's ApiMessageFormatter [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18695:
URL: https://github.com/apache/kafka/pull/18695#discussion_r2013562510


##########
tools/src/test/java/org/apache/kafka/tools/consumer/CoordinatorRecordMessageFormatterTest.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.tools.consumer;
+
+import org.apache.kafka.clients.consumer.ConsumerRecord;
+import org.apache.kafka.common.MessageFormatter;
+import org.apache.kafka.common.header.internals.RecordHeaders;
+import org.apache.kafka.common.record.TimestampType;
+
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import static java.util.Collections.emptyMap;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+public abstract class CoordinatorRecordMessageFormatterTest {
+    private static final String TOPIC = "TOPIC";
+
+    protected abstract CoordinatorRecordMessageFormatter formatter();
+    protected abstract Stream<Arguments> parameters();
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    public void testMessageFormatter(byte[] keyBuffer, byte[] valueBuffer, String expectedOutput) {

Review Comment:
   Sure. I added test cases to the concrete classes.



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