feat: use active users instead of total users in Template views (#3900)

This commit is contained in:
Ammar Bandukwala
2022-09-09 14:30:31 -05:00
committed by GitHub
parent 346583f13e
commit f6aa025a01
20 changed files with 251 additions and 142 deletions

View File

@ -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]
if dateEntry == nil {
dateEntry = make(map[string]struct{})
}
counts[date] = dateEntry
dateEntry[as.UserID.String()] = struct{}{}
dateEntry := seens[date]
if dateEntry == nil {
dateEntry = make(map[uuid.UUID]struct{})
}
dateEntry[as.UserID] = struct{}{}
seens[date] = dateEntry
}
countKeys := maps.Keys(counts)
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 {
rs = append(rs, database.GetTemplateDAUsRow{
Date: key,
Amount: int64(len(counts[key])),
})
for _, key := range seenKeys {
ids := seens[key]
for id := range ids {
rs = append(rs, database.GetTemplateDAUsRow{
Date: key,
UserID: id,
})
}
}
return rs, nil

View File

@ -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)

View File

@ -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;

View File

@ -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,9 +26,10 @@ type Cache struct {
database database.Store
log slog.Logger
templateDAUResponses atomic.Pointer[map[string]codersdk.TemplateDAUsResponse]
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
@ -41,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,
}
@ -49,34 +52,68 @@ 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 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 {
@ -90,30 +127,26 @@ func (c *Cache) refresh(ctx context.Context) error {
return err
}
templateDAUs := make(map[string]codersdk.TemplateDAUsResponse, len(templates))
var (
templateDAUs = make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates))
templateUniqueUsers = make(map[uuid.UUID]int)
)
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)
templateUniqueUsers[template.ID] = countUniqueUsers(rows)
}
c.templateDAUResponses.Store(&templateDAUs)
c.templateUniqueUsers.Store(&templateUniqueUsers)
return nil
}
func (c *Cache) run(ctx context.Context) {
defer close(c.doneCh)
defer close(c.done)
ticker := time.NewTicker(c.interval)
defer ticker.Stop()
@ -140,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
@ -150,23 +183,40 @@ 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.String()]
resp, ok := (*m)[id]
if !ok {
// Probably no data.
return codersdk.TemplateDAUsResponse{}
return nil, false
}
return resp
return &resp, true
}
// 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 {
// Data loading.
return -1, false
}
resp, ok := (*m)[id]
if !ok {
// Probably no data.
return -1, false
}
return resp, true
}

View File

@ -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 {
@ -157,7 +161,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()
@ -167,19 +171,29 @@ func TestCache(t *testing.T) {
ID: templateID,
})
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
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)
}, testutil.WaitShort, testutil.IntervalFast,
"GetDAUs() = %v, want %v", got, tt.want,
_, ok := cache.TemplateDAUs(templateID)
return ok
}, testutil.WaitShort, testutil.IntervalMedium,
"TemplateDAUs never populated",
)
gotUniqueUsers, ok = cache.TemplateUniqueUsers(templateID)
require.True(t, ok)
gotEntries, ok := cache.TemplateDAUs(templateID)
require.True(t, ok)
require.Equal(t, tt.want.entries, gotEntries.Entries)
require.Equal(t, tt.want.uniqueUsers, gotUniqueUsers)
})
}
}

View File

@ -79,7 +79,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) {
@ -304,7 +304,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 {
@ -375,7 +375,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) {
@ -429,7 +429,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) {
@ -557,7 +557,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) {
@ -567,9 +567,12 @@ func (api *API) templateDAUs(rw http.ResponseWriter, r *http.Request) {
return
}
resp := api.metricsCache.TemplateDAUs(template.ID)
if resp.Entries == nil {
resp.Entries = []codersdk.DAUEntry{}
resp, _ := api.metricsCache.TemplateDAUs(template.ID)
if resp == nil || resp.Entries == nil {
httpapi.Write(rw, http.StatusOK, &codersdk.TemplateDAUsResponse{
Entries: []codersdk.DAUEntry{},
})
return
}
httpapi.Write(rw, http.StatusOK, resp)
}
@ -726,7 +729,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 {
@ -735,24 +738,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,
@ -762,6 +768,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(),

View File

@ -593,6 +593,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)
@ -635,7 +637,7 @@ func TestTemplateDAUs(t *testing.T) {
require.NoError(t, err)
_ = sshConn.Close()
want := &codersdk.TemplateDAUsResponse{
wantDAUs := &codersdk.TemplateDAUsResponse{
Entries: []codersdk.DAUEntry{
{
@ -647,12 +649,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)