Skip to content

feat(coderd/database): use template_usage_stats in GetTemplateAppInsights query #12669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat(coderd/database): rewrite GetTemplateAppInsights to use `templ…
…ate_usage_stats`
  • Loading branch information
mafredri committed Mar 25, 2024
commit 40263d1ac073521755adbf3fd94f166a3649d24a
4 changes: 2 additions & 2 deletions coderd/database/dbmem/dbmem.go
Original file line number Diff line number Diff line change
Expand Up @@ -3000,8 +3000,8 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
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 != ""},
DisplayName: appKey.DisplayName,
Icon: appKey.Icon,
IsApp: appKey.Slug != "",
UsageSeconds: usage,
})
Expand Down
152 changes: 94 additions & 58 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

132 changes: 84 additions & 48 deletions coderd/database/queries/insights.sql
Original file line number Diff line number Diff line change
Expand Up @@ -193,57 +193,93 @@ GROUP BY template_id;
-- 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 app_stats_by_user_and_agent AS (
SELECT
s.start_time,
60 as 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 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
)
-- 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
WITH
app_insights AS (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you may want to drop a comment here, many joins around

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, in my opinion it's a self-contained sequence of joins and there is a comment inline, is there something more you would like explained in the comment? This is a common pattern when we need to lookup agent (or in our case app) -> template association.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, exactly this :)

prevent going through joins and "fast-forward" reading query code by human

SELECT
tus.user_id,
array_agg(DISTINCT tus.template_id)::uuid[] AS template_ids,
app_usage.key::text AS app_name,
COALESCE(wa.display_name, '') AS display_name,
COALESCE(wa.icon, '') AS icon,
(wa.slug IS NOT NULL)::boolean AS is_app,
LEAST(SUM(app_usage.value::int), 30) AS app_usage_mins
FROM
template_usage_stats AS tus, jsonb_each(app_usage_mins) AS app_usage
LEFT JOIN LATERAL (
-- Fetch the latest app info for each app based on slug and template.
SELECT
app.display_name,
app.icon,
app.slug
FROM
workspace_apps AS app
JOIN
workspace_agents AS agent
ON
agent.id = app.agent_id
JOIN
workspace_resources AS resource
ON
resource.id = agent.resource_id
JOIN
workspace_builds AS build
ON
build.job_id = resource.job_id
JOIN
workspaces AS workspace
ON
workspace.id = build.workspace_id
WHERE
-- Requires lateral join.
app.slug = app_usage.key
AND workspace.owner_id = tus.user_id
AND workspace.template_id = tus.template_id
ORDER BY
app.created_at DESC
LIMIT 1
) AS wa
ON
true
WHERE
tus.start_time >= @start_time::timestamptz
AND tus.end_time <= @end_time::timestamptz
AND CASE WHEN COALESCE(array_length(@template_ids::uuid[], 1), 0) > 0 THEN tus.template_id = ANY(@template_ids::uuid[]) ELSE TRUE END
GROUP BY
tus.start_time, tus.user_id, app_usage.key::text, wa.display_name, wa.icon, wa.slug
),
templates AS (
SELECT
app_name,
display_name,
icon,
is_app,
array_agg(DISTINCT template_id)::uuid[] AS template_ids
FROM
app_insights, unnest(template_ids) AS template_id
GROUP BY
app_name, display_name, icon, is_app
)
-- 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, 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;
t.template_ids,
array_agg(DISTINCT ai.user_id)::uuid[] AS active_user_ids,
''::text AS access_method, -- TODO(mafredri): Remove.
ai.app_name AS slug_or_port,
ai.display_name,
ai.icon,
ai.is_app,
(SUM(ai.app_usage_mins) * 60)::bigint AS usage_seconds
FROM
app_insights AS ai
JOIN
templates AS t
ON
ai.app_name = t.app_name
AND ai.display_name = t.display_name
AND ai.icon = t.icon
AND ai.is_app = t.is_app
GROUP BY
t.template_ids, ai.app_name, ai.display_name, ai.icon, ai.is_app;

-- name: GetTemplateAppInsightsByTemplate :many
WITH app_stats_by_user_and_agent AS (
Expand Down
12 changes: 6 additions & 6 deletions coderd/insights.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,11 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage
if a.SlugOrPort != b.SlugOrPort {
return strings.Compare(a.SlugOrPort, b.SlugOrPort)
}
if a.DisplayName.String != b.DisplayName.String {
return strings.Compare(a.DisplayName.String, b.DisplayName.String)
if a.DisplayName != b.DisplayName {
return strings.Compare(a.DisplayName, b.DisplayName)
}
if a.Icon.String != b.Icon.String {
return strings.Compare(a.Icon.String, b.Icon.String)
if a.Icon != b.Icon {
return strings.Compare(a.Icon, b.Icon)
}
if !a.IsApp && b.IsApp {
return -1
Expand All @@ -496,9 +496,9 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage
apps = append(apps, codersdk.TemplateAppUsage{
TemplateIDs: app.TemplateIDs,
Type: codersdk.TemplateAppsTypeApp,
DisplayName: app.DisplayName.String,
DisplayName: app.DisplayName,
Slug: app.SlugOrPort,
Icon: app.Icon.String,
Icon: app.Icon,
Seconds: app.UsageSeconds,
})
}
Expand Down