FrameworkBenchmarks icon indicating copy to clipboard operation
FrameworkBenchmarks copied to clipboard

TFB build runs aren't actually representative of requests made in benchmarking

Open daviddenton opened this issue 5 years ago • 4 comments

For http4k, we have just (hopefully) worked around an issue where the Host header sent with requests differs between the benchmarking suite and the build.

In the build and verification suite, the host header is set to "tfb-server:9000" In the benchmarking, the host header is set to "10.0.0.1"

For the Apache5 backend that we support, this change stopped the server responding to requests.

It would be best if both runs could match what they set in order that framework writers don't have to wait for an entire benchmark to run to get feedback.

OS (Please include kernel version)

Expected Behavior

Actual Behavior

Steps to reproduce behavior

Other details and logs

daviddenton avatar Sep 03 '20 15:09 daviddenton

Looking into this. It'd be nice to have parity there.

NateBrady23 avatar Sep 03 '20 16:09 NateBrady23

I'm going to backtrack on this. Azure and Citrine are both using IP addresses, "10.0.0.4" and "10.0.0.1", and I don't think that's an issue. I think we're expecting that your server be able to accept requests from any origin and the config should reflect that. I'd like to find a way to verify that during the verification step instead of forcing all configs to use tfb-server. Because server_host is an arg that can be changed, we don't want implementations breaking if someone tries to test your implementation where they have a different server_host config.

NateBrady23 avatar Sep 03 '20 17:09 NateBrady23

All that being said, after some discussion, it looks like we might just move to make tfb-server permanent and also follow the suggestion here #5982

We'll work this out over the next couple of days. Thanks!

NateBrady23 avatar Sep 03 '20 18:09 NateBrady23

we might just move to make tfb-server permanent

This is more difficult and messy to change than I thought, so I won't be making this change (for now) after all.

We still plan to use the tfb-server hostname in requests long term, but for now I suggest not making assertions about the value of the Host header in test implementations.

michaelhixson avatar Sep 22 '20 20:09 michaelhixson