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

make it possible to disable cookies and optimize startup #578

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- [#568](https://github.com/tag1consulting/goose/pull/568) don't panic when truncating non utf-8 string
- [#574](https://github.com/tag1consulting/goose/pull/574) update [`http`](https://docs.rs/http), [`itertools`](https://docs.rs/itertools) [`nix`](https://docs.rs/nix), [`rustls`](https://docs.rs/rustls/), and [`serial_test`](https://docs.rs/serial_test)
- [#575](https://github.com/tag1consulting/goose/pull/575) add test coverage for sessions and cookies, revert [#557](https://github.com/tag1consulting/goose/pull/557) to avoid sharing the CookieJar between all users
- [#578](https://github.com/tag1consulting/goose/pull/578) add `cookies` feature enabled by default, optimize startup if disabled

## 0.17.2 August 28, 2023
- [#557](https://github.com/tag1consulting/goose/pull/557) speed up user initialization on Linux
Expand Down
6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ num-format = "0.4"
rand = "0.8"
regex = "1"
reqwest = { version = "0.11", default-features = false, features = [
"cookies",
"gzip",
"json",
] }
Expand All @@ -52,8 +51,9 @@ tungstenite = "0.20"
url = "2"

[features]
default = ["reqwest/default-tls"]
rustls-tls = ["reqwest/rustls-tls", "tokio-tungstenite/rustls"]
default = ["reqwest/default-tls", "cookies"]
rustls-tls = ["reqwest/rustls-tls", "tokio-tungstenite/rustls", "cookies"]
cookies = ["reqwest/cookies"]

[dev-dependencies]
httpmock = "0.6"
Expand Down
10 changes: 10 additions & 0 deletions src/docs/goose-book/src/config/cookie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Cookies

By default, Goose enables the Reqwest `cookies` feature. However, if you are running a load test that doesn't use cookies, you can disable this feature. This disables the feature in Reqwest, and allows an optimization during Goose startup (see https://github.com/tag1consulting/goose/pull/557 for more information).

To disable client cookies and optimize startup performance, disable default features and pick a tls client in `Cargo.toml`, for example:

```toml
[dependencies]
goose = { version = "^0.17", default-features = false, features = ["reqwest/default-tls"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

When copying the example for testing the branch, I got this error below. Perhaps there should be additional cargo features that only enable default-tls/rustls-tls without including cookies?

error: failed to parse manifest at `Cargo.toml`

Caused by:
  feature `reqwest/default-tls` in dependency `goose` is not allowed to contain slashes
  If you want to enable features of a transitive dependency, the direct dependency needs to re-export those features from the `[features]` table.

```
11 changes: 7 additions & 4 deletions src/goose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2311,15 +2311,18 @@ pub(crate) fn create_reqwest_client(
GOOSE_REQUEST_TIMEOUT
};

Client::builder()
let client_builder = Client::builder()
.user_agent(APP_USER_AGENT)
.cookie_store(true)
.timeout(Duration::from_millis(timeout))
// Enable gzip unless `--no-gzip` flag is enabled.
.gzip(!configuration.no_gzip)
// Validate https certificates unless `--accept-invalid-certs` is enabled.
.danger_accept_invalid_certs(configuration.accept_invalid_certs)
.build()
.danger_accept_invalid_certs(configuration.accept_invalid_certs);

#[cfg(feature = "cookies")]
let client_builder = client_builder.cookie_store(true);

client_builder.build()
}

/// Defines the HTTP requests that Goose makes.
Expand Down
17 changes: 16 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,10 +751,25 @@ impl GooseAttack {
self.test_plan.total_users()
);

// Create a single Reqwest Client to share if `cookies` are disabled.
#[cfg(not(feature = "cookies"))]
let reqwest_client = goose::create_reqwest_client(&self.configuration)?;

let mut weighted_users = Vec::new();
let mut user_count = 0;
loop {
for scenarios_index in &weighted_scenarios {
// If the `cookies` feature is enabled, create a new reqwest client
// for each GooseUser, giving each their own CookieJar.
#[cfg(feature = "cookies")]
let client = goose::create_reqwest_client(&self.configuration)?;

// If the `cookies` feature is disabled, create a single client (see
// above, the client is created outside this loop) and clone it for each
// GooseUser.
#[cfg(not(feature = "cookies"))]
let client = reqwest_client.clone();

debug!(
"creating user state: {} ({})",
weighted_users.len(),
Expand All @@ -771,7 +786,7 @@ impl GooseAttack {
base_url,
&self.configuration,
self.metrics.hash,
Some(goose::create_reqwest_client(&self.configuration)?),
Some(client),
)?);
user_count += 1;
if user_count == total_users {
Expand Down
22 changes: 21 additions & 1 deletion tests/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,38 @@ const COOKIE_PATH: &str = "/cookie";
// Indexes for valid requests of above paths, used to validate tests.
const POST_SESSION_KEY: usize = 0;
const GET_SESSION_KEY: usize = 1;

#[cfg(feature = "cookies")]
const POST_COOKIE_KEY_0: usize = 2;
#[cfg(feature = "cookies")]
const GET_COOKIE_KEY_0: usize = 6;
#[cfg(feature = "cookies")]
const POST_COOKIE_KEY_1: usize = 7;
#[cfg(feature = "cookies")]
const GET_COOKIE_KEY_1: usize = 11;
#[cfg(feature = "cookies")]
const POST_COOKIE_KEY_2: usize = 12;
#[cfg(feature = "cookies")]
const GET_COOKIE_KEY_2: usize = 16;
#[cfg(feature = "cookies")]
const POST_COOKIE_KEY_3: usize = 17;
#[cfg(feature = "cookies")]
const GET_COOKIE_KEY_3: usize = 21;

// How many users to simulate, each with their own session.
const SESSION_USERS: &str = "10";

#[cfg(feature = "cookies")]
// How many users to simulate, each with their own cookie.
const COOKIE_USERS: &str = "4";

// There are multiple test variations in this file.
// By default, there are multiple test variations in this file. It is possible to
// disable the `cookies` feature, which will also disable the related tests.
#[derive(Clone)]
enum TestType {
// Test sessions.
Session,
#[cfg(feature = "cookies")]
// Test cookies.
Cookie,
}
Expand Down Expand Up @@ -85,6 +97,7 @@ pub async fn validate_session_data(user: &mut GooseUser) -> TransactionResult {
Ok(())
}

#[cfg(feature = "cookies")]
// Set a cookie that is unique per-user.
pub async fn set_cookie(user: &mut GooseUser) -> TransactionResult {
// Per-user cookie name.
Expand Down Expand Up @@ -296,6 +309,7 @@ fn common_build_configuration(
"--run-time",
"2",
],
#[cfg(feature = "cookies")]
TestType::Cookie => vec![
"--users",
COOKIE_USERS,
Expand All @@ -320,6 +334,7 @@ fn validate_requests(test_type: TestType, goose_metrics: &GooseMetrics, mock_end
TestType::Session => SESSION_USERS
.parse::<usize>()
.expect("must be a valid usize"),
#[cfg(feature = "cookies")]
TestType::Cookie => COOKIE_USERS
.parse::<usize>()
.expect("must be a valid usize"),
Expand All @@ -332,6 +347,7 @@ fn validate_requests(test_type: TestType, goose_metrics: &GooseMetrics, mock_end
// Confirm that each user validated their session multiple times.
assert!(mock_endpoints[GET_SESSION_KEY].hits() > users);
}
#[cfg(feature = "cookies")]
TestType::Cookie => {
// Confirm that each user set a cookie one and only one time.
assert!(mock_endpoints[POST_COOKIE_KEY_0].hits() == 1);
Expand All @@ -349,12 +365,14 @@ fn validate_requests(test_type: TestType, goose_metrics: &GooseMetrics, mock_end
// Extract the POST requests out of goose metrics.
let post_metrics = match test_type {
TestType::Session => goose_metrics.requests.get("POST create session").unwrap(),
#[cfg(feature = "cookies")]
TestType::Cookie => goose_metrics.requests.get("POST create cookie").unwrap(),
};

// Extract the GET requests out of goose metrics.
let get_metrics = match test_type {
TestType::Session => goose_metrics.requests.get("GET read session").unwrap(),
#[cfg(feature = "cookies")]
TestType::Cookie => goose_metrics.requests.get("GET read cookie").unwrap(),
};

Expand Down Expand Up @@ -386,6 +404,7 @@ fn get_scenarios(test_type: &TestType) -> Scenario {
// Validate the session repeateldy.
.register_transaction(transaction!(validate_session_data).set_name("read session"))
}
#[cfg(feature = "cookies")]
TestType::Cookie => {
scenario!("Cookie")
// Create the cookie only one time
Expand Down Expand Up @@ -435,6 +454,7 @@ async fn test_session() {
run_standalone_test(TestType::Session).await;
}

#[cfg(feature = "cookies")]
#[tokio::test]
// Test to confirm cookies are unique per GooseUser and last their lifetime.
async fn test_cookie() {
Expand Down