diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 5ad8dca748c3f..014388e6cd98e 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -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) })) } diff --git a/coderd/database/dbtime/dbtime.go b/coderd/database/dbtime/dbtime.go index 4d740ba941345..bda5a2263ce2b 100644 --- a/coderd/database/dbtime/dbtime.go +++ b/coderd/database/dbtime/dbtime.go @@ -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()) +} diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 7d489cf559220..00b189967f5a6 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -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") - - 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") + 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") + + 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) + } + } }) }) } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 90f9d8d4a165d..6ac9e2b9c69ba 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -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 } diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index c6e3862deed4f..8b4d8540cfb1a 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -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 diff --git a/coderd/insights.go b/coderd/insights.go index e4695f50495fb..9c9fdcfa3c200 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -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) {