Skip to content

Commit 50bf5ca

Browse files
SasSwartSasSwart
authored
fix(coderd/database): aggregate user engagement statistics by interval (coder#16150)
This PR addresses the TODO comment here: https://github.com/coder/coder/pull/16134/files#diff-1844f895bb005f036da11d800fe2a76b54bfddd481c5d8cb15f210c64679caa5R47 The backend now backfills entries for dates with no status changes.
1 parent 296dbf0 commit 50bf5ca

File tree

6 files changed

+142
-142
lines changed

6 files changed

+142
-142
lines changed

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,6 +1712,7 @@ func (s *MethodTestSuite) TestUser() {
17121712
check.Args(database.GetUserStatusCountsParams{
17131713
StartTime: time.Now().Add(-time.Hour * 24 * 30),
17141714
EndTime: time.Now(),
1715+
Interval: int32((time.Hour * 24).Seconds()),
17151716
}).Asserts(rbac.ResourceUser, policy.ActionRead)
17161717
}))
17171718
}

‎coderd/database/dbtime/dbtime.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,9 @@ func Now() time.Time {
1616
func Time(t time.Time) time.Time {
1717
return t.Round(time.Microsecond)
1818
}
19+
20+
// StartOfDay returns the first timestamp of the day of the input timestamp in its location.
21+
func StartOfDay(t time.Time) time.Time {
22+
year, month, day := t.Date()
23+
return time.Date(year, month, day, 0, 0, 0, 0, t.Location())
24+
}

‎coderd/database/querier_test.go

Lines changed: 115 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"database/sql"
88
"encoding/json"
99
"fmt"
10-
"maps"
1110
"sort"
1211
"testing"
1312
"time"
@@ -2411,12 +2410,9 @@ func TestGetUserStatusCounts(t *testing.T) {
24112410
db, _ := dbtestutil.NewDB(t)
24122411
ctx := testutil.Context(t, testutil.WaitShort)
24132412

2414-
end := dbtime.Now()
2415-
start := end.Add(-30 * 24 * time.Hour)
2416-
24172413
counts, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
2418-
StartTime: start,
2419-
EndTime: end,
2414+
StartTime: createdAt,
2415+
EndTime: today,
24202416
})
24212417
require.NoError(t, err)
24222418
require.Empty(t, counts, "should return no results when there are no users")
@@ -2457,23 +2453,31 @@ func TestGetUserStatusCounts(t *testing.T) {
24572453
UpdatedAt: createdAt,
24582454
})
24592455

2460-
// Query for the last 30 days
24612456
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
2462-
StartTime: createdAt,
2463-
EndTime: today,
2457+
StartTime: dbtime.StartOfDay(createdAt),
2458+
EndTime: dbtime.StartOfDay(today),
24642459
})
24652460
require.NoError(t, err)
2466-
require.NotEmpty(t, userStatusChanges, "should return results")
24672461

