Refactoring stats to handle custom percentiles #1477

vstepanov-lohika-tix · 2020-07-10T09:45:50Z

Goal
Currently, if we set a custom list of percentiles, we'll get a broken csv report & console stats for percentiles.
This PR is intended to:

Handling percentiles list in a single place
Get proper csv report with custom percentiles set
Get proper console stats with custom percentiles set
Decrease of duplication of percentile values in code

How-to check:
In the locust file, set the list of percentiles you need to get:

import locust.stats
locust.stats.PERCENTILES_TO_REPORT = [0.50, 0.90, 0.95, 0.99]

Both csv report & console stats should be correct with a proper columns number and data.

codecov · 2020-07-10T11:44:10Z

Codecov Report

Merging #1477 into master will decrease coverage by 0.24%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1477      +/-   ##
==========================================
- Coverage   81.47%   81.23%   -0.25%     
==========================================
  Files          27       27              
  Lines        2386     2398      +12     
  Branches      366      370       +4     
==========================================
+ Hits         1944     1948       +4     
- Misses        351      358       +7     
- Partials       91       92       +1

Impacted Files	Coverage Δ
locust/stats.py	`88.96% <100.00%> (-0.06%)`	⬇️
locust/clients.py	`90.19% <0.00%> (-4.91%)`	⬇️
locust/event.py	`87.50% <0.00%> (-4.40%)`	⬇️
locust/main.py	`18.66% <0.00%> (-0.19%)`	⬇️
locust/runners.py	`80.81% <0.00%> (+0.20%)`	⬆️
locust/user/task.py	`96.75% <0.00%> (+0.54%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b97c74...d85e0a0. Read the comment docs.

vstepanov-lohika-tix · 2020-07-10T11:47:36Z

Updated tests -> Passed. Moving to Open.

vstepanov-lohika-tix · 2020-07-10T13:18:55Z

Found issue with rounding:

Will fix in the next commit

vstepanov-lohika-tix · 2020-07-10T15:44:06Z

Fixed:

vstepanov-lohika-tix · 2020-07-10T22:36:06Z

Fixed console stats separator to any number of percentiles:

cyberw · 2020-07-12T11:13:12Z

Cool stuff! I think this may be better implemented as a command line switch, but I’m not wholly against it as is. It needs a proper test case (that actually modifies the setting and ensures it changes the output), and documentation.

cyberw · 2020-07-13T12:16:24Z

@heyman what do you think? Should this be a command line switch? I think we need to give some thought to how we manage settings in general (as the list of command line settings keeps growing)

vstepanov-lohika-tix · 2020-07-13T13:22:25Z

@cyberw Added a doc for customization of stats settings. Not sure about command-line options, but probably it's worth to add it to configuration file.

vstepanov-lohika-tix · 2020-07-21T12:15:40Z

@cyberw @heyman Please write if something else should be added/updated

lhupfeldt · 2020-08-05T21:51:50Z

locust/stats.py

-        '100%',
-    ))
-    console_logger.info("-" * (90 + STATS_NAME_WIDTH))
+    headers = ('Type', 'Name', '# reqs') + tuple([f"{round(percentile*100, 4)}%" for percentile in PERCENTILES_TO_REPORT])


Is a max number of decimal points assumed? Rounding like this can cause different percentiles to round to the same number. If a max number of decimal points is assumed, then there should be validation of the PERCENTILES_TO_REPORT.

At the moment maximum percentile in default list is 0.99999, that matching to 3 digits after point (99.999%)
I've updated to 6 digits and believe that no-one ever will try to use percentiles with higher precision, so I think to do the additional calculation from percentile list will be redundant.

That will probably do, but you should still verify the PERCENTILES_TO_REPORT so that the web interface does not break.

These changes assume that a user will manually set a list of percentiles based on the default list, and this assumes that a user understands what does he do and why. Otherwise, we should handle all possible wrong or not relevant values that could be input ( >1, <0, not a number etc). There's no reason to set a percentile precision over 99.999999% in a performance testing world, and even if it will happen - it's just will cause a wrong header name in results stats.
So, imho it's a very little sense to add additional calculation in code to cover a non-breaking issue that unlikely could occur.

In web.py there are assumptions about a few specific percentiles. I think it should be validated that those are present in the list. I agree that it is otherwise fine to assume that users will know to put relevant numbers in the list.

That's a good point, I'll check web.py additionally

lhupfeldt · 2020-08-05T22:00:50Z

stats_history_csv_header still has hardcoded list of percentiles

lhupfeldt · 2020-08-05T22:05:29Z

web.py has hardcoded percentiles in different places:

"ninetieth_response_time": s.get_response_time_percentile(0.9),

                report["current_response_time_percentile_95"] = environment.runner.stats.total.get_current_response_time_percentile(0.95)
                report["current_response_time_percentile_50"] = environment.runner.stats.total.get_current_response_time_percentile(0.5)

Not sure how median plays into this?
Maybe it should be required that the percentiles referenced from web.py must be in the percentiles list.

cyberw · 2020-08-07T20:24:08Z

This looks good to me, if nobody objects, I will merge it this weekend.

lhupfeldt · 2020-08-08T09:05:16Z

stats_history_csv_header still has hardcoded list of percentiles

cyberw · 2020-08-08T21:29:22Z

stats_history_csv_header still has hardcoded list of percentiles

Good point. I will hold off.

vstepanov-lohika-tix · 2020-08-10T07:48:08Z

Will check stats_history_csv_header and web.py additionally

vstepanov-lohika-tix · 2020-08-10T22:32:05Z

@cyberw @lhupfeldt Updated stats_history_csv_header as well and extracted repetative action to a function.
Regarding web.py - checked additionally, percentile values in this file are not linked to PERCENTILES_TO_REPORT parameter and calculated "on a fly", so looks there's no need to check those values according to this list.

cyberw · 2020-08-11T11:03:13Z

Nice. Let's give @heyman one last chance to comment before I merge :)

cyberw · 2020-08-13T18:26:54Z

Thanks!

vstepanov-lohika-tix added 4 commits July 10, 2020 12:37

Refactoring stats to handle custom percentiles

162f3ca

fix csv headers

4eb1d78

update to string interpolation

17fbc20

Fix tests for percentile stats

b9efbad

vstepanov-lohika-tix marked this pull request as ready for review July 10, 2020 11:47

fix float values

702cdcb

Adaptation separator to any number of percentiles

837085b

micro syntax fix

c363c29

vstepanov-lohika-tix added 2 commits July 13, 2020 13:52

adding test for custom percentile list

438390b

increase column width for percentiles to fit 99.999%

fd1dfbb

Adding doc for stats customization

faec786

vstepanov-lohika-tix added 4 commits July 13, 2020 16:34

Adding doc to configuration section

e89eb10

Moving doc section

0b53dd1

Adding PERCENTILES_TO_REPORT parameter to doc

70e35d1

align percentile width to column width

e8b052a

lhupfeldt reviewed Aug 5, 2020

View reviewed changes

Increase precision for percentile headers names

17c3991

Rework percentiles headers to use outer function

d85e0a0

cyberw merged commit d34fcc0 into locustio:master Aug 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring stats to handle custom percentiles #1477

Refactoring stats to handle custom percentiles #1477

vstepanov-lohika-tix commented Jul 10, 2020 •

edited

Loading

codecov bot commented Jul 10, 2020 •

edited

Loading

vstepanov-lohika-tix commented Jul 10, 2020

vstepanov-lohika-tix commented Jul 10, 2020

vstepanov-lohika-tix commented Jul 10, 2020

vstepanov-lohika-tix commented Jul 10, 2020

cyberw commented Jul 12, 2020

cyberw commented Jul 13, 2020

vstepanov-lohika-tix commented Jul 13, 2020

vstepanov-lohika-tix commented Jul 21, 2020 •

edited

Loading

lhupfeldt Aug 5, 2020 •

edited

Loading

vstepanov-lohika-tix Aug 6, 2020

lhupfeldt Aug 6, 2020 •

edited

Loading

vstepanov-lohika-tix Aug 7, 2020 •

edited

Loading

lhupfeldt Aug 7, 2020

vstepanov-lohika-tix Aug 10, 2020

lhupfeldt commented Aug 5, 2020 •

edited

Loading

lhupfeldt commented Aug 5, 2020

cyberw commented Aug 7, 2020

lhupfeldt commented Aug 8, 2020

cyberw commented Aug 8, 2020

vstepanov-lohika-tix commented Aug 10, 2020

vstepanov-lohika-tix commented Aug 10, 2020 •

edited

Loading

cyberw commented Aug 11, 2020

cyberw commented Aug 13, 2020

Refactoring stats to handle custom percentiles #1477

Refactoring stats to handle custom percentiles #1477

Conversation

vstepanov-lohika-tix commented Jul 10, 2020 • edited Loading

codecov bot commented Jul 10, 2020 • edited Loading

Codecov Report

vstepanov-lohika-tix commented Jul 10, 2020

vstepanov-lohika-tix commented Jul 10, 2020

vstepanov-lohika-tix commented Jul 10, 2020

vstepanov-lohika-tix commented Jul 10, 2020

cyberw commented Jul 12, 2020

cyberw commented Jul 13, 2020

vstepanov-lohika-tix commented Jul 13, 2020

vstepanov-lohika-tix commented Jul 21, 2020 • edited Loading

lhupfeldt Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

vstepanov-lohika-tix Aug 6, 2020

Choose a reason for hiding this comment

lhupfeldt Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

vstepanov-lohika-tix Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

lhupfeldt Aug 7, 2020

Choose a reason for hiding this comment

vstepanov-lohika-tix Aug 10, 2020

Choose a reason for hiding this comment

lhupfeldt commented Aug 5, 2020 • edited Loading

lhupfeldt commented Aug 5, 2020

cyberw commented Aug 7, 2020

lhupfeldt commented Aug 8, 2020

cyberw commented Aug 8, 2020

vstepanov-lohika-tix commented Aug 10, 2020

vstepanov-lohika-tix commented Aug 10, 2020 • edited Loading

cyberw commented Aug 11, 2020

cyberw commented Aug 13, 2020

vstepanov-lohika-tix commented Jul 10, 2020 •

edited

Loading

codecov bot commented Jul 10, 2020 •

edited

Loading

vstepanov-lohika-tix commented Jul 21, 2020 •

edited

Loading

lhupfeldt Aug 5, 2020 •

edited

Loading

lhupfeldt Aug 6, 2020 •

edited

Loading

vstepanov-lohika-tix Aug 7, 2020 •

edited

Loading

lhupfeldt commented Aug 5, 2020 •

edited

Loading

vstepanov-lohika-tix commented Aug 10, 2020 •

edited

Loading