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.

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

fix(charts): prevent displaying stats before requests are made #1853

Merged
merged 7 commits into from
Aug 19, 2021

Conversation

obradovichv
Copy link
Contributor

@obradovichv obradovichv commented Aug 17, 2021

Stat history would be charted by the client as a stream of consciousness
from the runners until they are stopped.
The client will no longer show unquantified stats (represented by the
number of requests).

Issue: #1852

State checking logic on the client has also been flattened to simplify the flow.

Considerations:

  • By only showing stats that contain request, there might be an appearance of unresponsiveness until requests are made because the chart will no longer be "ticking" without them.
  • The aggregated total stats are used to determine the number of requests, this is to allow a transition from ready state to running state to be displayed.
  • The times on the x-axis of charts diverge between a page that has been running for a while and a page refresh, this is because the client stores the time that it fetched the stats rather than the times of the stats themselves.

Stat history would be charted by the client as a stream of consciousness
from the runners until they are stopped.
The client will no longer show unquantified stats (represented by the
number of requests).

Issue: locustio#1852
@cyberw
Copy link
Collaborator

cyberw commented Aug 17, 2021

Thanks! This is, on the whole, a great improvement! (I dont really use the UI because I have an external reporting solution, and thus I havent really tried to improve it)

two things:

  • Is there some way (a vertical bar perhaps) to indicate the jump in time if someone has stopped and then restarted a test? Or at least disconnect the line somehow?
  • Is there a way to hide the initial dot in the response time graph?

image

@cyberw
Copy link
Collaborator

cyberw commented Aug 17, 2021

One possible way to indicate the time diff between test runs would be to listen for the test_stopped or test_started event and plot those somehow. If you do go down that route, it would be nice to plot spawning_complete as well :)

@obradovichv
Copy link
Contributor Author

* Is there some way (a vertical bar perhaps) to indicate the jump in time if someone has stopped and then restarted a test? 

echarts does have a markLine that I was considering for this purpose but locust does not seem to distinguish between runs in the data of /stats/requests so it was a challenge to determine where the line should be inserted in the graph. Is there a clear way to delineate between test runs on the client with the current data model?
I.e.:

{
  "current_response_time_percentile_50": 3, 
  "current_response_time_percentile_95": 12, 
  "errors": [], 
  "fail_ratio": 1.0, 
  "state": "running", 
  "stats": [
    {
      "avg_content_length": 0.0, 
      "avg_response_time": 9.050847440507383, 
      "current_fail_per_sec": 8.5, 
      "current_rps": 8.5, 
      "max_response_time": 44.0, 
      "median_response_time": 6, 
      "method": null, 
      "min_response_time": 1.0, 
      "name": "Aggregated", 
      "ninetieth_response_time": 17, 
      "num_failures": 186, 
      "num_requests": 186, 
      "safe_name": "Aggregated"
    }
  ], 
  "total_rps": 8.5, 
  "user_count": 10
}

One thought was to mark test runs based on the action of user clicking "New test", this would be based entirely in the client side but this might be fine because starting a new test run clears the old run's stats on the server anyway.

Or at least disconnect the line somehow?

I think some fake plots could be inserted to keep the lines disconnected (dependent on being able to distinguish between runs above)

* Is there a way to hide the initial dot in the response time graph?

image

This might be related to getting very fast response times (i.e.: from localhost). I'll investigate further.

@cyberw
Copy link
Collaborator

cyberw commented Aug 17, 2021

* Is there some way (a vertical bar perhaps) to indicate the jump in time if someone has stopped and then restarted a test? 

echarts does have a markLine that I was considering for this purpose but locust does not seem to distinguish between runs in the data of /stats/requests so it was a challenge to determine where the line should be inserted in the graph. Is there a clear way to delineate between test runs on the client with the current data model?

Probably not (although I’m not sure). But the events should work.

One thought was to mark test runs based on the action of user clicking "New test", this would be based entirely in the client side but this might be fine because starting a new test run clears the old run's stats on the server anyway.

Yes, that would also be acceptable imo.

Or at least disconnect the line somehow?

I think some fake plots could be inserted to keep the lines disconnected (dependent on being able to distinguish between runs above)

👍

* Is there a way to hide the initial dot in the response time graph?

This might be related to getting very fast response times (i.e.: from localhost). I'll investigate further.

I dont think it is a real data point. Maybe it is added just to get the same timespan as the rps graph.

test case test_get_current_response_time_percentile_outside_cache_window
verifies incorrect behaviour returning None instead of 0 when time is outside
window of cached times.