2468-
require.Len(t, userStatusChanges, 2, "should have 1 entry per status change plus and 1 entry for the end of the range = 2 entries")
2469-
2470-
require.Equal(t, userStatusChanges[0].Status, tc.status, "should have the correct status")
2471-
require.Equal(t, userStatusChanges[0].Count, int64(1), "should have 1 user")
2472-
require.True(t, userStatusChanges[0].Date.Equal(createdAt), "should have the correct date")
2473-
2474-
require.Equal(t, userStatusChanges[1].Status, tc.status, "should have the correct status")
2475-
require.Equal(t, userStatusChanges[1].Count, int64(1), "should have 1 user")
2476-
require.True(t, userStatusChanges[1].Date.Equal(today), "should have the correct date")
2462+
numDays := int(dbtime.StartOfDay(today).Sub(dbtime.StartOfDay(createdAt)).Hours() / 24)
2463+
require.Len(t, userStatusChanges, numDays+1, "should have 1 entry per day between the start and end time, including the end time")
2464+
2465+
for i, row := range userStatusChanges {
2466+
require.Equal(t, tc.status, row.Status, "should have the correct status")
2467+
require.True(
2468+
t,
2469+
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, i)),
2470+
"expected date %s, but got %s for row %n",
2471+
dbtime.StartOfDay(createdAt).AddDate(0, 0, i),
2472+
row.Date.In(location).String(),
2473+
i,
2474+
)
2475+
if row.Date.Before(createdAt) {
2476+
require.Equal(t, int64(0), row.Count, "should have 0 users before creation")
2477+
} else {
2478+
require.Equal(t, int64(1), row.Count, "should have 1 user after creation")
2479+
}
2480+
}
24772481
})
24782482
}
24792483
})
@@ -2627,24 +2631,38 @@ func TestGetUserStatusCounts(t *testing.T) {
26272631

26282632
// Query for the last 5 days
26292633
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
2630-
StartTime: createdAt,
2631-
EndTime: today,
2634+
StartTime: dbtime.StartOfDay(createdAt),
2635+
EndTime: dbtime.StartOfDay(today),
26322636
})
26332637
require.NoError(t, err)
2634-
require.NotEmpty(t, userStatusChanges, "should return results")
26352638

