From 2f2169ed19aff278cc047469824f5e7d68b8403e Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 12 Oct 2026 23:58:35 +0300 Subject: [PATCH] fix: deduplicate template insights family metrics (#30497) coderd_insights_applications_usage_seconds summed each family's per-app minutes, so a user with VS Code and Cursor open in the same minute was counted as two VS Code minutes. GetTemplateInsightsByTemplate now takes the registry's app-to-family mapping as jsonb, groups the raw agent stats by family, and counts each minute once per user and family. - Keep the metric's four built-in series (Visual Studio Code, JetBrains, Web Terminal, SSH) and their labels; only their values change. The HELP text now says built-in apps are grouped by family. - Add AppFamilyName.DisplayName() and remove the duplicate TemplateBuiltinAppDisplayName* constants from codersdk. The template insights handler returns the same display names. - Pin how the migration 000596 rollback approximates family minutes from per-app rows. Family values can drop wherever the old per-app sum double-counted overlapping apps. No migration or storage changes. --- .../migrations/migration000596_test.go | 97 +++++++++++++++++++ coderd/database/querier.go | 6 +- coderd/database/querier_test.go | 38 +++++--- coderd/database/queries.sql.go | 47 ++++----- coderd/database/queries/insights.sql | 32 +++--- coderd/database/session_usage_test.go | 6 +- coderd/database/sqlc.yaml | 2 +- coderd/insights.go | 10 +- coderd/insights_internal_test.go | 2 +- .../insights/metricscollector.go | 92 +++++++----------- .../metricscollector_internal_test.go | 25 ----- .../insights/metricscollector_test.go | 13 ++- codersdk/appname.go | 24 ++++- codersdk/insights.go | 9 -- docs/admin/integrations/prometheus.md | 2 +- scripts/metricsdocgen/generated_metrics | 2 +- site/src/api/typesGenerated.ts | 30 ------ 17 files changed, 241 insertions(+), 196 deletions(-) delete mode 100644 coderd/prometheusmetrics/insights/metricscollector_internal_test.go diff --git a/coderd/database/migrations/migration000596_test.go b/coderd/database/migrations/migration000596_test.go index 35cb41639e..48bedec6b1 100644 --- a/coderd/database/migrations/migration000596_test.go +++ b/coderd/database/migrations/migration000596_test.go @@ -356,3 +356,100 @@ func TestMigration000596ChainFrom589(t *testing.T) { require.NoError(t, err) require.Zero(t, leftovers) } + +// The rollback can't tell which minutes a family's apps shared, so it adds up +// their minutes in each 30-minute bucket and caps the total at 30. App names +// missing from the migration's copy of the registry count as SSH. +// +//nolint:tparallel,paralleltest // Subtests share one database with transaction-local fixtures. +func TestMigration000596DownOverlap(t *testing.T) { + t.Parallel() + + sqlDB := testSQLDB(t) + stepTo(t, sqlDB, 596) + + ctx := testutil.Context(t, testutil.WaitSuperLong) + down596, err := os.ReadFile("000596_template_usage_stats_session_usage.down.sql") + require.NoError(t, err) + + type familyMins struct { + ssh, sftp, reconnectingPTY, vscode, jetbrains int64 + } + tests := []struct { + name string + apps map[string]int64 + want familyMins + }{ + { + // vscode: cursor, vscode + name: "same family past the cap", + apps: map[string]int64{"cursor": 20, "vscode": 20}, + want: familyMins{vscode: 30}, + }, + { + // vscode: cursor, vscode + name: "same family under the cap", + apps: map[string]int64{"cursor": 5, "vscode": 5}, + want: familyMins{vscode: 10}, + }, + { + // ssh: ssh, zed, plus the unlisted some_new_ide, port_forwarding, + // overflow, unknown + name: "unlisted names count as ssh under the cap", + apps: map[string]int64{ + "ssh": 5, "zed": 6, "some_new_ide": 5, "port_forwarding": 4, "overflow": 3, "unknown": 2, + }, + want: familyMins{ssh: 25}, + }, + { + // jetbrains: goland, intellij; reconnecting_pty: reconnecting_pty; + // sftp: sftp; vscode: kiro + name: "families cap separately", + apps: map[string]int64{"goland": 25, "intellij": 10, "reconnecting_pty": 12, "sftp": 2, "kiro": 1}, + want: familyMins{sftp: 2, reconnectingPTY: 12, vscode: 1, jetbrains: 30}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tx, err := sqlDB.BeginTx(ctx, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = tx.Rollback() }) + + _, err = tx.ExecContext(ctx, ` + INSERT INTO template_usage_stats ( + start_time, end_time, template_id, user_id, median_latency_ms, + usage_mins, app_usage_mins + ) VALUES ( + date_trunc('hour', statement_timestamp()), + date_trunc('hour', statement_timestamp()) + interval '30 minutes', + '22222222-2222-2222-2222-222222222222'::uuid, + '11111111-1111-1111-1111-111111111111'::uuid, + NULL, 30, NULL + ) + `) + require.NoError(t, err) + for appName, mins := range tt.apps { + _, err = tx.ExecContext(ctx, ` + INSERT INTO template_usage_stats_session_apps ( + start_time, template_id, user_id, app_name, usage_mins + ) + SELECT start_time, template_id, user_id, $1, $2 + FROM template_usage_stats + `, appName, mins) + require.NoError(t, err) + } + + _, err = tx.ExecContext(ctx, string(down596)) + require.NoError(t, err) + + var got familyMins + err = tx.QueryRowContext(ctx, ` + SELECT ssh_mins, sftp_mins, reconnecting_pty_mins, vscode_mins, jetbrains_mins + FROM template_usage_stats + `).Scan(&got.ssh, &got.sftp, &got.reconnectingPTY, &got.vscode, &got.jetbrains) + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 8da87636b9..bb92223e40 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -907,8 +907,10 @@ type sqlcQuerier interface { // GetTemplateInsightsByTemplate is used for Prometheus metrics. Keep // in sync with GetTemplateInsights and UpsertTemplateUsageStats. // - // Session usage comes out per app name, as in GetTemplateInsights, so either - // query reports the same family totals once the names are grouped. + // Session usage is reported per app family. It reads the raw agent stats + // because the rollup stores minutes per app, which can't show whether two + // apps of one family were open in the same minute. @app_families maps each + // app name to its family; names missing from the map count as unknown. GetTemplateInsightsByTemplate(ctx context.Context, arg GetTemplateInsightsByTemplateParams) ([]GetTemplateInsightsByTemplateRow, 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/querier_test.go b/coderd/database/querier_test.go index 0b678387ef..385165abd1 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -857,20 +857,33 @@ func TestGetTemplateInsightsByTemplate(t *testing.T) { insertStat(15*time.Second, sharedConnectionTemplateID, sharedConnectionUserID, uuid.New(), 0, map[string]int64{"vscode": 1}) insertStat(30*time.Second, sharedConnectionTemplateID, sharedConnectionUserID, uuid.New(), 1, map[string]int64{"unknown": 1}) - insights, err := db.GetTemplateInsightsByTemplate(ctx, database.GetTemplateInsightsByTemplateParams{ - StartTime: startTime, - EndTime: endTime, - }) + // Apps of one family open in the same minute count that minute once. + overlapTemplateID := uuid.New() + overlapUserID := uuid.New() + insertStat(5*time.Second, overlapTemplateID, overlapUserID, uuid.New(), 1, map[string]int64{"vscode": 1, "cursor": 1, "zed": 1, "ssh": 1}) + insertStat(10*time.Second, overlapTemplateID, overlapUserID, uuid.New(), 0, map[string]int64{"port_forwarding": 1, "some_new_ide": 1, "overflow": 1}) + insertStat(time.Minute+5*time.Second, overlapTemplateID, overlapUserID, uuid.New(), 0, map[string]int64{"cursor": 1}) + + insights, err := db.GetTemplateInsightsByTemplate(ctx, templateInsightsByTemplateParams(t, startTime, endTime)) require.NoError(t, err) byTemplate := make(map[uuid.UUID]database.GetTemplateInsightsByTemplateRow) for _, row := range insights { byTemplate[row.TemplateID] = row } - require.Len(t, byTemplate, 2) + require.Len(t, byTemplate, 3) require.EqualValues(t, 2, byTemplate[templateID].ActiveUsers) - require.Equal(t, database.StringMapOfInt{"vscode": 120, "jetbrains": 60, "reconnecting_pty": 60, "ssh": 120, "unknown": 60}, byTemplate[templateID].SessionAppUsageSeconds) + require.Equal(t, database.StringMapOfInt{"vscode": 120, "jetbrains": 60, "reconnecting_pty": 60, "ssh": 120, "unknown": 60}, byTemplate[templateID].SessionFamilyUsageSeconds) require.EqualValues(t, 1, byTemplate[sharedConnectionTemplateID].ActiveUsers) - require.Equal(t, database.StringMapOfInt{"vscode": 60, "unknown": 60}, byTemplate[sharedConnectionTemplateID].SessionAppUsageSeconds) + require.Equal(t, database.StringMapOfInt{"vscode": 60, "unknown": 60}, byTemplate[sharedConnectionTemplateID].SessionFamilyUsageSeconds) + require.EqualValues(t, 1, byTemplate[overlapTemplateID].ActiveUsers) + require.Equal(t, database.StringMapOfInt{"vscode": 120, "ssh": 60, "port_forwarding": 60, "unknown": 60}, byTemplate[overlapTemplateID].SessionFamilyUsageSeconds) +} + +func templateInsightsByTemplateParams(t *testing.T, startTime, endTime time.Time) database.GetTemplateInsightsByTemplateParams { + t.Helper() + appFamilies, err := json.Marshal(codersdk.SessionCountAppFamilies()) + require.NoError(t, err) + return database.GetTemplateInsightsByTemplateParams{StartTime: startTime, EndTime: endTime, AppFamilies: appFamilies} } func TestGetWorkspaceAgentUsageStats(t *testing.T) { @@ -19474,24 +19487,21 @@ func TestSessionCountsAttributeByFamily(t *testing.T) { require.Zero(t, sessionFamilyCounts(t, stats.SessionCounts)["jetbrains"]) require.Zero(t, sessionFamilyCounts(t, stats.SessionCounts)["reconnecting_pty"]) - insights, err := db.GetTemplateInsightsByTemplate(ctx, database.GetTemplateInsightsByTemplateParams{ - StartTime: dbtime.Now().Add(-time.Hour), - EndTime: dbtime.Now().Add(time.Hour), - }) + insights, err := db.GetTemplateInsightsByTemplate(ctx, templateInsightsByTemplateParams(t, dbtime.Now().Add(-time.Hour), dbtime.Now().Add(time.Hour))) require.NoError(t, err) byTemplate := make(map[uuid.UUID]database.GetTemplateInsightsByTemplateRow, len(insights)) for _, row := range insights { byTemplate[row.TemplateID] = row } - require.Equal(t, database.StringMapOfInt{"cursor": 60}, byTemplate[cursorTemplate].SessionAppUsageSeconds) - require.Equal(t, database.StringMapOfInt{"zed": 60}, byTemplate[zedTemplate].SessionAppUsageSeconds) + require.Equal(t, database.StringMapOfInt{"vscode": 60}, byTemplate[cursorTemplate].SessionFamilyUsageSeconds) + require.Equal(t, database.StringMapOfInt{"ssh": 60}, byTemplate[zedTemplate].SessionFamilyUsageSeconds) // An app with no family is still activity, so the user is not counted idle. unknown, ok := byTemplate[unknownTemplate] require.True(t, ok, "a session with no family must still appear as usage") require.Equal(t, int64(1), unknown.ActiveUsers) - require.Equal(t, database.StringMapOfInt{"some_new_ide": 60}, unknown.SessionAppUsageSeconds) + require.Equal(t, database.StringMapOfInt{"unknown": 60}, unknown.SessionFamilyUsageSeconds) } // The rollup stores the app name the agent reported, whether or not the diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 75333614fa..4ba5434a31 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -18284,14 +18284,14 @@ WITH BOOL_OR(connection_count > 0) ), insights AS ( - -- A minute counts once per app however many of its sessions were open, - -- which COUNT(DISTINCT) does in the grouping. Deduplicating the + -- A minute counts once per family, however many of its apps were + -- open, which COUNT(DISTINCT) does in the grouping. Deduplicating the -- expanded rows first instead spills to disk once a deployment has a -- few thousand agents. SELECT was.template_id, was.user_id, - app_name, + COALESCE($3::jsonb ->> app_name, 'unknown') AS family, COUNT(DISTINCT date_trunc('minute', was.created_at)) AS usage_mins FROM workspace_agent_stats AS was @@ -18307,22 +18307,22 @@ WITH AND was.created_at < $2::timestamptz AND was.session_counts <> '{}'::jsonb GROUP BY - was.template_id, was.user_id, app_name + was.template_id, was.user_id, family ), - app_usage AS ( + app_family_usage AS ( SELECT template_id, - jsonb_object_agg(app_name, usage_seconds) AS session_app_usage_seconds + jsonb_object_agg(family, usage_seconds) AS session_family_usage_seconds FROM ( SELECT template_id, - app_name, + family, (SUM(usage_mins) * 60)::bigint AS usage_seconds FROM insights GROUP BY - template_id, app_name - ) AS app_seconds + template_id, family + ) AS family_seconds GROUP BY template_id ), @@ -18339,35 +18339,38 @@ WITH SELECT active_users.template_id, active_users.active_users, - app_usage.session_app_usage_seconds + app_family_usage.session_family_usage_seconds FROM active_users JOIN - -- Every counted template has at least one app; both sides come from + -- Every counted template has at least one family; both sides come from -- insights. - app_usage + app_family_usage ON - app_usage.template_id = active_users.template_id + app_family_usage.template_id = active_users.template_id ` type GetTemplateInsightsByTemplateParams 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"` + EndTime time.Time `db:"end_time" json:"end_time"` + AppFamilies json.RawMessage `db:"app_families" json:"app_families"` } type GetTemplateInsightsByTemplateRow struct { - TemplateID uuid.UUID `db:"template_id" json:"template_id"` - ActiveUsers int64 `db:"active_users" json:"active_users"` - SessionAppUsageSeconds StringMapOfInt `db:"session_app_usage_seconds" json:"session_app_usage_seconds"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + ActiveUsers int64 `db:"active_users" json:"active_users"` + SessionFamilyUsageSeconds StringMapOfInt `db:"session_family_usage_seconds" json:"session_family_usage_seconds"` } // GetTemplateInsightsByTemplate is used for Prometheus metrics. Keep // in sync with GetTemplateInsights and UpsertTemplateUsageStats. // -// Session usage comes out per app name, as in GetTemplateInsights, so either -// query reports the same family totals once the names are grouped. +// Session usage is reported per app family. It reads the raw agent stats +// because the rollup stores minutes per app, which can't show whether two +// apps of one family were open in the same minute. @app_families maps each +// app name to its family; names missing from the map count as unknown. func (q *sqlQuerier) GetTemplateInsightsByTemplate(ctx context.Context, arg GetTemplateInsightsByTemplateParams) ([]GetTemplateInsightsByTemplateRow, error) { - rows, err := q.db.QueryContext(ctx, getTemplateInsightsByTemplate, arg.StartTime, arg.EndTime) + rows, err := q.db.QueryContext(ctx, getTemplateInsightsByTemplate, arg.StartTime, arg.EndTime, arg.AppFamilies) if err != nil { return nil, err } @@ -18375,7 +18378,7 @@ func (q *sqlQuerier) GetTemplateInsightsByTemplate(ctx context.Context, arg GetT var items []GetTemplateInsightsByTemplateRow for rows.Next() { var i GetTemplateInsightsByTemplateRow - if err := rows.Scan(&i.TemplateID, &i.ActiveUsers, &i.SessionAppUsageSeconds); err != nil { + if err := rows.Scan(&i.TemplateID, &i.ActiveUsers, &i.SessionFamilyUsageSeconds); err != nil { return nil, err } items = append(items, i) diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 8a7e688683..5f37ce3c8e 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -211,8 +211,10 @@ FROM -- GetTemplateInsightsByTemplate is used for Prometheus metrics. Keep -- in sync with GetTemplateInsights and UpsertTemplateUsageStats. -- --- Session usage comes out per app name, as in GetTemplateInsights, so either --- query reports the same family totals once the names are grouped. +-- Session usage is reported per app family. It reads the raw agent stats +-- because the rollup stores minutes per app, which can't show whether two +-- apps of one family were open in the same minute. @app_families maps each +-- app name to its family; names missing from the map count as unknown. WITH connected AS ( -- NOTE(mafredri): connection_count covers one report interval, while @@ -235,14 +237,14 @@ WITH BOOL_OR(connection_count > 0) ), insights AS ( - -- A minute counts once per app however many of its sessions were open, - -- which COUNT(DISTINCT) does in the grouping. Deduplicating the + -- A minute counts once per family, however many of its apps were + -- open, which COUNT(DISTINCT) does in the grouping. Deduplicating the -- expanded rows first instead spills to disk once a deployment has a -- few thousand agents. SELECT was.template_id, was.user_id, - app_name, + COALESCE(@app_families::jsonb ->> app_name, 'unknown') AS family, COUNT(DISTINCT date_trunc('minute', was.created_at)) AS usage_mins FROM workspace_agent_stats AS was @@ -258,22 +260,22 @@ WITH AND was.created_at < @end_time::timestamptz AND was.session_counts <> '{}'::jsonb GROUP BY - was.template_id, was.user_id, app_name + was.template_id, was.user_id, family ), - app_usage AS ( + app_family_usage AS ( SELECT template_id, - jsonb_object_agg(app_name, usage_seconds) AS session_app_usage_seconds + jsonb_object_agg(family, usage_seconds) AS session_family_usage_seconds FROM ( SELECT template_id, - app_name, + family, (SUM(usage_mins) * 60)::bigint AS usage_seconds FROM insights GROUP BY - template_id, app_name - ) AS app_seconds + template_id, family + ) AS family_seconds GROUP BY template_id ), @@ -290,15 +292,15 @@ WITH SELECT active_users.template_id, active_users.active_users, - app_usage.session_app_usage_seconds + app_family_usage.session_family_usage_seconds FROM active_users JOIN - -- Every counted template has at least one app; both sides come from + -- Every counted template has at least one family; both sides come from -- insights. - app_usage + app_family_usage ON - app_usage.template_id = active_users.template_id; + app_family_usage.template_id = active_users.template_id; -- name: GetTemplateAppInsights :many -- GetTemplateAppInsights returns the aggregate usage of each app in a given diff --git a/coderd/database/session_usage_test.go b/coderd/database/session_usage_test.go index c8696057d8..c7e38139a8 100644 --- a/coderd/database/session_usage_test.go +++ b/coderd/database/session_usage_test.go @@ -140,10 +140,8 @@ func TestSessionUsageRollup(t *testing.T) { require.NoError(t, err) require.Empty(t, sessionUsageMins(ctx, t, sqlDB, start, user, template)) - // Live insights report the raw app names without half-hour caps. - live, err := db.GetTemplateInsightsByTemplate(ctx, database.GetTemplateInsightsByTemplateParams{ - StartTime: start.Add(30 * time.Second), EndTime: start.Add(30 * time.Minute), - }) + // Live insights count distinct minutes per family, with no 30-minute cap. + live, err := db.GetTemplateInsightsByTemplate(ctx, templateInsightsByTemplateParams(t, start.Add(30*time.Second), start.Add(30*time.Minute))) require.NoError(t, err) require.Len(t, live, 0, "no connected report occurs in this partial request window") } diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index ac50ea7b2a..9783f9a594 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -118,7 +118,7 @@ sql: - column: "template_usage_stats.app_usage_mins" go_type: type: "StringMapOfInt" - - column: "app_usage.session_app_usage_seconds" + - column: "app_family_usage.session_family_usage_seconds" go_type: type: "StringMapOfInt" - column: "tasks_with_status.workspace_user_acl" diff --git a/coderd/insights.go b/coderd/insights.go index 6ece26fea7..c7a506b21d 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -601,7 +601,7 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage { TemplateIDs: templateIDs(codersdk.AppFamilyVSCode), Type: codersdk.TemplateAppsTypeBuiltin, - DisplayName: codersdk.TemplateBuiltinAppDisplayNameVSCode, + DisplayName: codersdk.AppFamilyVSCode.DisplayName(), Slug: "vscode", Icon: "/icon/code.svg", Seconds: usageSeconds[codersdk.AppFamilyVSCode], @@ -609,7 +609,7 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage { TemplateIDs: templateIDs(codersdk.AppFamilyJetBrains), Type: codersdk.TemplateAppsTypeBuiltin, - DisplayName: codersdk.TemplateBuiltinAppDisplayNameJetBrains, + DisplayName: codersdk.AppFamilyJetBrains.DisplayName(), Slug: "jetbrains", Icon: "/icon/intellij.svg", Seconds: usageSeconds[codersdk.AppFamilyJetBrains], @@ -623,7 +623,7 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage { TemplateIDs: templateIDs(codersdk.AppFamilyReconnectingPTY), Type: codersdk.TemplateAppsTypeBuiltin, - DisplayName: codersdk.TemplateBuiltinAppDisplayNameWebTerminal, + DisplayName: codersdk.AppFamilyReconnectingPTY.DisplayName(), Slug: "reconnecting-pty", Icon: "/icon/terminal.svg", Seconds: usageSeconds[codersdk.AppFamilyReconnectingPTY], @@ -631,7 +631,7 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage { TemplateIDs: templateIDs(codersdk.AppFamilySSH), Type: codersdk.TemplateAppsTypeBuiltin, - DisplayName: codersdk.TemplateBuiltinAppDisplayNameSSH, + DisplayName: codersdk.AppFamilySSH.DisplayName(), Slug: "ssh", Icon: "/icon/terminal.svg", Seconds: usageSeconds[codersdk.AppFamilySSH], @@ -639,7 +639,7 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage { TemplateIDs: templateIDs(codersdk.AppFamilySFTP), Type: codersdk.TemplateAppsTypeBuiltin, - DisplayName: codersdk.TemplateBuiltinAppDisplayNameSFTP, + DisplayName: codersdk.AppFamilySFTP.DisplayName(), Slug: "sftp", Icon: "/icon/terminal.svg", Seconds: usageSeconds[codersdk.AppFamilySFTP], diff --git a/coderd/insights_internal_test.go b/coderd/insights_internal_test.go index f92c267c6a..81852f9945 100644 --- a/coderd/insights_internal_test.go +++ b/coderd/insights_internal_test.go @@ -399,7 +399,7 @@ func TestConvertTemplateInsightsApps(t *testing.T) { // from the old sftp_mins column still report it. TemplateIDs: []uuid.UUID{sftpTemplateID}, Type: codersdk.TemplateAppsTypeBuiltin, - DisplayName: codersdk.TemplateBuiltinAppDisplayNameSFTP, + DisplayName: codersdk.AppFamilySFTP.DisplayName(), Slug: "sftp", Icon: "/icon/terminal.svg", Seconds: 300, diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index 33d001b0ca..8ce46215c4 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -2,6 +2,7 @@ package insights import ( "context" + "encoding/json" "slices" "sync/atomic" "time" @@ -20,7 +21,7 @@ import ( var ( templatesActiveUsersDesc = prometheus.NewDesc("coderd_insights_templates_active_users", "The number of active users of the template.", []string{"template_name", "organization_name"}, nil) - applicationsUsageSecondsDesc = prometheus.NewDesc("coderd_insights_applications_usage_seconds", "The application usage per template.", []string{"template_name", "application_name", "slug", "organization_name"}, nil) + applicationsUsageSecondsDesc = prometheus.NewDesc("coderd_insights_applications_usage_seconds", "The application usage per template. Built-in apps are grouped by app family, such as Visual Studio Code, JetBrains, Web Terminal, or SSH, with an empty slug. A user-minute counts once per family.", []string{"template_name", "application_name", "slug", "organization_name"}, nil) parametersDesc = prometheus.NewDesc("coderd_insights_parameters", "The parameter usage per template.", []string{"template_name", "parameter_name", "parameter_type", "parameter_value", "organization_name"}, nil) ) @@ -29,12 +30,13 @@ type MetricsCollector struct { logger slog.Logger timeWindow time.Duration tickInterval time.Duration + appFamilies json.RawMessage data atomic.Pointer[insightsData] } type insightsData struct { - templates []templateInsightsRow + templates []database.GetTemplateInsightsByTemplateRow apps []database.GetTemplateAppInsightsByTemplateRow params []parameterRow @@ -42,14 +44,6 @@ type insightsData struct { organizationNames map[uuid.UUID]string // template ID → org name } -// templateInsightsRow is a GetTemplateInsightsByTemplateRow with its session -// usage decoded and grouped into families. -type templateInsightsRow struct { - templateID uuid.UUID - activeUsers int64 - usageSecondsByFamily map[codersdk.AppFamilyName]int64 -} - type parameterRow struct { templateID uuid.UUID name string @@ -71,12 +65,17 @@ func NewMetricsCollector(db database.Store, logger slog.Logger, timeWindow time. if tickInterval == 0 { tickInterval = timeWindow } + appFamilies, err := json.Marshal(codersdk.SessionCountAppFamilies()) + if err != nil { + return nil, xerrors.Errorf("marshal app families: %w", err) + } return &MetricsCollector{ database: db, logger: logger.Named("insights_metrics_collector"), timeWindow: timeWindow, tickInterval: tickInterval, + appFamilies: appFamilies, }, nil } @@ -99,21 +98,21 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { eg, egCtx := errgroup.WithContext(ctx) eg.SetLimit(3) - var templateInsights []templateInsightsRow + var templateInsights []database.GetTemplateInsightsByTemplateRow var appInsights []database.GetTemplateAppInsightsByTemplateRow var paramInsights []parameterRow eg.Go(func() error { - rows, err := mc.database.GetTemplateInsightsByTemplate(egCtx, database.GetTemplateInsightsByTemplateParams{ - StartTime: startTime, - EndTime: endTime, + var err error + templateInsights, err = mc.database.GetTemplateInsightsByTemplate(egCtx, database.GetTemplateInsightsByTemplateParams{ + StartTime: startTime, + EndTime: endTime, + AppFamilies: mc.appFamilies, }) if err != nil { mc.logger.Error(ctx, "unable to fetch template insights from database", slog.Error(err)) - return err } - templateInsights = convertTemplateInsights(rows) - return nil + return err }) eg.Go(func() error { var err error @@ -234,38 +233,21 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { appRow.DisplayName, appRow.SlugOrPort, data.organizationNames[appRow.TemplateID]) } - // Built-in apps + // Built-in apps, one series per app family. for _, templateRow := range data.templates { - orgName := data.organizationNames[templateRow.templateID] - - metricsCh <- prometheus.MustNewConstMetric(applicationsUsageSecondsDesc, prometheus.GaugeValue, - float64(templateRow.usageSecondsByFamily[codersdk.AppFamilyVSCode]), - data.templateNames[templateRow.templateID], - codersdk.TemplateBuiltinAppDisplayNameVSCode, - "", orgName) - - metricsCh <- prometheus.MustNewConstMetric(applicationsUsageSecondsDesc, prometheus.GaugeValue, - float64(templateRow.usageSecondsByFamily[codersdk.AppFamilyJetBrains]), - data.templateNames[templateRow.templateID], - codersdk.TemplateBuiltinAppDisplayNameJetBrains, - "", orgName) - - metricsCh <- prometheus.MustNewConstMetric(applicationsUsageSecondsDesc, prometheus.GaugeValue, - float64(templateRow.usageSecondsByFamily[codersdk.AppFamilyReconnectingPTY]), - data.templateNames[templateRow.templateID], - codersdk.TemplateBuiltinAppDisplayNameWebTerminal, - "", orgName) - - metricsCh <- prometheus.MustNewConstMetric(applicationsUsageSecondsDesc, prometheus.GaugeValue, - float64(templateRow.usageSecondsByFamily[codersdk.AppFamilySSH]), - data.templateNames[templateRow.templateID], - codersdk.TemplateBuiltinAppDisplayNameSSH, - "", orgName) + orgName := data.organizationNames[templateRow.TemplateID] + for _, family := range appFamilySeries { + metricsCh <- prometheus.MustNewConstMetric(applicationsUsageSecondsDesc, prometheus.GaugeValue, + float64(templateRow.SessionFamilyUsageSeconds[string(family)]), + data.templateNames[templateRow.TemplateID], + family.DisplayName(), + "", orgName) + } } // Templates for _, templateRow := range data.templates { - metricsCh <- prometheus.MustNewConstMetric(templatesActiveUsersDesc, prometheus.GaugeValue, float64(templateRow.activeUsers), data.templateNames[templateRow.templateID], data.organizationNames[templateRow.templateID]) + metricsCh <- prometheus.MustNewConstMetric(templatesActiveUsersDesc, prometheus.GaugeValue, float64(templateRow.ActiveUsers), data.templateNames[templateRow.TemplateID], data.organizationNames[templateRow.TemplateID]) } // Parameters @@ -276,10 +258,10 @@ func (mc *MetricsCollector) Collect(metricsCh chan<- prometheus.Metric) { // Helper functions below. -func uniqueTemplateIDs(templateInsights []templateInsightsRow, appInsights []database.GetTemplateAppInsightsByTemplateRow, paramInsights []parameterRow) []uuid.UUID { +func uniqueTemplateIDs(templateInsights []database.GetTemplateInsightsByTemplateRow, appInsights []database.GetTemplateAppInsightsByTemplateRow, paramInsights []parameterRow) []uuid.UUID { tids := map[uuid.UUID]bool{} for _, t := range templateInsights { - tids[t.templateID] = true + tids[t.TemplateID] = true } for _, t := range appInsights { tids[t.TemplateID] = true @@ -305,18 +287,12 @@ func onlyTemplateNames(templates []database.Template) map[uuid.UUID]string { return m } -// convertTemplateInsights groups each row's per-app session usage into -// families. -func convertTemplateInsights(rows []database.GetTemplateInsightsByTemplateRow) []templateInsightsRow { - converted := make([]templateInsightsRow, 0, len(rows)) - for _, row := range rows { - converted = append(converted, templateInsightsRow{ - templateID: row.TemplateID, - activeUsers: row.ActiveUsers, - usageSecondsByFamily: codersdk.SumByFamily(row.SessionAppUsageSeconds), - }) - } - return converted +// appFamilySeries lists the app families reported as built-in app series. +var appFamilySeries = []codersdk.AppFamilyName{ + codersdk.AppFamilyVSCode, + codersdk.AppFamilyJetBrains, + codersdk.AppFamilyReconnectingPTY, + codersdk.AppFamilySSH, } func convertParameterInsights(rows []database.GetTemplateParameterInsightsRow) []parameterRow { diff --git a/coderd/prometheusmetrics/insights/metricscollector_internal_test.go b/coderd/prometheusmetrics/insights/metricscollector_internal_test.go deleted file mode 100644 index 829b583a04..0000000000 --- a/coderd/prometheusmetrics/insights/metricscollector_internal_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package insights - -import ( - "testing" - - "github.com/google/uuid" - "github.com/stretchr/testify/require" - - "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/codersdk" -) - -func TestConvertTemplateInsights(t *testing.T) { - t.Parallel() - - rows := convertTemplateInsights([]database.GetTemplateInsightsByTemplateRow{ - { - TemplateID: uuid.New(), - SessionAppUsageSeconds: database.StringMapOfInt{"vscode": 300, "cursor": 120, "zed": 60}, - }, - }) - require.Len(t, rows, 1) - require.EqualValues(t, 420, rows[0].usageSecondsByFamily[codersdk.AppFamilyVSCode]) - require.EqualValues(t, 60, rows[0].usageSecondsByFamily[codersdk.AppFamilySSH]) -} diff --git a/coderd/prometheusmetrics/insights/metricscollector_test.go b/coderd/prometheusmetrics/insights/metricscollector_test.go index 8e0cb5c6ac..11bedc0d41 100644 --- a/coderd/prometheusmetrics/insights/metricscollector_test.go +++ b/coderd/prometheusmetrics/insights/metricscollector_test.go @@ -112,10 +112,15 @@ func TestCollectInsights(t *testing.T) { ConnectionCount: 1, ConnectionMedianLatencyMs: 15, // Session counts must be positive, but the exact value is ignored. - // Database query approximates it to 60s of usage. - SessionCountSsh: 99, - SessionCountJetbrains: 47, - SessionCountVscode: 34, + // The query counts 60s per family, however many of its apps + // report: cursor counts toward VS Code and zed toward SSH. + SessionCounts: map[string]int64{ + "vscode": 34, + "cursor": 2, + "jetbrains": 47, + "ssh": 99, + "zed": 1, + }, }, }) require.NoError(t, err, "unable to post fake stats") diff --git a/codersdk/appname.go b/codersdk/appname.go index 6c26855b3a..4b742efbcd 100644 --- a/codersdk/appname.go +++ b/codersdk/appname.go @@ -27,11 +27,26 @@ const ( AppFamilySSH AppFamilyName = "ssh" AppFamilyReconnectingPTY AppFamilyName = "reconnecting_pty" AppFamilyPortForwarding AppFamilyName = "port_forwarding" - // AppFamilySFTP only comes from history the sftp_mins column recorded. - AppFamilySFTP AppFamilyName = "sftp" - AppFamilyUnknown AppFamilyName = "unknown" + AppFamilySFTP AppFamilyName = "sftp" + AppFamilyUnknown AppFamilyName = "unknown" ) +// appFamilyDisplayNames names each family. +var appFamilyDisplayNames = map[AppFamilyName]string{ + AppFamilyVSCode: "Visual Studio Code", + AppFamilyJetBrains: "JetBrains", + AppFamilySSH: "SSH", + AppFamilyReconnectingPTY: "Web Terminal", + AppFamilyPortForwarding: "Port Forwarding", + AppFamilySFTP: "SFTP", + AppFamilyUnknown: "Unknown", +} + +// DisplayName returns the family's human-readable name. +func (f AppFamilyName) DisplayName() string { + return appFamilyDisplayNames[f] +} + // AppNameOverflow sums the app names past the per-report cap. const AppNameOverflow = "overflow" @@ -90,7 +105,8 @@ var sessionApps = map[string]sessionApp{ "dataspell": {AppFamilyJetBrains, "DataSpell", "/icon/dataspell.svg"}, "mps": {AppFamilyJetBrains, "MPS", ""}, "android_studio": {AppFamilyJetBrains, "Android Studio", "/icon/android-studio.svg"}, - // No agent reports sftp; the family covers the sftp_mins history. + // Coder's own clients don't report sftp, but other clients can. The + // family also holds usage from the old sftp_mins column. "sftp": {AppFamilySFTP, "SFTP", "/icon/terminal.svg"}, // Zed speaks SSH, so it counts toward the SSH total. "zed": {AppFamilySSH, "Zed", "/icon/zed.svg"}, diff --git a/codersdk/insights.go b/codersdk/insights.go index 730a72ccf9..2caeeca022 100644 --- a/codersdk/insights.go +++ b/codersdk/insights.go @@ -200,15 +200,6 @@ const ( TemplateAppsTypeApp TemplateAppsType = "app" ) -// Enums define the display name of the builtin app reported. -const ( - TemplateBuiltinAppDisplayNameVSCode string = "Visual Studio Code" - TemplateBuiltinAppDisplayNameJetBrains string = "JetBrains" - TemplateBuiltinAppDisplayNameWebTerminal string = "Web Terminal" - TemplateBuiltinAppDisplayNameSSH string = "SSH" - TemplateBuiltinAppDisplayNameSFTP string = "SFTP" -) - // TemplateAppUsage shows the usage of an app for one or more templates. type TemplateAppUsage struct { TemplateIDs []uuid.UUID `json:"template_ids" format:"uuid"` diff --git a/docs/admin/integrations/prometheus.md b/docs/admin/integrations/prometheus.md index eaed364f22..69ac214b23 100644 --- a/docs/admin/integrations/prometheus.md +++ b/docs/admin/integrations/prometheus.md @@ -263,7 +263,7 @@ The `coder_ai_gateway_cost_control_*` metrics are exported only by `coderd`. | `coderd_dbpurge_iteration_duration_seconds` | histogram | Duration of each dbpurge iteration in seconds. | `success` | | `coderd_dbpurge_records_purged_total` | counter | Total number of records purged by type. | `record_type` | | `coderd_experiments` | gauge | Indicates whether each experiment is enabled (1) or not (0) | `experiment` | -| `coderd_insights_applications_usage_seconds` | gauge | The application usage per template. | `application_name` `organization_name` `slug` `template_name` | +| `coderd_insights_applications_usage_seconds` | gauge | The application usage per template. Built-in apps are grouped by app family, such as Visual Studio Code, JetBrains, Web Terminal, or SSH, with an empty slug. A user-minute counts once per family. | `application_name` `organization_name` `slug` `template_name` | | `coderd_insights_parameters` | gauge | The parameter usage per template. | `organization_name` `parameter_name` `parameter_type` `parameter_value` `template_name` | | `coderd_insights_templates_active_users` | gauge | The number of active users of the template. | `organization_name` `template_name` | | `coderd_license_active_users` | gauge | The number of active users. | | diff --git a/scripts/metricsdocgen/generated_metrics b/scripts/metricsdocgen/generated_metrics index 6c008456b3..982af6e1e8 100644 --- a/scripts/metricsdocgen/generated_metrics +++ b/scripts/metricsdocgen/generated_metrics @@ -436,7 +436,7 @@ coderd_dbpurge_records_purged_total{record_type=""} 0 # HELP coderd_experiments Indicates whether each experiment is enabled (1) or not (0) # TYPE coderd_experiments gauge coderd_experiments{experiment=""} 0 -# HELP coderd_insights_applications_usage_seconds The application usage per template. +# HELP coderd_insights_applications_usage_seconds The application usage per template. Built-in apps are grouped by app family, such as Visual Studio Code, JetBrains, Web Terminal, or SSH, with an empty slug. A user-minute counts once per family. # TYPE coderd_insights_applications_usage_seconds gauge coderd_insights_applications_usage_seconds{template_name="",application_name="",slug="",organization_name=""} 0 # HELP coderd_insights_parameters The parameter usage per template. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index e2551de0e6..53e930c600 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -10143,36 +10143,6 @@ export const TemplateBuilderVariableTypes: TemplateBuilderVariableType[] = [ "string", ]; -// From codersdk/insights.go -/** - * Enums define the display name of the builtin app reported. - */ -export const TemplateBuiltinAppDisplayNameJetBrains = "JetBrains"; - -// From codersdk/insights.go -/** - * Enums define the display name of the builtin app reported. - */ -export const TemplateBuiltinAppDisplayNameSFTP = "SFTP"; - -// From codersdk/insights.go -/** - * Enums define the display name of the builtin app reported. - */ -export const TemplateBuiltinAppDisplayNameSSH = "SSH"; - -// From codersdk/insights.go -/** - * Enums define the display name of the builtin app reported. - */ -export const TemplateBuiltinAppDisplayNameVSCode = "Visual Studio Code"; - -// From codersdk/insights.go -/** - * Enums define the display name of the builtin app reported. - */ -export const TemplateBuiltinAppDisplayNameWebTerminal = "Web Terminal"; - // From codersdk/templates.go export interface TemplateExample { readonly id: string;