Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added output as JSON #375

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Added output as JSON #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7de3642
Added JSON output
ZuluPro Jan 24, 2019
1039c06
Added VIM file to .gitignore
ZuluPro Jan 24, 2019
134d2de
Update README about JSON
ZuluPro Jan 24, 2019
797961d
Added more infos and little fixes
ZuluPro Jan 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
obj/
wrk
*.sw[pon]
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

--latency: print detailed latency statistics

-j --json-format: print output as JSON format

--timeout: record a timeout if a response is not received within
this amount of time.

Expand Down
104 changes: 82 additions & 22 deletions src/wrk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ static struct config {
bool delay;
bool dynamic;
bool latency;
bool json_format;
char *host;
char *script;
SSL_CTX *ctx;
Expand Down Expand Up @@ -51,6 +52,7 @@ static void usage() {
" -s, --script <S> Load Lua script file \n"
" -H, --header <H> Add header to request \n"
" --latency Print latency statistics \n"
" -j --json-format Print output as JSON \n"
" --timeout <T> Socket/request timeout \n"
" -v, --version Print version details \n"
" \n"
Expand Down Expand Up @@ -135,8 +137,10 @@ int main(int argc, char **argv) {
sigaction(SIGINT, &sa, NULL);

char *time = format_time_s(cfg.duration);
printf("Running %s test @ %s\n", time, url);
printf(" %"PRIu64" threads and %"PRIu64" connections\n", cfg.threads, cfg.connections);
if (!cfg.json_format) {
printf("Running %s test @ %s\n", time, url);
printf(" %"PRIu64" threads and %"PRIu64" connections\n", cfg.threads, cfg.connections);
}

uint64_t start = time_us();
uint64_t complete = 0;
Expand Down Expand Up @@ -170,30 +174,81 @@ int main(int argc, char **argv) {
stats_correct(statistics.latency, interval);
}

print_stats_header();
print_stats("Latency", statistics.latency, format_time_us);
print_stats("Req/Sec", statistics.requests, format_metric);
if (cfg.latency) print_stats_latency(statistics.latency);
if (!cfg.json_format) {
print_stats_header();
print_stats("Latency", statistics.latency, format_time_us);
print_stats("Req/Sec", statistics.requests, format_metric);
if (cfg.latency) print_stats_latency(statistics.latency);

char *runtime_msg = format_time_us(runtime_us);
char *runtime_msg = format_time_us(runtime_us);

printf(" %"PRIu64" requests in %s, %sB read\n", complete, runtime_msg, format_binary(bytes));
if (errors.connect || errors.read || errors.write || errors.timeout) {
printf(" Socket errors: connect %d, read %d, write %d, timeout %d\n",
errors.connect, errors.read, errors.write, errors.timeout);
}
printf(" %"PRIu64" requests in %s, %sB read\n", complete, runtime_msg, format_binary(bytes));
if (errors.connect || errors.read || errors.write || errors.timeout) {
printf(" Socket errors: connect %d, read %d, write %d, timeout %d\n",
errors.connect, errors.read, errors.write, errors.timeout);
}

if (errors.status) {
printf(" Non-2xx or 3xx responses: %d\n", errors.status);
}
if (errors.status) {
printf(" Non-2xx or 3xx responses: %d\n", errors.status);
}

printf("Requests/sec: %9.2Lf\n", req_per_s);
printf("Transfer/sec: %10sB\n", format_binary(bytes_per_s));
printf("Requests/sec: %9.2Lf\n", req_per_s);
printf("Transfer/sec: %10sB\n", format_binary(bytes_per_s));

if (script_has_done(L)) {
script_summary(L, runtime_us, complete, bytes);
script_errors(L, &errors);
script_done(L, statistics.latency, statistics.requests);
if (script_has_done(L)) {
script_summary(L, runtime_us, complete, bytes);
script_errors(L, &errors);
script_done(L, statistics.latency, statistics.requests);
}
} else {
long double req_mean = stats_mean(statistics.requests);
long double req_stdev = stats_stdev(statistics.requests, req_mean);
long double req_within = stats_within_stdev(statistics.requests, req_mean, req_stdev, 1);
long double lat_mean = stats_mean(statistics.latency);
long double lat_stdev = stats_stdev(statistics.latency, lat_mean);
long double lat_within = stats_within_stdev(statistics.latency, lat_mean, lat_stdev, 1);
long double lat_perc_50 = stats_percentile(statistics.latency, 50.0) / 1000000.0;
long double lat_perc_75 = stats_percentile(statistics.latency, 75.0) / 1000000.0;
long double lat_perc_90 = stats_percentile(statistics.latency, 90.0) / 1000000.0;
long double lat_perc_95 = stats_percentile(statistics.latency, 95.0) / 1000000.0;
long double lat_perc_99 = stats_percentile(statistics.latency, 99.0) / 1000000.0;
printf("{\n");
printf(" \"url\": \"%s\",\n", url);
Copy link

Choose a reason for hiding this comment

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

Seems not be escaped here. Is this %s secure?

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean by secure ?

If you think about code injection, this is just JSON. And a parser would expect only 1 object.

Copy link

Choose a reason for hiding this comment

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

If someone do this.

#!/bin/sh
RESULT=$(wrk -j ...)
my-dommand $RESULT

With url is "} || sudo rm /etc/passwd #

Copy link
Author

Choose a reason for hiding this comment

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

This will give:

my-dommand {"url": ""} || sudo rm /etc/passwd", "script": null}

So the ending quote is missing

Feel free to propose an enhancement 👍

Copy link

Choose a reason for hiding this comment

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

Please look my code #

Copy link
Author

Choose a reason for hiding this comment

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

Which code are you talking about ?

If you're pointing me the ending #, the problem is still not here as the closing quote " is not here. In interactive terminal, it will wait for you to close quotes. And in non-interactive, it will raise a syntax error.

printf(" \"threads\": %"PRIu64",\n", cfg.threads);
printf(" \"connections\": %"PRIu64",\n", cfg.connections);
printf(" \"duration\": %"PRIu64",\n", cfg.duration);
printf(" \"timeout\": %"PRIu64",\n", cfg.timeout);
if (cfg.script) {
printf(" \"script\": \"%s\",\n", cfg.script);
} else {
printf(" \"script\": null,\n");
}
printf(" \"runtime_us\": %"PRIu64",\n", runtime_us);
printf(" \"bytes\": %"PRIu64",\n", bytes);
printf(" \"bytes_per_sec\": %Lf,\n", bytes_per_s);
Copy link

Choose a reason for hiding this comment

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

JSON is for machine. Why bytes_per_sec is needed?

Copy link
Author

Choose a reason for hiding this comment

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

Several reason, because:

  • It is declared in the original text output
  • it is internally calculated and without documentation, no one is expected to know how you get it
  • If the value could be given by the output instead of code the calculation elsewhere, it's better
  • There's no cost to give it to user

Copy link

@methane methane Jan 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

  • It is declared in the original text output

I don't think JSON output should be same to original text output. Text is mainly for human. But JSON is more for machine.

Too detailed information for human eye are omitted in text output, but it can be included in JSON.
On the other hand, values easily calculated by other values can be omitted in JSON.

People expect backward compatibility about JSON format.
Adding items later is easy. Removing or changing items later is difficult.
So you should be careful about choosing which value is included, and how to format it.

it is internally calculated and without documentation, no one is expected to know how you get it
If the value could be given by the output instead of code the calculation elsewhere, it's better
There's no cost to give it to user

All of these reasons seems not enough for inclusion.

Copy link
Author

Choose a reason for hiding this comment

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

@methane You don't say why it shouldn't be included.

Otherwise, even you know the runtime and wrk knows the number of transfered bytes. The throughput is still a value researched by a lot of people.

printf(" \"requests_count\": %"PRIu64",\n", statistics.requests->count);
printf(" \"requests_per_sec\": %Lf,\n", req_per_s);
printf(" \"requests_mean\": %Lf,\n", req_mean);
printf(" \"requests_stdev\": %Lf,\n", req_stdev);
printf(" \"requests_min\": %"PRIu64",\n", statistics.requests->min);
printf(" \"requests_max\": %"PRIu64",\n", statistics.requests->max);
printf(" \"requests_within_stdev\": %.2Lf,\n", req_within);
printf(" \"latency_mean\": %Lf,\n", lat_mean/1000000.0);
printf(" \"latency_stdev\": %Lf,\n", lat_stdev/1000000.0);
printf(" \"latency_min\": %f,\n", statistics.latency->min/1000000.0);
printf(" \"latency_max\": %f,\n", statistics.latency->max/1000000.0);
printf(" \"latency_within_stdev\": %.2Lf,\n", lat_within);
printf(" \"latency_percentile_50\": %.2Lf,\n", lat_perc_50);
printf(" \"latency_percentile_75\": %.2Lf,\n", lat_perc_75);
printf(" \"latency_percentile_90\": %.2Lf,\n", lat_perc_90);
printf(" \"latency_percentile_95\": %.2Lf,\n", lat_perc_95);
printf(" \"latency_percentile_99\": %.2Lf,\n", lat_perc_99);
Copy link

@methane methane Jan 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Is this really beautiful? How about

"latency_percentile": {
  "50": ...
  "75": ...
...

Copy link
Author

@ZuluPro ZuluPro Jan 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Beautiful or not, I think it in several aspects:

  • It gives exactly what a user could need.
  • I kept code simple and readable

I'm not against a new format, but let @wg decide about it.

printf(" \"errors_connect\": %d,\n", errors.connect);
printf(" \"errors_read\": %d,\n", errors.read);
printf(" \"errors_write\": %d,\n", errors.write);
printf(" \"errors_timeout\": %d,\n", errors.timeout);
printf(" \"errors_status\": %d\n", errors.status);
printf("}");
}

return 0;
Expand Down Expand Up @@ -474,6 +529,7 @@ static struct option longopts[] = {
{ "header", required_argument, NULL, 'H' },
{ "latency", no_argument, NULL, 'L' },
{ "timeout", required_argument, NULL, 'T' },
{ "json-format", no_argument, NULL, 'j' },
{ "help", no_argument, NULL, 'h' },
{ "version", no_argument, NULL, 'v' },
{ NULL, 0, NULL, 0 }
Expand All @@ -488,8 +544,9 @@ static int parse_args(struct config *cfg, char **url, struct http_parser_url *pa
cfg->connections = 10;
cfg->duration = 10;
cfg->timeout = SOCKET_TIMEOUT_MS;
cfg->json_format = false;

while ((c = getopt_long(argc, argv, "t:c:d:s:H:T:Lrv?", longopts, NULL)) != -1) {
while ((c = getopt_long(argc, argv, "t:c:d:s:H:T:Lrv?j", longopts, NULL)) != -1) {
switch (c) {
case 't':
if (scan_metric(optarg, &cfg->threads)) return -1;
Expand All @@ -509,6 +566,9 @@ static int parse_args(struct config *cfg, char **url, struct http_parser_url *pa
case 'L':
cfg->latency = true;
break;
case 'j':
cfg->json_format = true;
break;
case 'T':
if (scan_time(optarg, &cfg->timeout)) return -1;
cfg->timeout *= 1000;
Expand Down