Formatting of stats output

[spier]

The new stats features by #127 introduced two changes in the output, that might not have been intended.

I marked them in the screenshot below, and will try my best to describe them in text as well.

Label of 2nd column

Previous the label of the 2nd column of the stats table was "Value".
Now it is "Average".

However "Average" is not correct for rows 4-6 in the table, as those are just counts.

I don't have a great suggestion here. One could split up the table and introduce a new header for row 4-6.
Not sure how future-proof that would be either though.

Milliseconds in average/median/90th-percentile

The stats in the first 3 rows contain the milliseconds.

They reduce readability a bit, and I doubt that many users will need them.
My recommendation would be to remove the milliseconds.

One might even argue that for most purposes the seconds would not be needed either.
That might be a matter

Backwards compatibility?

@zkoppert had mentioned that introducing a new configuration option for the new stats would be good.

I guess that as the 2nd row still contains the average (like before), that idea was dropped?

Screenshot

[zkoppert]

Hey @spier! Thanks for opening this issue! Really important points that you bring up here.

I think for the column header being a mismatch of averages and counts, we could split the summary table you show in the screenshot into 2 summary tables. One table for the first 3 rows (4 rows if you include the header) and another table for the last 3 (containing counts). Any thoughts on that?

I think @MaineC decided not to implement the configurability piece and I was thinking that was okay since the original information (average) was still there. Maybe though with this significant formatting change, I should have rolled the version number to v3 to signify breaking changes. Love to hear you thoughts on that as well.

I also agree with removing the milliseconds. I can create a pull request for that.

[spier]

Hey @spier! Thanks for opening this issue! Really important points that you bring up here.

Hi @zkoppert :)

I think for the column header being a mismatch of averages and counts, we could split the summary table you show in the screenshot into 2 summary tables. One table for the first 3 rows (4 rows if you include the header) and another table for the last 3 (containing counts). Any thoughts on that?

That would work.

The first column of those two tables would then have different with and look misaligned. But not that big of a deal.

One implementation alternative might be to keep it in the same table but add a row in between with new headings?
(We would have to try that out in markdown yet to see how it looks)

I think @MaineC decided not to implement the configurability piece and I was thinking that was okay since the original information (average) was still there. Maybe though with this significant formatting change, I should have rolled the version number to v3 to signify breaking changes. Love to hear you thoughts on that as well.

Not sure about this either. How did this change impact the JSON output format?
As JSON might be used for processing the metrics further, that is the area where I can consider things to break.

If the users just consume the metrics "with their eyes", i.e. they look at them in the GitHub issue, then such a change does not really break anything, does it?

I also agree with removing the milliseconds. I can create a pull request for that.

Go for it! :)

[zkoppert]

Closing this for now. Will open a new issue if we find that users have broken workflows because json data changed.