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

Introduce --test-plan for more complex test plans #422

Merged
merged 13 commits into from
Apr 19, 2022

Conversation

jeremyandrews
Copy link
Member

@jeremyandrews jeremyandrews commented Mar 20, 2022

  • introduce --test-plan and GooseDefault::TestPlan for configuring test plans
  • closes Specify the traffic profile/shape? #365
  • use [FromStr] to auto convert --test-plan "{users},{timespan};{users},{timespan}", where {users} must be an integer, ie "100", and {timespan} can be integer seconds or "30s", "20m", "3h", "1h30m", etc, to internal Vec<(usize, usize)> representation
  • don't allow --test-plan together with --users, --startup-time, --hatch-rate or --run-time
  • add page to https://book.goose.rs/ explaining how to configure test plans

From the new book page:

The following command tells Goose to start 10 users over 60 seconds and then to run for 5 minutes before shutting down:

$ cargo run --release -- -H http://local.dev/ --startup-time 1m --users 10 --run-time 5m

The exact same behavior can be defined with the following test plan:

$ cargo run --release -- -H http://local.dev/ --test-plan "10,1m;10,5m;0,0"

Another possibility when specifying a test plan is to add load spikes into otherwise steady load. For example, in the following example Goose starts 500 users over 5 minutes and lets it run with a couple of traffic spikes to 2,500 users:

$ cargo run --release -- -H http://local.dev/ --test-plan "500,5m;500,5m;2500,45;500,45;500,5m;2500,45;500,45;500,5m;0,0"

In the current implementation, a test plan can only Increase and Maintain, the load test will shut down any time it defines a Decrease. This will be resolved in a followup.

An example run:

% cargo run --release --example drupal_memcache -- --host http://apache.fosciana/ --test-plan "10,5;20,5;30,5;30,5;60,10;60,2;0,0"

Includes the follow overview in the metrics:

 === OVERVIEW ===
 ------------------------------------------------------------------------------
 Action       Started             Stopped           Elapsed    Users
 ------------------------------------------------------------------------------
 Increasing:  22-04-18 07:07:06 - 22-04-18 07:07:11 (00:00:05, 0 -> 10)
 Increasing:  22-04-18 07:07:11 - 22-04-18 07:07:17 (00:00:06, 10 -> 20)
 Increasing:  22-04-18 07:07:17 - 22-04-18 07:07:22 (00:00:05, 20 -> 30)
 Maintaining: 22-04-18 07:07:22 - 22-04-18 07:07:27 (00:00:05, 30)
 Increasing:  22-04-18 07:07:27 - 22-04-18 07:07:40 (00:00:13, 30 -> 60)
 Maintaining: 22-04-18 07:07:40 - 22-04-18 07:07:42 (00:00:02, 60)
 Decreasing:  22-04-18 07:07:42 - 22-04-18 07:07:42 (00:00:00, 0 <- 60)

 Target host: http://apache.fosciana/
 goose v0.16.0-dev
 ------------------------------------------------------------------------------

@jeremyandrews jeremyandrews changed the title WIP: introduce --test-plan for more complex test plans Introduce --test-plan for more complex test plans Apr 18, 2022
Copy link
Collaborator

@slashrsm slashrsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and I can confirm that it works as expected (with the current limitations). Great job! This is not a small task!

It would be awesome to have some more automated tests for this as we're dealing with quite significant change.

I would also propose to create a list of follow-ups that are required and mark those that are release blocker. This way we'll avoid forgetting to fix something crucial.

CHANGELOG.md Outdated
@@ -13,6 +13,8 @@
- [#379](https://github.com/tag1consulting/goose/pull/379) **API change**: default to `INFO` level verbosity, introduce `-q` to reduce Goose verbosity
o **note**: `-v` now sets Goose to `DEBUG` level verbosity which when enabled will negatively impact load test performance; set `-q` to restore previous level of verbosity
- [#379](https://github.com/tag1consulting/goose/pull/379) **API change**: remove `.print()` which is no longer required to display metrics after a load test, disable with `--no-print-metrics` or `GooseDefault::NoPrintMetrics`
- @TODO: introduce `--test-plan` and `GooseDefault::TestPlan`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR # missing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed!

Comment on lines +2071 to +2075
writeln!(
fmt,
" {:<12} {:<19} {:<17} {:<10} Users",
"Action", "Started", "Stopped", "Elapsed",
)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if this output really belongs in this file. Are these metrics or something else?

metrics.rs is already a huge file and we might want to consider splitting it at some point? Maybe this is a good time to start this discussion (actually doing it is out of scope for this PR of course).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the ASCII display of the metrics. I suppose it could be moved into report.rs if that's a better fit?

src/config.rs Outdated
Comment on lines 381 to 387
pub(crate) struct TestPlan {
// A test plan is a vector of tuples each indicating a # of users and milliseconds.
pub(crate) steps: Vec<(usize, usize)>,
// Which step of the test_plan is currently running.
pub(crate) current: usize,
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put test plan and its logic in a separate file. I would also include TestPlanStepAction and TestPlanHistory that are currently in metrics.rs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides that I'd also add some test coverage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's very little logic here, but I suppose it's likely going to grow -- splitting it out works for me. (Especially as when I was writing this I could never remember which file it lived in).

And yes, it DEFINITELY needs tests! :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split it into its own file now -- definitely the right call, thanks.

@slashrsm
Copy link
Collaborator

Awesome work! 🎉

@slashrsm slashrsm merged commit 70cfc75 into tag1consulting:main Apr 19, 2022
@LionsAd
Copy link
Collaborator

LionsAd commented Apr 19, 2022

RTBM - retro — great work 👍

@jeremyandrews jeremyandrews deleted the shape branch April 19, 2022 11:28
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.

Specify the traffic profile/shape?
3 participants