Skip to content

Commit 8aea604

Browse files
f0sself0ssel
authored
fix: use unique workspace owners over unique users (#11044)
1 parent 091fdd6 commit 8aea604

File tree

10 files changed

+212
-2
lines changed

10 files changed

+212
-2
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2053,6 +2053,13 @@ func (q *querier) GetWorkspaceResourcesCreatedAfter(ctx context.Context, created
20532053
return q.db.GetWorkspaceResourcesCreatedAfter(ctx, createdAt)
20542054
}
20552055

2056+
func (q *querier) GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) {
2057+
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
2058+
return nil, err
2059+
}
2060+
return q.db.GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx, templateIds)
2061+
}
2062+
20562063
func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) {
20572064
prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceWorkspace.Type)
20582065
if err != nil {

‎coderd/database/dbmem/dbmem.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4520,6 +4520,36 @@ func (q *FakeQuerier) GetWorkspaceResourcesCreatedAfter(_ context.Context, after
45204520
return resources, nil
45214521
}
45224522

4523+
func (q *FakeQuerier) GetWorkspaceUniqueOwnerCountByTemplateIDs(_ context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) {
4524+
q.mutex.RLock()
4525+
defer q.mutex.RUnlock()
4526+
4527+
workspaceOwners := make(map[uuid.UUID]map[uuid.UUID]struct{})
4528+
for _, workspace := range q.workspaces {
4529+
if workspace.Deleted {
4530+
continue
4531+
}
4532+
if !slices.Contains(templateIds, workspace.TemplateID) {
4533+
continue
4534+
}
4535+
_, ok := workspaceOwners[workspace.TemplateID]
4536+
if !ok {
4537+
workspaceOwners[workspace.TemplateID] = make(map[uuid.UUID]struct{})
4538+
}
4539+
workspaceOwners[workspace.TemplateID][workspace.OwnerID] = struct{}{}
4540+
}
4541+
resp := make([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, 0)
4542+
for _, templateID := range templateIds {
4543+
count := len(workspaceOwners[templateID])
4544+
resp = append(resp, database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow{
4545+
TemplateID: templateID,
4546+
UniqueOwnersSum: int64(count),
4547+
})
4548+
}
4549+
4550+
return resp, nil
4551+
}
4552+
45234553
func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) {
45244554
if err := validateDatabaseType(arg); err != nil {
45254555
return nil, err

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 38 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaces.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,15 @@ WHERE
287287
AND LOWER("name") = LOWER(@name)
288288
ORDER BY created_at DESC;
289289

290+
-- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many
291+
SELECT
292+
template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum
293+
FROM
294+
workspaces
295+
WHERE
296+
template_id = ANY(@template_ids :: uuid[]) AND deleted = false
297+
GROUP BY template_id;
298+
290299
-- name: InsertWorkspace :one
291300
INSERT INTO
292301
workspaces (

‎coderd/metricscache/metricscache.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ type Cache struct {
5252
deploymentDAUResponses atomic.Pointer[map[int]codersdk.DAUsResponse]
5353
templateDAUResponses atomic.Pointer[map[int]map[uuid.UUID]codersdk.DAUsResponse]
5454
templateUniqueUsers atomic.Pointer[map[uuid.UUID]int]
55+
templateWorkspaceOwners atomic.Pointer[map[uuid.UUID]int]
5556
templateAverageBuildTime atomic.Pointer[map[uuid.UUID]database.GetTemplateAverageBuildTimeRow]
5657
deploymentStatsResponse atomic.Pointer[codersdk.DeploymentStats]
5758

@@ -206,6 +207,7 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error {
206207
var (
207208
templateDAUs = make(map[int]map[uuid.UUID]codersdk.DAUsResponse, len(templates))
208209
templateUniqueUsers = make(map[uuid.UUID]int)
210+
templateWorkspaceOwners = make(map[uuid.UUID]int)
209211
templateAverageBuildTimes = make(map[uuid.UUID]database.GetTemplateAverageBuildTimeRow)
210212
)
211213

@@ -214,7 +216,9 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error {
214216
return xerrors.Errorf("deployment daus: %w", err)
215217
}
216218

219+
ids := make([]uuid.UUID, 0, len(templates))
217220
for _, template := range templates {
221+
ids = append(ids, template.ID)
218222
for _, tzOffset := range templateTimezoneOffsets {
219223
rows, err := c.database.GetTemplateDAUs(ctx, database.GetTemplateDAUsParams{
220224
TemplateID: template.ID,
@@ -249,6 +253,17 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error {
249253
}
250254
templateAverageBuildTimes[template.ID] = templateAvgBuildTime
251255
}
256+
257+
owners, err := c.database.GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx, ids)
258+
if err != nil {
259+
return xerrors.Errorf("get workspace unique owner count by template ids: %w", err)
260+
}
261+
262+
for _, owner := range owners {
263+
templateWorkspaceOwners[owner.TemplateID] = int(owner.UniqueOwnersSum)
264+
}
265+
266+
c.templateWorkspaceOwners.Store(&templateWorkspaceOwners)
252267
c.templateDAUResponses.Store(&templateDAUs)
253268
c.templateUniqueUsers.Store(&templateUniqueUsers)
254269
c.templateAverageBuildTime.Store(&templateAverageBuildTimes)
@@ -469,6 +484,21 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS
469484
}
470485
}
471486

487+
func (c *Cache) TemplateWorkspaceOwners(id uuid.UUID) (int, bool) {
488+
m := c.templateWorkspaceOwners.Load()
489+
if m == nil {
490+
// Data loading.
491+
return -1, false
492+
}
493+
494+
resp, ok := (*m)[id]
495+
if !ok {
496+
// Probably no data.
497+
return -1, false
498+
}
499+
return resp, true
500+
}
501+
472502
func (c *Cache) DeploymentStats() (codersdk.DeploymentStats, bool) {
473503
deploymentStats := c.deploymentStatsResponse.Load()
474504
if deploymentStats == nil {

‎coderd/metricscache/metricscache_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,74 @@ func TestCache_TemplateUsers(t *testing.T) {
254254
}
255255
}
256256

257+
func TestCache_TemplateWorkspaceOwners(t *testing.T) {
258+
t.Parallel()
259+
var ()
260+
261+
var (
262+
db = dbmem.New()
263+
cache = metricscache.New(db, slogtest.Make(t, nil), metricscache.Intervals{
264+
TemplateDAUs: testutil.IntervalFast,
265+
})
266+
)
267+
268+
defer cache.Close()
269+
270+
user1 := dbgen.User(t, db, database.User{})
271+
user2 := dbgen.User(t, db, database.User{})
272+
template := dbgen.Template(t, db, database.Template{
273+
Provisioner: database.ProvisionerTypeEcho,
274+
})
275+
require.Eventuallyf(t, func() bool {
276+
count, ok := cache.TemplateWorkspaceOwners(template.ID)
277+
return ok && count == 0
278+
}, testutil.WaitShort, testutil.IntervalMedium,
279+
"TemplateWorkspaceOwners never populated 0 owners",
280+
)
281+
282+
dbgen.Workspace(t, db, database.Workspace{
283+
TemplateID: template.ID,
284+
OwnerID: user1.ID,
285+
})
286+
287+
require.Eventuallyf(t, func() bool {
288+
count, _ := cache.TemplateWorkspaceOwners(template.ID)
289+
return count == 1
290+
}, testutil.WaitShort, testutil.IntervalMedium,
291+
"TemplateWorkspaceOwners never populated 1 owner",
292+
)
293+
294+
workspace2 := dbgen.Workspace(t, db, database.Workspace{
295+
TemplateID: template.ID,
296+
OwnerID: user2.ID,
297+
})
298+
299+
require.Eventuallyf(t, func() bool {
300+
count, _ := cache.TemplateWorkspaceOwners(template.ID)
301+
return count == 2
302+
}, testutil.WaitShort, testutil.IntervalMedium,
303+
"TemplateWorkspaceOwners never populated 2 owners",
304+
)
305+
306+
// 3rd workspace should not be counted since we have the same owner as workspace2.
307+
dbgen.Workspace(t, db, database.Workspace{
308+
TemplateID: template.ID,
309+
OwnerID: user1.ID,
310+
})
311+
312+
db.UpdateWorkspaceDeletedByID(context.Background(), database.UpdateWorkspaceDeletedByIDParams{
313+
ID: workspace2.ID,
314+
Deleted: true,
315+
})
316+
317+
require.Eventuallyf(t, func() bool {
318+
count, _ := cache.TemplateWorkspaceOwners(template.ID)
319+
return count == 1
320+
}, testutil.WaitShort, testutil.IntervalMedium,
321+
"TemplateWorkspaceOwners never populated 1 owner after delete",
322+
)
323+
}
324+
257325
func clockTime(t time.Time, hour, minute, sec int) time.Time {
258326
return time.Date(t.Year(), t.Month(), t.Day(), hour, minute, sec, t.Nanosecond(), t.Location())
259327
}

‎coderd/templates.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,12 @@ func (api *API) convertTemplate(
831831
template database.Template,
832832
) codersdk.Template {
833833
templateAccessControl := (*(api.Options.AccessControlStore.Load())).GetTemplateAccessControl(template)
834-
activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID)
834+
835+
owners := 0
836+
o, ok := api.metricsCache.TemplateWorkspaceOwners(template.ID)
837+
if ok {
838+
owners = o
839+
}
835840

836841
buildTimeStats := api.metricsCache.TemplateBuildTimeStats(template.ID)
837842

@@ -849,7 +854,7 @@ func (api *API) convertTemplate(
849854
DisplayName: template.DisplayName,
850855
Provisioner: codersdk.ProvisionerType(template.Provisioner),
851856
ActiveVersionID: template.ActiveVersionID,
852-
ActiveUserCount: activeCount,
857+
ActiveUserCount: owners,
853858
BuildTimeStats: buildTimeStats,
854859
Description: template.Description,
855860
Icon: template.Icon,

0 commit comments

Comments
 (0)