-
Notifications
You must be signed in to change notification settings - Fork 889
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
5163ed5
to
7fe2e74
Compare
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
89e9f95
to
4f165be
Compare
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.
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, implementing type closeIdler interface { CloseIdleConnections() }
on headerTransport
and doing the client variant.)
defer http.DefaultTransport.(*http.Transport).CloseIdleConnections()
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 your trafficgen
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 that coder scaletest
is not hidden, so this shouldn't be in the // Hidden
section of the list.
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.
This PR adds a hidden
scaletest trafficgen
command for load testing.ReconnectingPTY
connection 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: