feat: normalize workspace agent session counts into a child table#27179
Draft
EhabY wants to merge 4 commits into
Draft
feat: normalize workspace agent session counts into a child table#27179EhabY wants to merge 4 commits into
EhabY wants to merge 4 commits into
Conversation
Adding a new IDE session type currently requires touching 15+ files across the stack. This replaces the fixed session_count_* columns on the ephemeral workspace_agent_stats table with a normalized workspace_agent_session_counts child table keyed by raw app name, and makes the agent report session counts via a dynamic map so new IDEs are tracked without schema or code changes. - Migration copies the ~1 day ephemeral buffer, drops the four columns, and trims the insights covering index (clean cut; no reporting gap). - Proto Stats gains map<string, int64> session_counts (field 13); fields 8-11 are deprecated but still converted server-side for old agents. - Agent counts sessions per raw type via sync.Map; JetBrains keeps its channel-watcher special case via family lookup; session metrics are labeled by family to bound cardinality. - App names are stored raw (casing preserved, null bytes stripped, truncated to 64 runes); well-known names and legacy spellings are canonicalized by the new coderd/idemetadata package. - The workspace usage API and coder ssh --usage-app accept arbitrary app names; codersdk.AllowedAppNames is removed. - All read queries keep their Row shapes, so the deployment stats API, Prometheus gauge names, and telemetry snapshots are unchanged. Part of the Scalable Approach for Adding New IDE Session Types RFC (phase 1 of 5). Later phases add the API map field and labeled metrics, migrate connection_logs off its type enum, and merge template_usage_stats *_mins into app_usage_mins.
- Cap session count reports at 64 distinct app names, aggregating the overflow under "other", so a malicious or buggy agent cannot fan out child-table rows. - Guard the batcher's positional JSONB contract at flush: on a length mismatch, drop the session counts for the batch (parent stats still insert) instead of misattributing them. - Replace the per-row lateral probes in UpsertTemplateUsageStats and GetTemplateInsightsByTemplate with a direct hash-joinable join; the inclusion filter becomes the join itself and the DISTINCT/MAX aggregates tolerate the fan-out. At 1.7M stats rows this takes the rollup from 292ms to 99ms and the insights query from 3.0s to 0.66s, with byte-identical results.
Complete the v2.11 plumbing so agents dial the version they report (fixes TestWorkspaceAgent_Startup), normalize well-known app names once at stats ingestion, and fold the idemetadata display/family layering into a single family map that groups stored names at read time, so new IDEs (Antigravity, Trae, Kiro, Devin) need no migration. Replace the per-row lateral pivots with plain joins where no fan-out is possible and backfill the child table in one pass.
Fold every app name to lowercase with hyphens as underscores at ingestion instead of only rewriting well-known spellings. Once the rollup starts persisting arbitrary names durably, raw storage would split aggregation keys across spellings forever; canonical storage removes the read-side folding obligation. Display casing returns via the phase-2 metadata map.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding a new IDE session type (Cursor, Windsurf, ...) currently requires changes to 15+ files across migrations, proto, server, SDK, and UI. This PR replaces the fixed
session_count_*columns on the ephemeralworkspace_agent_statstable with a normalizedworkspace_agent_session_countschild table keyed by app name, and makes the agent report session counts as a dynamic proto map. New session types then flow end-to-end (agent counters → stats pipeline → template usage rollup) with zero schema, proto, or server changes.Phase 1 of the Scalable Approach for Adding New IDE Session Types RFC. External surfaces are intentionally unchanged: all read queries keep their row shapes, so the deployment stats API, Prometheus gauge names, and telemetry snapshots are byte-identical. Later phases add the API map field and family-labeled metrics (2), migrate
connection_logsoff its type enum (3), mergetemplate_usage_stats*_minsintoapp_usage_mins(4), and version-gate the IDE clients (5).NOTE: The migration copies the ephemeral stats buffer (~1 day of rows, bounded by
dbpurge) into the child table in a single pass, drops the foursession_count_*columns, and recreates the insights covering index without them. There is no reporting gap. The down migration restores the four canonical app names.App names are stored in canonical form (supersedes the RFC's raw-storage rule)
Every app name is folded at stats ingestion: lowercased, hyphens folded to underscores, null bytes stripped, capped at 64 runes, empty becomes
unknown.Cursor,cursor, andmy-new-ide/my_new_ideare one storage key.This deliberately supersedes the RFC's "casing preserved, not rewritten at storage" decision, and the RFC needs a matching amendment. Rationale:
app_nameis an aggregation key (primary key component, rollup JSONB key, GROUP BY target), not a forensics field likeaudit_logs.user_agent. Once phase 4 rolls arbitrary names into the durableapp_usage_mins, raw storage would mean a name cataloged after its first appearance splits into two keys forever, and every read path carries a fold-or-undercount obligation. Canonical storage removes that obligation, keeps SQL exact-match, and costs nothing now: clients only send non-standard names after the phase-5 gate, so no data can exist under the old policy. The fold is deterministic and total (not the rejected regex sanitizer; it never rejects or drops), and display casing for cataloged IDEs returns via the phase-2 metadata map.Key changes:
workspace_agent_session_counts (workspace_agent_stats_id, app_name, count)withON DELETE CASCADE. App names canonicalized at ingestion as described above; no CHECK constraint.Stats.session_countsmap (field 13) as Agent API v2.11; fields 8–11 are deprecated and converted server-side byworkspacestats.SessionCountsFromProto, so old agents keep working through the upgrade window. Agents dial the version they report via the newConnectRPC211helpers; the unusedConnectRPC210WithRoleis removed.otherapp name (well-known names always kept, remainder chosen lexicographically for determinism). This bounds child-table fan-out from malicious or buggy agents; legitimate agents report a handful of entries.syncmap.Mapand omits idle types from reports; JetBrains keeps its channel-watcher special case via family lookup; agent session metrics are labeled by family to keep cardinality bounded.coderd/idemetadataleaf package: one map from canonical app names to families, grouping stored names at read time. Adding an IDE is one map entry, no migration, and applies retroactively to stored data; entries ship for Cursor, Windsurf, Positron, VSCodium, Antigravity, Trae, Kiro, and Devin. The agentssh magic session type constants derive from it, so the vocabularies cannot drift. Display names ship with their phase-2 consumers.connections_by_proto), exploded into the child table within the same insert statement.coder ssh --usage-appaccept arbitrary app names;codersdk.AllowedAppNamesis removed.Behavior changes, invisible until clients start sending non-standard names in phase 5: unknown app names are accepted and stored in canonical form instead of rejected (API) or coerced to
ssh(CLI), and unknown session types are counted instead of silently dropped.Benchmarks
Measured on PostgreSQL 13 with 1.73M stats rows (300 agents × 24h @ 15s) and 1.3M child rows; results byte-identical to the old queries in every case. The rollup and insights queries originally used per-row lateral probes and regressed 7.6×/7.7×; rewriting them as direct hash joins (the inclusion filter becomes the join, DISTINCT/MAX aggregates tolerate the fan-out) recovered most of it:
Since these measurements, the four pivots whose aggregates only touch child counts (deployment stats, per-agent stats, and both usage minute-bucket CTEs) were relaxed from per-row laterals to plain joins with
FILTERpivots, letting the planner choose between PK probes and hash joins; the two*AndLabelsqueries keep the lateral because they also aggregate parent columns.A covering
INCLUDE (count)index was evaluated and rejected: the hash-join plans never touch it, so it would be pure write overhead. Total storage at this scale dropped 697 MB → 666 MB, since the 40% of rows with no session activity now store nothing.Risks and mitigations
otheraggregation, and the ~1-day purge. Prometheus is unaffected (family labels).Implementation plan and decision log
Goal
Replace the four fixed
session_count_*columns on the ephemeralworkspace_agent_statstable with a normalized child table, make the agent report session counts dynamically via a proto map, and keep every external surface byte-identical (API JSON, Prometheus metric names, telemetry snapshot shape).Non-goals (later phases)
connection_logsENUM to TEXT migration, protoConnection.type_str(Phase 3)template_usage_stats*_minsmerge into JSONB +sftp_minsdrop (Phase 4)FeatureSetgate (Phase 5)Phase 1 invariant: nothing sends non-standard app names yet (clients are gated on the Phase 3 release), so this is a pure internal refactor with no observable behavior change at rollout.
Design summary
CROSS JOIN LATERAL (VALUES ...), counts > 0) → drop covering index → drop columns → recreate index with only theconnection_median_latency_msINCLUDE. Down migration re-adds columns, backfills the canonical four, restores the original index. Fixture seeds parent + child rows including a non-canonical name.map<string, int64> session_counts = 13; fields 8–11[deprecated = true], never reserved/removed. Documented as Agent API v2.11 intailnet/proto/version.go, withDRPCAgentClient211andConnectRPC211/ConnectRPC211WithRoleso agents dial the version they report. New agents populate the map only.extractMagicSessionType()canonicalizes (null-strip, truncate, lowercase, hyphen fold) instead of classifying; empty staysssh. Dynamic typedsyncmap.Mapcounters;ConnStats()returnsmap[string]int64omitting idle types;JetbrainsChannelWatchertakes the counter from the same store;ReportConnectionand session metric labels map by family.SessionCountsFromProtois the single ingestion point: it canonicalizes every app name (merging keys that fold together), converts old-agent fixed fields, and applies the 64-entry cap withotheraggregation. Used by the batcher, the reporter activity check (via the non-allocatingHasSessionCounts), and (viaClearSessionCounts) theExperimentWorkspaceUsagezeroing.postWorkspaceUsagepasses any app name through, replacing the allowlist 400 and the per-app switch.connectionsByProto; parent + child inserted in one CTE statement (jsonb_array_elementszipped withunnest(@id),jsonb_each_textexplodes each map,> 0filter).SUM(count) FILTER (WHERE app_name = ...); the two*AndLabelsqueries keepLEFT JOIN LATERALbecause they also aggregate parent columns (fan-out). The rollup and insights-by-template CTEs use a direct child join because their aggregates are DISTINCT/MAX (fan-out tolerant) and their inclusion filter is the join itself. Row shapes and output aliases preserved (load-bearing for positional struct conversions in metricscache/prometheusmetrics).UpsertTemplateUsageStatsstill writes the dedicated*_minscolumns until Phase 4.DeleteOldWorkspaceAgentStatsunchanged (cascade).dbgen.WorkspaceAgentStatseeds counts via a variadic map.Decisions (resolved during planning and review)
coderd/idemetadata(importable fromagent/, no heavy deps). Promote to codersdk only if external consumers ever need it.codersdk.AllowedAppNames: removed outright; it only fed the 400 path.UsageAppNameconstants stay.app_nameis an aggregation key, not a forensics field; once phase 4 makes names durable, raw storage would split keys across spellings permanently and oblige every read path to fold. The fold is total and deterministic (never rejects, unlike the regex sanitizer the RFC review rejected); spaces and unicode are left alone. Nothing sends arbitrary names before phase 5, so no data exists under the old policy.other(not dropped, notunknown), keeping totals accurate while bounding row fan-out.idemetadatasince nothing in phase 1 renders them; they ship with their phase-2 consumers (insights page, deployment banner) and will restore proper casing for cataloged IDEs. Families group at read time, so map additions need no migration and apply retroactively to stored rows.File-by-file
coderd/database/migrations/000543_*.{up,down}.sql+ fixturequeries/workspaceagentstats.sql,queries/insights.sqlqueries.sql.go,models.go,querier.go,dbmock,dbmetrics,dump.sql,agent.pb.gomake genagent/proto/agent.proto,agent/proto/agent_drpc_old.go,tailnet/proto/version.goagent/agentssh/{agentssh,metrics}.go,agent/agent.gocoderd/idemetadata(new)coderd/agentapi/stats.go,coderd/workspacestats/{batcher,reporter,sessioncounts}.go,coderd/workspaces.gocodersdk/workspaces.go,codersdk/agentsdk/agentsdk.go,cli/ssh.goConnectRPC211, usage-app passthroughcoderd/database/dbgen/dbgen.go🤖 This PR was generated by Coder Agents on behalf of @EhabY.