From f0155c59b75746785dd1e9d3c013a3fe05ce28b6 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 18 Mar 2024 20:27:21 +0200 Subject: [PATCH 1/4] feat(coderd/database): rewrite `GetUserActivityInsights` to use `template_usage_stats` --- coderd/database/querier.go | 10 +- coderd/database/queries.sql.go | 115 +++++++----------- coderd/database/queries/insights.sql | 111 +++++++---------- ..._workspaces_week_all_templates.json.golden | 2 +- ...orkspaces_week_deployment_wide.json.golden | 2 +- ...r_timezone_(S\303\243o_Paulo).json.golden" | 2 +- ...kly_aggregated_deployment_wide.json.golden | 2 +- ...es_weekly_aggregated_templates.json.golden | 2 +- 8 files changed, 100 insertions(+), 146 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 126b8e6d4de7b..929af475eb18b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -212,11 +212,11 @@ type sqlcQuerier interface { GetTemplatesWithFilter(ctx context.Context, arg GetTemplatesWithFilterParams) ([]Template, error) GetUnexpiredLicenses(ctx context.Context) ([]License, error) // GetUserActivityInsights returns the ranking with top active users. - // The result can be filtered on template_ids, meaning only user data from workspaces - // based on those templates will be included. - // Note: When selecting data from multiple templates or the entire deployment, - // be aware that it may lead to an increase in "usage" numbers (cumulative). In such cases, - // users may be counted multiple times for the same time interval if they have used multiple templates + // The result can be filtered on template_ids, meaning only user data + // from workspaces based on those templates will be included. + // Note: The usage_seconds and usage_seconds_cumulative differ only when + // requesting deployment-wide (or multiple template) data. Cumulative + // produces a bloated value if a user has used multiple templates // simultaneously. GetUserActivityInsights(ctx context.Context, arg GetUserActivityInsightsParams) ([]GetUserActivityInsightsRow, error) GetUserByEmailOrUsername(ctx context.Context, arg GetUserByEmailOrUsernameParams) (User, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8bee2470ddee2..81d0f0a0ef580 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2352,80 +2352,57 @@ func (q *sqlQuerier) GetTemplateUsageStats(ctx context.Context, arg GetTemplateU } const getUserActivityInsights = `-- name: GetUserActivityInsights :many -WITH app_stats AS ( +WITH deployment_stats AS ( SELECT - s.start_time, - was.user_id, - w.template_id, - 60 as seconds - FROM workspace_app_stats was - JOIN workspaces w ON ( - w.id = was.workspace_id - AND CASE WHEN COALESCE(array_length($1::uuid[], 1), 0) > 0 THEN w.template_id = ANY($1::uuid[]) ELSE TRUE END - ) - -- This table contains both 1 minute entries and >1 minute entries, - -- to calculate this with our uniqueness constraints, we generate series - -- for the longer intervals. - CROSS JOIN LATERAL generate_series( - date_trunc('minute', was.session_started_at), - -- Subtract 1 microsecond to avoid creating an extra series. - date_trunc('minute', was.session_ended_at - '1 microsecond'::interval), - '1 minute'::interval - ) s(start_time) - WHERE - s.start_time >= $2::timestamptz - -- Subtract one minute because the series only contains the start time. - AND s.start_time < ($3::timestamptz) - '1 minute'::interval - GROUP BY s.start_time, w.template_id, was.user_id -), session_stats AS ( - SELECT - date_trunc('minute', was.created_at) as start_time, - was.user_id, - was.template_id, - CASE WHEN - SUM(was.session_count_vscode) > 0 OR - SUM(was.session_count_jetbrains) > 0 OR - SUM(was.session_count_reconnecting_pty) > 0 OR - SUM(was.session_count_ssh) > 0 - THEN 60 ELSE 0 END as seconds - FROM workspace_agent_stats was - WHERE - was.created_at >= $2::timestamptz - AND was.created_at < $3::timestamptz - AND was.connection_count > 0 - AND CASE WHEN COALESCE(array_length($1::uuid[], 1), 0) > 0 THEN was.template_id = ANY($1::uuid[]) ELSE TRUE END - GROUP BY date_trunc('minute', was.created_at), was.user_id, was.template_id -), combined_stats AS ( - SELECT - user_id, - template_id, start_time, - seconds - FROM app_stats - UNION + user_id, + array_agg(template_id) AS template_ids, + -- See motivation in GetTemplateInsights for LEAST(SUM(n), 30). + LEAST(SUM(usage_mins), 30) AS usage_mins + FROM + template_usage_stats + WHERE + start_time >= $1::timestamptz + AND end_time <= $2::timestamptz + AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN template_id = ANY($3::uuid[]) ELSE TRUE END + GROUP BY + start_time, user_id +), template_ids AS ( SELECT user_id, - template_id, - start_time, - seconds - FROM session_stats + array_agg(DISTINCT template_id) AS ids + FROM + deployment_stats, unnest(template_ids) template_id + GROUP BY + user_id ) + SELECT - users.id as user_id, - users.username, - users.avatar_url, - array_agg(DISTINCT template_id)::uuid[] AS template_ids, - SUM(seconds) AS usage_seconds -FROM combined_stats -JOIN users ON (users.id = combined_stats.user_id) -GROUP BY users.id, username, avatar_url -ORDER BY user_id ASC + ds.user_id, + u.username, + u.avatar_url, + t.ids::uuid[] AS template_ids, + (SUM(ds.usage_mins) * 60)::bigint AS usage_seconds +FROM + deployment_stats ds +JOIN + users u +ON + u.id = ds.user_id +JOIN + template_ids t +ON + ds.user_id = t.user_id +GROUP BY + ds.user_id, u.username, u.avatar_url, t.ids +ORDER BY + ds.user_id ASC ` type GetUserActivityInsightsParams struct { - TemplateIDs []uuid.UUID `db:"template_ids" json:"template_ids"` StartTime time.Time `db:"start_time" json:"start_time"` EndTime time.Time `db:"end_time" json:"end_time"` + TemplateIDs []uuid.UUID `db:"template_ids" json:"template_ids"` } type GetUserActivityInsightsRow struct { @@ -2437,14 +2414,14 @@ type GetUserActivityInsightsRow struct { } // GetUserActivityInsights returns the ranking with top active users. -// The result can be filtered on template_ids, meaning only user data from workspaces -// based on those templates will be included. -// Note: When selecting data from multiple templates or the entire deployment, -// be aware that it may lead to an increase in "usage" numbers (cumulative). In such cases, -// users may be counted multiple times for the same time interval if they have used multiple templates +// The result can be filtered on template_ids, meaning only user data +// from workspaces based on those templates will be included. +// Note: The usage_seconds and usage_seconds_cumulative differ only when +// requesting deployment-wide (or multiple template) data. Cumulative +// produces a bloated value if a user has used multiple templates // simultaneously. func (q *sqlQuerier) GetUserActivityInsights(ctx context.Context, arg GetUserActivityInsightsParams) ([]GetUserActivityInsightsRow, error) { - rows, err := q.db.QueryContext(ctx, getUserActivityInsights, pq.Array(arg.TemplateIDs), arg.StartTime, arg.EndTime) + rows, err := q.db.QueryContext(ctx, getUserActivityInsights, arg.StartTime, arg.EndTime, pq.Array(arg.TemplateIDs)) if err != nil { return nil, err } diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index ffced53861cd9..ef36f16bcd324 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -27,80 +27,57 @@ ORDER BY -- name: GetUserActivityInsights :many -- GetUserActivityInsights returns the ranking with top active users. --- The result can be filtered on template_ids, meaning only user data from workspaces --- based on those templates will be included. --- Note: When selecting data from multiple templates or the entire deployment, --- be aware that it may lead to an increase in "usage" numbers (cumulative). In such cases, --- users may be counted multiple times for the same time interval if they have used multiple templates +-- The result can be filtered on template_ids, meaning only user data +-- from workspaces based on those templates will be included. +-- Note: The usage_seconds and usage_seconds_cumulative differ only when +-- requesting deployment-wide (or multiple template) data. Cumulative +-- produces a bloated value if a user has used multiple templates -- simultaneously. -WITH app_stats AS ( - SELECT - s.start_time, - was.user_id, - w.template_id, - 60 as seconds - FROM workspace_app_stats was - JOIN workspaces w ON ( - w.id = was.workspace_id - AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN w.template_id = ANY(@template_ids::uuid[]) ELSE TRUE END - ) - -- This table contains both 1 minute entries and >1 minute entries, - -- to calculate this with our uniqueness constraints, we generate series - -- for the longer intervals. - CROSS JOIN LATERAL generate_series( - date_trunc('minute', was.session_started_at), - -- Subtract 1 microsecond to avoid creating an extra series. - date_trunc('minute', was.session_ended_at - '1 microsecond'::interval), - '1 minute'::interval - ) s(start_time) - WHERE - s.start_time >= @start_time::timestamptz - -- Subtract one minute because the series only contains the start time. - AND s.start_time < (@end_time::timestamptz) - '1 minute'::interval - GROUP BY s.start_time, w.template_id, was.user_id -), session_stats AS ( - SELECT - date_trunc('minute', was.created_at) as start_time, - was.user_id, - was.template_id, - CASE WHEN - SUM(was.session_count_vscode) > 0 OR - SUM(was.session_count_jetbrains) > 0 OR - SUM(was.session_count_reconnecting_pty) > 0 OR - SUM(was.session_count_ssh) > 0 - THEN 60 ELSE 0 END as seconds - FROM workspace_agent_stats was - WHERE - was.created_at >= @start_time::timestamptz - AND was.created_at < @end_time::timestamptz - AND was.connection_count > 0 - AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN was.template_id = ANY(@template_ids::uuid[]) ELSE TRUE END - GROUP BY date_trunc('minute', was.created_at), was.user_id, was.template_id -), combined_stats AS ( +WITH deployment_stats AS ( SELECT - user_id, - template_id, start_time, - seconds - FROM app_stats - UNION + user_id, + array_agg(template_id) AS template_ids, + -- See motivation in GetTemplateInsights for LEAST(SUM(n), 30). + LEAST(SUM(usage_mins), 30) AS usage_mins + FROM + template_usage_stats + WHERE + start_time >= @start_time::timestamptz + AND end_time <= @end_time::timestamptz + AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN template_id = ANY(@template_ids::uuid[]) ELSE TRUE END + GROUP BY + start_time, user_id +), template_ids AS ( SELECT user_id, - template_id, - start_time, - seconds - FROM session_stats + array_agg(DISTINCT template_id) AS ids + FROM + deployment_stats, unnest(template_ids) template_id + GROUP BY + user_id ) + SELECT - users.id as user_id, - users.username, - users.avatar_url, - array_agg(DISTINCT template_id)::uuid[] AS template_ids, - SUM(seconds) AS usage_seconds -FROM combined_stats -JOIN users ON (users.id = combined_stats.user_id) -GROUP BY users.id, username, avatar_url -ORDER BY user_id ASC; + ds.user_id, + u.username, + u.avatar_url, + t.ids::uuid[] AS template_ids, + (SUM(ds.usage_mins) * 60)::bigint AS usage_seconds +FROM + deployment_stats ds +JOIN + users u +ON + u.id = ds.user_id +JOIN + template_ids t +ON + ds.user_id = t.user_id +GROUP BY + ds.user_id, u.username, u.avatar_url, t.ids +ORDER BY + ds.user_id ASC; -- name: GetTemplateInsights :one -- GetTemplateInsights returns the aggregate user-produced usage of all diff --git a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_all_templates.json.golden b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_all_templates.json.golden index 90177a0d6dc4a..6111b2198d289 100644 --- a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_all_templates.json.golden +++ b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_all_templates.json.golden @@ -16,7 +16,7 @@ "user_id": "00000000-0000-0000-0000-000000000004", "username": "user1", "avatar_url": "", - "seconds": 30540 + "seconds": 26820 }, { "template_ids": [ diff --git a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_deployment_wide.json.golden b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_deployment_wide.json.golden index 90177a0d6dc4a..6111b2198d289 100644 --- a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_deployment_wide.json.golden +++ b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_deployment_wide.json.golden @@ -16,7 +16,7 @@ "user_id": "00000000-0000-0000-0000-000000000004", "username": "user1", "avatar_url": "", - "seconds": 30540 + "seconds": 26820 }, { "template_ids": [ diff --git "a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_other_timezone_(S\303\243o_Paulo).json.golden" "b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_other_timezone_(S\303\243o_Paulo).json.golden" index 9c4c934feef18..c79482438bb19 100644 --- "a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_other_timezone_(S\303\243o_Paulo).json.golden" +++ "b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_week_other_timezone_(S\303\243o_Paulo).json.golden" @@ -16,7 +16,7 @@ "user_id": "00000000-0000-0000-0000-000000000004", "username": "user1", "avatar_url": "", - "seconds": 23280 + "seconds": 23160 }, { "template_ids": [ diff --git a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_deployment_wide.json.golden b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_deployment_wide.json.golden index 458b327c3c392..502110a0de3e1 100644 --- a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_deployment_wide.json.golden +++ b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_deployment_wide.json.golden @@ -16,7 +16,7 @@ "user_id": "00000000-0000-0000-0000-000000000004", "username": "user1", "avatar_url": "", - "seconds": 29820 + "seconds": 26100 }, { "template_ids": [ diff --git a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_templates.json.golden b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_templates.json.golden index b91414092f581..f543d371a98ed 100644 --- a/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_templates.json.golden +++ b/coderd/testdata/insights/user-activity/multiple_users_and_workspaces_weekly_aggregated_templates.json.golden @@ -16,7 +16,7 @@ "user_id": "00000000-0000-0000-0000-000000000004", "username": "user1", "avatar_url": "", - "seconds": 29820 + "seconds": 26100 }, { "template_ids": [ From 1e3c134627973c6d013b9d70d2ac2642d035d8ab Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 25 Mar 2024 11:52:15 +0000 Subject: [PATCH 2/4] fix dbmem query logic --- coderd/database/dbmem/dbmem.go | 222 ++++++++++++++------------- coderd/database/queries.sql.go | 50 +++--- coderd/database/queries/insights.sql | 50 +++--- 3 files changed, 168 insertions(+), 154 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index a895e9565b16d..366bf32491c67 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -4119,7 +4119,7 @@ func (q *FakeQuerier) GetUnexpiredLicenses(_ context.Context) ([]database.Licens return results, nil } -func (q *FakeQuerier) GetUserActivityInsights(ctx context.Context, arg database.GetUserActivityInsightsParams) ([]database.GetUserActivityInsightsRow, error) { +func (q *FakeQuerier) GetUserActivityInsights(_ context.Context, arg database.GetUserActivityInsightsParams) ([]database.GetUserActivityInsightsRow, error) { err := validateDatabaseType(arg) if err != nil { return nil, err @@ -4128,130 +4128,140 @@ func (q *FakeQuerier) GetUserActivityInsights(ctx context.Context, arg database. q.mutex.RLock() defer q.mutex.RUnlock() - type uniqueKey struct { - TemplateID uuid.UUID - UserID uuid.UUID - } - - combinedStats := make(map[uniqueKey]map[time.Time]int64) - - // Get application stats - for _, s := range q.workspaceAppStats { - if !(((s.SessionStartedAt.After(arg.StartTime) || s.SessionStartedAt.Equal(arg.StartTime)) && s.SessionStartedAt.Before(arg.EndTime)) || - (s.SessionEndedAt.After(arg.StartTime) && s.SessionEndedAt.Before(arg.EndTime)) || - (s.SessionStartedAt.Before(arg.StartTime) && (s.SessionEndedAt.After(arg.EndTime) || s.SessionEndedAt.Equal(arg.EndTime)))) { - continue - } - - w, err := q.getWorkspaceByIDNoLock(ctx, s.WorkspaceID) - if err != nil { - return nil, err - } - - if len(arg.TemplateIDs) > 0 && !slices.Contains(arg.TemplateIDs, w.TemplateID) { - continue - } - - key := uniqueKey{ - TemplateID: w.TemplateID, - UserID: s.UserID, - } - if combinedStats[key] == nil { - combinedStats[key] = make(map[time.Time]int64) - } + /* + WITH + */ + /* + deployment_stats AS ( + SELECT + start_time, + user_id, + array_agg(template_id) AS template_ids, + -- See motivation in GetTemplateInsights for LEAST(SUM(n), 30). + LEAST(SUM(usage_mins), 30) AS usage_mins + FROM + template_usage_stats + WHERE + start_time >= @start_time::timestamptz + AND end_time <= @end_time::timestamptz + AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN template_id = ANY(@template_ids::uuid[]) ELSE TRUE END + GROUP BY + start_time, user_id + ), + */ - t := s.SessionStartedAt.Truncate(time.Minute) - if t.Before(arg.StartTime) { - t = arg.StartTime - } - for t.Before(s.SessionEndedAt) && t.Before(arg.EndTime) { - combinedStats[key][t] = 60 - t = t.Add(1 * time.Minute) - } + type deploymentStatsGroupBy struct { + StartTime time.Time + UserID uuid.UUID } - - // Get session stats - for _, s := range q.workspaceAgentStats { - if s.CreatedAt.Before(arg.StartTime) || s.CreatedAt.Equal(arg.EndTime) || s.CreatedAt.After(arg.EndTime) { - continue - } - if len(arg.TemplateIDs) > 0 && !slices.Contains(arg.TemplateIDs, s.TemplateID) { + type deploymentStatsRow struct { + deploymentStatsGroupBy + TemplateIDs []uuid.UUID + UsageMins int16 + } + deploymentStatsRows := make(map[deploymentStatsGroupBy]deploymentStatsRow) + for _, stat := range q.templateUsageStats { + if stat.StartTime.Before(arg.StartTime) || stat.EndTime.After(arg.EndTime) { continue } - if s.ConnectionCount == 0 { + if len(arg.TemplateIDs) > 0 && !slices.Contains(arg.TemplateIDs, stat.TemplateID) { continue } - - key := uniqueKey{ - TemplateID: s.TemplateID, - UserID: s.UserID, - } - - if combinedStats[key] == nil { - combinedStats[key] = make(map[time.Time]int64) + key := deploymentStatsGroupBy{ + StartTime: stat.StartTime, + UserID: stat.UserID, } - - if s.SessionCountJetBrains > 0 || s.SessionCountVSCode > 0 || s.SessionCountReconnectingPTY > 0 || s.SessionCountSSH > 0 { - t := s.CreatedAt.Truncate(time.Minute) - combinedStats[key][t] = 60 + row, ok := deploymentStatsRows[key] + if !ok { + row = deploymentStatsRow{ + deploymentStatsGroupBy: key, + } } + row.TemplateIDs = append(row.TemplateIDs, stat.TemplateID) + row.UsageMins = least(row.UsageMins+stat.UsageMins, 30) + deploymentStatsRows[key] = row } - // Use temporary maps for aggregation purposes - mUserIDTemplateIDs := map[uuid.UUID]map[uuid.UUID]struct{}{} - mUserIDUsageSeconds := map[uuid.UUID]int64{} - - for key, times := range combinedStats { - if mUserIDTemplateIDs[key.UserID] == nil { - mUserIDTemplateIDs[key.UserID] = make(map[uuid.UUID]struct{}) - mUserIDUsageSeconds[key.UserID] = 0 - } - - if _, ok := mUserIDTemplateIDs[key.UserID][key.TemplateID]; !ok { - mUserIDTemplateIDs[key.UserID][key.TemplateID] = struct{}{} - } + /* + template_ids AS ( + SELECT + user_id, + array_agg(DISTINCT template_id) AS ids + FROM + deployment_stats, unnest(template_ids) template_id + GROUP BY + user_id + ) + */ - for _, t := range times { - mUserIDUsageSeconds[key.UserID] += t + type templateIDsRow struct { + UserID uuid.UUID + TemplateIDs []uuid.UUID + } + templateIDs := make(map[uuid.UUID]templateIDsRow) + for _, dsRow := range deploymentStatsRows { + row, ok := templateIDs[dsRow.UserID] + if !ok { + row = templateIDsRow{ + UserID: row.UserID, + } } + row.TemplateIDs = uniqueSortedUUIDs(append(row.TemplateIDs, dsRow.TemplateIDs...)) + templateIDs[dsRow.UserID] = row } - userIDs := make([]uuid.UUID, 0, len(mUserIDUsageSeconds)) - for userID := range mUserIDUsageSeconds { - userIDs = append(userIDs, userID) - } - sort.Slice(userIDs, func(i, j int) bool { - return userIDs[i].String() < userIDs[j].String() - }) + /* + SELECT + ds.user_id, + u.username, + u.avatar_url, + t.ids::uuid[] AS template_ids, + (SUM(ds.usage_mins) * 60)::bigint AS usage_seconds + FROM + deployment_stats ds + JOIN + users u + ON + u.id = ds.user_id + JOIN + template_ids t + ON + ds.user_id = t.user_id + GROUP BY + ds.user_id, u.username, u.avatar_url, t.ids + ORDER BY + ds.user_id ASC; + */ - // Finally, select stats var rows []database.GetUserActivityInsightsRow - - for _, userID := range userIDs { - user, err := q.getUserByIDNoLock(userID) - if err != nil { - return nil, err - } - - tids := mUserIDTemplateIDs[userID] - templateIDs := make([]uuid.UUID, 0, len(tids)) - for key := range tids { - templateIDs = append(templateIDs, key) - } - sort.Slice(templateIDs, func(i, j int) bool { - return templateIDs[i].String() < templateIDs[j].String() - }) - - row := database.GetUserActivityInsightsRow{ - UserID: user.ID, - Username: user.Username, - AvatarURL: user.AvatarURL, - TemplateIDs: templateIDs, - UsageSeconds: mUserIDUsageSeconds[userID], + groupedRows := make(map[uuid.UUID]database.GetUserActivityInsightsRow) + for _, dsRow := range deploymentStatsRows { + row, ok := groupedRows[dsRow.UserID] + if !ok { + user, err := q.getUserByIDNoLock(dsRow.UserID) + if err != nil { + return nil, err + } + row = database.GetUserActivityInsightsRow{ + UserID: user.ID, + Username: user.Username, + AvatarURL: user.AvatarURL, + TemplateIDs: templateIDs[user.ID].TemplateIDs, + } } - + row.UsageSeconds += int64(dsRow.UsageMins) * 60 + groupedRows[dsRow.UserID] = row + } + for _, row := range groupedRows { rows = append(rows, row) } + if len(rows) == 0 { + return nil, sql.ErrNoRows + } + slices.SortFunc(rows, func(a, b database.GetUserActivityInsightsRow) int { + return slice.Ascending(a.UserID.String(), b.UserID.String()) + }) + return rows, nil } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 81d0f0a0ef580..fb76533d1a783 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2352,30 +2352,32 @@ func (q *sqlQuerier) GetTemplateUsageStats(ctx context.Context, arg GetTemplateU } const getUserActivityInsights = `-- name: GetUserActivityInsights :many -WITH deployment_stats AS ( - SELECT - start_time, - user_id, - array_agg(template_id) AS template_ids, - -- See motivation in GetTemplateInsights for LEAST(SUM(n), 30). - LEAST(SUM(usage_mins), 30) AS usage_mins - FROM - template_usage_stats - WHERE - start_time >= $1::timestamptz - AND end_time <= $2::timestamptz - AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN template_id = ANY($3::uuid[]) ELSE TRUE END - GROUP BY - start_time, user_id -), template_ids AS ( - SELECT - user_id, - array_agg(DISTINCT template_id) AS ids - FROM - deployment_stats, unnest(template_ids) template_id - GROUP BY - user_id -) +WITH + deployment_stats AS ( + SELECT + start_time, + user_id, + array_agg(template_id) AS template_ids, + -- See motivation in GetTemplateInsights for LEAST(SUM(n), 30). + LEAST(SUM(usage_mins), 30) AS usage_mins + FROM + template_usage_stats + WHERE + start_time >= $1::timestamptz + AND end_time <= $2::timestamptz + AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN template_id = ANY($3::uuid[]) ELSE TRUE END + GROUP BY + start_time, user_id + ), + template_ids AS ( + SELECT + user_id, + array_agg(DISTINCT template_id) AS ids + FROM + deployment_stats, unnest(template_ids) template_id + GROUP BY + user_id + ) SELECT ds.user_id, diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index ef36f16bcd324..8491cdc7c4217 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -33,30 +33,32 @@ ORDER BY -- requesting deployment-wide (or multiple template) data. Cumulative -- produces a bloated value if a user has used multiple templates -- simultaneously. -WITH deployment_stats AS ( - SELECT - start_time, - user_id, - array_agg(template_id) AS template_ids, - -- See motivation in GetTemplateInsights for LEAST(SUM(n), 30). - LEAST(SUM(usage_mins), 30) AS usage_mins - FROM - template_usage_stats - WHERE - start_time >= @start_time::timestamptz - AND end_time <= @end_time::timestamptz - AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN template_id = ANY(@template_ids::uuid[]) ELSE TRUE END - GROUP BY - start_time, user_id -), template_ids AS ( - SELECT - user_id, - array_agg(DISTINCT template_id) AS ids - FROM - deployment_stats, unnest(template_ids) template_id - GROUP BY - user_id -) +WITH + deployment_stats AS ( + SELECT + start_time, + user_id, + array_agg(template_id) AS template_ids, + -- See motivation in GetTemplateInsights for LEAST(SUM(n), 30). + LEAST(SUM(usage_mins), 30) AS usage_mins + FROM + template_usage_stats + WHERE + start_time >= @start_time::timestamptz + AND end_time <= @end_time::timestamptz + AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN template_id = ANY(@template_ids::uuid[]) ELSE TRUE END + GROUP BY + start_time, user_id + ), + template_ids AS ( + SELECT + user_id, + array_agg(DISTINCT template_id) AS ids + FROM + deployment_stats, unnest(template_ids) template_id + GROUP BY + user_id + ) SELECT ds.user_id, From 4cae73026f3a3f9ea29841c0cfc3f92093a9ca13 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 25 Mar 2024 12:55:27 +0000 Subject: [PATCH 3/4] fix dbauthz --- coderd/database/dbauthz/dbauthz_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 19f970d2afb11..71345ccf09dc6 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -939,7 +939,7 @@ func (s *MethodTestSuite) TestTemplate() { check.Args(database.GetUserLatencyInsightsParams{}).Asserts(rbac.ResourceTemplateInsights, rbac.ActionRead) })) s.Run("GetUserActivityInsights", s.Subtest(func(db database.Store, check *expects) { - check.Args(database.GetUserActivityInsightsParams{}).Asserts(rbac.ResourceTemplateInsights, rbac.ActionRead) + check.Args(database.GetUserActivityInsightsParams{}).Asserts(rbac.ResourceTemplateInsights, rbac.ActionRead).Errors(sql.ErrNoRows) })) s.Run("GetTemplateParameterInsights", s.Subtest(func(db database.Store, check *expects) { check.Args(database.GetTemplateParameterInsightsParams{}).Asserts(rbac.ResourceTemplateInsights, rbac.ActionRead) From cb975fcdc1b4bf05ed6e13b3aefc8550f54dc909 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 25 Mar 2024 13:13:37 +0000 Subject: [PATCH 4/4] fix user-activity response on no rows --- coderd/insights.go | 14 ++++++++++++++ coderd/insights_test.go | 6 +++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/coderd/insights.go b/coderd/insights.go index 7231cb2f5d516..4da7a2a161dc9 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -2,6 +2,7 @@ package coderd import ( "context" + "database/sql" "fmt" "net/http" "strings" @@ -102,6 +103,19 @@ func (api *API) insightsUserActivity(rw http.ResponseWriter, r *http.Request) { TemplateIDs: templateIDs, }) if err != nil { + // No data is not an error. + if xerrors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusOK, codersdk.UserActivityInsightsResponse{ + Report: codersdk.UserActivityInsightsReport{ + StartTime: startTime, + EndTime: endTime, + TemplateIDs: []uuid.UUID{}, + Users: []codersdk.UserActivity{}, + }, + }) + return + } + // Check authorization. if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) return diff --git a/coderd/insights_test.go b/coderd/insights_test.go index 4eca480f279e9..be50083e0c834 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -136,7 +136,7 @@ func TestUserActivityInsights_SanityCheck(t *testing.T) { Pubsub: ps, Logger: &logger, IncludeProvisionerDaemon: true, - AgentStatsRefreshInterval: time.Millisecond * 50, + AgentStatsRefreshInterval: time.Millisecond * 100, DatabaseRolluper: dbrollup.New( logger.Named("dbrollup"), db, @@ -170,7 +170,7 @@ func TestUserActivityInsights_SanityCheck(t *testing.T) { y, m, d := time.Now().UTC().Date() today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC) - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitSuperLong) defer cancel() // Connect to the agent to generate usage/latency stats. @@ -212,7 +212,7 @@ func TestUserActivityInsights_SanityCheck(t *testing.T) { return false } return len(userActivities.Report.Users) > 0 && userActivities.Report.Users[0].Seconds > 0 - }, testutil.WaitMedium, testutil.IntervalFast, "user activity is missing") + }, testutil.WaitSuperLong, testutil.IntervalMedium, "user activity is missing") // We got our latency data, close the connection. _ = sess.Close()