2636-
gotCounts := map[time.Time]map[database.UserStatus]int64{}
2637-
for _, row := range userStatusChanges {
2638-
gotDateInLocation := row.Date.In(location)
2639-
if _, ok := gotCounts[gotDateInLocation]; !ok {
2640-
gotCounts[gotDateInLocation] = map[database.UserStatus]int64{}
2639+
for i, row := range userStatusChanges {
2640+
require.True(
2641+
t,
2642+
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, i/2)),
2643+
"expected date %s, but got %s for row %n",
2644+
dbtime.StartOfDay(createdAt).AddDate(0, 0, i/2),
2645+
row.Date.In(location).String(),
2646+
i,
2647+
)
2648+
if row.Date.Before(createdAt) {
2649+
require.Equal(t, int64(0), row.Count)
2650+
} else if row.Date.Before(firstTransitionTime) {
2651+
if row.Status == tc.initialStatus {
2652+
require.Equal(t, int64(1), row.Count)
2653+
} else if row.Status == tc.targetStatus {
2654+
require.Equal(t, int64(0), row.Count)
2655+
}
2656+
} else if !row.Date.After(today) {
2657+
if row.Status == tc.initialStatus {
2658+
require.Equal(t, int64(0), row.Count)
2659+
} else if row.Status == tc.targetStatus {
2660+
require.Equal(t, int64(1), row.Count)
2661+
}
2662+
} else {
2663+
t.Errorf("date %q beyond expected range end %q", row.Date, today)
26412664
}
2642-
if _, ok := gotCounts[gotDateInLocation][row.Status]; !ok {
2643-
gotCounts[gotDateInLocation][row.Status] = 0
2644-
}
2645-
gotCounts[gotDateInLocation][row.Status] += row.Count
26462665
}
2647-
require.Equal(t, tc.expectedCounts, gotCounts)
26482666
})
26492667
}
26502668
})
@@ -2725,6 +2743,7 @@ func TestGetUserStatusCounts(t *testing.T) {
27252743
tc := tc
27262744
t.Run(tc.name, func(t *testing.T) {
27272745
t.Parallel()
2746+
27282747
db, _ := dbtestutil.NewDB(t)
27292748
ctx := testutil.Context(t, testutil.WaitShort)
27302749

@@ -2756,66 +2775,48 @@ func TestGetUserStatusCounts(t *testing.T) {
27562775
require.NoError(t, err)
27572776

27582777
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
2759-
StartTime: createdAt,
2760-
EndTime: today,
2778+
StartTime: dbtime.StartOfDay(createdAt),
2779+
EndTime: dbtime.StartOfDay(today),
27612780
})
27622781
require.NoError(t, err)
27632782
require.NotEmpty(t, userStatusChanges)
2764-
gotCounts := map[time.Time]map[database.UserStatus]int64{
2765-
createdAt.In(location): {},
2766-
firstTransitionTime.In(location): {},
2767-
secondTransitionTime.In(location): {},
2768-
today.In(location): {},
2769-
}
2783+
gotCounts := map[time.Time]map[database.UserStatus]int64{}
27702784
for _, row := range userStatusChanges {
27712785
dateInLocation := row.Date.In(location)
2772-
switch {
2773-
case dateInLocation.Equal(createdAt.In(location)):
2774-
gotCounts[createdAt][row.Status] = row.Count
2775-
case dateInLocation.Equal(firstTransitionTime.In(location)):
2776-
gotCounts[firstTransitionTime][row.Status] = row.Count
2777-
case dateInLocation.Equal(secondTransitionTime.In(location)):
2778-
gotCounts[secondTransitionTime][row.Status] = row.Count
2779-
case dateInLocation.Equal(today.In(location)):
2780-
gotCounts[today][row.Status] = row.Count
2781-
default:
2782-
t.Fatalf("unexpected date %s", row.Date)
2786+
if gotCounts[dateInLocation] == nil {
2787+
gotCounts[dateInLocation] = map[database.UserStatus]int64{}
27832788
}
2789+
gotCounts[dateInLocation][row.Status] = row.Count
27842790
}
27852791

27862792
expectedCounts := map[time.Time]map[database.UserStatus]int64{}
2787-
for _, status := range []database.UserStatus{
2788-
tc.user1Transition.from,
2789-
tc.user1Transition.to,
2790-
tc.user2Transition.from,
2791-
tc.user2Transition.to,
2792-
} {
2793-
if _, ok := expectedCounts[createdAt]; !ok {
2794-
expectedCounts[createdAt] = map[database.UserStatus]int64{}
2793+
for d := dbtime.StartOfDay(createdAt); !d.After(dbtime.StartOfDay(today)); d = d.AddDate(0, 0, 1) {
2794+
expectedCounts[d] = map[database.UserStatus]int64{}
2795+
2796+
// Default values
2797+
expectedCounts[d][tc.user1Transition.from] = 0
2798+
expectedCounts[d][tc.user1Transition.to] = 0
2799+
expectedCounts[d][tc.user2Transition.from] = 0
2800+
expectedCounts[d][tc.user2Transition.to] = 0
2801+
2802+
// Counted Values
2803+
if d.Before(createdAt) {
2804+
continue
2805+
} else if d.Before(firstTransitionTime) {
2806+
expectedCounts[d][tc.user1Transition.from]++
2807+
expectedCounts[d][tc.user2Transition.from]++
2808+
} else if d.Before(secondTransitionTime) {
2809+
expectedCounts[d][tc.user1Transition.to]++
2810+
expectedCounts[d][tc.user2Transition.from]++
2811+
} else if d.Before(today) {
2812+
expectedCounts[d][tc.user1Transition.to]++
2813+
expectedCounts[d][tc.user2Transition.to]++
2814+
} else {
2815+
t.Fatalf("date %q beyond expected range end %q", d, today)
27952816
}
2796-
expectedCounts[createdAt][status] = 0
27972817
}
27982818

2799-
expectedCounts[createdAt][tc.user1Transition.from]++
2800-
expectedCounts[createdAt][tc.user2Transition.from]++
2801-
2802-
expectedCounts[firstTransitionTime] = map[database.UserStatus]int64{}
2803-
maps.Copy(expectedCounts[firstTransitionTime], expectedCounts[createdAt])
2804-
expectedCounts[firstTransitionTime][tc.user1Transition.from]--
2805-
expectedCounts[firstTransitionTime][tc.user1Transition.to]++
2806-
2807-
expectedCounts[secondTransitionTime] = map[database.UserStatus]int64{}
2808-
maps.Copy(expectedCounts[secondTransitionTime], expectedCounts[firstTransitionTime])
2809-
expectedCounts[secondTransitionTime][tc.user2Transition.from]--
2810-
expectedCounts[secondTransitionTime][tc.user2Transition.to]++
2811-
2812-
expectedCounts[today] = map[database.UserStatus]int64{}
2813-
maps.Copy(expectedCounts[today], expectedCounts[secondTransitionTime])
2814-
2815-
require.Equal(t, expectedCounts[createdAt], gotCounts[createdAt])
2816-
require.Equal(t, expectedCounts[firstTransitionTime], gotCounts[firstTransitionTime])
2817-
require.Equal(t, expectedCounts[secondTransitionTime], gotCounts[secondTransitionTime])
2818-
require.Equal(t, expectedCounts[today], gotCounts[today])
2819+
require.Equal(t, expectedCounts, gotCounts)
28192820
})
28202821
}
28212822
})
@@ -2832,16 +2833,23 @@ func TestGetUserStatusCounts(t *testing.T) {
28322833
})
28332834

