Add src gateway benchmark command #1124
Conversation
| var gatewayCommands commander | ||
|
|
||
| func init() { | ||
| usage := `'src gateway' manages Cody Gateway operations on a Sourcegraph instance. |
There was a problem hiding this comment.
Since Cody Gateway doesn't run as part of a Sourcegraph instance, I think this phrasing is maybe a bit confusing?
Perhaps "interacts with Cody Gateway directly or through a Sourcegraph instance" may be better?
| green = "\033[32m" | ||
| yellow = "\033[33m" | ||
| red = "\033[31m" | ||
| reset = "\033[0m" |
There was a problem hiding this comment.
I think we should be using the stuff in color.go here: https://sourcegraph.sourcegraph.com/github.com/sourcegraph/src-cli/-/blob/cmd/src/colors.go
| flagSet := flag.NewFlagSet("benchmark", flag.ExitOnError) | ||
|
|
||
| var ( | ||
| requestCount = flagSet.Int("requests", defaultRequestCount, "Number of requests to make per endpoint") |
There was a problem hiding this comment.
Maybe just write defaultRequestCount inline here instead of a constant (easier to see what the defautl is that way, less likely to conflict with other constants / files in the folder)
There was a problem hiding this comment.
Also may want to bump this to something statistically significant (e.x. 1000)
| @@ -53,6 +53,7 @@ The commands are: | |||
| config manages global, org, and user settings | |||
| extensions,ext manages extensions (experimental) | |||
| extsvc manages external services | |||
| gateway manages Cody Gateway | |||
There was a problem hiding this comment.
| gateway manages Cody Gateway | |
| gateway interact with Cody Gateway |
| Transport: &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, | ||
| }, | ||
| } |
There was a problem hiding this comment.
I think it's fine if we have a CLI option for --insecure to disable TLS for testing against local dev, but this wouldn't match a real prod environment (where TLS verification may be a substantial overhead) so let's make sure we have TLS verification enabled by default here
|
|
||
| endpoints := map[string]string{ | ||
| "HTTP": fmt.Sprintf("%s/cody-gateway-call-http", cfg.Endpoint), | ||
| "HTTP then WebSocket": fmt.Sprintf("%s/cody-gateway-call-http-then-websocket", cfg.Endpoint), |
There was a problem hiding this comment.
let's work torwards naming these endpoints whatever the real V2 API endpoint would be named (esp. before merging)
| if duration > 0 { | ||
| durations = append(durations, duration) | ||
| } | ||
| fmt.Printf("\rTesting %s: %d/%d", name, i+1, *requestCount) |
There was a problem hiding this comment.
let's drop this print statement, or make it only print on every 1/10th of the requestCount - to reduce the chance that printing each request adds non-deterministic latency.
| } | ||
| fmt.Println() | ||
|
|
||
| avg, p5, p95, median, total := calculateStats(durations) |
There was a problem hiding this comment.
let's make calculateStats return a struct with these as fields, so there's no chance we get the order wrong here and look at the wrong thing by accident.
| fmt.Printf("Error reading response body: %v\n", err) | ||
| return 0 | ||
| } | ||
| fmt.Printf("Response from %s: %s\n", url, body) |
There was a problem hiding this comment.
similarly, let's drop this print statement in the success path
| } | ||
|
|
||
| sort.Slice(durations, func(i, j int) bool { | ||
| return durations[i] < durations[j] |
There was a problem hiding this comment.
adding a note here for me to triple check we got the order right here; just don't want any chance we've got p95 and p5 inverted.
Actually; could you add p80 and p75 as well? that'd make it clear and might be useful to collect anyway.
| return yellow + value + reset | ||
| } | ||
|
|
||
| func printResults(results []endpointResult, requestCount *int) { |
There was a problem hiding this comment.
Can we add a CLI flag to dump the results as e.g. a CSV as well? When we hand this out to ~10 people to collect results it might be nicer when we try to import them into e.g. a google sheet
|
Cool, great starting point! can you get this client talking to / benchmarking 4 endpoints? e.g. with one test
i.e. if the user passes Then the default if you pass nothing can be to run against Cody Gateway prod and Sourcegraph.com For (2) and (4) you'll need to add a websocket client to this PR, you should be able to do that testing against any websocket server until I have a chance to debug the issue you brought up with doing it in the SG instance. |
Adds the
src gateway benchmarkcommand to the tool.This is a WIP.
Missing work to be done:
{codyGatewayUrl}/v1/hello-httpand{codyGatewayUrl}/v1/hello-websocket.Test plan
Tested manually:
go run ./cmd/srclists the newgatewaycommand correctly.go run ./cmd/src gatewaylists thebenchmarksubcommand correctly.go run ./cmd/src gateway benchmarkruns the benchmarks against the endpoint set in theSRC_ENDPOINTenv var correctly.go run ./cmd/src gateway benchmark --requests 2sets the number of requests correctly.SRC_ENDPOINT=http://localhost:3080 go run ./cmd/src gateway benchmark --requests 2sets the endpoint to my localhost and works correctly.