opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(coderd/database): aggregate user engagement statistics by interval (#16150)

Browse Source 
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.
This commit is contained in:
Sas Swart
2025-01-16 17:34:53 +02:00
committed by GitHub
parent 296dbf0918
commit 50bf5ca8fe
6 changed files with 141 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/database/dbauthz/dbauthz_test.go
+1
View File
@@ -1712,6 +1712,7 @@ func (s *MethodTestSuite) TestUser() {
check.Args(database.GetUserStatusCountsParams{
StartTime: time.Now().Add(-time.Hour * 24 * 30),
EndTime: time.Now(),
Interval: int32((time.Hour * 24).Seconds()),
}).Asserts(rbac.ResourceUser, policy.ActionRead)
}))
}
coderd/database/dbtime/dbtime.go
+6
View File
@@ -16,3 +16,9 @@ func Now() time.Time {
func Time(t time.Time) time.Time {
return t.Round(time.Microsecond)
}
// StartOfDay returns the first timestamp of the day of the input timestamp in its location.
func StartOfDay(t time.Time) time.Time {
year, month, day := t.Date()
return time.Date(year, month, day, 0, 0, 0, 0, t.Location())
}
coderd/database/querier_test.go
+114 -94
View File
@@ -7,7 +7,6 @@ import (
"database/sql"
"encoding/json"
"fmt"
"maps"
"sort"
"testing"
"time"
@@ -2411,12 +2410,9 @@ func TestGetUserStatusCounts(t *testing.T) {
db, _ := dbtestutil.NewDB(t)
ctx := testutil.Context(t, testutil.WaitShort)
end := dbtime.Now()
start := end.Add(-30 * 24 * time.Hour)
counts, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
StartTime: start,
EndTime: end,
StartTime: createdAt,
EndTime: today,
})
require.NoError(t, err)
require.Empty(t, counts, "should return no results when there are no users")
@@ -2457,23 +2453,31 @@ func TestGetUserStatusCounts(t *testing.T) {
UpdatedAt: createdAt,
})
// Query for the last 30 days
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
StartTime: createdAt,
EndTime: today,
StartTime: dbtime.StartOfDay(createdAt),
EndTime: dbtime.StartOfDay(today),
})
require.NoError(t, err)
require.NotEmpty(t, userStatusChanges, "should return results")
require.Len(t, userStatusChanges, 2, "should have 1 entry per status change plus and 1 entry for the end of the range = 2 entries")
numDays := int(dbtime.StartOfDay(today).Sub(dbtime.StartOfDay(createdAt)).Hours() / 24)
require.Len(t, userStatusChanges, numDays+1, "should have 1 entry per day between the start and end time, including the end time")
require.Equal(t, userStatusChanges[0].Status, tc.status, "should have the correct status")
require.Equal(t, userStatusChanges[0].Count, int64(1), "should have 1 user")
require.True(t, userStatusChanges[0].Date.Equal(createdAt), "should have the correct date")
require.Equal(t, userStatusChanges[1].Status, tc.status, "should have the correct status")
require.Equal(t, userStatusChanges[1].Count, int64(1), "should have 1 user")
require.True(t, userStatusChanges[1].Date.Equal(today), "should have the correct date")
for i, row := range userStatusChanges {
require.Equal(t, tc.status, row.Status, "should have the correct status")
require.True(
t,
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, i)),
"expected date %s, but got %s for row %n",
dbtime.StartOfDay(createdAt).AddDate(0, 0, i),
row.Date.In(location).String(),
i,
)
if row.Date.Before(createdAt) {
require.Equal(t, int64(0), row.Count, "should have 0 users before creation")
} else {
require.Equal(t, int64(1), row.Count, "should have 1 user after creation")
}
}
})
}
})
@@ -2627,24 +2631,38 @@ func TestGetUserStatusCounts(t *testing.T) {
// Query for the last 5 days
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
StartTime: createdAt,
EndTime: today,
StartTime: dbtime.StartOfDay(createdAt),
EndTime: dbtime.StartOfDay(today),
})
require.NoError(t, err)
require.NotEmpty(t, userStatusChanges, "should return results")
gotCounts := map[time.Time]map[database.UserStatus]int64{}
for _, row := range userStatusChanges {
gotDateInLocation := row.Date.In(location)
if _, ok := gotCounts[gotDateInLocation]; !ok {
gotCounts[gotDateInLocation] = map[database.UserStatus]int64{}
for i, row := range userStatusChanges {
require.True(
t,
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, i/2)),
"expected date %s, but got %s for row %n",
dbtime.StartOfDay(createdAt).AddDate(0, 0, i/2),
row.Date.In(location).String(),
i,
)
if row.Date.Before(createdAt) {
require.Equal(t, int64(0), row.Count)
} else if row.Date.Before(firstTransitionTime) {
if row.Status == tc.initialStatus {
require.Equal(t, int64(1), row.Count)
} else if row.Status == tc.targetStatus {
require.Equal(t, int64(0), row.Count)
}
} else if !row.Date.After(today) {
if row.Status == tc.initialStatus {
require.Equal(t, int64(0), row.Count)
} else if row.Status == tc.targetStatus {
require.Equal(t, int64(1), row.Count)
}
} else {
t.Errorf("date %q beyond expected range end %q", row.Date, today)
}
if _, ok := gotCounts[gotDateInLocation][row.Status]; !ok {
gotCounts[gotDateInLocation][row.Status] = 0
}
gotCounts[gotDateInLocation][row.Status] += row.Count
}
require.Equal(t, tc.expectedCounts, gotCounts)
})
}
})
@@ -2725,6 +2743,7 @@ func TestGetUserStatusCounts(t *testing.T) {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
db, _ := dbtestutil.NewDB(t)
ctx := testutil.Context(t, testutil.WaitShort)
@@ -2756,66 +2775,48 @@ func TestGetUserStatusCounts(t *testing.T) {
require.NoError(t, err)
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
StartTime: createdAt,
EndTime: today,
StartTime: dbtime.StartOfDay(createdAt),
EndTime: dbtime.StartOfDay(today),
})
require.NoError(t, err)
require.NotEmpty(t, userStatusChanges)
gotCounts := map[time.Time]map[database.UserStatus]int64{
createdAt.In(location): {},
firstTransitionTime.In(location): {},
secondTransitionTime.In(location): {},
today.In(location): {},
}
gotCounts := map[time.Time]map[database.UserStatus]int64{}
for _, row := range userStatusChanges {
dateInLocation := row.Date.In(location)
switch {
case dateInLocation.Equal(createdAt.In(location)):
gotCounts[createdAt][row.Status] = row.Count
case dateInLocation.Equal(firstTransitionTime.In(location)):
gotCounts[firstTransitionTime][row.Status] = row.Count
case dateInLocation.Equal(secondTransitionTime.In(location)):
gotCounts[secondTransitionTime][row.Status] = row.Count
case dateInLocation.Equal(today.In(location)):
gotCounts[today][row.Status] = row.Count
default:
t.Fatalf("unexpected date %s", row.Date)
if gotCounts[dateInLocation] == nil {
gotCounts[dateInLocation] = map[database.UserStatus]int64{}
}
gotCounts[dateInLocation][row.Status] = row.Count
}
expectedCounts := map[time.Time]map[database.UserStatus]int64{}
for _, status := range []database.UserStatus{
tc.user1Transition.from,
tc.user1Transition.to,
tc.user2Transition.from,
tc.user2Transition.to,
} {
if _, ok := expectedCounts[createdAt]; !ok {
expectedCounts[createdAt] = map[database.UserStatus]int64{}
for d := dbtime.StartOfDay(createdAt); !d.After(dbtime.StartOfDay(today)); d = d.AddDate(0, 0, 1) {
expectedCounts[d] = map[database.UserStatus]int64{}
// Default values
expectedCounts[d][tc.user1Transition.from] = 0
expectedCounts[d][tc.user1Transition.to] = 0
expectedCounts[d][tc.user2Transition.from] = 0
expectedCounts[d][tc.user2Transition.to] = 0
// Counted Values
if d.Before(createdAt) {
continue
} else if d.Before(firstTransitionTime) {
expectedCounts[d][tc.user1Transition.from]++
expectedCounts[d][tc.user2Transition.from]++
} else if d.Before(secondTransitionTime) {
expectedCounts[d][tc.user1Transition.to]++
expectedCounts[d][tc.user2Transition.from]++
} else if d.Before(today) {
expectedCounts[d][tc.user1Transition.to]++
expectedCounts[d][tc.user2Transition.to]++
} else {
t.Fatalf("date %q beyond expected range end %q", d, today)
}
expectedCounts[createdAt][status] = 0
}
expectedCounts[createdAt][tc.user1Transition.from]++
expectedCounts[createdAt][tc.user2Transition.from]++
expectedCounts[firstTransitionTime] = map[database.UserStatus]int64{}
maps.Copy(expectedCounts[firstTransitionTime], expectedCounts[createdAt])
expectedCounts[firstTransitionTime][tc.user1Transition.from]--
expectedCounts[firstTransitionTime][tc.user1Transition.to]++
expectedCounts[secondTransitionTime] = map[database.UserStatus]int64{}
maps.Copy(expectedCounts[secondTransitionTime], expectedCounts[firstTransitionTime])
expectedCounts[secondTransitionTime][tc.user2Transition.from]--
expectedCounts[secondTransitionTime][tc.user2Transition.to]++
expectedCounts[today] = map[database.UserStatus]int64{}
maps.Copy(expectedCounts[today], expectedCounts[secondTransitionTime])
require.Equal(t, expectedCounts[createdAt], gotCounts[createdAt])
require.Equal(t, expectedCounts[firstTransitionTime], gotCounts[firstTransitionTime])
require.Equal(t, expectedCounts[secondTransitionTime], gotCounts[secondTransitionTime])
require.Equal(t, expectedCounts[today], gotCounts[today])
require.Equal(t, expectedCounts, gotCounts)
})
}
})
@@ -2832,16 +2833,23 @@ func TestGetUserStatusCounts(t *testing.T) {
})
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
StartTime: createdAt.Add(time.Hour * 24),
EndTime: today,
StartTime: dbtime.StartOfDay(createdAt.Add(time.Hour * 24)),
EndTime: dbtime.StartOfDay(today),
})
require.NoError(t, err)
require.Len(t, userStatusChanges, 2)
require.Equal(t, userStatusChanges[0].Count, int64(1))
require.Equal(t, userStatusChanges[0].Status, database.UserStatusActive)
require.Equal(t, userStatusChanges[1].Count, int64(1))
require.Equal(t, userStatusChanges[1].Status, database.UserStatusActive)
for i, row := range userStatusChanges {
require.True(
t,
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, 1+i)),
"expected date %s, but got %s for row %n",
dbtime.StartOfDay(createdAt).AddDate(0, 0, 1+i),
row.Date.In(location).String(),
i,
)
require.Equal(t, database.UserStatusActive, row.Status)
require.Equal(t, int64(1), row.Count)
}
})
t.Run("User deleted before query range", func(t *testing.T) {
@@ -2881,16 +2889,28 @@ func TestGetUserStatusCounts(t *testing.T) {
require.NoError(t, err)
userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
StartTime: createdAt,
EndTime: today.Add(time.Hour * 24),
StartTime: dbtime.StartOfDay(createdAt),
EndTime: dbtime.StartOfDay(today.Add(time.Hour * 24)),
})
require.NoError(t, err)
require.Equal(t, userStatusChanges[0].Count, int64(1))
require.Equal(t, userStatusChanges[0].Status, database.UserStatusActive)
require.Equal(t, userStatusChanges[1].Count, int64(0))
require.Equal(t, userStatusChanges[1].Status, database.UserStatusActive)
require.Equal(t, userStatusChanges[2].Count, int64(0))
require.Equal(t, userStatusChanges[2].Status, database.UserStatusActive)
for i, row := range userStatusChanges {
require.True(
t,
row.Date.In(location).Equal(dbtime.StartOfDay(createdAt).AddDate(0, 0, i)),
"expected date %s, but got %s for row %n",
dbtime.StartOfDay(createdAt).AddDate(0, 0, i),
row.Date.In(location).String(),
i,
)
require.Equal(t, database.UserStatusActive, row.Status)
if row.Date.Before(createdAt) {
require.Equal(t, int64(0), row.Count)
} else if i == len(userStatusChanges)-1 {
require.Equal(t, int64(0), row.Count)
} else {
require.Equal(t, int64(1), row.Count)
}
}
})
})
}
coderd/database/queries.sql.go
+8 -21
View File
@@ -3099,25 +3099,11 @@ WITH
-- dates_of_interest defines all points in time that are relevant to the query.
-- It includes the start_time, all status changes, all deletions, and the end_time.
dates_of_interest AS (
SELECT $1::timestamptz AS date
UNION
SELECT DISTINCT changed_at AS date
FROM user_status_changes
WHERE changed_at > $1::timestamptz
AND changed_at < $2::timestamptz
UNION
SELECT DISTINCT deleted_at AS date
FROM user_deleted
WHERE deleted_at > $1::timestamptz
AND deleted_at < $2::timestamptz
UNION
SELECT $2::timestamptz AS date
SELECT date FROM generate_series(
$1::timestamptz,
$2::timestamptz,
(CASE WHEN $3::int <= 0 THEN 3600 * 24 ELSE $3::int END || ' seconds')::interval
) AS date
),
-- latest_status_before_range defines the status of each user before the start_time.
-- We do not include users who were deleted before the start_time. We use this to ensure that
@@ -3193,7 +3179,7 @@ ranked_status_change_per_user_per_date AS (
LEFT JOIN relevant_status_changes rsc1 ON rsc1.changed_at <= d.date
)
SELECT
rscpupd.date,
rscpupd.date::timestamptz AS date,
statuses.new_status AS status,
COUNT(rscpupd.user_id) FILTER (
WHERE rscpupd.rn = 1
@@ -3217,6 +3203,7 @@ ORDER BY rscpupd.date
type GetUserStatusCountsParams struct {
StartTime time.Time `db:"start_time" json:"start_time"`
EndTime time.Time `db:"end_time" json:"end_time"`
Interval int32 `db:"interval" json:"interval"`
}
type GetUserStatusCountsRow struct {
@@ -3238,7 +3225,7 @@ type GetUserStatusCountsRow struct {
// We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such,
// the result shows the total number of users in each status on any particular day.
func (q *sqlQuerier) GetUserStatusCounts(ctx context.Context, arg GetUserStatusCountsParams) ([]GetUserStatusCountsRow, error) {
rows, err := q.db.QueryContext(ctx, getUserStatusCounts, arg.StartTime, arg.EndTime)
rows, err := q.db.QueryContext(ctx, getUserStatusCounts, arg.StartTime, arg.EndTime, arg.Interval)
if err != nil {
return nil, err
}
coderd/database/queries/insights.sql
+6 -20
View File
@@ -789,25 +789,11 @@ WITH
-- dates_of_interest defines all points in time that are relevant to the query.
-- It includes the start_time, all status changes, all deletions, and the end_time.
dates_of_interest AS (
SELECT @start_time::timestamptz AS date
UNION
SELECT DISTINCT changed_at AS date
FROM user_status_changes
WHERE changed_at > @start_time::timestamptz
AND changed_at < @end_time::timestamptz
UNION
SELECT DISTINCT deleted_at AS date
FROM user_deleted
WHERE deleted_at > @start_time::timestamptz
AND deleted_at < @end_time::timestamptz
UNION
SELECT @end_time::timestamptz AS date
SELECT date FROM generate_series(
@start_time::timestamptz,
@end_time::timestamptz,
(CASE WHEN @interval::int <= 0 THEN 3600 * 24 ELSE @interval::int END || ' seconds')::interval
) AS date
),
-- latest_status_before_range defines the status of each user before the start_time.
-- We do not include users who were deleted before the start_time. We use this to ensure that
@@ -883,7 +869,7 @@ ranked_status_change_per_user_per_date AS (
LEFT JOIN relevant_status_changes rsc1 ON rsc1.changed_at <= d.date
)
SELECT
rscpupd.date,
rscpupd.date::timestamptz AS date,
statuses.new_status AS status,
COUNT(rscpupd.user_id) FILTER (
WHERE rscpupd.rn = 1
coderd/insights.go
+6 -6
View File
@@ -8,6 +8,8 @@ import (
"strings"
"time"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/google/uuid"
"golang.org/x/exp/slices"
"golang.org/x/sync/errgroup"
@@ -306,6 +308,7 @@ func (api *API) insightsUserStatusCounts(rw http.ResponseWriter, r *http.Request
p := httpapi.NewQueryParamParser()
vals := r.URL.Query()
tzOffset := p.Int(vals, 0, "tz_offset")
interval := p.Int(vals, int((24 * time.Hour).Seconds()), "interval")
p.ErrorExcessParams(vals)
if len(p.Errors) > 0 {
@@ -317,16 +320,13 @@ func (api *API) insightsUserStatusCounts(rw http.ResponseWriter, r *http.Request
}
loc := time.FixedZone("", tzOffset*3600)
// If the time is 14:01 or 14:31, we still want to include all the
// data between 14:00 and 15:00. Our rollups buckets are 30 minutes
// so this works nicely. It works just as well for 23:59 as well.
nextHourInLoc := time.Now().In(loc).Truncate(time.Hour).Add(time.Hour)
// Always return 60 days of data (2 months).
sixtyDaysAgo := nextHourInLoc.In(loc).Truncate(24*time.Hour).AddDate(0, 0, -60)
nextHourInLoc := dbtime.Now().Truncate(time.Hour).Add(time.Hour).In(loc)
sixtyDaysAgo := dbtime.StartOfDay(nextHourInLoc).AddDate(0, 0, -60)
rows, err := api.Database.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{
StartTime: sixtyDaysAgo,
EndTime: nextHourInLoc,
Interval: int32(interval),
})
if err != nil {
if httpapi.IsUnauthorizedError(err) {