From c6b087ac215639112ce38d5e60a438a946db4e6f Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 6 Sep 2022 01:17:02 +0000 Subject: [PATCH 1/5] Implement in metricscache --- .gitignore | 2 +- coderd/database/databasefake/databasefake.go | 25 +++---- coderd/database/queries.sql.go | 8 +-- coderd/database/queries/agentstats.sql | 4 +- coderd/metricscache/metricscache.go | 68 +++++++++++++------- coderd/metricscache/metricscache_test.go | 4 +- 6 files changed, 67 insertions(+), 44 deletions(-) diff --git a/.gitignore b/.gitignore index ae2c40f0a576c..a47ded4090358 100644 --- a/.gitignore +++ b/.gitignore @@ -39,7 +39,7 @@ site/out/ *.lock.hcl .terraform/ -.vscode/*.log +.vscode/* **/*.swp .coderv2/* **/__debug_bin diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index b4cf051fc8f1b..411e8cb2a9ce9 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -163,7 +163,7 @@ func (q *fakeQuerier) GetTemplateDAUs(_ context.Context, templateID uuid.UUID) ( q.mutex.Lock() defer q.mutex.Unlock() - counts := make(map[time.Time]map[string]struct{}) + seens := make(map[time.Time]map[uuid.UUID]struct{}) for _, as := range q.agentStats { if as.TemplateID != templateID { @@ -171,26 +171,29 @@ func (q *fakeQuerier) GetTemplateDAUs(_ context.Context, templateID uuid.UUID) ( } date := as.CreatedAt.Truncate(time.Hour * 24) - dateEntry := counts[date] + + dateEntry := seens[date] if dateEntry == nil { - dateEntry = make(map[string]struct{}) + dateEntry = make(map[uuid.UUID]struct{}) } - counts[date] = dateEntry - - dateEntry[as.UserID.String()] = struct{}{} + dateEntry[as.ID] = struct{}{} + seens[date] = dateEntry } - countKeys := maps.Keys(counts) + countKeys := maps.Keys(seens) sort.Slice(countKeys, func(i, j int) bool { return countKeys[i].Before(countKeys[j]) }) var rs []database.GetTemplateDAUsRow for _, key := range countKeys { - rs = append(rs, database.GetTemplateDAUsRow{ - Date: key, - Amount: int64(len(counts[key])), - }) + ids := seens[key] + for id := range ids { + rs = append(rs, database.GetTemplateDAUsRow{ + Date: key, + UserID: id, + }) + } } return rs, nil diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index de1d2d1812cea..45208c71717ed 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -27,19 +27,19 @@ func (q *sqlQuerier) DeleteOldAgentStats(ctx context.Context) error { const getTemplateDAUs = `-- name: GetTemplateDAUs :many select (created_at at TIME ZONE 'UTC')::date as date, - count(distinct(user_id)) as amount + user_id from agent_stats where template_id = $1 group by - date + date, user_id order by date asc ` type GetTemplateDAUsRow struct { Date time.Time `db:"date" json:"date"` - Amount int64 `db:"amount" json:"amount"` + UserID uuid.UUID `db:"user_id" json:"user_id"` } func (q *sqlQuerier) GetTemplateDAUs(ctx context.Context, templateID uuid.UUID) ([]GetTemplateDAUsRow, error) { @@ -51,7 +51,7 @@ func (q *sqlQuerier) GetTemplateDAUs(ctx context.Context, templateID uuid.UUID) var items []GetTemplateDAUsRow for rows.Next() { var i GetTemplateDAUsRow - if err := rows.Scan(&i.Date, &i.Amount); err != nil { + if err := rows.Scan(&i.Date, &i.UserID); err != nil { return nil, err } items = append(items, i) diff --git a/coderd/database/queries/agentstats.sql b/coderd/database/queries/agentstats.sql index ca500b7aa0a14..f1d960d512a5d 100644 --- a/coderd/database/queries/agentstats.sql +++ b/coderd/database/queries/agentstats.sql @@ -15,12 +15,12 @@ VALUES -- name: GetTemplateDAUs :many select (created_at at TIME ZONE 'UTC')::date as date, - count(distinct(user_id)) as amount + user_id from agent_stats where template_id = $1 group by - date + date, user_id order by date asc; diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 056bcdb80bae3..d3def6fc218de 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -5,6 +5,8 @@ import ( "sync/atomic" "time" + "golang.org/x/exp/maps" + "golang.org/x/exp/slices" "golang.org/x/xerrors" "github.com/google/uuid" @@ -24,7 +26,7 @@ type Cache struct { database database.Store log slog.Logger - templateDAUResponses atomic.Pointer[map[string]codersdk.TemplateDAUsResponse] + templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] doneCh chan struct{} cancel func() @@ -49,34 +51,60 @@ func New(db database.Store, log slog.Logger, interval time.Duration) *Cache { return c } -func fillEmptyDays(rows []database.GetTemplateDAUsRow) []database.GetTemplateDAUsRow { - var newRows []database.GetTemplateDAUsRow +func fillEmptyDays(sortedDates []time.Time) []time.Time { + var newDates []time.Time - for i, row := range rows { + for i, ti := range sortedDates { if i == 0 { - newRows = append(newRows, row) + newDates = append(newDates, ti) continue } - last := rows[i-1] + last := sortedDates[i-1] const day = time.Hour * 24 - diff := row.Date.Sub(last.Date) + diff := ti.Sub(last) for diff > day { if diff <= day { break } - last.Date = last.Date.Add(day) - last.Amount = 0 - newRows = append(newRows, last) + last = last.Add(day) + newDates = append(newDates, last) diff -= day } - newRows = append(newRows, row) + newDates = append(newDates, ti) continue } - return newRows + return newDates +} + +func convertDAUResponse(rows []database.GetTemplateDAUsRow) codersdk.TemplateDAUsResponse { + respMap := make(map[time.Time][]uuid.UUID) + for _, row := range rows { + uuids := respMap[row.Date] + if uuids == nil { + uuids = make([]uuid.UUID, 0, 8) + } + uuids = append(uuids, row.UserID) + respMap[row.Date] = uuids + } + + dates := maps.Keys(respMap) + slices.SortFunc(dates, func(a, b time.Time) bool { + return a.Before(b) + }) + + var resp codersdk.TemplateDAUsResponse + for _, date := range fillEmptyDays(dates) { + resp.Entries = append(resp.Entries, codersdk.DAUEntry{ + Date: date, + Amount: len(respMap[date]), + }) + } + + return resp } func (c *Cache) refresh(ctx context.Context) error { @@ -90,22 +118,14 @@ func (c *Cache) refresh(ctx context.Context) error { return err } - templateDAUs := make(map[string]codersdk.TemplateDAUsResponse, len(templates)) + templateDAUs := make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates)) for _, template := range templates { - daus, err := c.database.GetTemplateDAUs(ctx, template.ID) + rows, err := c.database.GetTemplateDAUs(ctx, template.ID) if err != nil { return err } - - var resp codersdk.TemplateDAUsResponse - for _, ent := range fillEmptyDays(daus) { - resp.Entries = append(resp.Entries, codersdk.DAUEntry{ - Date: ent.Date, - Amount: int(ent.Amount), - }) - } - templateDAUs[template.ID.String()] = resp + templateDAUs[template.ID] = convertDAUResponse(rows) } c.templateDAUResponses.Store(&templateDAUs) @@ -163,7 +183,7 @@ func (c *Cache) TemplateDAUs(id uuid.UUID) codersdk.TemplateDAUsResponse { return codersdk.TemplateDAUsResponse{} } - resp, ok := (*m)[id.String()] + resp, ok := (*m)[id] if !ok { // Probably no data. return codersdk.TemplateDAUsResponse{} diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 8111bcfcc3481..103f568d35953 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -157,7 +157,7 @@ func TestCache(t *testing.T) { t.Parallel() var ( db = databasefake.New() - cache = metricscache.New(db, slogtest.Make(t, nil), time.Millisecond*100) + cache = metricscache.New(db, slogtest.Make(t, nil), testutil.IntervalFast) ) defer cache.Close() @@ -177,7 +177,7 @@ func TestCache(t *testing.T) { require.Eventuallyf(t, func() bool { got = cache.TemplateDAUs(templateID) return reflect.DeepEqual(got.Entries, tt.want) - }, testutil.WaitShort, testutil.IntervalFast, + }, testutil.WaitShort, testutil.IntervalMedium, "GetDAUs() = %v, want %v", got, tt.want, ) }) From 055135a16b56634b38fc658820c7b12fb89a2fbc Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 6 Sep 2022 16:11:59 +0000 Subject: [PATCH 2/5] Finish metricscache tests --- .vscode/settings.json | 14 ++----- coderd/database/databasefake/databasefake.go | 2 +- coderd/metricscache/metricscache.go | 36 ++++++++++++++-- coderd/metricscache/metricscache_test.go | 43 ++++++++++++-------- 4 files changed, 65 insertions(+), 30 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 68d823b8cff81..1171b816a2c3c 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -161,16 +161,10 @@ // To reduce redundancy in tests, it's covered by other packages. // Since package coverage pairing can't be defined, all packages cover // all other packages. - "go.testFlags": ["-short", "-coverpkg=./..."], - "go.coverageDecorator": { - "type": "gutter", - "coveredHighlightColor": "rgba(64,128,128,0.5)", - "uncoveredHighlightColor": "rgba(128,64,64,0.25)", - "coveredBorderColor": "rgba(64,128,128,0.5)", - "uncoveredBorderColor": "rgba(128,64,64,0.25)", - "coveredGutterStyle": "blockgreen", - "uncoveredGutterStyle": "blockred" - }, + "go.testFlags": [ + "-short", + "-coverpkg=./..." + ], // We often use a version of TypeScript that's ahead of the version shipped // with VS Code. "typescript.tsdk": "./site/node_modules/typescript/lib" diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 411e8cb2a9ce9..88fe5f917f3b2 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -176,7 +176,7 @@ func (q *fakeQuerier) GetTemplateDAUs(_ context.Context, templateID uuid.UUID) ( if dateEntry == nil { dateEntry = make(map[uuid.UUID]struct{}) } - dateEntry[as.ID] = struct{}{} + dateEntry[as.UserID] = struct{}{} seens[date] = dateEntry } diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index d3def6fc218de..76b44d4e07665 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -27,6 +27,7 @@ type Cache struct { log slog.Logger templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] + templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] doneCh chan struct{} cancel func() @@ -107,6 +108,14 @@ func convertDAUResponse(rows []database.GetTemplateDAUsRow) codersdk.TemplateDAU return resp } +func countUniqueUsers(rows []database.GetTemplateDAUsRow) int { + seen := make(map[uuid.UUID]struct{}, len(rows)) + for _, row := range rows { + seen[row.UserID] = struct{}{} + } + return len(seen) +} + func (c *Cache) refresh(ctx context.Context) error { err := c.database.DeleteOldAgentStats(ctx) if err != nil { @@ -118,17 +127,21 @@ func (c *Cache) refresh(ctx context.Context) error { return err } - templateDAUs := make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates)) - + var ( + templateDAUs = make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates)) + templateUniqueUsers = make(map[uuid.UUID]int) + ) for _, template := range templates { rows, err := c.database.GetTemplateDAUs(ctx, template.ID) if err != nil { return err } templateDAUs[template.ID] = convertDAUResponse(rows) + templateUniqueUsers[template.ID] = countUniqueUsers(rows) } - c.templateDAUResponses.Store(&templateDAUs) + c.templateUniqueUsers.Store(&templateUniqueUsers) + return nil } @@ -190,3 +203,20 @@ func (c *Cache) TemplateDAUs(id uuid.UUID) codersdk.TemplateDAUsResponse { } return resp } + +// TemplateUniqueUsers returns the total number of unique users for the template, +// from all the Cache data. +func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) { + m := c.templateUniqueUsers.Load() + if m == nil { + // Data loading. + return -1, false + } + + resp, ok := (*m)[id] + if !ok { + // Probably no data. + return -1, false + } + return resp, true +} diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 103f568d35953..557815e246c07 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -2,7 +2,6 @@ package metricscache_test import ( "context" - "reflect" "testing" "time" @@ -25,19 +24,23 @@ func TestCache(t *testing.T) { t.Parallel() var ( - zebra = uuid.New() - tiger = uuid.New() + zebra = uuid.UUID{1} + tiger = uuid.UUID{2} ) type args struct { rows []database.InsertAgentStatParams } + type want struct { + entries []codersdk.DAUEntry + uniqueUsers int + } tests := []struct { name string args args - want []codersdk.DAUEntry + want want }{ - {"empty", args{}, nil}, + {"empty", args{}, want{nil, 0}}, {"one hole", args{ rows: []database.InsertAgentStatParams{ { @@ -49,7 +52,7 @@ func TestCache(t *testing.T) { UserID: zebra, }, }, - }, []codersdk.DAUEntry{ + }, want{[]codersdk.DAUEntry{ { Date: date(2022, 8, 27), Amount: 1, @@ -66,7 +69,8 @@ func TestCache(t *testing.T) { Date: date(2022, 8, 30), Amount: 1, }, - }}, + }, 1}, + }, {"no holes", args{ rows: []database.InsertAgentStatParams{ { @@ -82,7 +86,7 @@ func TestCache(t *testing.T) { UserID: zebra, }, }, - }, []codersdk.DAUEntry{ + }, want{[]codersdk.DAUEntry{ { Date: date(2022, 8, 27), Amount: 1, @@ -95,7 +99,7 @@ func TestCache(t *testing.T) { Date: date(2022, 8, 29), Amount: 1, }, - }}, + }, 1}}, {"holes", args{ rows: []database.InsertAgentStatParams{ { @@ -119,7 +123,7 @@ func TestCache(t *testing.T) { UserID: tiger, }, }, - }, []codersdk.DAUEntry{ + }, want{[]codersdk.DAUEntry{ { Date: date(2022, 1, 1), Amount: 2, @@ -148,7 +152,7 @@ func TestCache(t *testing.T) { Date: date(2022, 1, 7), Amount: 2, }, - }}, + }, 2}}, } for _, tt := range tests { @@ -167,19 +171,26 @@ func TestCache(t *testing.T) { ID: templateID, }) + gotUniqueUsers, ok := cache.TemplateUniqueUsers(templateID) + require.False(t, ok, "template shouldn't have loaded yet") + for _, row := range tt.args.rows { row.TemplateID = templateID db.InsertAgentStat(context.Background(), row) } - var got codersdk.TemplateDAUsResponse - require.Eventuallyf(t, func() bool { - got = cache.TemplateDAUs(templateID) - return reflect.DeepEqual(got.Entries, tt.want) + return len(cache.TemplateDAUs(templateID).Entries) > 0 }, testutil.WaitShort, testutil.IntervalMedium, - "GetDAUs() = %v, want %v", got, tt.want, + "TemplateDAUs never populated", ) + + gotUniqueUsers, ok = cache.TemplateUniqueUsers(templateID) + require.True(t, ok) + + gotEntries := cache.TemplateDAUs(templateID) + require.Equal(t, tt.want.entries, gotEntries.Entries) + require.Equal(t, tt.want.uniqueUsers, gotUniqueUsers) }) } } From c5787bed1ccc0db0970c0609d71849f869a088e0 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 6 Sep 2022 17:08:46 +0000 Subject: [PATCH 3/5] Complete frontend --- .gitignore | 3 +- cli/create.go | 8 ++--- cli/portforward.go | 3 +- cli/templates.go | 4 +-- coderd/metricscache/metricscache.go | 22 +++++++------- coderd/metricscache/metricscache_test.go | 7 +++-- coderd/templates.go | 28 +++++++++-------- coderd/templates_test.go | 16 +++++++--- codersdk/templates.go | 30 ++++++++++--------- site/src/api/typesGenerated.ts | 1 + .../TemplateStats/TemplateStats.stories.tsx | 11 ++++++- .../TemplateStats/TemplateStats.tsx | 7 ++--- .../TemplatesPageView.stories.tsx | 5 ++-- .../pages/TemplatesPage/TemplatesPageView.tsx | 5 ++-- site/src/testHelpers/entities.ts | 3 +- site/src/util/templates.ts | 7 +++++ 16 files changed, 99 insertions(+), 61 deletions(-) create mode 100644 site/src/util/templates.ts diff --git a/.gitignore b/.gitignore index a47ded4090358..6c2cc61f86abb 100644 --- a/.gitignore +++ b/.gitignore @@ -39,7 +39,8 @@ site/out/ *.lock.hcl .terraform/ -.vscode/* +.vscode/*.log +.vscode/launch.json **/*.swp .coderv2/* **/__debug_bin diff --git a/cli/create.go b/cli/create.go index 1829cd71f099b..90a73c98270c6 100644 --- a/cli/create.go +++ b/cli/create.go @@ -72,7 +72,7 @@ func create() *cobra.Command { } slices.SortFunc(templates, func(a, b codersdk.Template) bool { - return a.WorkspaceOwnerCount > b.WorkspaceOwnerCount + return a.ActiveUserCount > b.ActiveUserCount }) templateNames := make([]string, 0, len(templates)) @@ -81,13 +81,13 @@ func create() *cobra.Command { for _, template := range templates { templateName := template.Name - if template.WorkspaceOwnerCount > 0 { + if template.ActiveUserCount > 0 { developerText := "developer" - if template.WorkspaceOwnerCount != 1 { + if template.ActiveUserCount != 1 { developerText = "developers" } - templateName += cliui.Styles.Placeholder.Render(fmt.Sprintf(" (used by %d %s)", template.WorkspaceOwnerCount, developerText)) + templateName += cliui.Styles.Placeholder.Render(fmt.Sprintf(" (used by %d %s)", template.ActiveUserCount, developerText)) } templateNames = append(templateNames, templateName) diff --git a/cli/portforward.go b/cli/portforward.go index 3b78fa8d11a65..ae5daeb9541bd 100644 --- a/cli/portforward.go +++ b/cli/portforward.go @@ -12,11 +12,12 @@ import ( "sync" "syscall" - "cdr.dev/slog" "github.com/pion/udp" "github.com/spf13/cobra" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/agent" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" diff --git a/cli/templates.go b/cli/templates.go index ce8d7247b3edd..b2b903b37f05b 100644 --- a/cli/templates.go +++ b/cli/templates.go @@ -65,7 +65,7 @@ func displayTemplates(filterColumns []string, templates ...codersdk.Template) (s rows := make([]templateTableRow, len(templates)) for i, template := range templates { suffix := "" - if template.WorkspaceOwnerCount != 1 { + if template.ActiveUserCount != 1 { suffix = "s" } @@ -76,7 +76,7 @@ func displayTemplates(filterColumns []string, templates ...codersdk.Template) (s OrganizationID: template.OrganizationID, Provisioner: template.Provisioner, ActiveVersionID: template.ActiveVersionID, - UsedBy: cliui.Styles.Fuchsia.Render(fmt.Sprintf("%d developer%s", template.WorkspaceOwnerCount, suffix)), + UsedBy: cliui.Styles.Fuchsia.Render(fmt.Sprintf("%d developer%s", template.ActiveUserCount, suffix)), MaxTTL: (time.Duration(template.MaxTTLMillis) * time.Millisecond), MinAutostartInterval: (time.Duration(template.MinAutostartIntervalMillis) * time.Millisecond), } diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 76b44d4e07665..da38f54c840f0 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -29,7 +29,7 @@ type Cache struct { templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] - doneCh chan struct{} + done chan struct{} cancel func() interval time.Duration @@ -44,7 +44,7 @@ func New(db database.Store, log slog.Logger, interval time.Duration) *Cache { c := &Cache{ database: db, log: log, - doneCh: make(chan struct{}), + done: make(chan struct{}), cancel: cancel, interval: interval, } @@ -146,7 +146,7 @@ func (c *Cache) refresh(ctx context.Context) error { } func (c *Cache) run(ctx context.Context) { - defer close(c.doneCh) + defer close(c.done) ticker := time.NewTicker(c.interval) defer ticker.Stop() @@ -173,7 +173,7 @@ func (c *Cache) run(ctx context.Context) { select { case <-ticker.C: - case <-c.doneCh: + case <-c.done: return case <-ctx.Done(): return @@ -183,29 +183,29 @@ func (c *Cache) run(ctx context.Context) { func (c *Cache) Close() error { c.cancel() - <-c.doneCh + <-c.done return nil } // TemplateDAUs returns an empty response if the template doesn't have users // or is loading for the first time. -func (c *Cache) TemplateDAUs(id uuid.UUID) codersdk.TemplateDAUsResponse { +func (c *Cache) TemplateDAUs(id uuid.UUID) (*codersdk.TemplateDAUsResponse, bool) { m := c.templateDAUResponses.Load() if m == nil { // Data loading. - return codersdk.TemplateDAUsResponse{} + return nil, false } resp, ok := (*m)[id] if !ok { // Probably no data. - return codersdk.TemplateDAUsResponse{} + return nil, false } - return resp + return &resp, true } -// TemplateUniqueUsers returns the total number of unique users for the template, -// from all the Cache data. +// TemplateUniqueUsers returns the number of unique Template users +// from all Cache data. func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) { m := c.templateUniqueUsers.Load() if m == nil { diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 557815e246c07..70d926702e77f 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -173,6 +173,7 @@ func TestCache(t *testing.T) { gotUniqueUsers, ok := cache.TemplateUniqueUsers(templateID) require.False(t, ok, "template shouldn't have loaded yet") + require.EqualValues(t, -1, gotUniqueUsers) for _, row := range tt.args.rows { row.TemplateID = templateID @@ -180,7 +181,8 @@ func TestCache(t *testing.T) { } require.Eventuallyf(t, func() bool { - return len(cache.TemplateDAUs(templateID).Entries) > 0 + _, ok := cache.TemplateDAUs(templateID) + return ok }, testutil.WaitShort, testutil.IntervalMedium, "TemplateDAUs never populated", ) @@ -188,7 +190,8 @@ func TestCache(t *testing.T) { gotUniqueUsers, ok = cache.TemplateUniqueUsers(templateID) require.True(t, ok) - gotEntries := cache.TemplateDAUs(templateID) + gotEntries, ok := cache.TemplateDAUs(templateID) + require.True(t, ok) require.Equal(t, tt.want.entries, gotEntries.Entries) require.Equal(t, tt.want.uniqueUsers, gotUniqueUsers) }) diff --git a/coderd/templates.go b/coderd/templates.go index 64b10ca83f1d2..cfdd9111e044f 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -78,7 +78,7 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(rw, http.StatusOK, convertTemplate(template, count, createdByNameMap[template.ID.String()])) + httpapi.Write(rw, http.StatusOK, api.convertTemplate(template, count, createdByNameMap[template.ID.String()])) } func (api *API) deleteTemplate(rw http.ResponseWriter, r *http.Request) { @@ -268,7 +268,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque return xerrors.Errorf("get creator name: %w", err) } - template = convertTemplate(dbTemplate, 0, createdByNameMap[dbTemplate.ID.String()]) + template = api.convertTemplate(dbTemplate, 0, createdByNameMap[dbTemplate.ID.String()]) return nil }) if err != nil { @@ -339,7 +339,7 @@ func (api *API) templatesByOrganization(rw http.ResponseWriter, r *http.Request) return } - httpapi.Write(rw, http.StatusOK, convertTemplates(templates, workspaceCounts, createdByNameMap)) + httpapi.Write(rw, http.StatusOK, api.convertTemplates(templates, workspaceCounts, createdByNameMap)) } func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Request) { @@ -393,7 +393,7 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re return } - httpapi.Write(rw, http.StatusOK, convertTemplate(template, count, createdByNameMap[template.ID.String()])) + httpapi.Write(rw, http.StatusOK, api.convertTemplate(template, count, createdByNameMap[template.ID.String()])) } func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { @@ -514,7 +514,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(rw, http.StatusOK, convertTemplate(updated, count, createdByNameMap[updated.ID.String()])) + httpapi.Write(rw, http.StatusOK, api.convertTemplate(updated, count, createdByNameMap[updated.ID.String()])) } func (api *API) templateDAUs(rw http.ResponseWriter, r *http.Request) { @@ -524,7 +524,7 @@ func (api *API) templateDAUs(rw http.ResponseWriter, r *http.Request) { return } - resp := api.metricsCache.TemplateDAUs(template.ID) + resp, _ := api.metricsCache.TemplateDAUs(template.ID) if resp.Entries == nil { resp.Entries = []codersdk.DAUEntry{} } @@ -683,7 +683,7 @@ func getCreatedByNamesByTemplateIDs(ctx context.Context, db database.Store, temp return creators, nil } -func convertTemplates(templates []database.Template, workspaceCounts []database.GetWorkspaceOwnerCountsByTemplateIDsRow, createdByNameMap map[string]string) []codersdk.Template { +func (api *API) convertTemplates(templates []database.Template, workspaceCounts []database.GetWorkspaceOwnerCountsByTemplateIDsRow, createdByNameMap map[string]string) []codersdk.Template { apiTemplates := make([]codersdk.Template, 0, len(templates)) for _, template := range templates { @@ -692,24 +692,27 @@ func convertTemplates(templates []database.Template, workspaceCounts []database. if workspaceCount.TemplateID.String() != template.ID.String() { continue } - apiTemplates = append(apiTemplates, convertTemplate(template, uint32(workspaceCount.Count), createdByNameMap[template.ID.String()])) + apiTemplates = append(apiTemplates, api.convertTemplate(template, uint32(workspaceCount.Count), createdByNameMap[template.ID.String()])) found = true break } if !found { - apiTemplates = append(apiTemplates, convertTemplate(template, uint32(0), createdByNameMap[template.ID.String()])) + apiTemplates = append(apiTemplates, api.convertTemplate(template, uint32(0), createdByNameMap[template.ID.String()])) } } - // Sort templates by WorkspaceOwnerCount DESC + // Sort templates by ActiveUserCount DESC sort.SliceStable(apiTemplates, func(i, j int) bool { - return apiTemplates[i].WorkspaceOwnerCount > apiTemplates[j].WorkspaceOwnerCount + return apiTemplates[i].ActiveUserCount > apiTemplates[j].ActiveUserCount }) return apiTemplates } -func convertTemplate(template database.Template, workspaceOwnerCount uint32, createdByName string) codersdk.Template { +func (api *API) convertTemplate( + template database.Template, workspaceOwnerCount uint32, createdByName string, +) codersdk.Template { + activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID) return codersdk.Template{ ID: template.ID, CreatedAt: template.CreatedAt, @@ -719,6 +722,7 @@ func convertTemplate(template database.Template, workspaceOwnerCount uint32, cre Provisioner: codersdk.ProvisionerType(template.Provisioner), ActiveVersionID: template.ActiveVersionID, WorkspaceOwnerCount: workspaceOwnerCount, + ActiveUserCount: activeCount, Description: template.Description, Icon: template.Icon, MaxTTLMillis: time.Duration(template.MaxTtl).Milliseconds(), diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 2147d0fc4d6a2..76ae1cc0bce8a 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -575,6 +575,8 @@ func TestTemplateDAUs(t *testing.T) { }}, }) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + require.Equal(t, -1, template.ActiveUserCount) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) @@ -617,7 +619,7 @@ func TestTemplateDAUs(t *testing.T) { require.NoError(t, err) _ = sshConn.Close() - want := &codersdk.TemplateDAUsResponse{ + wantDAUs := &codersdk.TemplateDAUsResponse{ Entries: []codersdk.DAUEntry{ { @@ -629,12 +631,18 @@ func TestTemplateDAUs(t *testing.T) { require.Eventuallyf(t, func() bool { daus, err = client.TemplateDAUs(ctx, template.ID) require.NoError(t, err) - - return assert.ObjectsAreEqual(want, daus) + return len(daus.Entries) > 0 }, testutil.WaitShort, testutil.IntervalFast, - "got %+v != %+v", daus, want, + "template daus never loaded", ) + gotDAUs, err := client.TemplateDAUs(ctx, template.ID) + require.NoError(t, err) + require.Equal(t, gotDAUs, wantDAUs) + + template, err = client.Template(ctx, template.ID) + require.NoError(t, err) + require.Equal(t, 1, template.ActiveUserCount) workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) diff --git a/codersdk/templates.go b/codersdk/templates.go index dc1240fe7484c..3af058cc19719 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -14,20 +14,22 @@ import ( // Template is the JSON representation of a Coder template. This type matches the // database object for now, but is abstracted for ease of change later on. type Template struct { - ID uuid.UUID `json:"id"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - OrganizationID uuid.UUID `json:"organization_id"` - Name string `json:"name"` - Provisioner ProvisionerType `json:"provisioner"` - ActiveVersionID uuid.UUID `json:"active_version_id"` - WorkspaceOwnerCount uint32 `json:"workspace_owner_count"` - Description string `json:"description"` - Icon string `json:"icon"` - MaxTTLMillis int64 `json:"max_ttl_ms"` - MinAutostartIntervalMillis int64 `json:"min_autostart_interval_ms"` - CreatedByID uuid.UUID `json:"created_by_id"` - CreatedByName string `json:"created_by_name"` + ID uuid.UUID `json:"id"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + OrganizationID uuid.UUID `json:"organization_id"` + Name string `json:"name"` + Provisioner ProvisionerType `json:"provisioner"` + ActiveVersionID uuid.UUID `json:"active_version_id"` + WorkspaceOwnerCount uint32 `json:"workspace_owner_count"` + // ActiveUserCount is set to -1 when loading. + ActiveUserCount int `json:"active_user_count"` + Description string `json:"description"` + Icon string `json:"icon"` + MaxTTLMillis int64 `json:"max_ttl_ms"` + MinAutostartIntervalMillis int64 `json:"min_autostart_interval_ms"` + CreatedByID uuid.UUID `json:"created_by_id"` + CreatedByName string `json:"created_by_name"` } type UpdateActiveTemplateVersion struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 2c69d43b127d2..6755277e13d53 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -367,6 +367,7 @@ export interface Template { readonly provisioner: ProvisionerType readonly active_version_id: string readonly workspace_owner_count: number + readonly active_user_count: number readonly description: string readonly icon: string readonly max_ttl_ms: number diff --git a/site/src/components/TemplateStats/TemplateStats.stories.tsx b/site/src/components/TemplateStats/TemplateStats.stories.tsx index 6dc5481454712..db6e0ae5c59e4 100644 --- a/site/src/components/TemplateStats/TemplateStats.stories.tsx +++ b/site/src/components/TemplateStats/TemplateStats.stories.tsx @@ -19,7 +19,16 @@ export const UsedByMany = Template.bind({}) UsedByMany.args = { template: { ...Mocks.MockTemplate, - workspace_owner_count: 15, + active_user_count: 15, + }, + activeVersion: Mocks.MockTemplateVersion, +} + +export const ActiveUsersNotLoaded = Template.bind({}) +ActiveUsersNotLoaded.args = { + template: { + ...Mocks.MockTemplate, + active_user_count: -1, }, activeVersion: Mocks.MockTemplateVersion, } diff --git a/site/src/components/TemplateStats/TemplateStats.tsx b/site/src/components/TemplateStats/TemplateStats.tsx index 83fc8f49cb986..0256d5164b5a3 100644 --- a/site/src/components/TemplateStats/TemplateStats.tsx +++ b/site/src/components/TemplateStats/TemplateStats.tsx @@ -1,6 +1,7 @@ import { makeStyles } from "@material-ui/core/styles" import { FC } from "react" import { createDayString } from "util/createDayString" +import { formatTemplateActiveDevelopers } from "util/templates" import { Template, TemplateVersion } from "../../api/typesGenerated" import { MONOSPACE_FONT_FAMILY } from "../../theme/constants" @@ -27,10 +28,8 @@ export const TemplateStats: FC = ({ template, activeVersion {Language.usedByLabel} - {template.workspace_owner_count}{" "} - {template.workspace_owner_count === 1 - ? Language.developerSingular - : Language.developerPlural} + {formatTemplateActiveDevelopers(template.active_user_count)}{" "} + {template.active_user_count === 1 ? Language.developerSingular : Language.developerPlural}
diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx index e45d6cf3906a0..f2a22f28d099d 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx @@ -16,12 +16,13 @@ AllStates.args = { MockTemplate, { ...MockTemplate, - description: "🚀 Some magical template that does some magical things!", + active_user_count: -1, + description: "🚀 Some new template that has no activity data", icon: "/icon/goland.svg", }, { ...MockTemplate, - workspace_owner_count: 150, + active_user_count: 150, description: "😮 Wow, this one has a bunch of usage!", icon: "", }, diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index 4309ec275c19e..5bf45071736d7 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -11,6 +11,7 @@ import useTheme from "@material-ui/styles/useTheme" import { FC } from "react" import { useNavigate } from "react-router-dom" import { createDayString } from "util/createDayString" +import { formatTemplateActiveDevelopers } from "util/templates" import * as TypesGen from "../../api/typesGenerated" import { AvatarData } from "../../components/AvatarData/AvatarData" import { CodeExample } from "../../components/CodeExample/CodeExample" @@ -34,7 +35,7 @@ import { export const Language = { developerCount: (ownerCount: number): string => { - return `${ownerCount} developer${ownerCount !== 1 ? "s" : ""}` + return `${formatTemplateActiveDevelopers(ownerCount)} developer${ownerCount !== 1 ? "s" : ""}` }, nameLabel: "Name", usedByLabel: "Used by", @@ -176,7 +177,7 @@ export const TemplatesPageView: FC - {Language.developerCount(template.workspace_owner_count)} + {Language.developerCount(template.active_user_count)} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 66642bf2bcdd2..1ba15258d4c53 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -174,7 +174,8 @@ export const MockTemplate: TypesGen.Template = { name: "test-template", provisioner: MockProvisioner.provisioners[0], active_version_id: MockTemplateVersion.id, - workspace_owner_count: 1, + workspace_owner_count: 2, + active_user_count: 1, description: "This is a test description.", max_ttl_ms: 24 * 60 * 60 * 1000, min_autostart_interval_ms: 60 * 60 * 1000, diff --git a/site/src/util/templates.ts b/site/src/util/templates.ts new file mode 100644 index 0000000000000..473142d1276c8 --- /dev/null +++ b/site/src/util/templates.ts @@ -0,0 +1,7 @@ +export const formatTemplateActiveDevelopers = (num?: number): string => { + if (num === undefined || num < 0) { + // Loading + return "-" + } + return num.toString() +} From 2fcdb0977156f6281ea48fc4ba8029532c968ba4 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 9 Sep 2022 17:54:55 +0000 Subject: [PATCH 4/5] Address Asher's review comments --- cli/create.go | 12 ++++++------ cli/templates.go | 8 +------- cli/util.go | 17 +++++++++++++++++ coderd/database/databasefake/databasefake.go | 8 ++++---- coderd/templates.go | 2 +- .../pages/TemplatesPage/TemplatesPageView.tsx | 4 ++-- 6 files changed, 31 insertions(+), 20 deletions(-) diff --git a/cli/create.go b/cli/create.go index 90a73c98270c6..86e8ff016ce4e 100644 --- a/cli/create.go +++ b/cli/create.go @@ -82,12 +82,12 @@ func create() *cobra.Command { templateName := template.Name if template.ActiveUserCount > 0 { - developerText := "developer" - if template.ActiveUserCount != 1 { - developerText = "developers" - } - - templateName += cliui.Styles.Placeholder.Render(fmt.Sprintf(" (used by %d %s)", template.ActiveUserCount, developerText)) + templateName += cliui.Styles.Placeholder.Render( + fmt.Sprintf( + " (used by %s)", + formatActiveDevelopers(template.ActiveUserCount), + ), + ) } templateNames = append(templateNames, templateName) diff --git a/cli/templates.go b/cli/templates.go index b2b903b37f05b..cb4a028f2605c 100644 --- a/cli/templates.go +++ b/cli/templates.go @@ -1,7 +1,6 @@ package cli import ( - "fmt" "time" "github.com/google/uuid" @@ -64,11 +63,6 @@ type templateTableRow struct { func displayTemplates(filterColumns []string, templates ...codersdk.Template) (string, error) { rows := make([]templateTableRow, len(templates)) for i, template := range templates { - suffix := "" - if template.ActiveUserCount != 1 { - suffix = "s" - } - rows[i] = templateTableRow{ Name: template.Name, CreatedAt: template.CreatedAt.Format("January 2, 2006"), @@ -76,7 +70,7 @@ func displayTemplates(filterColumns []string, templates ...codersdk.Template) (s OrganizationID: template.OrganizationID, Provisioner: template.Provisioner, ActiveVersionID: template.ActiveVersionID, - UsedBy: cliui.Styles.Fuchsia.Render(fmt.Sprintf("%d developer%s", template.ActiveUserCount, suffix)), + UsedBy: cliui.Styles.Fuchsia.Render(formatActiveDevelopers(template.ActiveUserCount)), MaxTTL: (time.Duration(template.MaxTTLMillis) * time.Millisecond), MinAutostartInterval: (time.Duration(template.MinAutostartIntervalMillis) * time.Millisecond), } diff --git a/cli/util.go b/cli/util.go index c7c75ae1c06bd..982b3fdba3a5b 100644 --- a/cli/util.go +++ b/cli/util.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "strconv" "strings" "time" @@ -175,3 +176,19 @@ func parseTime(s string) (time.Time, error) { } return time.Time{}, errInvalidTimeFormat } + +func formatActiveDevelopers(n int) string { + developerText := "developer" + if n != 1 { + developerText = "developers" + } + + var nStr string + if n < 0 { + nStr = "-" + } else { + nStr = strconv.Itoa(n) + } + + return fmt.Sprintf("%s active %s", nStr, developerText) +} diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index cd52ff49cfd41..1cbac61863a26 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -180,13 +180,13 @@ func (q *fakeQuerier) GetTemplateDAUs(_ context.Context, templateID uuid.UUID) ( seens[date] = dateEntry } - countKeys := maps.Keys(seens) - sort.Slice(countKeys, func(i, j int) bool { - return countKeys[i].Before(countKeys[j]) + seenKeys := maps.Keys(seens) + sort.Slice(seenKeys, func(i, j int) bool { + return seenKeys[i].Before(seenKeys[j]) }) var rs []database.GetTemplateDAUsRow - for _, key := range countKeys { + for _, key := range seenKeys { ids := seens[key] for id := range ids { rs = append(rs, database.GetTemplateDAUsRow{ diff --git a/coderd/templates.go b/coderd/templates.go index fb87bf70d2210..4c63a07c70805 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -568,7 +568,7 @@ func (api *API) templateDAUs(rw http.ResponseWriter, r *http.Request) { } resp, _ := api.metricsCache.TemplateDAUs(template.ID) - if resp.Entries == nil { + if resp == nil || resp.Entries == nil { resp.Entries = []codersdk.DAUEntry{} } httpapi.Write(rw, http.StatusOK, resp) diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index 41d046e75bc8a..b40dab7b21d2d 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -34,8 +34,8 @@ import { } from "../../components/Tooltips/HelpTooltip/HelpTooltip" export const Language = { - developerCount: (ownerCount: number): string => { - return `${formatTemplateActiveDevelopers(ownerCount)} developer${ownerCount !== 1 ? "s" : ""}` + developerCount: (activeCount: number): string => { + return `${formatTemplateActiveDevelopers(activeCount)} developer${activeCount !== 1 ? "s" : ""}` }, nameLabel: "Name", usedByLabel: "Used by", From 899482429d2eb7cf71711b725f4defe01273e7ad Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 9 Sep 2022 19:01:01 +0000 Subject: [PATCH 5/5] fixup! Address Asher's review comments --- coderd/templates.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/coderd/templates.go b/coderd/templates.go index 4c63a07c70805..c48531a25c226 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -569,7 +569,10 @@ func (api *API) templateDAUs(rw http.ResponseWriter, r *http.Request) { resp, _ := api.metricsCache.TemplateDAUs(template.ID) if resp == nil || resp.Entries == nil { - resp.Entries = []codersdk.DAUEntry{} + httpapi.Write(rw, http.StatusOK, &codersdk.TemplateDAUsResponse{ + Entries: []codersdk.DAUEntry{}, + }) + return } httpapi.Write(rw, http.StatusOK, resp) }