kafka-5068: Optionally print out metrics after running the perf tests #2860
Conversation
Added a config `--print.metrics` to control whether ProducerPerformance prints out metrics at the end of the test.
|
The output is like this: |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
There was a problem hiding this comment.
@Amethystic : Thanks for the patch. Looks good. Just a minor comment. It would be useful to add the same option for consumerPerf test.
| int maxLengthOfDisplayName = 0; | ||
| for (Metric metric : metrics.values()) { | ||
| MetricName mName = metric.metricName(); | ||
| String mergedName = mName.group() + ":" + mName.name(); |
There was a problem hiding this comment.
The tags in the MetricName can be useful too. For example, for the per node metrics, it tells us the broker id.
added code to show tags and print out metrics for consumer
|
@junrao addressed your comments by adding code to display tags and print out metrics for consumer. Please review again. Thanks. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
There was a problem hiding this comment.
@Amethystic : Thanks for the patch. Looks good. Just a couple of minor comments.
| for (Metric metric : metrics.values()) { | ||
| MetricName mName = metric.metricName(); | ||
| System.out.println(String.format(outputFormat, mName.group() + ":" + mName.name() + ":" + mName.tags(), metric.value())); | ||
| } |
There was a problem hiding this comment.
Perhaps we can pull this code into a ToolsUtils.printMetrics(Map<MetricName, ? extends Metric> metrics) method and share between the consumerPerf and producerPerf. It would also be useful to print out the metrics in alphabetical order.
There was a problem hiding this comment.
kafka-client does not depend on kafka-core, so client code cannot see ToolsUtils. Do we need to create a peer helper class on client side? There is no counterpart of ConsumerPerformance in the client codebase right now
There was a problem hiding this comment.
Ok, perhaps we can just create a util method in the tools package and use it in producerPerf. We can duplicate the code in consumerPerf for now.
| @@ -223,9 +232,32 @@ private static ArgumentParser argParser() { | |||
| .dest("producerConfigFile") | |||
| .help("producer config properties file."); | |||
|
|
|||
| parser.addArgument("--print.metrics") | |||
There was a problem hiding this comment.
fixed. Always use print-metrics in both producer and consumer perf test.
1. fixed config name to 'print-metrics' 2. Add org.apache.kafka.tools.ToolsUtils#printMetrics 3. Honor alphabetical order
|
@junrao Please review again. Thanks. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
@Amethystic : Thanks for the updated patch. Could you fix the error in style check? |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
@junrao Already passed the check. Please take a look at it again. Thanks! |
|
@Amethystic : Thanks for the patch. LGTM. |
@junrao added a config
--print.metricsto control whether ProducerPerformance prints out metrics at the end of the test. If its okay, will add the code counterpart for consumer.