- Notifications
You must be signed in to change notification settings - Fork927
feat(cli): add trafficgen command for load testing#7307
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
5163ed5 to7fe2e74Compare| Data string `json:"data,omitempty"` | ||
| Height uint16 `json:"height,omitempty"` | ||
| Width uint16 `json:"width,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
review: Added omitempty here to reduce the size of the payload.
| r.vscodeSSH(), | ||
| r.workspaceAgent(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
review: I'm assuming all of this wants to be sorted alphabetically.
| var ( | ||
| pageNumber = 0 | ||
| limit = 100 | ||
| workspaces []codersdk.Workspace | ||
| ) | ||
| for { | ||
| page, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ | ||
| Name: "scaletest-", | ||
| Offset: pageNumber * limit, | ||
| Limit: limit, | ||
| }) | ||
| if err != nil { | ||
| return xerrors.Errorf("fetch scaletest workspaces page %d: %w", pageNumber, err) | ||
| } | ||
| pageNumber++ | ||
| if len(page.Workspaces) == 0 { | ||
| break | ||
| } | ||
| pageWorkspaces := make([]codersdk.Workspace, 0, len(page.Workspaces)) | ||
| for _, w := range page.Workspaces { | ||
| if isScaleTestWorkspace(w) { | ||
| pageWorkspaces = append(pageWorkspaces, w) | ||
| } | ||
| } | ||
| workspaces = append(workspaces, pageWorkspaces...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
review: extracted to function
| pageNumber = 0 | ||
| limit = 100 | ||
| var users []codersdk.User | ||
| for { | ||
| page, err := client.Users(ctx, codersdk.UsersRequest{ | ||
| Search: "scaletest-", | ||
| Pagination: codersdk.Pagination{ | ||
| Offset: pageNumber * limit, | ||
| Limit: limit, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return xerrors.Errorf("fetch scaletest users page %d: %w", pageNumber, err) | ||
| } | ||
| pageNumber++ | ||
| if len(page.Users) == 0 { | ||
| break | ||
| } | ||
| pageUsers := make([]codersdk.User, 0, len(page.Users)) | ||
| for _, u := range page.Users { | ||
| if isScaleTestUser(u) { | ||
| pageUsers = append(pageUsers, u) | ||
| } | ||
| } | ||
| users = append(users, pageUsers...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
review: extracted to function
Uh oh!
There was an error while loading.Please reload this page.
89e9f95 to4f165beCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's a bit unclear to me why we need the context based read/writes and why we're doing so one character at a time, but other than that the PR looks good. 👍🏻
| codersdk.BypassRatelimitHeader: {"true"}, | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Suggestion: Since header transport isn't implementing close idler,client.HTTPClient.CloseIdleConnections() won't work. This might be a good addition to avoid leaks in tests:
(Or alternatively, implementingtype closeIdler interface { CloseIdleConnections() } onheaderTransport and doing the client variant.)
deferhttp.DefaultTransport.(*http.Transport).CloseIdleConnections()
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
cli/root.go Outdated
| r.gitssh(), | ||
| r.scaletest(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
scaletest is no longer hidden, even if yourtrafficgen subcommand is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Un-hidden!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry, I meant thatcoder scaletest is not hidden, so this shouldn't be in the// Hidden section of the list.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
found an outdated comment, but I don't need to review again before merging.
scaletest/workspacetraffic/config.go Outdated
| // Duration is the total duration for which to send traffic to the agent. | ||
| Duration time.Duration `json:"duration"` | ||
| // TicksInterval specifies how many times per second we send traffic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this comment is outdated; this is now the interval/period, not the frequency.
Uh oh!
There was an error while loading.Please reload this page.
This PR adds a hidden
scaletest trafficgencommand for load testing.ReconnectingPTYconnection to each scaletest workspace, and concurrently writes and reads random data to/from the PTY.#${RANDOM_ALPHANUMERIC_STRING}, which essentially drops garbage comments in the remote shell, and should not result in any commands being executed.Future work: