search

tag1consulting / goose

Load testing framework, inspired by Locust
https://tag1.com/goose
Apache License 2.0
813 stars 71 forks source link

Refactor reports/metrics, add JSON and markdown report #600

Closed ctron closed 3 months ago

ctron commented 4 months ago

This PR refactors the code of the metrics and report module a bit. It extracts the common part into a common module, allowing to prepare the data consistently and then rendering it out into HTML, and also serializing the data into JSON.

For that the --report-file argument has been changed into a Vec<String> to allow providing more than one file. It uses the file extension as an indicator of which file type should be created.

In addition, this should make it easier to implement a markdown report too.

ctron commented 4 months ago

I updated the PR to make clippy happy.

Although these are not files touched by the PR, it looks like newer Rust versions brought in new lints that now fail.

ctron commented 4 months ago

I updated the PR to support markdown as well. It should be pretty close to the HTML report, but without the charts.

jeremyandrews commented 4 months ago

This looks great from a read through on my phone. I’m traveling internationally but will find time to test it and get it merged.

In a follow up PR, it would be great to update the book documentation too.

ctron commented 4 months ago

This looks great from a read through on my phone. I’m traveling internationally but will find time to test it and get it merged.

In a follow up PR, it would be great to update the book documentation too.

Cool. Take your time. I can update the PR over time. I am currently also trying to add the baseline feature. We can split that off, but it's based on the other changes anyway.

JimFuller-RedHat commented 4 months ago

does this mean a new release as well ? nudge nudge

ctron commented 4 months ago

I pushed the change for baseline comparison too. Of course we can split this off. Looks like this:

image

jeremyandrews commented 4 months ago

I pushed the change for baseline comparison too. Of course we can split this off.

Please split this off -- I'd like to get the first bits committed first.

ctron commented 3 months ago

I pushed the change for baseline comparison too. Of course we can split this off.

Please split this off -- I'd like to get the first bits committed first.

I split it off (#602) and rebased the PR.

jeremyandrews commented 3 months ago

I'm finally starting to test this now. We need to improve the documentation, as it's non-obvious what format of report files are supported. Further, it feels like there should be some sort of validation? For example, if you accidentally type --report-file report.htm it will create the file but not log anything there. While this isn't the expected filename on a Unix-based system, wouldn't it be legit on Windows? And, if you type --report-file report.foo it will happily create that filename, but then it won't write data to it.

ctron commented 3 months ago

Updated the PR, and the docs as well.

jeremyandrews commented 3 months ago

This looks great, thanks for all your updates! I'm only confused about the test failures: locally I can't duplicate this, and these tests that are failing should be ignored. Any ideas?

I see, we're running with --all-features, so adding back the gaggle feature is causing these tests to run, while the contained code is no longer supported (for now).

jeremyandrews commented 3 months ago

We can easily fix the gaggle issue in a follow-up, I've confirmed all other tests run correctly and without errors.

This new functionality is greatly appreciated, thanks for the contribution!