Skip to content

Commit 1705138

Browse files
mafredrimafredri
committed
update plumbing to fix wsproxy tests
1 parent 8926dd0 commit 1705138

File tree

9 files changed

+88
-61
lines changed

9 files changed

+88
-61
lines changed

‎coderd/coderd.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ type Options struct {
163163
UpdateAgentMetrics func(ctx context.Context, username, workspaceName, agentName string, metrics []agentsdk.AgentMetric)
164164
StatsBatcher *batchstats.Batcher
165165

166-
WorkspaceAppsStatsCollector *workspaceapps.StatsCollector
166+
WorkspaceAppsStatsCollectorOptions workspaceapps.StatsCollectorOptions
167167
}
168168

169169
// @title Coder API
@@ -418,13 +418,16 @@ func New(options *Options) *API {
418418
Cache: wsconncache.New(api._dialWorkspaceAgentTailnet, 0),
419419
}
420420
}
421+
421422
workspaceAppsLogger := options.Logger.Named("workspaceapps")
422-
if options.WorkspaceAppsStatsCollector == nil {
423-
options.WorkspaceAppsStatsCollector = workspaceapps.NewStatsCollector(workspaceapps.StatsCollectorOptions{
424-
Logger: workspaceAppsLogger.Named("stats_collector"),
425-
Reporter: workspaceapps.NewStatsDBReporter(options.Database, workspaceapps.DefaultStatsDBReporterBatchSize),
426-
})
423+
if options.WorkspaceAppsStatsCollectorOptions.Logger == nil {
424+
named := workspaceAppsLogger.Named("stats_collector")
425+
options.WorkspaceAppsStatsCollectorOptions.Logger = &named
427426
}
427+
if options.WorkspaceAppsStatsCollectorOptions.Reporter == nil {
428+
options.WorkspaceAppsStatsCollectorOptions.Reporter = workspaceapps.NewStatsDBReporter(options.Database, workspaceapps.DefaultStatsDBReporterBatchSize)
429+
}
430+
428431
api.workspaceAppServer = &workspaceapps.Server{
429432
Logger: workspaceAppsLogger,
430433

@@ -437,7 +440,7 @@ func New(options *Options) *API {
437440
SignedTokenProvider: api.WorkspaceAppsProvider,
438441
AgentProvider: api.agentProvider,
439442
AppSecurityKey: options.AppSecurityKey,
440-
StatsCollector: options.WorkspaceAppsStatsCollector,
443+
StatsCollector: workspaceapps.NewStatsCollector(options.WorkspaceAppsStatsCollectorOptions),
441444

442445
DisablePathApps: options.DeploymentValues.DisablePathApps.Value(),
443446
SecureAuthCookie: options.DeploymentValues.SecureAuthCookie.Value(),

‎coderd/coderdtest/coderdtest.go

+33-33
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ type Options struct {
145145
Logger *slog.Logger
146146
StatsBatcher *batchstats.Batcher
147147

148-
WorkspaceAppsStatsCollector *workspaceapps.StatsCollector
148+
WorkspaceAppsStatsCollectorOptions workspaceapps.StatsCollectorOptions
149149
}
150150

151151
// New constructs a codersdk client connected to an in-memory API instance.
@@ -396,38 +396,38 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
396396
Pubsub: options.Pubsub,
397397
GitAuthConfigs: options.GitAuthConfigs,
398398

399-
Auditor: options.Auditor,
400-
AWSCertificates: options.AWSCertificates,
401-
AzureCertificates: options.AzureCertificates,
402-
GithubOAuth2Config: options.GithubOAuth2Config,
403-
RealIPConfig: options.RealIPConfig,
404-
OIDCConfig: options.OIDCConfig,
405-
GoogleTokenValidator: options.GoogleTokenValidator,
406-
SSHKeygenAlgorithm: options.SSHKeygenAlgorithm,
407-
DERPServer: derpServer,
408-
APIRateLimit: options.APIRateLimit,
409-
LoginRateLimit: options.LoginRateLimit,
410-
FilesRateLimit: options.FilesRateLimit,
411-
Authorizer: options.Authorizer,
412-
Telemetry: telemetry.NewNoop(),
413-
TemplateScheduleStore: &templateScheduleStore,
414-
TLSCertificates: options.TLSCertificates,
415-
TrialGenerator: options.TrialGenerator,
416-
TailnetCoordinator: options.Coordinator,
417-
BaseDERPMap: derpMap,
418-
DERPMapUpdateFrequency: 150 * time.Millisecond,
419-
MetricsCacheRefreshInterval: options.MetricsCacheRefreshInterval,
420-
AgentStatsRefreshInterval: options.AgentStatsRefreshInterval,
421-
DeploymentValues: options.DeploymentValues,
422-
UpdateCheckOptions: options.UpdateCheckOptions,
423-
SwaggerEndpoint: options.SwaggerEndpoint,
424-
AppSecurityKey: AppSecurityKey,
425-
SSHConfig: options.ConfigSSH,
426-
HealthcheckFunc: options.HealthcheckFunc,
427-
HealthcheckTimeout: options.HealthcheckTimeout,
428-
HealthcheckRefresh: options.HealthcheckRefresh,
429-
StatsBatcher: options.StatsBatcher,
430-
WorkspaceAppsStatsCollector: options.WorkspaceAppsStatsCollector,
399+
Auditor: options.Auditor,
400+
AWSCertificates: options.AWSCertificates,
401+
AzureCertificates: options.AzureCertificates,
402+
GithubOAuth2Config: options.GithubOAuth2Config,
403+
RealIPConfig: options.RealIPConfig,
404+
OIDCConfig: options.OIDCConfig,
405+
GoogleTokenValidator: options.GoogleTokenValidator,
406+
SSHKeygenAlgorithm: options.SSHKeygenAlgorithm,
407+
DERPServer: derpServer,
408+
APIRateLimit: options.APIRateLimit,
409+
LoginRateLimit: options.LoginRateLimit,
410+
FilesRateLimit: options.FilesRateLimit,
411+
Authorizer: options.Authorizer,
412+
Telemetry: telemetry.NewNoop(),
413+
TemplateScheduleStore: &templateScheduleStore,
414+
TLSCertificates: options.TLSCertificates,
415+
TrialGenerator: options.TrialGenerator,
416+
TailnetCoordinator: options.Coordinator,
417+
BaseDERPMap: derpMap,
418+
DERPMapUpdateFrequency: 150 * time.Millisecond,
419+
MetricsCacheRefreshInterval: options.MetricsCacheRefreshInterval,
420+
AgentStatsRefreshInterval: options.AgentStatsRefreshInterval,
421+
DeploymentValues: options.DeploymentValues,
422+
UpdateCheckOptions: options.UpdateCheckOptions,
423+
SwaggerEndpoint: options.SwaggerEndpoint,
424+
AppSecurityKey: AppSecurityKey,
425+
SSHConfig: options.ConfigSSH,
426+
HealthcheckFunc: options.HealthcheckFunc,
427+
HealthcheckTimeout: options.HealthcheckTimeout,
428+
HealthcheckRefresh: options.HealthcheckRefresh,
429+
StatsBatcher: options.StatsBatcher,
430+
WorkspaceAppsStatsCollectorOptions: options.WorkspaceAppsStatsCollectorOptions,
431431
}
432432
}
433433

‎coderd/workspaceapps/apptest/apptest.go

+15-7
Original file line numberDiff line numberDiff line change
@@ -1411,14 +1411,17 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
14111411
t.Run("ReportStats", func(t *testing.T) {
14121412
t.Parallel()
14131413

1414+
flush := make(chan chan<- struct{}, 1)
1415+
14141416
reporter := &fakeStatsReporter{}
1415-
collector := workspaceapps.NewStatsCollector(workspaceapps.StatsCollectorOptions{
1416-
Reporter: reporter,
1417-
ReportInterval: time.Second,
1418-
RollupWindow: time.Minute,
1419-
})
14201417
appDetails := setupProxyTest(t, &DeploymentOptions{
1421-
StatsCollector: collector,
1418+
StatsCollectorOptions: workspaceapps.StatsCollectorOptions{
1419+
Reporter: reporter,
1420+
ReportInterval: time.Hour,
1421+
RollupWindow: time.Minute,
1422+
1423+
Flush: flush,
1424+
},
14221425
})
14231426

14241427
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
@@ -1433,8 +1436,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
14331436
require.Equal(t, http.StatusOK, resp.StatusCode)
14341437

14351438
require.Eventually(t, func() bool {
1439+
// Keep flushing until we get a non-empty stats report.
1440+
flushDone := make(chan struct{}, 1)
1441+
flush <- flushDone
1442+
<-flushDone
1443+
14361444
return len(reporter.stats()) > 0
1437-
}, testutil.WaitLong, testutil.IntervalMedium, "stats not reported")
1445+
}, testutil.WaitLong, testutil.IntervalFast, "stats not reported")
14381446
})
14391447
}
14401448

