- Notifications
You must be signed in to change notification settings - Fork3.2k
Add Duration-Based Testing and Enhanced TLS Logging#5228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
- 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.
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this 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 therdkafka_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-cflag - 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.
| /* Print warnings and errors to stderr */ | ||
| fprintf(stderr,"%%%d|%s|%s: %s\n",level,rd_kafka_name(rk),fac,buf); |
CopilotAINov 3, 2025
There was a problem hiding this comment.
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.
| /* 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 */ | |
| intmin_level; | |
| if (verbosity >=4) | |
| min_level=7;/* RD_KAFKA_LOG_PRINT, print all */ | |
| elseif (verbosity==3) | |
| min_level=6;/* RD_KAFKA_LOG_DEBUG */ | |
| elseif (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); |
| case'E': | ||
| test_duration=atoi(optarg); | ||
| if (test_duration <=0) { | ||
| fprintf(stderr, | ||
| "%% Invalid test duration: %s " | ||
| "(must be > 0)\n", | ||
| optarg); | ||
| exit(1); | ||
| } | ||
| break; |
CopilotAINov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Usingatoi() 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. Usestrtol() with error checking to properly validate numeric input and provide accurate error messages for malformed input.
| 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; | |
| longval; | |
| 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; | |
| } |
| if (test_duration>0) { | ||
| /* For time-based tests, flush remaining messages with | ||
| * timeout */ | ||
| rd_kafka_flush(rk,5000);/* 5 second flush timeout */ |
CopilotAINov 3, 2025
There was a problem hiding this comment.
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.
Duration-based testing for
rdkafka_performanceNew
-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.txtTLS connection visibility
Added logger callback to display TLS version information during connection establishment, improving observability for SSL/TLS debugging.