Add requests per second graph to the HTML report. #381

slashrsm · 2021-11-08T14:36:13Z

Closes #246.

TODO

~~Graph color scheme~~
~~Only display starting/stopping phase when --no-reset-metrics is used~~
Investigate spike on transition between starting and started phase
Confirm that metrics are correct (when throttling is enabled RPS numbers do not match exactly)
Automated tests
Add additional graphs.

Screenshots

With `--no-reset-metrics`

Without `--no-reset-metrics`

src/config.rs

slashrsm · 2021-11-08T14:38:52Z

src/metrics.rs

+
+    /// Record request in requests per second metric.
+    pub(crate) fn record_requests_per_second(&mut self, bucket_size: usize) {
+        let time_bucket = (Utc::now().timestamp() - self.start_time) as usize / bucket_size;


Instead of the actual time of the request we are using the time when the request metric is reported.

It would be better to collect the time when the request happened, but I think that this data currently isn't reported by the worker.

src/report.rs

jeremyandrews

It's fantastic seeing this feature come together!

As we only use the requests-per-second (and tasks-per-second and users-per-second etc) data when writing the html-report, perhaps we simply tie it to the existing --report-file option.

Specifying a bucket size doesn't seem to scale well, we need a mechanism that works regardless of if the load test runs for 5 minutes or 5 hours.

The code is clean and when it works it looks great -- very nice addition to the load test report!

src/metrics.rs

src/config.rs

src/report.rs

src/config.rs

src/report.rs

src/metrics.rs

slashrsm · 2021-11-10T20:15:22Z

I removed the options that were added in the initial version of this pull request and now we rely on --report-file to enable/disable this.

I also changed how the data is stored. Now we are storing the number of requests for each second and the (flawed) concept of "buckets" is now gone. Let me know what you think.

Also, what do you think about using something like this for the chart style: https://echarts.apache.org/examples/en/editor.html?c=area-simple. I imagine that it could be quite useful specially for long-running load tests (ability to zoom in, etc.).

jeremyandrews

This is much cleaner, thanks for reworking!

The graph should include ramp-up, running, and ramp-down (and ideally should visually indicate each somehow). I also like the idea of a different graph type that allows you to zoom in.

src/config.rs

jeremyandrews · 2021-11-11T06:02:06Z

src/metrics.rs

@@ -2415,6 +2434,9 @@ impl GooseAttack {
            if self.configuration.status_codes {
                merge_request.set_status_code(request_metric.status_code);
            }
+            if !self.configuration.report_file.is_empty() {
+                merge_request.record_requests_per_second(self.metrics.duration);


We already have self, so no need to pass self.metrics.duration

The graph should include ramp-up time and ramp-down time -- in the following test I ran with the following parameters:

cargo run --release --example drupal_memcache -- --host http://apache/ -v -u5 -t10 --report-file report.html --throttle-requests 11

It's not including the ramp-up time, and it needs to. Also, I think we're missing the first second, as we should see 10 seconds of requests for ~11 rps:

We already have self, so no need to pass self.metrics.duration

record_requests_per_second() is a function on a different struct so self represents something else there. And duration is not available there, so we need to send it.

jeremyandrews · 2021-11-11T08:41:41Z

I also changed how the data is stored. Now we are storing the number of requests for each second and the (flawed) concept of "buckets" is now gone. Let me know what you think.

This is much simpler, I like it!

Note that the data is still getting cut off: I ran a load test for 1 hour (3600 seconds) but the data was cut off -- I manually added the missing bit of the report to see the data that was recorded. Likely this is a limitation of the current --report-file implementation, and a buffer or something will need to be increase to support larger file sizes.

Also, what do you think about using something like this for the chart style: https://echarts.apache.org/examples/en/editor.html?c=area-simple. I imagine that it could be quite useful specially for long-running load tests (ability to zoom in, etc.).

I like that idea very much, it would add richness to the charts being able to zoom in.

Perhaps the X axis should be actual timestamps though, instead of seconds since the test started? I wonder which is more useful? (The latter would be easier to match up against data in New Relic etc)

slashrsm · 2021-11-11T08:53:38Z

I am still trying to figure out the cutting off problem (I am able to reproduce it). I fell that there is something wrong with JSON serialization or maybe format! when the data series grows over certain point. Looking into it as we speak, but is quite time consuming, since I can only reproduce it if I run a load test for a while.

slashrsm · 2021-11-11T20:39:28Z

I improved the appearance of the graph:

Improved styling of the line
Added zoom support
Added tooltip on mouse hover
Marked starting and stopping period (I can't make the latter to appear as it completes so quickly)
Added "Save as image" function.

I noticed that there is always a spike of requests exactly at the point when the load test was started up and --no-reset-metrics. Is it possible that we are recording some requests twice in this case?

With `--no-reset-metrics`

Without `--no-reset-metrics`

jeremyandrews · 2021-11-12T10:54:22Z

I think your graphs in the PR are labeled backwards -- the first one looks like --no-reset-metrics IS enabled, the second looks like --no-reset-metrics is NOT enabled.

jeremyandrews · 2021-11-12T11:02:32Z

Any idea what this spike is at the start of the load test? It seems to show up in the graphs semi-regularly (in this case I'm using the following options: -v --report-file report.html -u10 -s10 -t10 --no-reset-metrics:

jeremyandrews · 2021-11-12T11:02:36Z

Any idea what this spike is at the start of the load test? It seems to show up in the graphs semi-regularly (in this case I'm using the following options: -v --report-file report.html -u10 -s10 -t10 --no-reset-metrics:

jeremyandrews

This is looking very nice! I like the new charts much better.

When we are displaying starting/stopping, perhaps we could use "gray" instead of a red shade, as the latter draws more attention to it than it likely should
It would be ideal to create an automated test that confirms the data in the report is truly accurate
We either need to preserve a copy of requests made during Startup when running with "--no-reset-metrics" so we can show all data in the chart, or not show that section of time, as it's confusing/wrong to show it blank
I was able to run an hour long load test and got full data, the report file was only 117k

src/metrics.rs

jeremyandrews · 2021-11-12T10:50:24Z

src/metrics.rs

+                && Some(stopped) == self.metrics.stopped
+            {
+                let mut count = 0;
+                for (_path, path_metric) in self.metrics.requests.iter() {


In src/metrics.rs see reset_metrics() which includes this logic:

self.metrics.requests = HashMap::new();

Thus, in the default configuration all metrics from starting are not available to you by the time the html report is written. That explains the following chart:

If --report-file is enabled and --no-reset-metrics is not enabled, we should store a copy of the metrics.requests "somewhere else" before we reset the metrics so we can generate a complete chart. Alternatively, in this configuration perhaps we should simply not display the "Starting" portion of the chart as it will always be blank.

… default and remove additional options.

…ritten.

…ng and stopping period.

… reset.

slashrsm · 2021-11-18T20:54:39Z

I played with the color scheme and aspect ratio and I think that graph looks better now. Starting phase is only displayed when --no-reset-metrics is used and stopping phase is only displayed if it lasts for long enough to actually be visible on the graph.

Screenshots

With `--no-reset-metrics`

Without `--no-reset-metrics`

jeremyandrews · 2021-11-19T14:21:07Z

This is a very nice enhancement, and only affects the HTML report generated when the load test ends. Committing as is: the rest of your TODO will make great followups -- thanks!

slashrsm commented Nov 8, 2021

View reviewed changes

src/config.rs Outdated Show resolved Hide resolved

slashrsm commented Nov 8, 2021

View reviewed changes

src/report.rs Show resolved Hide resolved

slashrsm force-pushed the 246_html_report_graph branch from 38c81fc to 537e093 Compare November 8, 2021 14:44

jeremyandrews requested changes Nov 9, 2021

View reviewed changes

slashrsm force-pushed the 246_html_report_graph branch 2 times, most recently from b87c207 to f54fc73 Compare November 10, 2021 20:01

jeremyandrews requested changes Nov 11, 2021

View reviewed changes

jeremyandrews requested changes Nov 12, 2021

View reviewed changes

slashrsm added 8 commits November 18, 2021 18:08

tag1consulting#246: Add requests per second graph to the HTML report.

d923bb4

tag1consulting#246: Remove unnecessary use statement.

45fa7e9

tag1consulting#246: Enable requests per second graph on the report by…

923c6d0

… default and remove additional options.

tag1consulting#246: Record RPS for each second.

c81f9b1

tag1consulting#246: Mention eChars in the report file docs.

bb61d16

tag1consulting#246: Make sure that HTML reports of every length get w…

92e3920

…ritten.

tag1consulting#246: Undo unwanted changes.

164cb29

tag1consulting#246: Improve graph - appearance, zoom, tooltip, starti…

93fcf3d

…ng and stopping period.

slashrsm force-pushed the 246_html_report_graph branch from 7ec8fc0 to 93fcf3d Compare November 18, 2021 18:08

slashrsm added 4 commits November 18, 2021 18:23

tag1consulting#246: Minor cosmetic fix and a comment.

c71a082

tag1consulting#246: Improve graph color scheme.

eeecef0

tag1consulting#246: Skip starting part on the graphs when metrics are…

1350194

… reset.

tag1consulting#246: Make graph shorter (1:2 aspect ratio).

0a12dea

jeremyandrews marked this pull request as ready for review November 19, 2021 14:19

jeremyandrews merged commit 4988799 into tag1consulting:main Nov 19, 2021

This was referenced Nov 19, 2021

add charts to HTML report #246

Closed

Investigate spike or requests on transition between starting and started phase #386

Closed

jeremyandrews mentioned this pull request Nov 19, 2021

release 0.15.1 #389

Merged

slashrsm mentioned this pull request Nov 21, 2021

Add automated tests for requests per second graph in HTML report #392

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add requests per second graph to the HTML report. #381

Add requests per second graph to the HTML report. #381

slashrsm commented Nov 8, 2021 •

edited

Loading

slashrsm Nov 8, 2021

jeremyandrews left a comment

slashrsm commented Nov 10, 2021

jeremyandrews left a comment

jeremyandrews Nov 11, 2021

jeremyandrews Nov 11, 2021

slashrsm Nov 11, 2021

jeremyandrews commented Nov 11, 2021

slashrsm commented Nov 11, 2021

slashrsm commented Nov 11, 2021 •

edited

Loading

jeremyandrews commented Nov 12, 2021

jeremyandrews commented Nov 12, 2021

jeremyandrews commented Nov 12, 2021

jeremyandrews left a comment

jeremyandrews Nov 12, 2021

jeremyandrews Nov 12, 2021

slashrsm commented Nov 18, 2021

jeremyandrews commented Nov 19, 2021

Add requests per second graph to the HTML report. #381

Add requests per second graph to the HTML report. #381

Conversation

slashrsm commented Nov 8, 2021 • edited Loading

TODO

Screenshots

With --no-reset-metrics

Without --no-reset-metrics

slashrsm Nov 8, 2021

Choose a reason for hiding this comment

jeremyandrews left a comment

Choose a reason for hiding this comment

slashrsm commented Nov 10, 2021

jeremyandrews left a comment

Choose a reason for hiding this comment

jeremyandrews Nov 11, 2021

Choose a reason for hiding this comment

jeremyandrews Nov 11, 2021

Choose a reason for hiding this comment

slashrsm Nov 11, 2021

Choose a reason for hiding this comment

jeremyandrews commented Nov 11, 2021

slashrsm commented Nov 11, 2021

slashrsm commented Nov 11, 2021 • edited Loading

With --no-reset-metrics

Without --no-reset-metrics

jeremyandrews commented Nov 12, 2021

jeremyandrews commented Nov 12, 2021

jeremyandrews commented Nov 12, 2021

jeremyandrews left a comment

Choose a reason for hiding this comment

jeremyandrews Nov 12, 2021

Choose a reason for hiding this comment

jeremyandrews Nov 12, 2021

Choose a reason for hiding this comment

slashrsm commented Nov 18, 2021

Screenshots

With --no-reset-metrics

Without --no-reset-metrics

jeremyandrews commented Nov 19, 2021

slashrsm commented Nov 8, 2021 •

edited

Loading

With `--no-reset-metrics`

Without `--no-reset-metrics`

slashrsm commented Nov 11, 2021 •

edited

Loading

With `--no-reset-metrics`

Without `--no-reset-metrics`

With `--no-reset-metrics`

Without `--no-reset-metrics`