From cece437c57beba6db68fabcc52d0073e29f81f50 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 16 Aug 2023 20:04:04 +0000 Subject: [PATCH 01/10] feat(coderd): report template app usage insights Fixes #8658 --- coderd/apidoc/docs.go | 6 +- coderd/apidoc/swagger.json | 4 +- coderd/database/dbauthz/dbauthz.go | 19 +++ coderd/database/dbfake/dbfake.go | 18 +- coderd/database/dbmetrics/dbmetrics.go | 7 + coderd/database/dbmock/dbmock.go | 15 ++ coderd/database/querier.go | 7 +- coderd/database/queries.sql.go | 218 ++++++++++++++++++++----- coderd/database/queries/insights.sql | 161 +++++++++++++----- coderd/database/sqlc.yaml | 1 + coderd/insights.go | 82 +++++++++- codersdk/insights.go | 3 +- docs/api/schemas.md | 1 + site/src/api/typesGenerated.ts | 4 +- 14 files changed, 447 insertions(+), 99 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index ea0979333b451..247000282b388 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -9589,10 +9589,12 @@ const docTemplate = `{ "codersdk.TemplateAppsType": { "type": "string", "enum": [ - "builtin" + "builtin", + "app" ], "x-enum-varnames": [ - "TemplateAppsTypeBuiltin" + "TemplateAppsTypeBuiltin", + "TemplateAppsTypeApp" ] }, "codersdk.TemplateBuildTimeStats": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 375be4c013fff..b21fd94032a90 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -8659,8 +8659,8 @@ }, "codersdk.TemplateAppsType": { "type": "string", - "enum": ["builtin"], - "x-enum-varnames": ["TemplateAppsTypeBuiltin"] + "enum": ["builtin", "app"], + "x-enum-varnames": ["TemplateAppsTypeBuiltin", "TemplateAppsTypeApp"] }, "codersdk.TemplateBuildTimeStats": { "type": "object", diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 38766121c443c..a29523804c363 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1173,6 +1173,25 @@ func (q *querier) GetTailnetClientsForAgent(ctx context.Context, agentID uuid.UU return q.db.GetTailnetClientsForAgent(ctx, agentID) } +func (q *querier) GetTemplateAppInsights(ctx context.Context, arg database.GetTemplateAppInsightsParams) ([]database.GetTemplateAppInsightsRow, error) { + for _, templateID := range arg.TemplateIDs { + template, err := q.db.GetTemplateByID(ctx, templateID) + if err != nil { + return nil, err + } + + if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { + return nil, err + } + } + if len(arg.TemplateIDs) == 0 { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil { + return nil, err + } + } + return q.db.GetTemplateAppInsights(ctx, arg) +} + // Only used by metrics cache. func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 818656496184f..4086f21c8f938 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -1966,6 +1966,15 @@ func (*FakeQuerier) GetTailnetClientsForAgent(context.Context, uuid.UUID) ([]dat return nil, ErrUnimplemented } +func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.GetTemplateAppInsightsParams) ([]database.GetTemplateAppInsightsRow, error) { + err := validateDatabaseType(arg) + if err != nil { + return nil, err + } + + panic("not implemented") +} + func (q *FakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { if err := validateDatabaseType(arg); err != nil { return database.GetTemplateAverageBuildTimeRow{}, err @@ -2201,9 +2210,14 @@ func (q *FakeQuerier) GetTemplateInsights(_ context.Context, arg database.GetTem slices.SortFunc(templateIDs, func(a, b uuid.UUID) int { return slice.Ascending(a.String(), b.String()) }) + activeUserIDs := make([]uuid.UUID, 0, len(appUsageIntervalsByUser)) + for userID := range appUsageIntervalsByUser { + activeUserIDs = append(activeUserIDs, userID) + } + result := database.GetTemplateInsightsRow{ - TemplateIDs: templateIDs, - ActiveUsers: int64(len(appUsageIntervalsByUser)), + TemplateIDs: templateIDs, + ActiveUserIDs: activeUserIDs, } for _, intervals := range appUsageIntervalsByUser { for _, interval := range intervals { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index f2c98f6594c43..adb12cba4e627 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -599,6 +599,13 @@ func (m metricsStore) GetTailnetClientsForAgent(ctx context.Context, agentID uui return m.s.GetTailnetClientsForAgent(ctx, agentID) } +func (m metricsStore) GetTemplateAppInsights(ctx context.Context, arg database.GetTemplateAppInsightsParams) ([]database.GetTemplateAppInsightsRow, error) { + start := time.Now() + r0, r1 := m.s.GetTemplateAppInsights(ctx, arg) + m.queryLatencies.WithLabelValues("GetTemplateAppInsights").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { start := time.Now() buildTime, err := m.s.GetTemplateAverageBuildTime(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 45db2e6bd4f58..a388ea0f1031d 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1196,6 +1196,21 @@ func (mr *MockStoreMockRecorder) GetTailnetClientsForAgent(arg0, arg1 interface{ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTailnetClientsForAgent", reflect.TypeOf((*MockStore)(nil).GetTailnetClientsForAgent), arg0, arg1) } +// GetTemplateAppInsights mocks base method. +func (m *MockStore) GetTemplateAppInsights(arg0 context.Context, arg1 database.GetTemplateAppInsightsParams) ([]database.GetTemplateAppInsightsRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTemplateAppInsights", arg0, arg1) + ret0, _ := ret[0].([]database.GetTemplateAppInsightsRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTemplateAppInsights indicates an expected call of GetTemplateAppInsights. +func (mr *MockStoreMockRecorder) GetTemplateAppInsights(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTemplateAppInsights", reflect.TypeOf((*MockStore)(nil).GetTemplateAppInsights), arg0, arg1) +} + // GetTemplateAverageBuildTime mocks base method. func (m *MockStore) GetTemplateAverageBuildTime(arg0 context.Context, arg1 database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 3a8f97307114d..1a37b48fbd3cf 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -107,6 +107,10 @@ type sqlcQuerier interface { GetServiceBanner(ctx context.Context) (string, error) GetTailnetAgents(ctx context.Context, id uuid.UUID) ([]TailnetAgent, error) GetTailnetClientsForAgent(ctx context.Context, agentID uuid.UUID) ([]TailnetClient, error) + // GetTemplateAppInsights returns the aggregate usage of each app in a given + // timeframe. The result can be filtered on template_ids, meaning only user data + // from workspaces based on those templates will be included. + GetTemplateAppInsights(ctx context.Context, arg GetTemplateAppInsightsParams) ([]GetTemplateAppInsightsRow, error) GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (GetTemplateAverageBuildTimeRow, error) GetTemplateByID(ctx context.Context, id uuid.UUID) (Template, error) GetTemplateByOrganizationAndName(ctx context.Context, arg GetTemplateByOrganizationAndNameParams) (Template, error) @@ -117,7 +121,8 @@ type sqlcQuerier interface { // interval/template, it will be included in the results with 0 active users. GetTemplateDailyInsights(ctx context.Context, arg GetTemplateDailyInsightsParams) ([]GetTemplateDailyInsightsRow, error) // GetTemplateInsights has a granularity of 5 minutes where if a session/app was - // in use, we will add 5 minutes to the total usage for that session (per user). + // in use during a minute, we will add 5 minutes to the total usage for that + // session/app (per user). GetTemplateInsights(ctx context.Context, arg GetTemplateInsightsParams) (GetTemplateInsightsRow, error) // GetTemplateParameterInsights does for each template in a given timeframe, // look for the latest workspace build (for every workspace) that has been diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 1a8c7598f7f59..7ad7a413908c2 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1461,21 +1461,143 @@ func (q *sqlQuerier) UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDPar return i, err } +const getTemplateAppInsights = `-- name: GetTemplateAppInsights :many +WITH ts AS ( + SELECT + d::timestamptz AS from_, + (d::timestamptz + '5 minute'::interval) AS to_, + EXTRACT(epoch FROM '5 minute'::interval) AS seconds + FROM + -- Subtract 1 second from end_time to avoid including the next interval in the results. + generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '5 minute'::interval) d +), app_stats_by_user_and_agent AS ( + SELECT + ts.from_, + ts.to_, + ts.seconds, + w.template_id, + was.user_id, + was.agent_id, + was.access_method, + was.slug_or_port, + wa.display_name, + wa.icon, + (wa.slug IS NOT NULL)::boolean AS is_app + FROM ts + JOIN workspace_app_stats was ON ( + (was.session_started_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + ) + JOIN workspaces w ON ( + w.id = was.workspace_id + AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN w.template_id = ANY($3::uuid[]) ELSE TRUE END + ) + -- We do a left join here because we want to include user IDs that have used + -- e.g. ports when counting active users. + LEFT JOIN workspace_apps wa ON ( + wa.agent_id = was.agent_id + AND wa.slug = was.slug_or_port + ) + WHERE + -- We already handle timeframe in the join, but we use an additional + -- check against a static timeframe to help speed up the query. + (was.session_started_at BETWEEN $1 AND $2) + OR (was.session_ended_at BETWEEN $1 AND $2) + OR (was.session_started_at < $1 AND was.session_ended_at > $2) + GROUP BY ts.from_, ts.to_, ts.seconds, w.template_id, was.user_id, was.agent_id, was.access_method, was.slug_or_port, wa.display_name, wa.icon, wa.slug +) + +SELECT + array_agg(DISTINCT template_id)::uuid[] AS template_ids, + -- Return IDs so we can combine this with GetTemplateInsights. + array_agg(DISTINCT user_id)::uuid[] AS active_user_ids, + access_method, + slug_or_port, + display_name, + icon, + is_app, + SUM(seconds) AS usage_seconds +FROM app_stats_by_user_and_agent +GROUP BY access_method, slug_or_port, display_name, icon, is_app +` + +type GetTemplateAppInsightsParams struct { + 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 GetTemplateAppInsightsRow struct { + TemplateIDs []uuid.UUID `db:"template_ids" json:"template_ids"` + ActiveUserIDs []uuid.UUID `db:"active_user_ids" json:"active_user_ids"` + AccessMethod string `db:"access_method" json:"access_method"` + SlugOrPort string `db:"slug_or_port" json:"slug_or_port"` + DisplayName sql.NullString `db:"display_name" json:"display_name"` + Icon sql.NullString `db:"icon" json:"icon"` + IsApp bool `db:"is_app" json:"is_app"` + UsageSeconds int64 `db:"usage_seconds" json:"usage_seconds"` +} + +// GetTemplateAppInsights returns the aggregate usage of each app in a given +// timeframe. The result can be filtered on template_ids, meaning only user data +// from workspaces based on those templates will be included. +func (q *sqlQuerier) GetTemplateAppInsights(ctx context.Context, arg GetTemplateAppInsightsParams) ([]GetTemplateAppInsightsRow, error) { + rows, err := q.db.QueryContext(ctx, getTemplateAppInsights, arg.StartTime, arg.EndTime, pq.Array(arg.TemplateIDs)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetTemplateAppInsightsRow + for rows.Next() { + var i GetTemplateAppInsightsRow + if err := rows.Scan( + pq.Array(&i.TemplateIDs), + pq.Array(&i.ActiveUserIDs), + &i.AccessMethod, + &i.SlugOrPort, + &i.DisplayName, + &i.Icon, + &i.IsApp, + &i.UsageSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getTemplateDailyInsights = `-- name: GetTemplateDailyInsights :many -WITH d AS ( - -- sqlc workaround, use SELECT generate_series instead of SELECT * FROM generate_series. - -- Subtract 1 second from end_time to avoid including the next interval in the results. - SELECT generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '1 day'::interval) AS d -), ts AS ( +WITH ts AS ( SELECT d::timestamptz AS from_, - CASE WHEN (d + '1 day'::interval)::timestamptz <= $2::timestamptz THEN (d + '1 day'::interval)::timestamptz ELSE $2::timestamptz END AS to_ - FROM d -), usage_by_day AS ( + CASE + WHEN (d::timestamptz + '1 day'::interval) <= $1::timestamptz + THEN (d::timestamptz + '1 day'::interval) + ELSE $1::timestamptz + END AS to_ + FROM + -- Subtract 1 second from end_time to avoid including the next interval in the results. + generate_series($2::timestamptz, ($1::timestamptz) - '1 second'::interval, '1 day'::interval) AS d +), unflattened_usage_by_day AS ( + -- We select data from both workspace agent stats and workspace app stats to + -- get a complete picture of usage. This matches how usage is calculated by + -- the combination of GetTemplateInsights and GetTemplateAppInsights. We use + -- a union all to avoid a costly distinct operation. + -- + -- Note that one query must perform a left join so that all intervals are + -- present at least once. SELECT ts.from_, ts.to_, - was.user_id, - array_agg(was.template_id) AS template_ids + was.template_id, + was.user_id FROM ts LEFT JOIN workspace_agent_stats was ON ( was.created_at >= ts.from_ @@ -1483,33 +1605,39 @@ WITH d AS ( AND was.connection_count > 0 AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN was.template_id = ANY($3::uuid[]) ELSE TRUE END ) - GROUP BY ts.from_, ts.to_, was.user_id -), template_ids AS ( + GROUP BY ts.from_, ts.to_, was.template_id, was.user_id + + UNION ALL + SELECT - template_usage_by_day.from_, - array_agg(template_id) AS ids - FROM ( - SELECT DISTINCT - from_, - unnest(template_ids) AS template_id - FROM usage_by_day - ) AS template_usage_by_day - WHERE template_id IS NOT NULL - GROUP BY template_usage_by_day.from_ + ts.from_, ts.to_, + w.template_id, + was.user_id + FROM ts + JOIN workspace_app_stats was ON ( + (was.session_started_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + ) + JOIN workspaces w ON ( + w.id = was.workspace_id + AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN w.template_id = ANY($3::uuid[]) ELSE TRUE END + ) + GROUP BY ts.from_, ts.to_, w.template_id, was.user_id ) SELECT from_ AS start_time, to_ AS end_time, - COALESCE((SELECT template_ids.ids FROM template_ids WHERE template_ids.from_ = usage_by_day.from_), '{}')::uuid[] AS template_ids, + COALESCE(array_agg(DISTINCT template_id) FILTER (WHERE template_id IS NOT NULL), '{}')::uuid[] AS template_ids, COUNT(DISTINCT user_id) AS active_users -FROM usage_by_day +FROM unflattened_usage_by_day GROUP BY from_, to_ ` type GetTemplateDailyInsightsParams struct { - StartTime time.Time `db:"start_time" json:"start_time"` EndTime time.Time `db:"end_time" json:"end_time"` + StartTime time.Time `db:"start_time" json:"start_time"` TemplateIDs []uuid.UUID `db:"template_ids" json:"template_ids"` } @@ -1525,7 +1653,7 @@ type GetTemplateDailyInsightsRow struct { // that interval will be less than 24 hours. If there is no data for a selected // interval/template, it will be included in the results with 0 active users. func (q *sqlQuerier) GetTemplateDailyInsights(ctx context.Context, arg GetTemplateDailyInsightsParams) ([]GetTemplateDailyInsightsRow, error) { - rows, err := q.db.QueryContext(ctx, getTemplateDailyInsights, arg.StartTime, arg.EndTime, pq.Array(arg.TemplateIDs)) + rows, err := q.db.QueryContext(ctx, getTemplateDailyInsights, arg.EndTime, arg.StartTime, pq.Array(arg.TemplateIDs)) if err != nil { return nil, err } @@ -1553,16 +1681,15 @@ func (q *sqlQuerier) GetTemplateDailyInsights(ctx context.Context, arg GetTempla } const getTemplateInsights = `-- name: GetTemplateInsights :one -WITH d AS ( - -- Subtract 1 second from end_time to avoid including the next interval in the results. - SELECT generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '5 minute'::interval) AS d -), ts AS ( +WITH ts AS ( SELECT d::timestamptz AS from_, - (d + '5 minute'::interval)::timestamptz AS to_, + (d::timestamptz + '5 minute'::interval) AS to_, EXTRACT(epoch FROM '5 minute'::interval) AS seconds - FROM d -), usage_by_user AS ( + FROM + -- Subtract 1 second from end_time to avoid including the next interval in the results. + generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '5 minute'::interval) d +), agent_stats_by_interval_and_user AS ( SELECT ts.from_, ts.to_, @@ -1579,21 +1706,27 @@ WITH d AS ( AND was.connection_count > 0 AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN was.template_id = ANY($3::uuid[]) ELSE TRUE END ) + WHERE + -- We already handle created_at in the join, but we use an additional + -- check against a static timeframe to help speed up the query. + was.created_at >= $1 + AND was.created_at < $2 GROUP BY ts.from_, ts.to_, ts.seconds, was.user_id ), template_ids AS ( SELECT array_agg(DISTINCT template_id) AS ids - FROM usage_by_user, unnest(template_ids) template_id + FROM agent_stats_by_interval_and_user, unnest(template_ids) template_id WHERE template_id IS NOT NULL ) SELECT COALESCE((SELECT ids FROM template_ids), '{}')::uuid[] AS template_ids, - COUNT(DISTINCT user_id) AS active_users, + -- Return IDs so we can combine this with GetTemplateAppInsights. + array_agg(DISTINCT user_id)::uuid[] AS active_user_ids, COALESCE(SUM(usage_vscode_seconds), 0)::bigint AS usage_vscode_seconds, COALESCE(SUM(usage_jetbrains_seconds), 0)::bigint AS usage_jetbrains_seconds, COALESCE(SUM(usage_reconnecting_pty_seconds), 0)::bigint AS usage_reconnecting_pty_seconds, COALESCE(SUM(usage_ssh_seconds), 0)::bigint AS usage_ssh_seconds -FROM usage_by_user +FROM agent_stats_by_interval_and_user ` type GetTemplateInsightsParams struct { @@ -1604,7 +1737,7 @@ type GetTemplateInsightsParams struct { type GetTemplateInsightsRow struct { TemplateIDs []uuid.UUID `db:"template_ids" json:"template_ids"` - ActiveUsers int64 `db:"active_users" json:"active_users"` + ActiveUserIDs []uuid.UUID `db:"active_user_ids" json:"active_user_ids"` UsageVscodeSeconds int64 `db:"usage_vscode_seconds" json:"usage_vscode_seconds"` UsageJetbrainsSeconds int64 `db:"usage_jetbrains_seconds" json:"usage_jetbrains_seconds"` UsageReconnectingPtySeconds int64 `db:"usage_reconnecting_pty_seconds" json:"usage_reconnecting_pty_seconds"` @@ -1612,13 +1745,14 @@ type GetTemplateInsightsRow struct { } // GetTemplateInsights has a granularity of 5 minutes where if a session/app was -// in use, we will add 5 minutes to the total usage for that session (per user). +// in use during a minute, we will add 5 minutes to the total usage for that +// session/app (per user). func (q *sqlQuerier) GetTemplateInsights(ctx context.Context, arg GetTemplateInsightsParams) (GetTemplateInsightsRow, error) { row := q.db.QueryRowContext(ctx, getTemplateInsights, arg.StartTime, arg.EndTime, pq.Array(arg.TemplateIDs)) var i GetTemplateInsightsRow err := row.Scan( pq.Array(&i.TemplateIDs), - &i.ActiveUsers, + pq.Array(&i.ActiveUserIDs), &i.UsageVscodeSeconds, &i.UsageJetbrainsSeconds, &i.UsageReconnectingPtySeconds, @@ -1634,15 +1768,15 @@ WITH latest_workspace_builds AS ( wbmax.template_id, wb.template_version_id FROM ( - SELECT - tv.template_id, wbmax.workspace_id, MAX(wbmax.build_number) as max_build_number + SELECT + tv.template_id, wbmax.workspace_id, MAX(wbmax.build_number) as max_build_number FROM workspace_builds wbmax JOIN template_versions tv ON (tv.id = wbmax.template_version_id) WHERE wbmax.created_at >= $1::timestamptz AND wbmax.created_at < $2::timestamptz AND CASE WHEN COALESCE(array_length($3::uuid[], 1), 0) > 0 THEN tv.template_id = ANY($3::uuid[]) ELSE TRUE END - GROUP BY tv.template_id, wbmax.workspace_id + GROUP BY tv.template_id, wbmax.workspace_id ) wbmax JOIN workspace_builds wb ON ( wb.workspace_id = wbmax.workspace_id diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index ad8c581161e99..78d0657e20e86 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -23,17 +23,17 @@ ORDER BY user_id ASC; -- name: GetTemplateInsights :one -- GetTemplateInsights has a granularity of 5 minutes where if a session/app was --- in use, we will add 5 minutes to the total usage for that session (per user). -WITH d AS ( - -- Subtract 1 second from end_time to avoid including the next interval in the results. - SELECT generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '5 minute'::interval) AS d -), ts AS ( +-- in use during a minute, we will add 5 minutes to the total usage for that +-- session/app (per user). +WITH ts AS ( SELECT d::timestamptz AS from_, - (d + '5 minute'::interval)::timestamptz AS to_, + (d::timestamptz + '5 minute'::interval) AS to_, EXTRACT(epoch FROM '5 minute'::interval) AS seconds - FROM d -), usage_by_user AS ( + FROM + -- Subtract 1 second from end_time to avoid including the next interval in the results. + generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '5 minute'::interval) d +), agent_stats_by_interval_and_user AS ( SELECT ts.from_, ts.to_, @@ -50,41 +50,119 @@ WITH d AS ( 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 ) + WHERE + -- We already handle created_at in the join, but we use an additional + -- check against a static timeframe to help speed up the query. + was.created_at >= @start_time + AND was.created_at < @end_time GROUP BY ts.from_, ts.to_, ts.seconds, was.user_id ), template_ids AS ( SELECT array_agg(DISTINCT template_id) AS ids - FROM usage_by_user, unnest(template_ids) template_id + FROM agent_stats_by_interval_and_user, unnest(template_ids) template_id WHERE template_id IS NOT NULL ) SELECT COALESCE((SELECT ids FROM template_ids), '{}')::uuid[] AS template_ids, - COUNT(DISTINCT user_id) AS active_users, + -- Return IDs so we can combine this with GetTemplateAppInsights. + array_agg(DISTINCT user_id)::uuid[] AS active_user_ids, COALESCE(SUM(usage_vscode_seconds), 0)::bigint AS usage_vscode_seconds, COALESCE(SUM(usage_jetbrains_seconds), 0)::bigint AS usage_jetbrains_seconds, COALESCE(SUM(usage_reconnecting_pty_seconds), 0)::bigint AS usage_reconnecting_pty_seconds, COALESCE(SUM(usage_ssh_seconds), 0)::bigint AS usage_ssh_seconds -FROM usage_by_user; +FROM agent_stats_by_interval_and_user; + +-- name: GetTemplateAppInsights :many +-- GetTemplateAppInsights returns the aggregate usage of each app in a given +-- timeframe. The result can be filtered on template_ids, meaning only user data +-- from workspaces based on those templates will be included. +WITH ts AS ( + SELECT + d::timestamptz AS from_, + (d::timestamptz + '5 minute'::interval) AS to_, + EXTRACT(epoch FROM '5 minute'::interval) AS seconds + FROM + -- Subtract 1 second from end_time to avoid including the next interval in the results. + generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '5 minute'::interval) d +), app_stats_by_user_and_agent AS ( + SELECT + ts.from_, + ts.to_, + ts.seconds, + w.template_id, + was.user_id, + was.agent_id, + was.access_method, + was.slug_or_port, + wa.display_name, + wa.icon, + (wa.slug IS NOT NULL)::boolean AS is_app + FROM ts + JOIN workspace_app_stats was ON ( + (was.session_started_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + ) + 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 + ) + -- We do a left join here because we want to include user IDs that have used + -- e.g. ports when counting active users. + LEFT JOIN workspace_apps wa ON ( + wa.agent_id = was.agent_id + AND wa.slug = was.slug_or_port + ) + WHERE + -- We already handle timeframe in the join, but we use an additional + -- check against a static timeframe to help speed up the query. + (was.session_started_at BETWEEN @start_time AND @end_time) + OR (was.session_ended_at BETWEEN @start_time AND @end_time) + OR (was.session_started_at < @start_time AND was.session_ended_at > @end_time) + GROUP BY ts.from_, ts.to_, ts.seconds, w.template_id, was.user_id, was.agent_id, was.access_method, was.slug_or_port, wa.display_name, wa.icon, wa.slug +) + +SELECT + array_agg(DISTINCT template_id)::uuid[] AS template_ids, + -- Return IDs so we can combine this with GetTemplateInsights. + array_agg(DISTINCT user_id)::uuid[] AS active_user_ids, + access_method, + slug_or_port, + display_name, + icon, + is_app, + SUM(seconds) AS usage_seconds +FROM app_stats_by_user_and_agent +GROUP BY access_method, slug_or_port, display_name, icon, is_app; -- name: GetTemplateDailyInsights :many -- GetTemplateDailyInsights returns all daily intervals between start and end -- time, if end time is a partial day, it will be included in the results and -- that interval will be less than 24 hours. If there is no data for a selected -- interval/template, it will be included in the results with 0 active users. -WITH d AS ( - -- sqlc workaround, use SELECT generate_series instead of SELECT * FROM generate_series. - -- Subtract 1 second from end_time to avoid including the next interval in the results. - SELECT generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '1 day'::interval) AS d -), ts AS ( +WITH ts AS ( SELECT d::timestamptz AS from_, - CASE WHEN (d + '1 day'::interval)::timestamptz <= @end_time::timestamptz THEN (d + '1 day'::interval)::timestamptz ELSE @end_time::timestamptz END AS to_ - FROM d -), usage_by_day AS ( + CASE + WHEN (d::timestamptz + '1 day'::interval) <= @end_time::timestamptz + THEN (d::timestamptz + '1 day'::interval) + ELSE @end_time::timestamptz + END AS to_ + FROM + -- Subtract 1 second from end_time to avoid including the next interval in the results. + generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '1 day'::interval) AS d +), unflattened_usage_by_day AS ( + -- We select data from both workspace agent stats and workspace app stats to + -- get a complete picture of usage. This matches how usage is calculated by + -- the combination of GetTemplateInsights and GetTemplateAppInsights. We use + -- a union all to avoid a costly distinct operation. + -- + -- Note that one query must perform a left join so that all intervals are + -- present at least once. SELECT ts.*, - was.user_id, - array_agg(was.template_id) AS template_ids + was.template_id, + was.user_id FROM ts LEFT JOIN workspace_agent_stats was ON ( was.created_at >= ts.from_ @@ -92,30 +170,35 @@ WITH d AS ( 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 ts.from_, ts.to_, was.user_id -), template_ids AS ( + GROUP BY ts.from_, ts.to_, was.template_id, was.user_id + + UNION ALL + SELECT - template_usage_by_day.from_, - array_agg(template_id) AS ids - FROM ( - SELECT DISTINCT - from_, - unnest(template_ids) AS template_id - FROM usage_by_day - ) AS template_usage_by_day - WHERE template_id IS NOT NULL - GROUP BY template_usage_by_day.from_ + ts.*, + w.template_id, + was.user_id + FROM ts + JOIN workspace_app_stats was ON ( + (was.session_started_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + ) + 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 + ) + GROUP BY ts.from_, ts.to_, w.template_id, was.user_id ) SELECT from_ AS start_time, to_ AS end_time, - COALESCE((SELECT template_ids.ids FROM template_ids WHERE template_ids.from_ = usage_by_day.from_), '{}')::uuid[] AS template_ids, + COALESCE(array_agg(DISTINCT template_id) FILTER (WHERE template_id IS NOT NULL), '{}')::uuid[] AS template_ids, COUNT(DISTINCT user_id) AS active_users -FROM usage_by_day +FROM unflattened_usage_by_day GROUP BY from_, to_; - -- name: GetTemplateParameterInsights :many -- GetTemplateParameterInsights does for each template in a given timeframe, -- look for the latest workspace build (for every workspace) that has been @@ -127,15 +210,15 @@ WITH latest_workspace_builds AS ( wbmax.template_id, wb.template_version_id FROM ( - SELECT - tv.template_id, wbmax.workspace_id, MAX(wbmax.build_number) as max_build_number + SELECT + tv.template_id, wbmax.workspace_id, MAX(wbmax.build_number) as max_build_number FROM workspace_builds wbmax JOIN template_versions tv ON (tv.id = wbmax.template_version_id) WHERE wbmax.created_at >= @start_time::timestamptz AND wbmax.created_at < @end_time::timestamptz AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN tv.template_id = ANY(@template_ids::uuid[]) ELSE TRUE END - GROUP BY tv.template_id, wbmax.workspace_id + GROUP BY tv.template_id, wbmax.workspace_id ) wbmax JOIN workspace_builds wb ON ( wb.workspace_id = wbmax.workspace_id diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 526f62d3a5ca5..ca1a2324e9ab0 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -71,6 +71,7 @@ overrides: eof: EOF locked_ttl: LockedTTL template_ids: TemplateIDs + active_user_ids: ActiveUserIDs sql: - schema: "./dump.sql" diff --git a/coderd/insights.go b/coderd/insights.go index b452b963277ed..9aaf9ff367c9e 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -188,6 +188,7 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) { } var usage database.GetTemplateInsightsRow + var appUsage []database.GetTemplateAppInsightsRow var dailyUsage []database.GetTemplateDailyInsightsRow // Use a transaction to ensure that we get consistent data between @@ -215,6 +216,15 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) { return xerrors.Errorf("get template insights: %w", err) } + appUsage, err = tx.GetTemplateAppInsights(ctx, database.GetTemplateAppInsightsParams{ + StartTime: startTime, + EndTime: endTime, + TemplateIDs: templateIDs, + }) + if err != nil { + return xerrors.Errorf("get template app insights: %w", err) + } + return nil }, nil) if httpapi.Is404Error(err) { @@ -257,9 +267,9 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) { Report: codersdk.TemplateInsightsReport{ StartTime: startTime, EndTime: endTime, - TemplateIDs: usage.TemplateIDs, - ActiveUsers: usage.ActiveUsers, - AppsUsage: convertTemplateInsightsBuiltinApps(usage), + TemplateIDs: convertTemplateInsightsTemplateIDs(usage, appUsage), + ActiveUsers: convertTemplateInsightsActiveUsers(usage, appUsage), + AppsUsage: convertTemplateInsightsApps(usage, appUsage), ParametersUsage: parametersUsage, }, IntervalReports: []codersdk.TemplateInsightsIntervalReport{}, @@ -276,10 +286,45 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, resp) } -// convertTemplateInsightsBuiltinApps builds the list of builtin apps from the -// database row, these are apps that are implicitly a part of all templates. -func convertTemplateInsightsBuiltinApps(usage database.GetTemplateInsightsRow) []codersdk.TemplateAppUsage { - return []codersdk.TemplateAppUsage{ +func convertTemplateInsightsTemplateIDs(usage database.GetTemplateInsightsRow, appUsage []database.GetTemplateAppInsightsRow) []uuid.UUID { + templateIDSet := make(map[uuid.UUID]struct{}) + for _, id := range usage.TemplateIDs { + templateIDSet[id] = struct{}{} + } + for _, app := range appUsage { + for _, id := range app.TemplateIDs { + templateIDSet[id] = struct{}{} + } + } + templateIDs := make([]uuid.UUID, 0, len(templateIDSet)) + for id := range templateIDSet { + templateIDs = append(templateIDs, id) + } + slices.SortFunc(templateIDs, func(a, b uuid.UUID) int { + return slice.Ascending(a.String(), b.String()) + }) + return templateIDs +} + +func convertTemplateInsightsActiveUsers(usage database.GetTemplateInsightsRow, appUsage []database.GetTemplateAppInsightsRow) int64 { + activeUserIDSet := make(map[uuid.UUID]struct{}) + for _, id := range usage.ActiveUserIDs { + activeUserIDSet[id] = struct{}{} + } + for _, app := range appUsage { + for _, id := range app.ActiveUserIDs { + activeUserIDSet[id] = struct{}{} + } + } + return int64(len(activeUserIDSet)) +} + +// convertTemplateInsightsApps builds the list of builtin apps and template apps +// from the provided database rows, builtin apps are implicitly a part of all +// templates. +func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage []database.GetTemplateAppInsightsRow) []codersdk.TemplateAppUsage { + // Builtin apps. + apps := []codersdk.TemplateAppUsage{ { TemplateIDs: usage.TemplateIDs, Type: codersdk.TemplateAppsTypeBuiltin, @@ -296,6 +341,12 @@ func convertTemplateInsightsBuiltinApps(usage database.GetTemplateInsightsRow) [ Icon: "/icon/intellij.svg", Seconds: usage.UsageJetbrainsSeconds, }, + // TODO(mafredri): We could take Web Terminal usage from appUsage since + // that should be more accurate. The difference is that this reflects + // the rpty session as seen by the agent (can live past the connection), + // whereas appUsage reflects the lifetime of the client connection. The + // condition finding the corresponding app entry in appUsage is: + // !app.IsApp && app.AccessMethod == "terminal" && app.SlugOrPort == "" { TemplateIDs: usage.TemplateIDs, Type: codersdk.TemplateAppsTypeBuiltin, @@ -313,6 +364,23 @@ func convertTemplateInsightsBuiltinApps(usage database.GetTemplateInsightsRow) [ Seconds: usage.UsageSshSeconds, }, } + + // Template apps. + for _, app := range appUsage { + if !app.IsApp { + continue + } + apps = append(apps, codersdk.TemplateAppUsage{ + TemplateIDs: app.TemplateIDs, + Type: codersdk.TemplateAppsTypeApp, + DisplayName: app.DisplayName.String, + Slug: app.SlugOrPort, + Icon: app.Icon.String, + Seconds: app.UsageSeconds, + }) + } + + return apps } // parseInsightsStartAndEndTime parses the start and end time query parameters diff --git a/codersdk/insights.go b/codersdk/insights.go index 24e2ba140dd76..f4739f3abf5e6 100644 --- a/codersdk/insights.go +++ b/codersdk/insights.go @@ -118,8 +118,7 @@ type TemplateAppsType string // TemplateAppsType enums. const ( TemplateAppsTypeBuiltin TemplateAppsType = "builtin" - // TODO(mafredri): To be introduced in a future pull request. - // TemplateAppsTypeApp TemplateAppsType = "app" + TemplateAppsTypeApp TemplateAppsType = "app" ) // TemplateAppUsage shows the usage of an app for one or more templates. diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 8d73e64e84aa6..dca1dd3d6c1be 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -4315,6 +4315,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | Value | | --------- | | `builtin` | +| `app` | ## codersdk.TemplateBuildTimeStats diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index ccb37bc923004..73a50ad3db11c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1812,8 +1812,8 @@ export const ServerSentEventTypes: ServerSentEventType[] = [ ] // From codersdk/insights.go -export type TemplateAppsType = "builtin" -export const TemplateAppsTypes: TemplateAppsType[] = ["builtin"] +export type TemplateAppsType = "app" | "builtin" +export const TemplateAppsTypes: TemplateAppsType[] = ["app", "builtin"] // From codersdk/templates.go export type TemplateRole = "" | "admin" | "use" From 0db240ac22b8a9ec2691984a517656ebdbf5cc2a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 16 Aug 2023 20:06:56 +0000 Subject: [PATCH 02/10] use 1 minute granularity instead of 5 --- coderd/database/querier.go | 4 ++-- coderd/database/queries.sql.go | 16 ++++++++-------- coderd/database/queries/insights.sql | 16 ++++++++-------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 1a37b48fbd3cf..185ebf41f539e 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -120,8 +120,8 @@ type sqlcQuerier interface { // that interval will be less than 24 hours. If there is no data for a selected // interval/template, it will be included in the results with 0 active users. GetTemplateDailyInsights(ctx context.Context, arg GetTemplateDailyInsightsParams) ([]GetTemplateDailyInsightsRow, error) - // GetTemplateInsights has a granularity of 5 minutes where if a session/app was - // in use during a minute, we will add 5 minutes to the total usage for that + // GetTemplateInsights has a granularity of 1 minute where if a session/app was + // in use during a minute, we will add 1 minute to the total usage for that // session/app (per user). GetTemplateInsights(ctx context.Context, arg GetTemplateInsightsParams) (GetTemplateInsightsRow, error) // GetTemplateParameterInsights does for each template in a given timeframe, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 7ad7a413908c2..819544490517b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1465,11 +1465,11 @@ const getTemplateAppInsights = `-- name: GetTemplateAppInsights :many WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '5 minute'::interval) AS to_, - EXTRACT(epoch FROM '5 minute'::interval) AS seconds + (d::timestamptz + '1 minute'::interval) AS to_, + EXTRACT(epoch FROM '1 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '5 minute'::interval) d + generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '1 minute'::interval) d ), app_stats_by_user_and_agent AS ( SELECT ts.from_, @@ -1684,11 +1684,11 @@ const getTemplateInsights = `-- name: GetTemplateInsights :one WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '5 minute'::interval) AS to_, - EXTRACT(epoch FROM '5 minute'::interval) AS seconds + (d::timestamptz + '1 minute'::interval) AS to_, + EXTRACT(epoch FROM '1 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '5 minute'::interval) d + generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '1 minute'::interval) d ), agent_stats_by_interval_and_user AS ( SELECT ts.from_, @@ -1744,8 +1744,8 @@ type GetTemplateInsightsRow struct { UsageSshSeconds int64 `db:"usage_ssh_seconds" json:"usage_ssh_seconds"` } -// GetTemplateInsights has a granularity of 5 minutes where if a session/app was -// in use during a minute, we will add 5 minutes to the total usage for that +// GetTemplateInsights has a granularity of 1 minute where if a session/app was +// in use during a minute, we will add 1 minute to the total usage for that // session/app (per user). func (q *sqlQuerier) GetTemplateInsights(ctx context.Context, arg GetTemplateInsightsParams) (GetTemplateInsightsRow, error) { row := q.db.QueryRowContext(ctx, getTemplateInsights, arg.StartTime, arg.EndTime, pq.Array(arg.TemplateIDs)) diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 78d0657e20e86..504bb255374ae 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -22,17 +22,17 @@ GROUP BY workspace_agent_stats.user_id, users.username, users.avatar_url ORDER BY user_id ASC; -- name: GetTemplateInsights :one --- GetTemplateInsights has a granularity of 5 minutes where if a session/app was --- in use during a minute, we will add 5 minutes to the total usage for that +-- GetTemplateInsights has a granularity of 1 minute where if a session/app was +-- in use during a minute, we will add 1 minute to the total usage for that -- session/app (per user). WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '5 minute'::interval) AS to_, - EXTRACT(epoch FROM '5 minute'::interval) AS seconds + (d::timestamptz + '1 minute'::interval) AS to_, + EXTRACT(epoch FROM '1 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '5 minute'::interval) d + generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '1 minute'::interval) d ), agent_stats_by_interval_and_user AS ( SELECT ts.from_, @@ -79,11 +79,11 @@ FROM agent_stats_by_interval_and_user; WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '5 minute'::interval) AS to_, - EXTRACT(epoch FROM '5 minute'::interval) AS seconds + (d::timestamptz + '1 minute'::interval) AS to_, + EXTRACT(epoch FROM '1 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '5 minute'::interval) d + generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '1 minute'::interval) d ), app_stats_by_user_and_agent AS ( SELECT ts.from_, From 91dce4f9f23436c3a5013202e366b5ee12f966d6 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 17 Aug 2023 10:14:04 +0000 Subject: [PATCH 03/10] fix coalesce and cleanup with array remove --- coderd/database/queries.sql.go | 4 ++-- coderd/database/queries/insights.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 819544490517b..61f4fc73d5470 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1629,7 +1629,7 @@ WITH ts AS ( SELECT from_ AS start_time, to_ AS end_time, - COALESCE(array_agg(DISTINCT template_id) FILTER (WHERE template_id IS NOT NULL), '{}')::uuid[] AS template_ids, + array_remove(array_agg(DISTINCT template_id), NULL)::uuid[] AS template_ids, COUNT(DISTINCT user_id) AS active_users FROM unflattened_usage_by_day GROUP BY from_, to_ @@ -1721,7 +1721,7 @@ WITH ts AS ( SELECT COALESCE((SELECT ids FROM template_ids), '{}')::uuid[] AS template_ids, -- Return IDs so we can combine this with GetTemplateAppInsights. - array_agg(DISTINCT user_id)::uuid[] AS active_user_ids, + COALESCE(array_agg(DISTINCT user_id), '{}')::uuid[] AS active_user_ids, COALESCE(SUM(usage_vscode_seconds), 0)::bigint AS usage_vscode_seconds, COALESCE(SUM(usage_jetbrains_seconds), 0)::bigint AS usage_jetbrains_seconds, COALESCE(SUM(usage_reconnecting_pty_seconds), 0)::bigint AS usage_reconnecting_pty_seconds, diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 504bb255374ae..a99ea1ced761c 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -65,7 +65,7 @@ WITH ts AS ( SELECT COALESCE((SELECT ids FROM template_ids), '{}')::uuid[] AS template_ids, -- Return IDs so we can combine this with GetTemplateAppInsights. - array_agg(DISTINCT user_id)::uuid[] AS active_user_ids, + COALESCE(array_agg(DISTINCT user_id), '{}')::uuid[] AS active_user_ids, COALESCE(SUM(usage_vscode_seconds), 0)::bigint AS usage_vscode_seconds, COALESCE(SUM(usage_jetbrains_seconds), 0)::bigint AS usage_jetbrains_seconds, COALESCE(SUM(usage_reconnecting_pty_seconds), 0)::bigint AS usage_reconnecting_pty_seconds, @@ -194,7 +194,7 @@ WITH ts AS ( SELECT from_ AS start_time, to_ AS end_time, - COALESCE(array_agg(DISTINCT template_id) FILTER (WHERE template_id IS NOT NULL), '{}')::uuid[] AS template_ids, + array_remove(array_agg(DISTINCT template_id), NULL)::uuid[] AS template_ids, COUNT(DISTINCT user_id) AS active_users FROM unflattened_usage_by_day GROUP BY from_, to_; From 98a835c37e8309ac83e8ea26f4137150e825921a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 17 Aug 2023 10:14:19 +0000 Subject: [PATCH 04/10] Revert "use 1 minute granularity instead of 5" This reverts commit 77e8382e6757512914a3e71d9d8274e511ebf501. --- coderd/database/querier.go | 4 ++-- coderd/database/queries.sql.go | 16 ++++++++-------- coderd/database/queries/insights.sql | 16 ++++++++-------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 185ebf41f539e..1a37b48fbd3cf 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -120,8 +120,8 @@ type sqlcQuerier interface { // that interval will be less than 24 hours. If there is no data for a selected // interval/template, it will be included in the results with 0 active users. GetTemplateDailyInsights(ctx context.Context, arg GetTemplateDailyInsightsParams) ([]GetTemplateDailyInsightsRow, error) - // GetTemplateInsights has a granularity of 1 minute where if a session/app was - // in use during a minute, we will add 1 minute to the total usage for that + // GetTemplateInsights has a granularity of 5 minutes where if a session/app was + // in use during a minute, we will add 5 minutes to the total usage for that // session/app (per user). GetTemplateInsights(ctx context.Context, arg GetTemplateInsightsParams) (GetTemplateInsightsRow, error) // GetTemplateParameterInsights does for each template in a given timeframe, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 61f4fc73d5470..4035ece097638 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1465,11 +1465,11 @@ const getTemplateAppInsights = `-- name: GetTemplateAppInsights :many WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '1 minute'::interval) AS to_, - EXTRACT(epoch FROM '1 minute'::interval) AS seconds + (d::timestamptz + '5 minute'::interval) AS to_, + EXTRACT(epoch FROM '5 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '1 minute'::interval) d + generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '5 minute'::interval) d ), app_stats_by_user_and_agent AS ( SELECT ts.from_, @@ -1684,11 +1684,11 @@ const getTemplateInsights = `-- name: GetTemplateInsights :one WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '1 minute'::interval) AS to_, - EXTRACT(epoch FROM '1 minute'::interval) AS seconds + (d::timestamptz + '5 minute'::interval) AS to_, + EXTRACT(epoch FROM '5 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '1 minute'::interval) d + generate_series($1::timestamptz, ($2::timestamptz) - '1 second'::interval, '5 minute'::interval) d ), agent_stats_by_interval_and_user AS ( SELECT ts.from_, @@ -1744,8 +1744,8 @@ type GetTemplateInsightsRow struct { UsageSshSeconds int64 `db:"usage_ssh_seconds" json:"usage_ssh_seconds"` } -// GetTemplateInsights has a granularity of 1 minute where if a session/app was -// in use during a minute, we will add 1 minute to the total usage for that +// GetTemplateInsights has a granularity of 5 minutes where if a session/app was +// in use during a minute, we will add 5 minutes to the total usage for that // session/app (per user). func (q *sqlQuerier) GetTemplateInsights(ctx context.Context, arg GetTemplateInsightsParams) (GetTemplateInsightsRow, error) { row := q.db.QueryRowContext(ctx, getTemplateInsights, arg.StartTime, arg.EndTime, pq.Array(arg.TemplateIDs)) diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index a99ea1ced761c..63d0acebb3824 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -22,17 +22,17 @@ GROUP BY workspace_agent_stats.user_id, users.username, users.avatar_url ORDER BY user_id ASC; -- name: GetTemplateInsights :one --- GetTemplateInsights has a granularity of 1 minute where if a session/app was --- in use during a minute, we will add 1 minute to the total usage for that +-- GetTemplateInsights has a granularity of 5 minutes where if a session/app was +-- in use during a minute, we will add 5 minutes to the total usage for that -- session/app (per user). WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '1 minute'::interval) AS to_, - EXTRACT(epoch FROM '1 minute'::interval) AS seconds + (d::timestamptz + '5 minute'::interval) AS to_, + EXTRACT(epoch FROM '5 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '1 minute'::interval) d + generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '5 minute'::interval) d ), agent_stats_by_interval_and_user AS ( SELECT ts.from_, @@ -79,11 +79,11 @@ FROM agent_stats_by_interval_and_user; WITH ts AS ( SELECT d::timestamptz AS from_, - (d::timestamptz + '1 minute'::interval) AS to_, - EXTRACT(epoch FROM '1 minute'::interval) AS seconds + (d::timestamptz + '5 minute'::interval) AS to_, + EXTRACT(epoch FROM '5 minute'::interval) AS seconds FROM -- Subtract 1 second from end_time to avoid including the next interval in the results. - generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '1 minute'::interval) d + generate_series(@start_time::timestamptz, (@end_time::timestamptz) - '1 second'::interval, '5 minute'::interval) d ), app_stats_by_user_and_agent AS ( SELECT ts.from_, From cacebb9078d359e8f59aa10e4bd7f5092274a8fc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 18 Aug 2023 14:38:00 +0000 Subject: [PATCH 05/10] fix query joins, add basic test for app usage --- coderd/database/queries.sql.go | 18 ++-- coderd/database/queries/insights.sql | 18 ++-- coderd/insights_test.go | 118 ++++++++++++++++++++++++++- 3 files changed, 132 insertions(+), 22 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4035ece097638..e6f152bb84ad2 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1485,9 +1485,9 @@ WITH ts AS ( (wa.slug IS NOT NULL)::boolean AS is_app FROM ts JOIN workspace_app_stats was ON ( - (was.session_started_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_) + OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_) ) JOIN workspaces w ON ( w.id = was.workspace_id @@ -1502,9 +1502,9 @@ WITH ts AS ( WHERE -- We already handle timeframe in the join, but we use an additional -- check against a static timeframe to help speed up the query. - (was.session_started_at BETWEEN $1 AND $2) - OR (was.session_ended_at BETWEEN $1 AND $2) - OR (was.session_started_at < $1 AND was.session_ended_at > $2) + (was.session_started_at >= $1 AND was.session_started_at < $2) + OR (was.session_ended_at > $1 AND was.session_ended_at < $2) + OR (was.session_started_at < $1 AND was.session_ended_at >= $2) GROUP BY ts.from_, ts.to_, ts.seconds, w.template_id, was.user_id, was.agent_id, was.access_method, was.slug_or_port, wa.display_name, wa.icon, wa.slug ) @@ -1615,9 +1615,9 @@ WITH ts AS ( was.user_id FROM ts JOIN workspace_app_stats was ON ( - (was.session_started_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_) + OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_) ) JOIN workspaces w ON ( w.id = was.workspace_id diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 63d0acebb3824..8f1f3e3bb0b60 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -99,9 +99,9 @@ WITH ts AS ( (wa.slug IS NOT NULL)::boolean AS is_app FROM ts JOIN workspace_app_stats was ON ( - (was.session_started_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_) + OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_) ) JOIN workspaces w ON ( w.id = was.workspace_id @@ -116,9 +116,9 @@ WITH ts AS ( WHERE -- We already handle timeframe in the join, but we use an additional -- check against a static timeframe to help speed up the query. - (was.session_started_at BETWEEN @start_time AND @end_time) - OR (was.session_ended_at BETWEEN @start_time AND @end_time) - OR (was.session_started_at < @start_time AND was.session_ended_at > @end_time) + (was.session_started_at >= @start_time AND was.session_started_at < @end_time) + OR (was.session_ended_at > @start_time AND was.session_ended_at < @end_time) + OR (was.session_started_at < @start_time AND was.session_ended_at >= @end_time) GROUP BY ts.from_, ts.to_, ts.seconds, w.template_id, was.user_id, was.agent_id, was.access_method, was.slug_or_port, wa.display_name, wa.icon, wa.slug ) @@ -180,9 +180,9 @@ WITH ts AS ( was.user_id FROM ts JOIN workspace_app_stats was ON ( - (was.session_started_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_ended_at BETWEEN ts.from_ AND ts.to_) - OR (was.session_started_at < ts.from_ AND was.session_ended_at > ts.to_) + (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_) + OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_) + OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_) ) JOIN workspaces w ON ( w.id = was.workspace_id diff --git a/coderd/insights_test.go b/coderd/insights_test.go index 91bca4b364a08..145f2bc2edd8a 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -17,7 +17,9 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/workspaceapps" "github.com/coder/coder/codersdk" "github.com/coder/coder/codersdk/agentsdk" "github.com/coder/coder/provisioner/echo" @@ -257,6 +259,11 @@ func TestTemplateInsights(t *testing.T) { thirdParameterOptionValue2 = "bbb" thirdParameterOptionName3 = "This is CCC" thirdParameterOptionValue3 = "ccc" + + testAppSlug = "test-app" + testAppName = "Test App" + testAppIcon = "/icon.png" + testAppURL = "http://127.1.0.1:65536" // Not used. ) logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) @@ -264,7 +271,7 @@ func TestTemplateInsights(t *testing.T) { IncludeProvisionerDaemon: true, AgentStatsRefreshInterval: time.Millisecond * 100, } - client := coderdtest.New(t, opts) + client, _, coderdAPI := coderdtest.NewWithAPI(t, opts) user := coderdtest.CreateFirstUser(t, client) authToken := uuid.NewString() @@ -287,7 +294,32 @@ func TestTemplateInsights(t *testing.T) { }, }, }, - ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Resources: []*proto.Resource{{ + Name: "example", + Type: "aws_instance", + Agents: []*proto.Agent{{ + Id: uuid.NewString(), + Name: "dev", + Auth: &proto.Agent_Token{ + Token: authToken, + }, + Apps: []*proto.App{ + { + Slug: testAppSlug, + DisplayName: testAppName, + Icon: testAppIcon, + SharingLevel: proto.AppSharingLevel_OWNER, + Url: testAppURL, + }, + }, + }}, + }}, + }, + }, + }}, }) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) require.Empty(t, template.BuildTimeStats[codersdk.WorkspaceTransitionStart]) @@ -320,10 +352,70 @@ func TestTemplateInsights(t *testing.T) { // the day changes so that we get the relevant stats faster. y, m, d := time.Now().UTC().Date() today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC) + requestStartTime := today + requestEndTime := time.Now().UTC().Truncate(time.Hour).Add(time.Hour) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() + // TODO(mafredri): We should prefer to set up an app and generate + // data by accessing it. + // Insert entries within and outside timeframe. + reporter := workspaceapps.NewStatsDBReporter(coderdAPI.Database, workspaceapps.DefaultStatsDBReporterBatchSize) + err := reporter.Report(dbauthz.AsSystemRestricted(ctx), []workspaceapps.StatsReport{ + { + UserID: user.UserID, + WorkspaceID: workspace.ID, + AgentID: resources[0].Agents[0].ID, + AccessMethod: workspaceapps.AccessMethodPath, + SlugOrPort: testAppSlug, + SessionID: uuid.New(), + // Outside report range. + SessionStartedAt: requestStartTime.Add(-1 * time.Minute), + SessionEndedAt: requestStartTime, + Requests: 1, + }, + { + UserID: user.UserID, + WorkspaceID: workspace.ID, + AgentID: resources[0].Agents[0].ID, + AccessMethod: workspaceapps.AccessMethodPath, + SlugOrPort: testAppSlug, + SessionID: uuid.New(), + // One minute of usage (rounded up to 5 due to query intervals). + // TODO(mafredri): We'll fix this in a future refactor so that it's + // 1 minute increments instead of 5. + SessionStartedAt: requestStartTime, + SessionEndedAt: requestStartTime.Add(1 * time.Minute), + Requests: 1, + }, + { + UserID: user.UserID, + WorkspaceID: workspace.ID, + AgentID: resources[0].Agents[0].ID, + AccessMethod: workspaceapps.AccessMethodPath, + SlugOrPort: testAppSlug, + SessionID: uuid.New(), + // Five additional minutes of usage. + SessionStartedAt: requestStartTime.Add(10 * time.Minute), + SessionEndedAt: requestStartTime.Add(15 * time.Minute), + Requests: 1, + }, + { + UserID: user.UserID, + WorkspaceID: workspace.ID, + AgentID: resources[0].Agents[0].ID, + AccessMethod: workspaceapps.AccessMethodPath, + SlugOrPort: testAppSlug, + SessionID: uuid.New(), + // Outside report range. + SessionStartedAt: requestEndTime, + SessionEndedAt: requestEndTime.Add(1 * time.Minute), + Requests: 1, + }, + }) + require.NoError(t, err, "want no error inserting stats") + // Connect to the agent to generate usage/latency stats. conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, &codersdk.DialWorkspaceAgentOptions{ Logger: logger.Named("client"), @@ -362,8 +454,8 @@ func TestTemplateInsights(t *testing.T) { waitForAppSeconds := func(slug string) func() bool { return func() bool { req = codersdk.TemplateInsightsRequest{ - StartTime: today, - EndTime: time.Now().UTC().Truncate(time.Hour).Add(time.Hour), + StartTime: requestStartTime, + EndTime: requestEndTime, Interval: codersdk.InsightsReportIntervalDay, } resp, err = client.TemplateInsights(ctx, req) @@ -390,13 +482,31 @@ func TestTemplateInsights(t *testing.T) { assert.WithinDuration(t, req.StartTime, resp.Report.StartTime, 0) assert.WithinDuration(t, req.EndTime, resp.Report.EndTime, 0) assert.Equal(t, resp.Report.ActiveUsers, int64(1), "want one active user") + var gotApps []codersdk.TemplateAppUsage + // Check builtin apps usage. for _, app := range resp.Report.AppsUsage { + if app.Type != codersdk.TemplateAppsTypeBuiltin { + gotApps = append(gotApps, app) + continue + } if slices.Contains([]string{"reconnecting-pty", "ssh"}, app.Slug) { assert.Equal(t, app.Seconds, int64(300), "want app %q to have 5 minutes of usage", app.Slug) } else { assert.Equal(t, app.Seconds, int64(0), "want app %q to have 0 minutes of usage", app.Slug) } } + // Check app usage. + assert.Equal(t, gotApps, []codersdk.TemplateAppUsage{ + { + TemplateIDs: []uuid.UUID{template.ID}, + Type: codersdk.TemplateAppsTypeApp, + Slug: testAppSlug, + DisplayName: testAppName, + Icon: testAppIcon, + Seconds: 300 + 300, // Two times 5 minutes of usage (actually 1 + 5, but see TODO above). + }, + }, "want app usage to match") + // The full timeframe is <= 24h, so the interval matches exactly. require.Len(t, resp.IntervalReports, 1, "want one interval report") assert.WithinDuration(t, req.StartTime, resp.IntervalReports[0].StartTime, 0) From 6fd3d937899b1af7f0122c2d6f0bd262c4923de8 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 18 Aug 2023 14:40:03 +0000 Subject: [PATCH 06/10] nolint --- coderd/insights_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/insights_test.go b/coderd/insights_test.go index 145f2bc2edd8a..5fb8b2591dba4 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -362,6 +362,7 @@ func TestTemplateInsights(t *testing.T) { // data by accessing it. // Insert entries within and outside timeframe. reporter := workspaceapps.NewStatsDBReporter(coderdAPI.Database, workspaceapps.DefaultStatsDBReporterBatchSize) + //nolint:gocritic // This is a test. err := reporter.Report(dbauthz.AsSystemRestricted(ctx), []workspaceapps.StatsReport{ { UserID: user.UserID, From 3bc91b50f57da3ce34b9c45135c8ce8773803427 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 18 Aug 2023 16:10:46 +0000 Subject: [PATCH 07/10] test produce another active user via app usage --- coderd/insights_test.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/coderd/insights_test.go b/coderd/insights_test.go index 5fb8b2591dba4..d360d52cc7623 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -274,6 +274,7 @@ func TestTemplateInsights(t *testing.T) { client, _, coderdAPI := coderdtest.NewWithAPI(t, opts) user := coderdtest.CreateFirstUser(t, client) + _, otherUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) authToken := uuid.NewString() version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ Parse: echo.ParseComplete, @@ -390,6 +391,20 @@ func TestTemplateInsights(t *testing.T) { SessionEndedAt: requestStartTime.Add(1 * time.Minute), Requests: 1, }, + { + // Other use is using users workspace, this will result in an + // additional active user and more time spent in app. + UserID: otherUser.ID, + WorkspaceID: workspace.ID, + AgentID: resources[0].Agents[0].ID, + AccessMethod: workspaceapps.AccessMethodPath, + SlugOrPort: testAppSlug, + SessionID: uuid.New(), + // One minute of usage (rounded up to 5 due to query intervals). + SessionStartedAt: requestStartTime, + SessionEndedAt: requestStartTime.Add(1 * time.Minute), + Requests: 1, + }, { UserID: user.UserID, WorkspaceID: workspace.ID, @@ -482,7 +497,7 @@ func TestTemplateInsights(t *testing.T) { assert.WithinDuration(t, req.StartTime, resp.Report.StartTime, 0) assert.WithinDuration(t, req.EndTime, resp.Report.EndTime, 0) - assert.Equal(t, resp.Report.ActiveUsers, int64(1), "want one active user") + assert.Equal(t, int64(2), resp.Report.ActiveUsers, "want two active users") var gotApps []codersdk.TemplateAppUsage // Check builtin apps usage. for _, app := range resp.Report.AppsUsage { @@ -504,7 +519,7 @@ func TestTemplateInsights(t *testing.T) { Slug: testAppSlug, DisplayName: testAppName, Icon: testAppIcon, - Seconds: 300 + 300, // Two times 5 minutes of usage (actually 1 + 5, but see TODO above). + Seconds: 300 + 300 + 300, // Three times 5 minutes of usage (actually 1 + 1 + 5, but see TODO above). }, }, "want app usage to match") @@ -512,7 +527,7 @@ func TestTemplateInsights(t *testing.T) { require.Len(t, resp.IntervalReports, 1, "want one interval report") assert.WithinDuration(t, req.StartTime, resp.IntervalReports[0].StartTime, 0) assert.WithinDuration(t, req.EndTime, resp.IntervalReports[0].EndTime, 0) - assert.Equal(t, resp.IntervalReports[0].ActiveUsers, int64(1), "want one active user in the interval report") + assert.Equal(t, int64(2), resp.IntervalReports[0].ActiveUsers, "want two active users in the interval report") // The workspace uses 3 parameters require.Len(t, resp.Report.ParametersUsage, 3) From ae20a468aa7c2e03fb868be091bf64ef9cd741ec Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 18 Aug 2023 17:29:33 +0000 Subject: [PATCH 08/10] implement fakedb --- coderd/database/dbfake/dbfake.go | 154 ++++++++++++++++++++++++++++++- coderd/insights_test.go | 4 +- 2 files changed, 153 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 4086f21c8f938..a915416a866fd 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -1972,7 +1972,117 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G return nil, err } - panic("not implemented") + q.mutex.RLock() + defer q.mutex.RUnlock() + + type appKey struct { + AccessMethod string + SlugOrPort string + Slug string + DisplayName string + Icon string + } + type uniqueKey struct { + TemplateID uuid.UUID + UserID uuid.UUID + AgentID uuid.UUID + AppKey appKey + } + + appUsageIntervalsByUserAgentApp := make(map[uniqueKey]map[time.Time]int64) + for _, s := range q.workspaceAppStats { + // (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_) + // OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_) + // OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_) + 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 + } + + app, _ := q.getWorkspaceAppByAgentIDAndSlugNoLock(ctx, database.GetWorkspaceAppByAgentIDAndSlugParams{ + AgentID: s.AgentID, + Slug: s.SlugOrPort, + }) + + key := uniqueKey{ + TemplateID: w.TemplateID, + UserID: s.UserID, + AgentID: s.AgentID, + AppKey: appKey{ + AccessMethod: s.AccessMethod, + SlugOrPort: s.SlugOrPort, + Slug: app.Slug, + DisplayName: app.DisplayName, + Icon: app.Icon, + }, + } + if appUsageIntervalsByUserAgentApp[key] == nil { + appUsageIntervalsByUserAgentApp[key] = make(map[time.Time]int64) + } + + t := s.SessionStartedAt.Truncate(5 * time.Minute) + if t.Before(arg.StartTime) { + t = arg.StartTime + } + for t.Before(s.SessionEndedAt) && t.Before(arg.EndTime) { + appUsageIntervalsByUserAgentApp[key][t] = 300 // 5 minutes. + t = t.Add(5 * time.Minute) + } + } + + appUsageTemplateIDs := make(map[appKey]map[uuid.UUID]struct{}) + appUsageUserIDs := make(map[appKey]map[uuid.UUID]struct{}) + appUsage := make(map[appKey]int64) + for uniqueKey, usage := range appUsageIntervalsByUserAgentApp { + for _, seconds := range usage { + if appUsageTemplateIDs[uniqueKey.AppKey] == nil { + appUsageTemplateIDs[uniqueKey.AppKey] = make(map[uuid.UUID]struct{}) + } + appUsageTemplateIDs[uniqueKey.AppKey][uniqueKey.TemplateID] = struct{}{} + if appUsageUserIDs[uniqueKey.AppKey] == nil { + appUsageUserIDs[uniqueKey.AppKey] = make(map[uuid.UUID]struct{}) + } + appUsageUserIDs[uniqueKey.AppKey][uniqueKey.UserID] = struct{}{} + appUsage[uniqueKey.AppKey] += seconds + } + } + + var rows []database.GetTemplateAppInsightsRow + for appKey, usage := range appUsage { + templateIDs := make([]uuid.UUID, 0, len(appUsageTemplateIDs[appKey])) + for templateID := range appUsageTemplateIDs[appKey] { + templateIDs = append(templateIDs, templateID) + } + slices.SortFunc(templateIDs, func(a, b uuid.UUID) int { + return slice.Ascending(a.String(), b.String()) + }) + activeUserIDs := make([]uuid.UUID, 0, len(appUsageUserIDs[appKey])) + for userID := range appUsageUserIDs[appKey] { + activeUserIDs = append(activeUserIDs, userID) + } + slices.SortFunc(activeUserIDs, func(a, b uuid.UUID) int { + return slice.Ascending(a.String(), b.String()) + }) + + rows = append(rows, database.GetTemplateAppInsightsRow{ + TemplateIDs: templateIDs, + ActiveUserIDs: activeUserIDs, + AccessMethod: appKey.AccessMethod, + SlugOrPort: appKey.SlugOrPort, + DisplayName: sql.NullString{String: appKey.DisplayName, Valid: appKey.DisplayName != ""}, + Icon: sql.NullString{String: appKey.Icon, Valid: appKey.Icon != ""}, + IsApp: appKey.Slug != "", + UsageSeconds: usage, + }) + } + + return rows, nil } func (q *FakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { @@ -2102,12 +2212,15 @@ func (q *FakeQuerier) GetTemplateDAUs(_ context.Context, arg database.GetTemplat return rs, nil } -func (q *FakeQuerier) GetTemplateDailyInsights(_ context.Context, arg database.GetTemplateDailyInsightsParams) ([]database.GetTemplateDailyInsightsRow, error) { +func (q *FakeQuerier) GetTemplateDailyInsights(ctx context.Context, arg database.GetTemplateDailyInsightsParams) ([]database.GetTemplateDailyInsightsRow, error) { err := validateDatabaseType(arg) if err != nil { return nil, err } + q.mutex.RLock() + defer q.mutex.RUnlock() + type dailyStat struct { startTime, endTime time.Time userSet map[uuid.UUID]struct{} @@ -2142,6 +2255,37 @@ func (q *FakeQuerier) GetTemplateDailyInsights(_ context.Context, arg database.G } } + for _, s := range q.workspaceAppStats { + // (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_) + // OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_) + // OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_) + 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 + } + + for _, ds := range dailyStats { + // (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_) + // OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_) + // OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_) + 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 + } + + ds.userSet[s.UserID] = struct{}{} + ds.templateIDSet[w.TemplateID] = struct{}{} + break + } + } + var result []database.GetTemplateDailyInsightsRow for _, ds := range dailyStats { templateIDs := make([]uuid.UUID, 0, len(ds.templateIDSet)) @@ -3089,7 +3233,7 @@ func (q *FakeQuerier) GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx context.C return agents, nil } -func (q *FakeQuerier) GetWorkspaceAppByAgentIDAndSlug(_ context.Context, arg database.GetWorkspaceAppByAgentIDAndSlugParams) (database.WorkspaceApp, error) { +func (q *FakeQuerier) GetWorkspaceAppByAgentIDAndSlug(ctx context.Context, arg database.GetWorkspaceAppByAgentIDAndSlugParams) (database.WorkspaceApp, error) { if err := validateDatabaseType(arg); err != nil { return database.WorkspaceApp{}, err } @@ -3097,6 +3241,10 @@ func (q *FakeQuerier) GetWorkspaceAppByAgentIDAndSlug(_ context.Context, arg dat q.mutex.RLock() defer q.mutex.RUnlock() + return q.getWorkspaceAppByAgentIDAndSlugNoLock(ctx, arg) +} + +func (q *FakeQuerier) getWorkspaceAppByAgentIDAndSlugNoLock(_ context.Context, arg database.GetWorkspaceAppByAgentIDAndSlugParams) (database.WorkspaceApp, error) { for _, app := range q.workspaceApps { if app.AgentID != arg.AgentID { continue diff --git a/coderd/insights_test.go b/coderd/insights_test.go index d360d52cc7623..dceb9ae85acab 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -512,7 +512,7 @@ func TestTemplateInsights(t *testing.T) { } } // Check app usage. - assert.Equal(t, gotApps, []codersdk.TemplateAppUsage{ + assert.Equal(t, []codersdk.TemplateAppUsage{ { TemplateIDs: []uuid.UUID{template.ID}, Type: codersdk.TemplateAppsTypeApp, @@ -521,7 +521,7 @@ func TestTemplateInsights(t *testing.T) { Icon: testAppIcon, Seconds: 300 + 300 + 300, // Three times 5 minutes of usage (actually 1 + 1 + 5, but see TODO above). }, - }, "want app usage to match") + }, gotApps, "want app usage to match") // The full timeframe is <= 24h, so the interval matches exactly. require.Len(t, resp.IntervalReports, 1, "want one interval report") From 6b8fe1a00d754684f26da2fffa8184d4aceea641 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 18 Aug 2023 17:34:24 +0000 Subject: [PATCH 09/10] strict --- coderd/insights_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/insights_test.go b/coderd/insights_test.go index dceb9ae85acab..b8343d0610761 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -512,6 +512,7 @@ func TestTemplateInsights(t *testing.T) { } } // Check app usage. + assert.Len(t, gotApps, 1, "want one app") assert.Equal(t, []codersdk.TemplateAppUsage{ { TemplateIDs: []uuid.UUID{template.ID}, From 1904fbe36b756fdc7f62c92c0f3da20dec807c92 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 18 Aug 2023 17:44:33 +0000 Subject: [PATCH 10/10] fix gen --- coderd/database/dbfake/dbfake.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index a915416a866fd..26b6f175e4885 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -549,6 +549,19 @@ func (q *FakeQuerier) getWorkspaceAgentsByResourceIDsNoLock(_ context.Context, r return workspaceAgents, nil } +func (q *FakeQuerier) getWorkspaceAppByAgentIDAndSlugNoLock(_ context.Context, arg database.GetWorkspaceAppByAgentIDAndSlugParams) (database.WorkspaceApp, error) { + for _, app := range q.workspaceApps { + if app.AgentID != arg.AgentID { + continue + } + if app.Slug != arg.Slug { + continue + } + return app, nil + } + return database.WorkspaceApp{}, sql.ErrNoRows +} + func (q *FakeQuerier) getProvisionerJobByIDNoLock(_ context.Context, id uuid.UUID) (database.ProvisionerJob, error) { for _, provisionerJob := range q.provisionerJobs { if provisionerJob.ID != id { @@ -3244,19 +3257,6 @@ func (q *FakeQuerier) GetWorkspaceAppByAgentIDAndSlug(ctx context.Context, arg d return q.getWorkspaceAppByAgentIDAndSlugNoLock(ctx, arg) } -func (q *FakeQuerier) getWorkspaceAppByAgentIDAndSlugNoLock(_ context.Context, arg database.GetWorkspaceAppByAgentIDAndSlugParams) (database.WorkspaceApp, error) { - for _, app := range q.workspaceApps { - if app.AgentID != arg.AgentID { - continue - } - if app.Slug != arg.Slug { - continue - } - return app, nil - } - return database.WorkspaceApp{}, sql.ErrNoRows -} - func (q *FakeQuerier) GetWorkspaceAppsByAgentID(_ context.Context, id uuid.UUID) ([]database.WorkspaceApp, error) { q.mutex.RLock() defer q.mutex.RUnlock()