Issue: locustio#1852
@obradovichv
Copy link
Contributor Author

For very fast response times the percentile calculation would not find the time in the initial cached response times window and would return None instead of 0 (the default return was missing).
Verified this behavior with a failing test case and resolved the issue. You should now see a contiguous line on the chart for response times.

I'm still tinkering with the separation of test runs on the client UI.

@cyberw
Copy link
Collaborator

cyberw commented Aug 18, 2021

Hmm... I'm not sure the last commits were an improvement, because the gap in response times between test runs that was there before at least gave you the chance to infer that something strange happened at that time (test stop and then restart), but now it looks like one big continous test run :)

Also, now it looks like response times start at zero which is not correct.

And maybe there is something wrong with the way RPS is grouped, because response times show up earlier than RPS (this is probably an old issue though, so feel free to ignore it)

image

Creates dummy placeholders in the stats history to space between the marker
and the line data.

Issue: locustio#1852
…window

This prevents the data from being displayed on the UI chart during the
response time calculation's indeterminate state.

Issue: locustio#1852
@obradovichv
Copy link
Contributor Author

obradovichv commented Aug 18, 2021

The issue was caused by initial runner stats of 0 being populated, my assumption was that response times outside the window of response times should match this rather than vanish (e.g.: 0 => null => 1) because that is how the disjointed graph would end up being drawn.
Instead, stats history will now not be injected on the client page unless there has been requests made. This keeps it consistent because initial runner stats are cleared when a test run is started.

Also added line marker to distinguish between test runs on the client.

@obradovichv
Copy link
Contributor Author

There might be differing views on the correct representation of the {ready -> spawning -> running} states, that is, there are a number of runners executing but they have not yet made requests, or their requests have not yet completed:

  1. Represent the response times during this period as zero
  2. Do not plot the response times until they exist

This PR implements option 2 but I believe it addresses the issue raised in #1702.
The initial graph will no longer be disconnected but will instead only be plotted after requests are made.

Possibly fixes #1702

@cyberw
Copy link
Collaborator

cyberw commented Aug 19, 2021

Looks amazing now! One tiny thing: Shouldnt it say Run #2,3,... on the label, instead of Run #1,2,..., as it marks the start of the new run?

@obradovichv
Copy link
Contributor Author

obradovichv commented Aug 19, 2021

Looks amazing now! One tiny thing: Shouldnt it say Run #2,3,... on the label, instead of Run #1,2,..., as it marks the start of the new run?

It depends on whether you consider the lines as end-of-run or start-of-run markers 😄

I wanted to avoid cluttering single-run graphs so I excluded the Run# marker before the first test run. This meant the first line marker would start with Run #2 but there would be no Run #1 on the graph - this might be equally confusing.

Here are some of other ideas I considered:

  1. Change label text to Start of run #n or End of run #n - at the cost of more visual noise from a longer label text. Start of run #n implies a line marker before the first run.
  2. Omit the label - at the cost of user not being sure what the line means.
  3. Omit the label but have a tooltip - at the cost of the information not being visible in downloaded images, plus passing run# information to echarts is quite complicated since markLine is not passed as part of the arguments to the tooltip formatter ( https://echarts.apache.org/en/option.html#tooltip.formatter ) which increases code complexity substantially (storing run numbers on each plotted series) unless I am missing something.
  4. Plot line marker before the first run - at the cost of more visual nose and less space for single-run graphs.

I'm happy to implement either.

In the future we might be able to label a run ( "Warm up", "Scale-down test", etc ) so either a leading or trailing marker would be useful.

@cyberw
Copy link
Collaborator

cyberw commented Aug 19, 2021

The line is shown at the time when the second test was started, so it should be 2, 3,... imho. The user should be able to infer that it refers to the start of a run, and not the end of one, so a longer text is not necessary. The missing #1 is a little confusing maybe, but it should be kind of obvious why it is not there (reducing clutter).

Idk if there is the option of showing the Run #1 only once there actually IS a run 2, if so then that is even better, but I dont consider the missing 1 so bad.

Omitting the label entirely (your option 2) is acceptable, but I prefer doing it as above.

Mark start of first run on the chart once we start the second run.
Hook stop/start markers of test runs into update loop of stats.

Issue: locustio#1852
@obradovichv
Copy link
Contributor Author

Agreed.
Test run markers now mark the start of a test run (leading).
The first marker will only be shown once there is a second run to show.

Hooked run markers into the stats update loop to make the x-axis times manageable.

@cyberw
Copy link
Collaborator

cyberw commented Aug 19, 2021

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants