KAFKA-19011: Improve EndToEndLatency Tool with argument parser and message key/header support by Rancho-7 · Pull Request #20301 · apache/kafka

Rancho-7 · 2025-08-04T13:50:23Z

jira: https://issues.apache.org/jira/browse/KAFKA-19011 kip:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-1172%3A+Improve+EndToEndLatency+tool

This PR improves the usability and maintainability of the
kafka-e2e-latency.sh tool:

Replaces fixed-index argument parsing with a proper argument parser
(joptsimple)
Adds support for configuring:
- -record-key-size: size of the message key
- -num-headers: number of headers per message
- -record-header-key-size: size of each header key
- -record-header-size: size of each header value
Renames existing arguments to align with Kafka CLI conventions:
- broker_list → bootstrap-server
- num_messages → num-records
- message_size_bytes → record-size
- properties_file → command-config

Reviewers: Jhen-Yung Hsu jhenyunghsu@gmail.com, Ken Huang
s7133700@gmail.com, Chia-Ping Tsai chia7712@gmail.com

…ssage key/header support

Rancho-7 · 2025-08-04T13:53:36Z

The updated usage of the tool with argument parser:

(base) lansg:bin/ (KAFKA-19011✗) $ ./kafka-e2e-latency.sh  --bootstrap-server localhost:9092 --topic test-topic --num-records 1000 --producer-acks 1 --record-size 512 --record-key-size 64 --num-headers 5 --record-header-key-size 64 --record-header-size 128
0       26.187541
Avg latency: 1.2853 ms
Percentiles: 50th = 1, 99th = 3, 99.9th = 26

It's compatible with the legacy version:

(base) lansg:bin/ (KAFKA-19011✗) $ ./kafka-e2e-latency.sh localhost:9092 test-topic 1000 1 512                                                       [21:21:28]
WARNING: Positional argument usage is deprecated and will be removed in Apache Kafka 5.0. Please use named arguments instead: --bootstrap-server, --topic, --num-records, --producer-acks, --record-size, --command-config
0       45.907666
Avg latency: 1.5202 ms
Percentiles: 50th = 1, 99th = 6, 99.9th = 88

Yunyung

Thanks for the PR. Partial review, some comments left. Also, could you add the KIP to the title or description?

Yunyung · 2025-08-04T16:17:37Z

+
+            // validate 'producer-acks'
+            String acksValue = options.valueOf(acksOpt);
+            if (!List.of("1", "all").contains(acksValue)) {


What if -1?

The legacy version only had these two options, so I keep it the same. Since -1 has the same effect as all, I think one is enough?

+1 to keep this behavior

Yunyung · 2025-08-04T16:28:06Z

    }
+
+    // Visible for testing
+    static String[] convertLegacyArgsIfNeeded(String[] args) throws Exception {


Should we add Deprecated tag?

Thanks for pointing that out! Added it.

chia7712 · 2025-08-07T16:12:26Z

@Rancho-7 could you please run e2e on you local?

Rancho-7 · 2025-08-12T09:44:07Z

@chia7712 I ran the tests with:

TC_PATHS="tests/kafkatest/sanity_checks/test_performance_services.py tests/kafkatest/benchmarks/core/benchmark_test.py" bash tests/docker/run_tests.sh

e2e result:

kafkatest@fair-sun-wilts-fin-03:/opt/kafka/kafka/results/latest$ cat report.txt
====================================================================================================
SESSION REPORT (ALL TESTS)
ducktape version: 0.12.0
session_id:       2025-08-12--002
run time:         73 minutes 38.234 seconds
tests run:        78
passed:           78
flaky:            0
failed:           0
ignored:          0
====================================================================================================

m1a2st

Thanks @Rancho-7 for this patch, a little comments

chia7712

@Rancho-7 thanks for this patch.

chia7712 · 2025-08-21T19:10:31Z

+
+            // validate 'producer-acks'
+            String acksValue = options.valueOf(acksOpt);
+            if (!List.of("1", "all").contains(acksValue)) {


+1 to keep this behavior

chia7712 · 2025-08-21T19:20:22Z

+        // optional
+        Optional<String> propertiesFile = opts.options.has(opts.commandConfigOpt) ?
+                Optional.of(opts.options.valueOf(opts.commandConfigOpt)) : Optional.empty();
+        int recordKeySize = 0;


Why not add a default value to recordKeyOpt instead?

You're right, using a default value would be much cleaner. Updated it.

chia7712 · 2025-08-21T19:21:15Z

+
+        if (sentRecordKey != null) {
+            if (record.key() == null) {
+                throw new RuntimeException("Expected message key but received null");


Should we commit the offsets too?

chia7712 · 2025-08-21T19:28:22Z

+
+        for (int i = 0; i < numHeaders; i++) {
+            String headerKey = new String(randomBytesOfLen(random, keySize), StandardCharsets.UTF_8);
+            byte[] headerValue = randomBytesOfLen(random, valueSize);


Perhaps we should treat -1 as null for header value, since null is legal.

Good point. Done.

chia7712 · 2025-08-21T19:32:39Z

+            return args;
+        }
+
+        boolean hasNamedArgs = Arrays.stream(args).anyMatch(arg -> arg.startsWith("--"));


- is legal character in topic names, so it could cause an issue if users create a topic like topic--aaa. Perhaps we should check the required ops instead?

Absolutely right! This would be a much safer approach. Thanks for catching this!

chia7712

@Rancho-7 thanks for this patch!

chia7712 · 2025-08-25T11:51:32Z

+        newArgs.add(legacyArgs[4]);
+
+        // properties_file -> --command-config
+        if (legacyArgs.length == 6 && !legacyArgs[5].trim().isEmpty()) {


Excuse me, why to add !legacyArgs[5].trim().isEmpty() condition?

Removed it, thanks!

chia7712 · 2025-08-25T11:55:01Z

+                    .withOptionalArg()
+                    .describedAs("bytes")
+                    .ofType(Integer.class)
+                    .defaultsTo(0);


This is not discussed in the KIP. By default, the number of records is one, so should we create a non-empty header by default?

Yes, that does feel a bit odd. Since the header is optional, maybe we should set the default num-headers to 0, and leave record-header-key-size and record-header-size empty?

chia7712 · 2025-08-25T11:55:20Z

+                    .withOptionalArg()
+                    .describedAs("count")
+                    .ofType(Integer.class)
+                    .defaultsTo(1);


Could you please alos add those default values to the KIP?

The KIP mentions that the default value for num-headers is set to 1, but it doesn't specify the default value for the other arguments. I will add that information to the KIP. Thanks!

chia7712 · 2025-08-25T11:57:35Z

+        for (int i = 0; i < numHeaders; i++) {
+            String headerKey = new String(randomBytesOfLen(random, keySize), StandardCharsets.UTF_8);
+            byte[] headerValue = valueSize == -1 ? null : randomBytesOfLen(random, valueSize);
+            headers.add(new RecordHeader(headerKey, headerValue));


RecordHeader is internal class. Perhaps we could add a record class instead

chia7712 · 2025-08-25T11:57:58Z

+        List<Header> headers = new ArrayList<>();
+
+        for (int i = 0; i < numHeaders; i++) {
+            String headerKey = new String(randomBytesOfLen(random, keySize), StandardCharsets.UTF_8);


Have you considered directly generating a random string?

Not yet. The randomBytesOfLen function was used in the earlier version, and I found it could be reused, so I kept it.

chia7712

@Rancho-7 thanks for the updates!

chia7712 · 2025-08-30T02:55:02Z

-            throw new IllegalArgumentException("Latency testing requires synchronous acknowledgement. Please use 1 or all");
-        }
+        // optional
+        Optional<String> propertiesFile = opts.options.has(opts.commandConfigOpt) ?


Optional<String> propertiesFile = Optional.ofNullable(opts.options.valueOf(opts.commandConfigOpt));

chia7712 · 2025-08-30T02:58:30Z

+            if (options.has(recordHeaderKeySizeOpt) && options.valueOf(recordHeaderKeySizeOpt) < 0) {
+                CommandLineUtils.printUsageAndExit(parser, "Value for --record-header-key-size must be a non-negative integer.");
+            }
+            if (options.has(recordHeaderValueSizeOpt) && options.valueOf(recordHeaderValueSizeOpt) < -1) {


Is options.has(recordHeaderValueSizeOpt) necessary since recordHeaderValueSizeOpt has default value?

Thanks for catching this! I have removed all these unnecessary checks.

…sage key/header support (apache#20301) jira: https://issues.apache.org/jira/browse/KAFKA-19011 kip: https://cwiki.apache.org/confluence/display/KAFKA/KIP-1172%3A+Improve+EndToEndLatency+tool This PR improves the usability and maintainability of the `kafka-e2e-latency.sh` tool: - Replaces fixed-index argument parsing with a proper argument parser (joptsimple) - Adds support for configuring: - -record-key-size: size of the message key - -num-headers: number of headers per message - -record-header-key-size: size of each header key - -record-header-size: size of each header value - Renames existing arguments to align with Kafka CLI conventions: - broker_list → bootstrap-server - num_messages → num-records - message_size_bytes → record-size - properties_file → command-config - Reviewers: Jhen-Yung Hsu <jhenyunghsu@gmail.com>, Ken Huang <s7133700@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>

KAFKA-19011: Improve EndToEndLatency Tool with argument parser and me…

facbbe3

…ssage key/header support

github-actions Bot added triage PRs from the community tools labels Aug 4, 2025

m1a2st added the ci-approved label Aug 4, 2025

Yunyung reviewed Aug 4, 2025

View reviewed changes

github-actions Bot removed the triage PRs from the community label Aug 5, 2025

Rancho-7 added 2 commits August 5, 2025 22:44

Merge branch 'upstream-trunk' into KAFKA-19011

2485ccd

address comments

0c57140

m1a2st reviewed Aug 15, 2025

View reviewed changes

Comment thread tools/src/test/java/org/apache/kafka/tools/EndToEndLatencyTest.java Outdated

Comment thread tools/src/test/java/org/apache/kafka/tools/EndToEndLatencyTest.java

Rancho-7 added 2 commits August 17, 2025 22:37

Merge branch 'upstream-trunk' into KAFKA-19011

4fef3a8

address comments

8fd5dce

chia7712 reviewed Aug 21, 2025

View reviewed changes

Rancho-7 added 2 commits August 24, 2025 13:49

Merge branch 'upstream-trunk' into KAFKA-19011

dec9740

address comments

7a62f60

chia7712 reviewed Aug 25, 2025

View reviewed changes

Rancho-7 added 3 commits August 26, 2025 22:44

Merge branch 'upstream-trunk' into KAFKA-19011

37d814c

address comments

09e4784

rename argument

301df40

chia7712 reviewed Aug 30, 2025

View reviewed changes

Rancho-7 added 2 commits August 30, 2025 15:10

Merge branch 'upstream-trunk' into KAFKA-19011

969f03c

clean up

18eb850

chia7712 approved these changes Sep 3, 2025

View reviewed changes

chia7712 merged commit ef10a52 into apache:trunk Sep 3, 2025
24 checks passed

Uh oh!

Conversation

Rancho-7 commented Aug 4, 2025 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rancho-7 commented Aug 4, 2025

Uh oh!

Yunyung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Yunyung Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rancho-7 Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chia7712 commented Aug 7, 2025

Uh oh!

Rancho-7 commented Aug 12, 2025

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rancho-7 Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Rancho-7 commented Aug 4, 2025 •

edited by github-actions Bot

Loading

Yunyung Aug 4, 2025 •

edited

Loading

Rancho-7 Aug 5, 2025 •

edited

Loading

Rancho-7 Aug 25, 2025 •

edited

Loading