‎coderd/workspaceapps/apptest/setup.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ type DeploymentOptions struct {
5252
DangerousAllowPathAppSiteOwnerAccess bool
5353
ServeHTTPS bool
5454

55-
StatsCollector *workspaceapps.StatsCollector
55+
StatsCollectorOptions workspaceapps.StatsCollectorOptions
5656

5757
// The following fields are only used by setupProxyTestWithFactory.
5858
noWorkspace bool

‎coderd/workspaceapps/stats.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ type StatsCollector struct {
147147
}
148148

149149
type StatsCollectorOptions struct {
150-
Logger slog.Logger
150+
Logger *slog.Logger
151151
Reporter StatsReporter
152152
// ReportInterval is the interval at which stats are reported, both partial
153153
// and fully formed stats.
@@ -163,6 +163,9 @@ type StatsCollectorOptions struct {
163163
}
164164

165165
func NewStatsCollector(opts StatsCollectorOptions) *StatsCollector {
166+
if opts.Logger == nil {
167+
opts.Logger = &slog.Logger{}
168+
}
166169
if opts.ReportInterval == 0 {
167170
opts.ReportInterval = DefaultStatsCollectorReportInterval
168171
}

‎enterprise/coderd/coderdenttest/proxytest.go

+5
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ func NewWorkspaceProxy(t *testing.T, coderdAPI *coderd.API, owner *codersdk.Clie
110110
})
111111
require.NoError(t, err, "failed to create workspace proxy")
112112

113+
// Inherit collector options from coderd, but keep the wsproxy reporter.
114+
statsCollectorOptions := coderdAPI.Options.WorkspaceAppsStatsCollectorOptions
115+
statsCollectorOptions.Reporter = nil
116+
113117
wssrv, err := wsproxy.New(ctx, &wsproxy.Options{
114118
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
115119
Experiments: options.Experiments,
@@ -129,6 +133,7 @@ func NewWorkspaceProxy(t *testing.T, coderdAPI *coderd.API, owner *codersdk.Clie
129133
DERPEnabled: !options.DerpDisabled,
130134
DERPOnly: options.DerpOnly,
131135
DERPServerRelayAddress: accessURL.String(),
136+
StatsCollectorOptions: statsCollectorOptions,
132137
})
133138
require.NoError(t, err)
134139
t.Cleanup(func() {

‎enterprise/coderd/workspaceproxy.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -511,14 +511,15 @@ func (api *API) workspaceProxyReportAppStats(rw http.ResponseWriter, r *http.Req
511511
ctx := r.Context()
512512
_ = httpmw.WorkspaceProxy(r) // Ensure the proxy is authenticated.
513513

514-
var stats []workspaceapps.StatsReport
515-
if !httpapi.Read(ctx, rw, r, &stats) {
514+
var req wsproxysdk.ReportAppStatsRequest
515+
if !httpapi.Read(ctx, rw, r, &req) {
516516
return
517517
}
518518

519-
// Reporter has no state, so we can use an ephemeral instance here.
520-
reporter := workspaceapps.NewStatsDBReporter(api.Database, workspaceapps.DefaultStatsDBReporterBatchSize)
521-
if err := reporter.Report(ctx, stats); err != nil {
519+
api.Logger.Debug(ctx, "report app stats", slog.F("stats", req.Stats))
520+
521+
reporter := api.WorkspaceAppsStatsCollectorOptions.Reporter
522+
if err := reporter.Report(ctx, req.Stats); err != nil {
522523
api.Logger.Error(ctx, "report app stats failed", slog.Error(err))
523524
httpapi.InternalServerError(rw, err)
524525
return

‎enterprise/wsproxy/wsproxy.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ type Options struct {
7979
// By default, CORs is set to accept external requests
8080
// from the dashboardURL. This should only be used in development.
8181
AllowAllCors bool
82+
83+
StatsCollectorOptions workspaceapps.StatsCollectorOptions
8284
}
8385

8486
func (o *Options) Validate() error {
@@ -262,6 +264,14 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
262264
}
263265

264266
workspaceAppsLogger := opts.Logger.Named("workspaceapps")
267+
if opts.StatsCollectorOptions.Logger == nil {
268+
named := workspaceAppsLogger.Named("stats_collector")
269+
opts.StatsCollectorOptions.Logger = &named
270+
}
271+
if opts.StatsCollectorOptions.Reporter == nil {
272+
opts.StatsCollectorOptions.Reporter = &appStatsReporter{Client: client}
273+
}
274+
265275
s.AppServer = &workspaceapps.Server{
266276
Logger: workspaceAppsLogger,
267277
DashboardURL: opts.DashboardURL,
@@ -282,11 +292,8 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
282292
DisablePathApps: opts.DisablePathApps,
283293
SecureAuthCookie: opts.SecureAuthCookie,
284294

285-
AgentProvider: agentProvider,
286-
StatsCollector: workspaceapps.NewStatsCollector(workspaceapps.StatsCollectorOptions{
287-
Logger: workspaceAppsLogger.Named("stats_collector"),
288-
Reporter: &appStatsReporter{Client: client},
289-
}),
295+
AgentProvider: agentProvider,
296+
StatsCollector: workspaceapps.NewStatsCollector(opts.StatsCollectorOptions),
290297
}
291298

292299
derpHandler := derphttp.Handler(derpServer)

‎enterprise/wsproxy/wsproxy_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ func TestWorkspaceProxyWorkspaceApps_Wsconncache(t *testing.T) {
478478
"CF-Connecting-IP",
479479
},
480480
},
481-
WorkspaceAppsStatsCollector: opts.StatsCollector,
481+
WorkspaceAppsStatsCollectorOptions: opts.StatsCollectorOptions,
482482
},
483483
LicenseOptions: &coderdenttest.LicenseOptions{
484484
Features: license.Features{
@@ -537,7 +537,7 @@ func TestWorkspaceProxyWorkspaceApps_SingleTailnet(t *testing.T) {
537537
"CF-Connecting-IP",
538538
},
539539
},
540-
WorkspaceAppsStatsCollector: opts.StatsCollector,
540+
WorkspaceAppsStatsCollectorOptions: opts.StatsCollectorOptions,
541541
},
542542
LicenseOptions: &coderdenttest.LicenseOptions{
543543
Features: license.Features{

0 commit comments

Comments
 (0)