From f867dd7ba2c3ad9c766bf011fe4fef0bca1c38c3 Mon Sep 17 00:00:00 2001 From: ___ Date: Fri, 9 Jun 2023 17:54:58 +0000 Subject: [PATCH 1/5] feat: add template tags to agent up metric So we can track template and version usage for all running workspaces. Right now, we can only track it by workspace builds. --- coderd/database/queries.sql.go | 5 +- coderd/database/queries/workspaces.sql | 3 +- coderd/prometheusmetrics/prometheusmetrics.go | 68 +++++++++++++++++-- 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d27802827763b..5c23f04feac49 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -7871,7 +7871,7 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace const getWorkspaces = `-- name: GetWorkspaces :many SELECT - workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, COUNT(*) OVER () as count + workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, latest_build.template_version_id as template_version_id, COUNT(*) OVER () as count FROM workspaces JOIN @@ -7881,6 +7881,7 @@ ON LEFT JOIN LATERAL ( SELECT workspace_builds.transition, + workspace_builds.template_version_id, provisioner_jobs.id AS provisioner_job_id, provisioner_jobs.started_at, provisioner_jobs.updated_at, @@ -8081,6 +8082,7 @@ type GetWorkspacesRow struct { AutostartSchedule sql.NullString `db:"autostart_schedule" json:"autostart_schedule"` Ttl sql.NullInt64 `db:"ttl" json:"ttl"` LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` + TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` Count int64 `db:"count" json:"count"` } @@ -8117,6 +8119,7 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) &i.AutostartSchedule, &i.Ttl, &i.LastUsedAt, + &i.TemplateVersionID, &i.Count, ); err != nil { return nil, err diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index f5d7c35cfd929..762676eeca77c 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -75,7 +75,7 @@ WHERE -- name: GetWorkspaces :many SELECT - workspaces.*, COUNT(*) OVER () as count + workspaces.*, latest_build.template_version_id as template_version_id, COUNT(*) OVER () as count FROM workspaces JOIN @@ -85,6 +85,7 @@ ON LEFT JOIN LATERAL ( SELECT workspace_builds.transition, + workspace_builds.template_version_id, provisioner_jobs.id AS provisioner_job_id, provisioner_jobs.started_at, provisioner_jobs.updated_at, diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index 4fba21b7c337f..43b4796a70a5a 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -153,7 +153,7 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis Subsystem: "agents", Name: "up", Help: "The number of active agents per workspace.", - }, []string{usernameLabel, workspaceNameLabel})) + }, []string{usernameLabel, workspaceNameLabel, "template_name", "template_version"})) err := registerer.Register(agentsGauge) if err != nil { return nil, err @@ -225,6 +225,10 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis logger.Debug(ctx, "Agent metrics collection is starting") timer := prometheus.NewTimer(metricsCollectorAgents) + // Need to define these ahead of time bc of the use of gotos below + var templateNamesByID map[uuid.UUID]string + var templateVersionNamesByID map[uuid.UUID]string + workspaceRows, err := db.GetWorkspaces(ctx, database.GetWorkspacesParams{ AgentInactiveDisconnectTimeoutSeconds: int64(agentInactiveDisconnectTimeout.Seconds()), }) @@ -233,30 +237,44 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis goto done } + templateNamesByID, templateVersionNamesByID, err = getTemplatesAndVersionNamesFromWorkspaces(ctx, db, workspaceRows) + if err != nil { + logger.Error(ctx, "can't get template info", slog.Error(err)) + goto done + } + for _, workspace := range workspaceRows { + templateName, found := templateNamesByID[workspace.TemplateID] + if !found { + templateName = "unknown" + } + templateVersionName, found := templateVersionNamesByID[workspace.TemplateID] + if !found { + templateVersionName = "unknown" + } user, err := db.GetUserByID(ctx, workspace.OwnerID) if err != nil { logger.Error(ctx, "can't get user", slog.F("user_id", workspace.OwnerID), slog.Error(err)) - agentsGauge.WithLabelValues(VectorOperationAdd, 0, user.Username, workspace.Name) + agentsGauge.WithLabelValues(VectorOperationAdd, 0, user.Username, workspace.Name, templateName, templateVersionName) continue } agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, workspace.ID) if err != nil { logger.Error(ctx, "can't get workspace agents", slog.F("workspace_id", workspace.ID), slog.Error(err)) - agentsGauge.WithLabelValues(VectorOperationAdd, 0, user.Username, workspace.Name) + agentsGauge.WithLabelValues(VectorOperationAdd, 0, user.Username, workspace.Name, templateName, templateVersionName) continue } if len(agents) == 0 { logger.Debug(ctx, "workspace agents are unavailable", slog.F("workspace_id", workspace.ID)) - agentsGauge.WithLabelValues(VectorOperationAdd, 0, user.Username, workspace.Name) + agentsGauge.WithLabelValues(VectorOperationAdd, 0, user.Username, workspace.Name, templateName, templateVersionName) continue } for _, agent := range agents { // Collect information about agents - agentsGauge.WithLabelValues(VectorOperationAdd, 1, user.Username, workspace.Name) + agentsGauge.WithLabelValues(VectorOperationAdd, 1, user.Username, workspace.Name, templateName, templateVersionName) connectionStatus := agent.Status(agentInactiveDisconnectTimeout) node := (*coordinator.Load()).Node(agent.ID) @@ -325,6 +343,46 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis }, nil } +func getTemplatesAndVersionNamesFromWorkspaces(ctx context.Context, db database.Store, workspaceRows []database.GetWorkspacesRow) (map[uuid.UUID]string, map[uuid.UUID]string, error) { + // Aggregate the used template and version IDs to minimize DB calls + usedTemplateIDs := map[uuid.UUID]struct{}{} + usedTemplateVersionIDs := map[uuid.UUID]struct{}{} + for _, workspace := range workspaceRows { + usedTemplateIDs[workspace.TemplateID] = struct{}{} + usedTemplateVersionIDs[workspace.TemplateVersionID] = struct{}{} + } + templatesToGet := make([]uuid.UUID, 0, len(usedTemplateIDs)) + for id := range usedTemplateIDs { + templatesToGet = append(templatesToGet, id) + } + templateVersionsToGet := make([]uuid.UUID, 0, len(usedTemplateVersionIDs)) + for id := range usedTemplateVersionIDs { + templateVersionsToGet = append(templateVersionsToGet, id) + } + + templates, err := db.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ + IDs: templatesToGet, + }) + if err != nil { + return nil, nil, err + } + templateNamesByID := make(map[uuid.UUID]string, len(templates)) + for _, template := range templates { + templateNamesByID[template.ID] = template.Name + } + + versions, err := db.GetTemplateVersionsByIDs(ctx, templateVersionsToGet) + if err != nil { + return nil, nil, err + } + templateVersionNamesByID := make(map[uuid.UUID]string, len(versions)) + for _, version := range versions { + templateVersionNamesByID[version.ID] = version.Name + } + + return templateNamesByID, templateVersionNamesByID, nil +} + func AgentStats(ctx context.Context, logger slog.Logger, registerer prometheus.Registerer, db database.Store, initialCreateAfter time.Time, duration time.Duration) (func(), error) { if duration == 0 { duration = 1 * time.Minute From b705b6984d7bd18d56ee21629c7914a9e96ffd59 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Mon, 10 Jul 2023 23:12:19 +0000 Subject: [PATCH 2/5] fix tests --- coderd/database/dbfake/dbfake.go | 37 ++++++++++-- coderd/database/queries.sql.go | 47 ++++++++++----- coderd/database/queries/workspaces.sql | 15 ++++- coderd/prometheusmetrics/prometheusmetrics.go | 58 ++----------------- .../prometheusmetrics_test.go | 8 ++- 5 files changed, 86 insertions(+), 79 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 55c4973cb74b2..b787af6324d99 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -607,12 +607,12 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. } if arg.Limit > 0 { if int(arg.Limit) > len(workspaces) { - return convertToWorkspaceRows(workspaces, int64(beforePageCount)), nil + return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount)), nil } workspaces = workspaces[:arg.Limit] } - return convertToWorkspaceRows(workspaces, int64(beforePageCount)), nil + return q.convertToWorkspaceRowsNoLock(ctx, workspaces, int64(beforePageCount)), nil } // mapAgentStatus determines the agent status based on different timestamps like created_at, last_connected_at, disconnected_at, etc. @@ -649,10 +649,10 @@ func mapAgentStatus(dbAgent database.WorkspaceAgent, agentInactiveDisconnectTime return status } -func convertToWorkspaceRows(workspaces []database.Workspace, count int64) []database.GetWorkspacesRow { - rows := make([]database.GetWorkspacesRow, len(workspaces)) - for i, w := range workspaces { - rows[i] = database.GetWorkspacesRow{ +func (q *fakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspaces []database.Workspace, count int64) []database.GetWorkspacesRow { + rows := make([]database.GetWorkspacesRow, 0, len(workspaces)) + for _, w := range workspaces { + wr := database.GetWorkspacesRow{ ID: w.ID, CreatedAt: w.CreatedAt, UpdatedAt: w.UpdatedAt, @@ -666,6 +666,31 @@ func convertToWorkspaceRows(workspaces []database.Workspace, count int64) []data LastUsedAt: w.LastUsedAt, Count: count, } + + for _, t := range q.templates { + if t.ID == w.TemplateID { + wr.TemplateName = sql.NullString{ + Valid: true, + String: t.Name, + } + break + } + } + + if build, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, w.ID); err == nil { + for _, tv := range q.templateVersions { + if tv.ID == build.TemplateVersionID { + wr.TemplateVersionID = tv.ID + wr.TemplateVersionName = sql.NullString{ + Valid: true, + String: tv.Name, + } + break + } + } + } + + rows = append(rows, wr) } return rows } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5f48493aba33d..5e98088e88ebf 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8391,7 +8391,11 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace const getWorkspaces = `-- name: GetWorkspaces :many SELECT - workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.locked_at, latest_build.template_version_id as template_version_id, COUNT(*) OVER () as count + workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.locked_at, + templates.name AS template_name, + latest_build.template_version_id, + latest_build.template_version_name, + COUNT(*) OVER () as count FROM workspaces JOIN @@ -8402,6 +8406,7 @@ LEFT JOIN LATERAL ( SELECT workspace_builds.transition, workspace_builds.template_version_id, + template_versions.name AS template_version_name, provisioner_jobs.id AS provisioner_job_id, provisioner_jobs.started_at, provisioner_jobs.updated_at, @@ -8414,6 +8419,10 @@ LEFT JOIN LATERAL ( provisioner_jobs ON provisioner_jobs.id = workspace_builds.job_id + LEFT JOIN + template_versions + ON + template_versions.id = workspace_builds.template_version_id WHERE workspace_builds.workspace_id = workspaces.id ORDER BY @@ -8421,6 +8430,10 @@ LEFT JOIN LATERAL ( LIMIT 1 ) latest_build ON TRUE +LEFT JOIN + templates +ON + templates.id = workspaces.template_id WHERE -- Optionally include deleted workspaces workspaces.deleted = $1 @@ -8591,20 +8604,22 @@ type GetWorkspacesParams struct { } type GetWorkspacesRow struct { - ID uuid.UUID `db:"id" json:"id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - TemplateID uuid.UUID `db:"template_id" json:"template_id"` - Deleted bool `db:"deleted" json:"deleted"` - Name string `db:"name" json:"name"` - AutostartSchedule sql.NullString `db:"autostart_schedule" json:"autostart_schedule"` - Ttl sql.NullInt64 `db:"ttl" json:"ttl"` - LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` - LockedAt sql.NullTime `db:"locked_at" json:"locked_at"` - TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` - Count int64 `db:"count" json:"count"` + ID uuid.UUID `db:"id" json:"id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + Deleted bool `db:"deleted" json:"deleted"` + Name string `db:"name" json:"name"` + AutostartSchedule sql.NullString `db:"autostart_schedule" json:"autostart_schedule"` + Ttl sql.NullInt64 `db:"ttl" json:"ttl"` + LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` + LockedAt sql.NullTime `db:"locked_at" json:"locked_at"` + TemplateName sql.NullString `db:"template_name" json:"template_name"` + TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` + TemplateVersionName sql.NullString `db:"template_version_name" json:"template_version_name"` + Count int64 `db:"count" json:"count"` } func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]GetWorkspacesRow, error) { @@ -8641,7 +8656,9 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.TemplateName, &i.TemplateVersionID, + &i.TemplateVersionName, &i.Count, ); err != nil { return nil, err diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index c68d5c93c17ef..50b8e4547e83c 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -75,7 +75,11 @@ WHERE -- name: GetWorkspaces :many SELECT - workspaces.*, latest_build.template_version_id as template_version_id, COUNT(*) OVER () as count + workspaces.*, + templates.name AS template_name, + latest_build.template_version_id, + latest_build.template_version_name, + COUNT(*) OVER () as count FROM workspaces JOIN @@ -86,6 +90,7 @@ LEFT JOIN LATERAL ( SELECT workspace_builds.transition, workspace_builds.template_version_id, + template_versions.name AS template_version_name, provisioner_jobs.id AS provisioner_job_id, provisioner_jobs.started_at, provisioner_jobs.updated_at, @@ -98,6 +103,10 @@ LEFT JOIN LATERAL ( provisioner_jobs ON provisioner_jobs.id = workspace_builds.job_id + LEFT JOIN + template_versions + ON + template_versions.id = workspace_builds.template_version_id WHERE workspace_builds.workspace_id = workspaces.id ORDER BY @@ -105,6 +114,10 @@ LEFT JOIN LATERAL ( LIMIT 1 ) latest_build ON TRUE +LEFT JOIN + templates +ON + templates.id = workspaces.template_id WHERE -- Optionally include deleted workspaces workspaces.deleted = @deleted diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index 59a3abcb486e2..cebee16d48815 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -225,10 +225,6 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis logger.Debug(ctx, "agent metrics collection is starting") timer := prometheus.NewTimer(metricsCollectorAgents) - // Need to define these ahead of time bc of the use of gotos below - var templateNamesByID map[uuid.UUID]string - var templateVersionNamesByID map[uuid.UUID]string - workspaceRows, err := db.GetWorkspaces(ctx, database.GetWorkspacesParams{ AgentInactiveDisconnectTimeoutSeconds: int64(agentInactiveDisconnectTimeout.Seconds()), }) @@ -237,19 +233,13 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis goto done } - templateNamesByID, templateVersionNamesByID, err = getTemplatesAndVersionNamesFromWorkspaces(ctx, db, workspaceRows) - if err != nil { - logger.Error(ctx, "can't get template info", slog.Error(err)) - goto done - } - for _, workspace := range workspaceRows { - templateName, found := templateNamesByID[workspace.TemplateID] - if !found { + templateName := workspace.TemplateName.String + if !workspace.TemplateName.Valid { templateName = "unknown" } - templateVersionName, found := templateVersionNamesByID[workspace.TemplateID] - if !found { + templateVersionName := workspace.TemplateVersionName.String + if !workspace.TemplateVersionName.Valid { templateVersionName = "unknown" } user, err := db.GetUserByID(ctx, workspace.OwnerID) @@ -343,46 +333,6 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis }, nil } -func getTemplatesAndVersionNamesFromWorkspaces(ctx context.Context, db database.Store, workspaceRows []database.GetWorkspacesRow) (map[uuid.UUID]string, map[uuid.UUID]string, error) { - // Aggregate the used template and version IDs to minimize DB calls - usedTemplateIDs := map[uuid.UUID]struct{}{} - usedTemplateVersionIDs := map[uuid.UUID]struct{}{} - for _, workspace := range workspaceRows { - usedTemplateIDs[workspace.TemplateID] = struct{}{} - usedTemplateVersionIDs[workspace.TemplateVersionID] = struct{}{} - } - templatesToGet := make([]uuid.UUID, 0, len(usedTemplateIDs)) - for id := range usedTemplateIDs { - templatesToGet = append(templatesToGet, id) - } - templateVersionsToGet := make([]uuid.UUID, 0, len(usedTemplateVersionIDs)) - for id := range usedTemplateVersionIDs { - templateVersionsToGet = append(templateVersionsToGet, id) - } - - templates, err := db.GetTemplatesWithFilter(ctx, database.GetTemplatesWithFilterParams{ - IDs: templatesToGet, - }) - if err != nil { - return nil, nil, err - } - templateNamesByID := make(map[uuid.UUID]string, len(templates)) - for _, template := range templates { - templateNamesByID[template.ID] = template.Name - } - - versions, err := db.GetTemplateVersionsByIDs(ctx, templateVersionsToGet) - if err != nil { - return nil, nil, err - } - templateVersionNamesByID := make(map[uuid.UUID]string, len(versions)) - for _, version := range versions { - templateVersionNamesByID[version.ID] = version.Name - } - - return templateNamesByID, templateVersionNamesByID, nil -} - func AgentStats(ctx context.Context, logger slog.Logger, registerer prometheus.Registerer, db database.Store, initialCreateAfter time.Time, duration time.Duration) (func(), error) { if duration == 0 { duration = 1 * time.Minute diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 9101288cca570..9ee8c4c9009bb 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -312,7 +312,7 @@ func TestAgents(t *testing.T) { // when closeFunc, err := prometheusmetrics.Agents(ctx, slogtest.Make(t, &slogtest.Options{ IgnoreErrors: true, - }), registry, db, &coordinatorPtr, derpMap, agentInactiveDisconnectTimeout, time.Millisecond) + }), registry, db, &coordinatorPtr, derpMap, agentInactiveDisconnectTimeout, time.Second) require.NoError(t, err) t.Cleanup(closeFunc) @@ -332,8 +332,10 @@ func TestAgents(t *testing.T) { for _, metric := range metrics { switch metric.GetName() { case "coderd_agents_up": - assert.Equal(t, "testuser", metric.Metric[0].Label[0].GetValue()) // Username - assert.Equal(t, workspace.Name, metric.Metric[0].Label[1].GetValue()) // Workspace name + assert.Equal(t, template.Name, metric.Metric[0].Label[0].GetValue()) // Template name + assert.Equal(t, version.Name, metric.Metric[0].Label[1].GetValue()) // Template version name + assert.Equal(t, "testuser", metric.Metric[0].Label[2].GetValue()) // Username + assert.Equal(t, workspace.Name, metric.Metric[0].Label[3].GetValue()) // Workspace name assert.Equal(t, 1, int(metric.Metric[0].Gauge.GetValue())) // Metric value agentsUp = true case "coderd_agents_connections": From b967f47f17ac45809944ea08d32db8aeb9044732 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Mon, 10 Jul 2023 23:14:31 +0000 Subject: [PATCH 3/5] fixup! fix tests --- coderd/prometheusmetrics/prometheusmetrics_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 9ee8c4c9009bb..5b53fcaa047e4 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -312,7 +312,7 @@ func TestAgents(t *testing.T) { // when closeFunc, err := prometheusmetrics.Agents(ctx, slogtest.Make(t, &slogtest.Options{ IgnoreErrors: true, - }), registry, db, &coordinatorPtr, derpMap, agentInactiveDisconnectTimeout, time.Second) + }), registry, db, &coordinatorPtr, derpMap, agentInactiveDisconnectTimeout, 50*time.Millisecond) require.NoError(t, err) t.Cleanup(closeFunc) From 20b208a85e500aeb7724d6326a3d4de34a29fff2 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Mon, 10 Jul 2023 23:56:00 +0000 Subject: [PATCH 4/5] fixup! fix tests --- coderd/database/modelqueries.go | 3 +++ coderd/database/queries.sql.go | 12 ++++++------ coderd/database/queries/workspaces.sql | 12 ++++++------ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 4ff6e6e2d154a..28a56b825f34e 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -236,6 +236,9 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa &i.Ttl, &i.LastUsedAt, &i.LockedAt, + &i.TemplateName, + &i.TemplateVersionID, + &i.TemplateVersionName, &i.Count, ); err != nil { return nil, err diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 5e98088e88ebf..3a975166d6788 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8505,13 +8505,13 @@ WHERE -- Filter by owner_id AND CASE WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - owner_id = $3 + workspaces.owner_id = $3 ELSE true END -- Filter by owner_name AND CASE WHEN $4 :: text != '' THEN - owner_id = (SELECT id FROM users WHERE lower(username) = lower($4) AND deleted = false) + workspaces.owner_id = (SELECT id FROM users WHERE lower(username) = lower($4) AND deleted = false) ELSE true END -- Filter by template_name @@ -8519,19 +8519,19 @@ WHERE -- Use the organization filter to restrict to 1 org if needed. AND CASE WHEN $5 :: text != '' THEN - template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower($5) AND deleted = false) + workspaces.template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower($5) AND deleted = false) ELSE true END -- Filter by template_ids AND CASE WHEN array_length($6 :: uuid[], 1) > 0 THEN - template_id = ANY($6) + workspaces.template_id = ANY($6) ELSE true END -- Filter by name, matching on substring AND CASE WHEN $7 :: text != '' THEN - name ILIKE '%' || $7 || '%' + workspaces.name ILIKE '%' || $7 || '%' ELSE true END -- Filter by agent status @@ -8579,7 +8579,7 @@ ORDER BY latest_build.error IS NULL AND latest_build.transition = 'start'::workspace_transition) DESC, LOWER(users.username) ASC, - LOWER(name) ASC + LOWER(workspaces.name) ASC LIMIT CASE WHEN $11 :: integer > 0 THEN diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 50b8e4547e83c..ff05537a34ce4 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -189,13 +189,13 @@ WHERE -- Filter by owner_id AND CASE WHEN @owner_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - owner_id = @owner_id + workspaces.owner_id = @owner_id ELSE true END -- Filter by owner_name AND CASE WHEN @owner_username :: text != '' THEN - owner_id = (SELECT id FROM users WHERE lower(username) = lower(@owner_username) AND deleted = false) + workspaces.owner_id = (SELECT id FROM users WHERE lower(username) = lower(@owner_username) AND deleted = false) ELSE true END -- Filter by template_name @@ -203,19 +203,19 @@ WHERE -- Use the organization filter to restrict to 1 org if needed. AND CASE WHEN @template_name :: text != '' THEN - template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower(@template_name) AND deleted = false) + workspaces.template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower(@template_name) AND deleted = false) ELSE true END -- Filter by template_ids AND CASE WHEN array_length(@template_ids :: uuid[], 1) > 0 THEN - template_id = ANY(@template_ids) + workspaces.template_id = ANY(@template_ids) ELSE true END -- Filter by name, matching on substring AND CASE WHEN @name :: text != '' THEN - name ILIKE '%' || @name || '%' + workspaces.name ILIKE '%' || @name || '%' ELSE true END -- Filter by agent status @@ -263,7 +263,7 @@ ORDER BY latest_build.error IS NULL AND latest_build.transition = 'start'::workspace_transition) DESC, LOWER(users.username) ASC, - LOWER(name) ASC + LOWER(workspaces.name) ASC LIMIT CASE WHEN @limit_ :: integer > 0 THEN From 3c78569d04fe14a910f4d70518ea5617231bca6d Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Tue, 11 Jul 2023 00:16:26 +0000 Subject: [PATCH 5/5] fixup! fix tests --- coderd/database/dbfake/dbfake.go | 5 +---- coderd/database/queries.sql.go | 16 ++++++++++------ coderd/database/queries/workspaces.sql | 14 +++++++++----- coderd/prometheusmetrics/prometheusmetrics.go | 7 ++----- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index b787af6324d99..49e6735c4cf06 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -669,10 +669,7 @@ func (q *fakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac for _, t := range q.templates { if t.ID == w.TemplateID { - wr.TemplateName = sql.NullString{ - Valid: true, - String: t.Name, - } + wr.TemplateName = t.Name break } } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3a975166d6788..7bbb5dd2d3404 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -8392,7 +8392,7 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace const getWorkspaces = `-- name: GetWorkspaces :many SELECT workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.locked_at, - templates.name AS template_name, + COALESCE(template_name.template_name, 'unknown') as template_name, latest_build.template_version_id, latest_build.template_version_name, COUNT(*) OVER () as count @@ -8430,10 +8430,14 @@ LEFT JOIN LATERAL ( LIMIT 1 ) latest_build ON TRUE -LEFT JOIN - templates -ON - templates.id = workspaces.template_id +LEFT JOIN LATERAL ( + SELECT + templates.name AS template_name + FROM + templates + WHERE + templates.id = workspaces.template_id +) template_name ON true WHERE -- Optionally include deleted workspaces workspaces.deleted = $1 @@ -8616,7 +8620,7 @@ type GetWorkspacesRow struct { Ttl sql.NullInt64 `db:"ttl" json:"ttl"` LastUsedAt time.Time `db:"last_used_at" json:"last_used_at"` LockedAt sql.NullTime `db:"locked_at" json:"locked_at"` - TemplateName sql.NullString `db:"template_name" json:"template_name"` + TemplateName string `db:"template_name" json:"template_name"` TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` TemplateVersionName sql.NullString `db:"template_version_name" json:"template_version_name"` Count int64 `db:"count" json:"count"` diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index ff05537a34ce4..ef3d84adebfec 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -76,7 +76,7 @@ WHERE -- name: GetWorkspaces :many SELECT workspaces.*, - templates.name AS template_name, + COALESCE(template_name.template_name, 'unknown') as template_name, latest_build.template_version_id, latest_build.template_version_name, COUNT(*) OVER () as count @@ -114,10 +114,14 @@ LEFT JOIN LATERAL ( LIMIT 1 ) latest_build ON TRUE -LEFT JOIN - templates -ON - templates.id = workspaces.template_id +LEFT JOIN LATERAL ( + SELECT + templates.name AS template_name + FROM + templates + WHERE + templates.id = workspaces.template_id +) template_name ON true WHERE -- Optionally include deleted workspaces workspaces.deleted = @deleted diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index cebee16d48815..454934f1dd7b4 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -15,7 +15,6 @@ import ( "tailscale.com/tailcfg" "cdr.dev/slog" - "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/db2sdk" "github.com/coder/coder/coderd/database/dbauthz" @@ -234,14 +233,12 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis } for _, workspace := range workspaceRows { - templateName := workspace.TemplateName.String - if !workspace.TemplateName.Valid { - templateName = "unknown" - } + templateName := workspace.TemplateName templateVersionName := workspace.TemplateVersionName.String if !workspace.TemplateVersionName.Valid { templateVersionName = "unknown" } + user, err := db.GetUserByID(ctx, workspace.OwnerID) if err != nil { logger.Error(ctx, "can't get user from the database", slog.F("user_id", workspace.OwnerID), slog.Error(err))