28342835
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
2835-
StartTime: createdAt.Add(time.Hour * 24),
2836-
EndTime: today,
2836+
StartTime: dbtime.StartOfDay(createdAt.Add(time.Hour * 24)),
2837+
EndTime: dbtime.StartOfDay(today),
28372838
})
28382839
require.NoError(t, err)
28392840

2840-
require.Len(t, userStatusChanges, 2)
2841-
require.Equal(t, userStatusChanges[0].Count, int64(1))
2842-
require.Equal(t, userStatusChanges[0].Status, database.UserStatusActive)
2843-
require.Equal(t, userStatusChanges[1].Count, int64(1))
2844-
require.Equal(t, userStatusChanges[1].Status, database.UserStatusActive)
2841+
for i, row := range userStatusChanges {
2842+
require.True(
2843+
t,
2844+
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, 1+i)),
2845+
"expected date %s, but got %s for row %n",
2846+
dbtime.StartOfDay(createdAt).AddDate(0, 0, 1+i),
2847+
row.Date.In(location).String(),
2848+
i,
2849+
)
2850+
require.Equal(t, database.UserStatusActive, row.Status)
2851+
require.Equal(t, int64(1), row.Count)
2852+
}
28452853
})
28462854

28472855
t.Run("User deleted before query range", func(t *testing.T) {
@@ -2881,16 +2889,28 @@ func TestGetUserStatusCounts(t *testing.T) {
28812889
require.NoError(t, err)
28822890

28832891
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
2884-
StartTime: createdAt,
2885-
EndTime: today.Add(time.Hour * 24),
2892+
StartTime: dbtime.StartOfDay(createdAt),
2893+
EndTime: dbtime.StartOfDay(today.Add(time.Hour * 24)),
28862894
})
28872895
require.NoError(t, err)
2888-
require.Equal(t, userStatusChanges[0].Count, int64(1))
2889-
require.Equal(t, userStatusChanges[0].Status, database.UserStatusActive)
2890-
require.Equal(t, userStatusChanges[1].Count, int64(0))
2891-
require.Equal(t, userStatusChanges[1].Status, database.UserStatusActive)
2892-
require.Equal(t, userStatusChanges[2].Count, int64(0))
2893-
require.Equal(t, userStatusChanges[2].Status, database.UserStatusActive)
2896+
for i, row := range userStatusChanges {
2897+
require.True(
2898+
t,
2899+
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, i)),
2900+
"expected date %s, but got %s for row %n",
2901+
dbtime.StartOfDay(createdAt).AddDate(0, 0, i),
2902+
row.Date.In(location).String(),
2903+
i,
2904+
)
2905+
require.Equal(t, database.UserStatusActive, row.Status)
2906+
if row.Date.Before(createdAt) {
2907+
require.Equal(t, int64(0), row.Count)
2908+
} else if i == len(userStatusChanges)-1 {
2909+
require.Equal(t, int64(0), row.Count)
2910+
} else {
2911+
require.Equal(t, int64(1), row.Count)
2912+
}
2913+
}
28942914
})
28952915
})
28962916
}

‎coderd/database/queries.sql.go

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

0 commit comments

Comments
 (0)