Skip to content

Commit d76fd80

Browse files
spikecurtisspikecurtis
committed
fix: use separate HTTP clients in scale test load generators
1 parent 871ed12 commit d76fd80

File tree

7 files changed

+162
-68
lines changed

7 files changed

+162
-68
lines changed

‎cli/exp_scaletest.go‎

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ import (
4848

4949
const scaletestTracerName = "coder_scaletest"
5050

51+
var BypassHeader = map[string][]string{codersdk.BypassRatelimitHeader: {"true"}}
52+
5153
func (r *RootCmd) scaletestCmd() *serpent.Command {
5254
cmd := &serpent.Command{
5355
Use: "scaletest",
@@ -690,15 +692,6 @@ func (r *RootCmd) scaletestCreateWorkspaces() *serpent.Command {
690692
return err
691693
}
692694

693-
client.HTTPClient = &http.Client{
694-
Transport: &codersdk.HeaderTransport{
695-
Transport: http.DefaultTransport,
696-
Header: map[string][]string{
697-
codersdk.BypassRatelimitHeader: {"true"},
698-
},
699-
},
700-
}
701-
702695
if count <= 0 {
703696
return xerrors.Errorf("--count is required and must be greater than 0")
704697
}
@@ -810,7 +803,13 @@ func (r *RootCmd) scaletestCreateWorkspaces() *serpent.Command {
810803
return xerrors.Errorf("validate config: %w", err)
811804
}
812805

813-
var runner harness.Runnable = createworkspaces.NewRunner(client, config)
806+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
807+
// requests being unbalanced among Coder instances.
808+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
809+
if err != nil {
810+
return xerrors.Errorf("create runner client: %w", err)
811+
}
812+
var runner harness.Runnable = createworkspaces.NewRunner(runnerClient, config)
814813
if tracingEnabled {
815814
runner = &runnableTraceWrapper{
816815
tracer: tracer,
@@ -1011,15 +1010,6 @@ func (r *RootCmd) scaletestWorkspaceUpdates() *serpent.Command {
10111010
return err
10121011
}
10131012

1014-
client.HTTPClient = &http.Client{
1015-
Transport: &codersdk.HeaderTransport{
1016-
Transport: http.DefaultTransport,
1017-
Header: map[string][]string{
1018-
codersdk.BypassRatelimitHeader: {"true"},
1019-
},
1020-
},
1021-
}
1022-
10231013
if workspaceCount <= 0 {
10241014
return xerrors.Errorf("--workspace-count must be greater than 0")
10251015
}
@@ -1158,7 +1148,14 @@ func (r *RootCmd) scaletestWorkspaceUpdates() *serpent.Command {
11581148
for i, config := range configs {
11591149
name := fmt.Sprintf("workspaceupdates-%dw", config.WorkspaceCount)
11601150
id := strconv.Itoa(i)
1161-
var runner harness.Runnable = workspaceupdates.NewRunner(client, config)
1151+
1152+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
1153+
// requests being unbalanced among Coder instances.
1154+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
1155+
if err != nil {
1156+
return xerrors.Errorf("create runner client: %w", err)
1157+
}
1158+
var runner harness.Runnable = workspaceupdates.NewRunner(runnerClient, config)
11621159
if tracingEnabled {
11631160
runner = &runnableTraceWrapper{
11641161
tracer: tracer,
@@ -1421,7 +1418,13 @@ func (r *RootCmd) scaletestWorkspaceTraffic() *serpent.Command {
14211418
if err := config.Validate(); err != nil {
14221419
return xerrors.Errorf("validate config: %w", err)
14231420
}
1424-
var runner harness.Runnable = workspacetraffic.NewRunner(client, config)
1421+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
1422+
// requests being unbalanced among Coder instances.
1423+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
1424+
if err != nil {
1425+
return xerrors.Errorf("create runner client: %w", err)
1426+
}
1427+
var runner harness.Runnable = workspacetraffic.NewRunner(runnerClient, config)
14251428
if tracingEnabled {
14261429
runner = &runnableTraceWrapper{
14271430
tracer: tracer,
@@ -1609,9 +1612,13 @@ func (r *RootCmd) scaletestDashboard() *serpent.Command {
16091612
return xerrors.Errorf("create token for user: %w", err)
16101613
}
16111614

1612-
userClient := codersdk.New(client.URL,
1613-
codersdk.WithSessionToken(userTokResp.Key),
1614-
)
1615+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
1616+
// requests being unbalanced among Coder instances.
1617+
userClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
1618+
if err != nil {
1619+
return xerrors.Errorf("create runner client: %w", err)
1620+
}
1621+
codersdk.WithSessionToken(userTokResp.Key)(userClient)
16151622

16161623
config := dashboard.Config{
16171624
Interval: interval,
@@ -1758,15 +1765,6 @@ func (r *RootCmd) scaletestAutostart() *serpent.Command {
17581765
return err
17591766
}
17601767

1761-
client.HTTPClient = &http.Client{
1762-
Transport: &codersdk.HeaderTransport{
1763-
Transport: http.DefaultTransport,
1764-
Header: map[string][]string{
1765-
codersdk.BypassRatelimitHeader: {"true"},
1766-
},
1767-
},
1768-
}
1769-
17701768
if workspaceCount <= 0 {
17711769
return xerrors.Errorf("--workspace-count must be greater than zero")
17721770
}
@@ -1832,7 +1830,13 @@ func (r *RootCmd) scaletestAutostart() *serpent.Command {
18321830
if err := config.Validate(); err != nil {
18331831
return xerrors.Errorf("validate config: %w", err)
18341832
}
1835-
var runner harness.Runnable = autostart.NewRunner(client, config)
1833+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
1834+
// requests being unbalanced among Coder instances.
1835+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
1836+
if err != nil {
1837+
return xerrors.Errorf("create runner client: %w", err)
1838+
}
1839+
var runner harness.Runnable = autostart.NewRunner(runnerClient, config)
18361840
if tracingEnabled {
18371841
runner = &runnableTraceWrapper{
18381842
tracer: tracer,

‎cli/exp_scaletest_dynamicparameters.go‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ package cli
44

55
import (
66
"fmt"
7-
"net/http"
87
"time"
98

9+
"github.com/coder/coder/v2/scaletest/loadtestutil"
1010
"github.com/prometheus/client_golang/prometheus"
1111
"github.com/prometheus/client_golang/prometheus/promhttp"
1212
"golang.org/x/xerrors"
@@ -15,7 +15,6 @@ import (
1515
"cdr.dev/slog/sloggers/sloghuman"
1616
"github.com/coder/serpent"
1717

18-
"github.com/coder/coder/v2/codersdk"
1918
"github.com/coder/coder/v2/scaletest/dynamicparameters"
2019
"github.com/coder/coder/v2/scaletest/harness"
2120
)
@@ -72,15 +71,6 @@ func (r *RootCmd) scaletestDynamicParameters() *serpent.Command {
7271
return err
7372
}
7473

75-
client.HTTPClient = &http.Client{
76-
Transport: &codersdk.HeaderTransport{
77-
Transport: http.DefaultTransport,
78-
Header: map[string][]string{
79-
codersdk.BypassRatelimitHeader: {"true"},
80-
},
81-
},
82-
}
83-
8474
reg := prometheus.NewRegistry()
8575
metrics := dynamicparameters.NewMetrics(reg, "concurrent_evaluations")
8676

@@ -122,7 +112,13 @@ func (r *RootCmd) scaletestDynamicParameters() *serpent.Command {
122112
Metrics: metrics,
123113
MetricLabelValues: []string{fmt.Sprintf("%d", part.ConcurrentEvaluations)},
124114
}
125-
var runner harness.Runnable = dynamicparameters.NewRunner(client, cfg)
115+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
116+
// requests being unbalanced among Coder instances.
117+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
118+
if err != nil {
119+
return xerrors.Errorf("create runner client: %w", err)
120+
}
121+
var runner harness.Runnable = dynamicparameters.NewRunner(runnerClient, cfg)
126122
if tracingEnabled {
127123
runner = &runnableTraceWrapper{
128124
tracer: tracer,

‎cli/exp_scaletest_notifications.go‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"sync"
1414
"time"
1515

16+
"github.com/coder/coder/v2/scaletest/loadtestutil"
1617
"github.com/google/uuid"
1718
"github.com/prometheus/client_golang/prometheus"
1819
"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -66,15 +67,6 @@ func (r *RootCmd) scaletestNotifications() *serpent.Command {
6667
return err
6768
}
6869

69-
client.HTTPClient = &http.Client{
70-
Transport: &codersdk.HeaderTransport{
71-
Transport: http.DefaultTransport,
72-
Header: map[string][]string{
73-
codersdk.BypassRatelimitHeader: {"true"},
74-
},
75-
},
76-
}
77-
7870
if userCount <= 0 {
7971
return xerrors.Errorf("--user-count must be greater than 0")
8072
}
@@ -206,7 +198,13 @@ func (r *RootCmd) scaletestNotifications() *serpent.Command {
206198
for i, config := range configs {
207199
id := strconv.Itoa(i)
208200
name := fmt.Sprintf("notifications-%s", id)
209-
var runner harness.Runnable = notifications.NewRunner(client, config)
201+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
202+
// requests being unbalanced among Coder instances.
203+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
204+
if err != nil {
205+
return xerrors.Errorf("create runner client: %w", err)
206+
}
207+
var runner harness.Runnable = notifications.NewRunner(runnerClient, config)
210208
if tracingEnabled {
211209
runner = &runnableTraceWrapper{
212210
tracer: tracer,

‎cli/exp_scaletest_prebuilds.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ package cli
44

55
import (
66
"fmt"
7-
"net/http"
87
"os/signal"
98
"strconv"
109
"sync"
1110
"time"
1211

12+
"github.com/coder/coder/v2/scaletest/loadtestutil"
1313
"github.com/prometheus/client_golang/prometheus"
1414
"github.com/prometheus/client_golang/prometheus/promhttp"
1515
"golang.org/x/xerrors"
@@ -56,15 +56,6 @@ func (r *RootCmd) scaletestPrebuilds() *serpent.Command {
5656
return err
5757
}
5858

59-
client.HTTPClient = &http.Client{
60-
Transport: &codersdk.HeaderTransport{
61-
Transport: http.DefaultTransport,
62-
Header: map[string][]string{
63-
codersdk.BypassRatelimitHeader: {"true"},
64-
},
65-
},
66-
}
67-
6859
if numTemplates <= 0 {
6960
return xerrors.Errorf("--num-templates must be greater than 0")
7061
}
@@ -140,7 +131,13 @@ func (r *RootCmd) scaletestPrebuilds() *serpent.Command {
140131
return xerrors.Errorf("validate config: %w", err)
141132
}
142133

143-
var runner harness.Runnable = prebuilds.NewRunner(client, cfg)
134+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
135+
// requests being unbalanced among Coder instances.
136+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
137+
if err != nil {
138+
return xerrors.Errorf("create runner client: %w", err)
139+
}
140+
var runner harness.Runnable = prebuilds.NewRunner(runnerClient, cfg)
144141
if tracingEnabled {
145142
runner = &runnableTraceWrapper{
146143
tracer: tracer,

‎cli/exp_scaletest_taskstatus.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"sync"
1010
"time"
1111

12+
"github.com/coder/coder/v2/scaletest/loadtestutil"
1213
"github.com/google/uuid"
1314
"github.com/prometheus/client_golang/prometheus"
1415
"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -143,7 +144,13 @@ After all runners connect, it waits for the baseline duration before triggering
143144
return xerrors.Errorf("validate config for runner %d: %w", i, err)
144145
}
145146

146-
var runner harness.Runnable = taskstatus.NewRunner(client, cfg)
147+
// use an independent client for each Runner, so they don't reuse TCP connections. This can lead to
148+
// requests being unbalanced among Coder instances.
149+
runnerClient, err := loadtestutil.DupClientCopyingHeaders(client, BypassHeader)
150+
if err != nil {
151+
return xerrors.Errorf("create runner client: %w", err)
152+
}
153+
var runner harness.Runnable = taskstatus.NewRunner(runnerClient, cfg)
147154
if tracingEnabled {
148155
runner = &runnableTraceWrapper{
149156
tracer: tracer,

‎scaletest/loadtestutil/client.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package loadtestutil
2+
3+
import (
4+
"maps"
5+
"net/http"
6+
7+
"github.com/coder/coder/v2/codersdk"
8+
"golang.org/x/xerrors"
9+
)
10+
11+
// DupClientCopyingHeaders duplicates the Client, but with an independent underlying HTTP transport, so that it will not
12+
// share connections with the client being duplicated. It copies any headers already on the existing transport as
13+
// [codersdk.HeaderTransport] and add the headers in the argument.
14+
func DupClientCopyingHeaders(client *codersdk.Client, header http.Header) (*codersdk.Client, error) {
15+
nc := codersdk.New(client.URL, codersdk.WithLogger(client.Logger()))
16+
nc.SessionTokenProvider = client.SessionTokenProvider
17+
newHeader, t, err := extractHeaderAndInnerTransport(client.HTTPClient.Transport)
18+
if err != nil {
19+
return nil, xerrors.Errorf("extract headers: %w", err)
20+
}
21+
maps.Copy(newHeader, header)
22+
23+
nc.HTTPClient.Transport = &codersdk.HeaderTransport{
24+
Transport: t.Clone(),
25+
Header: newHeader,
26+
}
27+
return nc, nil
28+
}
29+
30+
func extractHeaderAndInnerTransport(rt http.RoundTripper) (http.Header, *http.Transport, error) {
31+
if t, ok := rt.(*http.Transport); ok {
32+
// base case
33+
return make(http.Header), t, nil
34+
}
35+
if ht, ok := rt.(*codersdk.HeaderTransport); ok {
36+
headers, t, err := extractHeaderAndInnerTransport(ht.Transport)
37+
if err != nil {
38+
return nil, nil, err
39+
}
40+
maps.Copy(headers, ht.Header)
41+
return headers, t, nil
42+
}
43+
return nil, nil, xerrors.New("round tripper is neither HeaderTransport nor Transport")
44+
}

‎scaletest/loadtestutil/client_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package loadtestutil_test
2+
3+
import (
4+
"net/http"
5+
"net/url"
6+
"testing"
7+
8+
"github.com/coder/coder/v2/codersdk"
9+
"github.com/coder/coder/v2/scaletest/loadtestutil"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestDupClientCopyingHeaders(t *testing.T) {
14+
httpClient := &http.Client{
15+
Transport: &codersdk.HeaderTransport{
16+
Transport: &codersdk.HeaderTransport{
17+
Transport: http.DefaultTransport,
18+
Header: map[string][]string{
19+
"X-Coder-Test": {"foo"},
20+
"X-Coder-Test3": {"socks"},
21+
},
22+
},
23+
Header: map[string][]string{
24+
"X-Coder-Test": {"bar"},
25+
"X-Coder-Test2": {"baz"},
26+
},
27+
},
28+
}
29+
serverURL, err := url.Parse("http://coder.example.com")
30+
require.NoError(t, err)
31+
sdkClient := codersdk.New(serverURL,
32+
codersdk.WithSessionToken("test-token"), codersdk.WithHTTPClient(httpClient))
33+
34+
dup, err := loadtestutil.DupClientCopyingHeaders(sdkClient, map[string][]string{
35+
"X-Coder-Test3": {"clocks"},
36+
"X-Coder-Test4": {"bears"},
37+
})
38+
require.NoError(t, err)
39+
require.Equal(t, "http://coder.example.com", dup.URL.String())
40+
require.Equal(t, "test-token", dup.SessionToken())
41+
ht, ok := dup.HTTPClient.Transport.(*codersdk.HeaderTransport)
42+
require.True(t, ok)
43+
require.Equal(t, "bar", ht.Header.Get("X-Coder-Test"))
44+
require.Equal(t, "baz", ht.Header.Get("X-Coder-Test2"))
45+
require.Equal(t, "clocks", ht.Header.Get("X-Coder-Test3"))
46+
require.Equal(t, "bears", ht.Header.Get("X-Coder-Test4"))
47+
require.NotEqual(t, http.DefaultTransport, ht.Transport)
48+
}

0 commit comments

Comments
 (0)