Skip to content

Conversation

@Ankith-Confluent
Copy link
Member

Duration-based testing for rdkafka_performance

New -E <seconds> flag allows running producer tests for a specified duration (in seconds) instead of message count. Includes mutual exclusivity validation with -c flag with real-time countdown display.

Example
./rdkafka_performance -P -t anl-1 -p 0 -s 1000 -E 600 -X file=test.conf -X compression.type=none -X linger.ms=5 -l -A latency_L2-long.txt

TLS connection visibility

Added logger callback to display TLS version information during connection establishment, improving observability for SSL/TLS debugging.

- Introduced a new command-line option '-E' to specify the test duration in seconds, allowing for time-based message production.
- Implemented a logger callback to capture and display TLS version information during connections.
- Updated print_stats function to include remaining time for duration-based tests.
- Added validation to prevent simultaneous use of message count and test duration options.
… SSL verification logging

- Set both minimum and maximum TLS version to 1.3 for testing purposes.
- Log the TLS version and cipher used during SSL certificate verification for improved debugging.
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@Ankith-Confluent Ankith-Confluent marked this pull request as ready for review November 3, 2025 14:49
@Ankith-Confluent Ankith-Confluent requested a review from a team as a code owner November 3, 2025 14:49
Copilot AI review requested due to automatic review settings November 3, 2025 14:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds duration-based testing capability to the rdkafka_performance tool and enhances TLS connection logging for better observability.

Key Changes:

  • Introduces a new -E <seconds> flag for running producer tests for a specified duration instead of message count, with mutual exclusivity validation against the -c flag
  • Adds a logger callback to display TLS version and cipher information during SSL/TLS connection establishment
  • Implements real-time countdown display showing remaining test time during duration-based tests

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/rdkafka_ssl.c Added debug logging to display TLS version and cipher information after successful SSL certificate verification
examples/rdkafka_performance.c Implemented duration-based testing with -E flag, added logger callback for TLS info display, and integrated countdown timer in stats output

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +99 to +100
/* Print warnings and errors to stderr */
fprintf(stderr, "%%%d|%s|%s: %s\n", level, rd_kafka_name(rk), fac, buf);
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logger callback always prints all log messages to stderr, which will produce excessive output. This should be conditional based on log level or verbosity settings to avoid flooding the console with debug messages. Consider adding a level check (e.g., if (level <= LOG_WARNING)) or respecting the global verbosity setting.

Suggested change
/* Print warnings and errors to stderr */
fprintf(stderr, "%%%d|%s|%s: %s\n", level, rd_kafka_name(rk), fac, buf);
/* Only print warnings and errors by default, or more if verbosity is high */
/* Map verbosity: 1 = warnings/errors, 2 = info, 3 = debug, 4+ = all */
int min_level;
if (verbosity >= 4)
min_level = 7; /* RD_KAFKA_LOG_PRINT, print all */
else if (verbosity == 3)
min_level = 6; /* RD_KAFKA_LOG_DEBUG */
else if (verbosity == 2)
min_level = 5; /* RD_KAFKA_LOG_INFO */
else
min_level = 4; /* RD_KAFKA_LOG_WARNING */
if (level <= min_level)
fprintf(stderr, "%%%d|%s|%s: %s\n", level, rd_kafka_name(rk), fac, buf);

Copilot uses AI. Check for mistakes.
Comment on lines +1153 to +1162
case 'E':
test_duration = atoi(optarg);
if (test_duration <= 0) {
fprintf(stderr,
"%% Invalid test duration: %s "
"(must be > 0)\n",
optarg);
exit(1);
}
break;
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using atoi() for parsing user input doesn't detect invalid input. If the user provides non-numeric input, atoi() returns 0, which will pass the <= 0 check but gives a misleading error message. Use strtol() with error checking to properly validate numeric input and provide accurate error messages for malformed input.

Suggested change
case 'E':
test_duration = atoi(optarg);
if (test_duration <= 0) {
fprintf(stderr,
"%% Invalid test duration: %s "
"(must be > 0)\n",
optarg);
exit(1);
}
break;
case 'E': {
char *endptr = NULL;
long val;
errno = 0;
val = strtol(optarg, &endptr, 10);
if (errno == ERANGE || val > INT_MAX || val <= 0 || endptr == optarg || *endptr != '\0') {
fprintf(stderr,
"%% Invalid test duration: %s "
"(must be a positive integer)\n",
optarg);
exit(1);
}
test_duration = (int)val;
break;
}

Copilot uses AI. Check for mistakes.
if (test_duration > 0) {
/* For time-based tests, flush remaining messages with
* timeout */
rd_kafka_flush(rk, 5000); /* 5 second flush timeout */
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The 5-second flush timeout is hardcoded with no explanation of why this specific duration was chosen. Consider documenting the rationale for this timeout value or making it configurable, especially since duration-based tests might involve different message volumes that could require different flush timeouts.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants