search

tag1consulting / goose

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

Allow shared reqwest client configuration #357

Open finnefloyd opened 3 years ago

finnefloyd commented 3 years ago

From Reqwest official documentation:

The Client holds a connection pool internally, so it is advised that you create one and reuse it. You do not have to wrap the Client it in an Rc or Arc to reuse it, because it already uses an Arc internally.

This PR, use a shared reqwest client among all GooseUser and allow the user the configure the global client.

jeremyandrews commented 3 years ago

Merge conflict needs to be resolved

jeremyandrews commented 3 years ago

It's unclear to me if this will result in cookies and headers being shared between all GooseUsers, which would be undesirable.

finnefloyd commented 3 years ago

It's unclear to me if this will result in cookies and headers being shared between all GooseUsers, which would be undesirable.

Goose point, I did not think about it and I'm not using cookies in my current load tests, I didn't detect it. I moved the cookies handling to the GooseUser so we can shared the connection pool

jeremyandrews commented 3 years ago

This addresses cookies, but there’s still a problem from shared headers. For example, if using http header authentication, with this PR once one GooseUser logs in all would be.

jeremyandrews commented 3 years ago

For example, as documented here it's currently possible to set different headers with different GooseUsers, and this is required functionality for many load tests: https://docs.rs/goose/*/goose/goose/struct.GooseUser.html#example-18

jeremyandrews commented 3 years ago

This example will also need to be updated to show how to set custom cookies: https://docs.rs/goose/*/goose/goose/struct.GooseUser.html#example-19

finnefloyd commented 3 years ago

I added support for default headers and custom cookies to the GooseUser.

LionsAd commented 3 years ago

@finnefloyd Thanks for all your contributions so far!

I share Jeremy's sentiment that I would like to understand the motivation of the change a little bit more, before considering this PR.

netom commented 1 year ago

@finnefloyd Thanks for all your contributions so far!

I share Jeremy's sentiment that I would like to understand the motivation of the change a little bit more, before considering this PR.

I used Goose recently, and I needed to create thoudands of users. Starting Goose took about thirty seconds. The tests themselves were not terribly long, so the startup time was significant.

I solved this by monkey-patching goose to use a single reqwest client. (Cookies and headers weren't an issue for me.) This way the statup time disappeared.

I know this is a corner case, but IMHO worth implementing client sharing.

The general argument would be "because it's just more resource efficient".