From 843d65035f0c1c436e24ef2ac5121b7c5b58784d Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 25 Mar 2024 16:46:25 +0200 Subject: [PATCH 01/13] Add metric to show operators what statuses workspaces are in, with relevant detail Light refactoring Signed-off-by: Danny Kopping --- cli/server.go | 2 +- coderd/database/modelqueries.go | 1 + coderd/database/queries.sql.go | 62 ++++++++-------- coderd/database/queries/workspaces.sql | 6 +- coderd/prometheusmetrics/prometheusmetrics.go | 74 +++++++++++++++---- .../prometheusmetrics_test.go | 2 +- 6 files changed, 100 insertions(+), 47 deletions(-) diff --git a/cli/server.go b/cli/server.go index f2178d470a078..9eae4d114f0f2 100644 --- a/cli/server.go +++ b/cli/server.go @@ -209,7 +209,7 @@ func enablePrometheus( } afterCtx(ctx, closeUsersFunc) - closeWorkspacesFunc, err := prometheusmetrics.Workspaces(ctx, options.PrometheusRegistry, options.Database, 0) + closeWorkspacesFunc, err := prometheusmetrics.Workspaces(ctx, options.Logger.Named("workspaces_metrics"), options.PrometheusRegistry, options.Database, 0) if err != nil { return nil, xerrors.Errorf("register workspaces prometheus metric: %w", err) } diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 40c953375d8a0..ca38505b28ef0 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -266,6 +266,7 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa &i.LatestBuildCanceledAt, &i.LatestBuildError, &i.LatestBuildTransition, + &i.LatestBuildStatus, &i.Count, ); err != nil { return nil, err diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2112b05133fb1..be777dcc330a4 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -12178,7 +12178,8 @@ SELECT latest_build.completed_at as latest_build_completed_at, latest_build.canceled_at as latest_build_canceled_at, latest_build.error as latest_build_error, - latest_build.transition as latest_build_transition + latest_build.transition as latest_build_transition, + latest_build.job_status as latest_build_status FROM workspaces JOIN @@ -12405,7 +12406,7 @@ WHERE -- @authorize_filter ), filtered_workspaces_order AS ( SELECT - fw.id, fw.created_at, fw.updated_at, fw.owner_id, fw.organization_id, fw.template_id, fw.deleted, fw.name, fw.autostart_schedule, fw.ttl, fw.last_used_at, fw.dormant_at, fw.deleting_at, fw.automatic_updates, fw.favorite, fw.template_name, fw.template_version_id, fw.template_version_name, fw.username, fw.latest_build_completed_at, fw.latest_build_canceled_at, fw.latest_build_error, fw.latest_build_transition + fw.id, fw.created_at, fw.updated_at, fw.owner_id, fw.organization_id, fw.template_id, fw.deleted, fw.name, fw.autostart_schedule, fw.ttl, fw.last_used_at, fw.dormant_at, fw.deleting_at, fw.automatic_updates, fw.favorite, fw.template_name, fw.template_version_id, fw.template_version_name, fw.username, fw.latest_build_completed_at, fw.latest_build_canceled_at, fw.latest_build_error, fw.latest_build_transition, fw.latest_build_status FROM filtered_workspaces fw ORDER BY @@ -12426,7 +12427,7 @@ WHERE $19 ), filtered_workspaces_order_with_summary AS ( SELECT - fwo.id, fwo.created_at, fwo.updated_at, fwo.owner_id, fwo.organization_id, fwo.template_id, fwo.deleted, fwo.name, fwo.autostart_schedule, fwo.ttl, fwo.last_used_at, fwo.dormant_at, fwo.deleting_at, fwo.automatic_updates, fwo.favorite, fwo.template_name, fwo.template_version_id, fwo.template_version_name, fwo.username, fwo.latest_build_completed_at, fwo.latest_build_canceled_at, fwo.latest_build_error, fwo.latest_build_transition + fwo.id, fwo.created_at, fwo.updated_at, fwo.owner_id, fwo.organization_id, fwo.template_id, fwo.deleted, fwo.name, fwo.autostart_schedule, fwo.ttl, fwo.last_used_at, fwo.dormant_at, fwo.deleting_at, fwo.automatic_updates, fwo.favorite, fwo.template_name, fwo.template_version_id, fwo.template_version_name, fwo.username, fwo.latest_build_completed_at, fwo.latest_build_canceled_at, fwo.latest_build_error, fwo.latest_build_transition, fwo.latest_build_status FROM filtered_workspaces_order fwo -- Return a technical summary row with total count of workspaces. @@ -12456,7 +12457,8 @@ WHERE '0001-01-01 00:00:00+00'::timestamptz, -- latest_build_completed_at, '0001-01-01 00:00:00+00'::timestamptz, -- latest_build_canceled_at, '', -- latest_build_error - 'start'::workspace_transition -- latest_build_transition + 'start'::workspace_transition, -- latest_build_transition + 'unknown'::provisioner_job_status -- latest_build_status WHERE $21 :: boolean = true ), total_count AS ( @@ -12466,7 +12468,7 @@ WHERE filtered_workspaces ) SELECT - fwos.id, fwos.created_at, fwos.updated_at, fwos.owner_id, fwos.organization_id, fwos.template_id, fwos.deleted, fwos.name, fwos.autostart_schedule, fwos.ttl, fwos.last_used_at, fwos.dormant_at, fwos.deleting_at, fwos.automatic_updates, fwos.favorite, fwos.template_name, fwos.template_version_id, fwos.template_version_name, fwos.username, fwos.latest_build_completed_at, fwos.latest_build_canceled_at, fwos.latest_build_error, fwos.latest_build_transition, + fwos.id, fwos.created_at, fwos.updated_at, fwos.owner_id, fwos.organization_id, fwos.template_id, fwos.deleted, fwos.name, fwos.autostart_schedule, fwos.ttl, fwos.last_used_at, fwos.dormant_at, fwos.deleting_at, fwos.automatic_updates, fwos.favorite, fwos.template_name, fwos.template_version_id, fwos.template_version_name, fwos.username, fwos.latest_build_completed_at, fwos.latest_build_canceled_at, fwos.latest_build_error, fwos.latest_build_transition, fwos.latest_build_status, tc.count FROM filtered_workspaces_order_with_summary fwos @@ -12499,30 +12501,31 @@ 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"` - DormantAt sql.NullTime `db:"dormant_at" json:"dormant_at"` - DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` - AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` - Favorite bool `db:"favorite" json:"favorite"` - 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"` - Username string `db:"username" json:"username"` - LatestBuildCompletedAt sql.NullTime `db:"latest_build_completed_at" json:"latest_build_completed_at"` - LatestBuildCanceledAt sql.NullTime `db:"latest_build_canceled_at" json:"latest_build_canceled_at"` - LatestBuildError sql.NullString `db:"latest_build_error" json:"latest_build_error"` - LatestBuildTransition WorkspaceTransition `db:"latest_build_transition" json:"latest_build_transition"` - 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"` + DormantAt sql.NullTime `db:"dormant_at" json:"dormant_at"` + DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` + AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` + Favorite bool `db:"favorite" json:"favorite"` + 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"` + Username string `db:"username" json:"username"` + LatestBuildCompletedAt sql.NullTime `db:"latest_build_completed_at" json:"latest_build_completed_at"` + LatestBuildCanceledAt sql.NullTime `db:"latest_build_canceled_at" json:"latest_build_canceled_at"` + LatestBuildError sql.NullString `db:"latest_build_error" json:"latest_build_error"` + LatestBuildTransition WorkspaceTransition `db:"latest_build_transition" json:"latest_build_transition"` + LatestBuildStatus NullProvisionerJobStatus `db:"latest_build_status" json:"latest_build_status"` + Count int64 `db:"count" json:"count"` } // build_params is used to filter by build parameters if present. @@ -12583,6 +12586,7 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) &i.LatestBuildCanceledAt, &i.LatestBuildError, &i.LatestBuildTransition, + &i.LatestBuildStatus, &i.Count, ); err != nil { return nil, err diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 767280634f1e3..29255d475b410 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -96,7 +96,8 @@ SELECT latest_build.completed_at as latest_build_completed_at, latest_build.canceled_at as latest_build_canceled_at, latest_build.error as latest_build_error, - latest_build.transition as latest_build_transition + latest_build.transition as latest_build_transition, + latest_build.job_status as latest_build_status FROM workspaces JOIN @@ -374,7 +375,8 @@ WHERE '0001-01-01 00:00:00+00'::timestamptz, -- latest_build_completed_at, '0001-01-01 00:00:00+00'::timestamptz, -- latest_build_canceled_at, '', -- latest_build_error - 'start'::workspace_transition -- latest_build_transition + 'start'::workspace_transition, -- latest_build_transition + 'unknown'::provisioner_job_status -- latest_build_status WHERE @with_summary :: boolean = true ), total_count AS ( diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index b2c4b46677eb6..a4349f99b97eb 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -72,36 +72,42 @@ func ActiveUsers(ctx context.Context, registerer prometheus.Registerer, db datab } // Workspaces tracks the total number of workspaces with labels on status. -func Workspaces(ctx context.Context, registerer prometheus.Registerer, db database.Store, duration time.Duration) (func(), error) { +func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.Registerer, db database.Store, duration time.Duration) (func(), error) { if duration == 0 { duration = 5 * time.Minute } - gauge := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + workspacesByStatus := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", Subsystem: "api", Name: "workspace_latest_build_total", - Help: "The latest workspace builds with a status.", + Help: "The current number of workspace builds by status.", }, []string{"status"}) - err := registerer.Register(gauge) - if err != nil { + if err := registerer.Register(workspacesByStatus); err != nil { + return nil, err + } + + workspacesDetail := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: "coderd", + Subsystem: "api", + Name: "workspace_detail", + Help: "The current workspace details by template, transition, owner, and status.", + }, []string{"status", "template_name", "template_version", "workspace_name", "workspace_owner", "workspace_transition"}) + if err := registerer.Register(workspacesDetail); err != nil { return nil, err } // This exists so the prometheus metric exports immediately when set. // It helps with tests so they don't have to wait for a tick. - gauge.WithLabelValues("pending").Set(0) + workspacesByStatus.WithLabelValues(string(database.ProvisionerJobStatusPending)).Set(0) + workspacesDetail.WithLabelValues(string(database.ProvisionerJobStatusPending), "", "", "", "", "").Set(0) ctx, cancelFunc := context.WithCancel(ctx) done := make(chan struct{}) - // Use time.Nanosecond to force an initial tick. It will be reset to the - // correct duration after executing once. - ticker := time.NewTicker(time.Nanosecond) - doTick := func() { - defer ticker.Reset(duration) - + updateWorkspacesByStatus := func() { builds, err := db.GetLatestWorkspaceBuilds(ctx) if err != nil { + logger.Warn(ctx, "failed to load latest workspace builds", slog.Error(err)) return } jobIDs := make([]uuid.UUID, 0, len(builds)) @@ -110,16 +116,56 @@ func Workspaces(ctx context.Context, registerer prometheus.Registerer, db databa } jobs, err := db.GetProvisionerJobsByIDs(ctx, jobIDs) if err != nil { + ids := make([]string, 0, len(jobIDs)) + for _, id := range jobIDs { + ids = append(ids, id.String()) + } + + logger.Warn(ctx, "failed to load provisioner jobs", slog.F("ids", ids), slog.Error(err)) return } - gauge.Reset() + workspacesByStatus.Reset() for _, job := range jobs { status := codersdk.ProvisionerJobStatus(job.JobStatus) - gauge.WithLabelValues(string(status)).Add(1) + workspacesByStatus.WithLabelValues(string(status)).Add(1) } } + updateWorkspacesDetail := func() { + ws, err := db.GetWorkspaces(ctx, database.GetWorkspacesParams{ + Deleted: false, + WithSummary: false, + }) + if err != nil { + logger.Warn(ctx, "failed to load active workspaces", slog.Error(err)) + return + } + + workspacesDetail.Reset() + for _, w := range ws { + // TODO: there may be a more elegant/idiomatic way to do this? + buildStatus := string(database.ProvisionerJobStatusUnknown) + if val, err := w.LatestBuildStatus.Value(); err == nil { + if status, ok := val.(string); ok { + buildStatus = status + } + } + + workspacesDetail.WithLabelValues(buildStatus, w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1) + } + } + + // Use time.Nanosecond to force an initial tick. It will be reset to the + // correct duration after executing once. + ticker := time.NewTicker(time.Nanosecond) + doTick := func() { + defer ticker.Reset(duration) + + updateWorkspacesByStatus() + updateWorkspacesDetail() + } + go func() { defer close(done) defer ticker.Stop() diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 32e97f84c32b1..fe9ec13181d54 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -229,7 +229,7 @@ func TestWorkspaces(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() registry := prometheus.NewRegistry() - closeFunc, err := prometheusmetrics.Workspaces(context.Background(), registry, tc.Database(), time.Millisecond) + closeFunc, err := prometheusmetrics.Workspaces(context.Background(), slogtest.Make(t, nil), registry, tc.Database(), time.Millisecond) require.NoError(t, err) t.Cleanup(closeFunc) From 2ed42a351116638b56af838661e7aff0c439db78 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 26 Mar 2024 12:16:23 +0200 Subject: [PATCH 02/13] Clear gauges if no db entries are found Light refactoring Signed-off-by: Danny Kopping --- coderd/prometheusmetrics/prometheusmetrics.go | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index a4349f99b97eb..79eafd582b9e9 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -77,36 +77,37 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R duration = 5 * time.Minute } - workspacesByStatus := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + workspaceStatuses := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", Subsystem: "api", Name: "workspace_latest_build_total", Help: "The current number of workspace builds by status.", }, []string{"status"}) - if err := registerer.Register(workspacesByStatus); err != nil { + if err := registerer.Register(workspaceStatuses); err != nil { return nil, err } - workspacesDetail := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + workspaceDetails := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", Subsystem: "api", Name: "workspace_detail", Help: "The current workspace details by template, transition, owner, and status.", }, []string{"status", "template_name", "template_version", "workspace_name", "workspace_owner", "workspace_transition"}) - if err := registerer.Register(workspacesDetail); err != nil { + if err := registerer.Register(workspaceDetails); err != nil { return nil, err } - // This exists so the prometheus metric exports immediately when set. - // It helps with tests so they don't have to wait for a tick. - workspacesByStatus.WithLabelValues(string(database.ProvisionerJobStatusPending)).Set(0) - workspacesDetail.WithLabelValues(string(database.ProvisionerJobStatusPending), "", "", "", "", "").Set(0) ctx, cancelFunc := context.WithCancel(ctx) done := make(chan struct{}) - updateWorkspacesByStatus := func() { + updateWorkspaceStatuses := func() { builds, err := db.GetLatestWorkspaceBuilds(ctx) if err != nil { + if errors.Is(err, sql.ErrNoRows) { + // clear all series if there are no database entries + workspaceStatuses.Reset() + } + logger.Warn(ctx, "failed to load latest workspace builds", slog.Error(err)) return } @@ -125,24 +126,29 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R return } - workspacesByStatus.Reset() + workspaceStatuses.Reset() for _, job := range jobs { status := codersdk.ProvisionerJobStatus(job.JobStatus) - workspacesByStatus.WithLabelValues(string(status)).Add(1) + workspaceStatuses.WithLabelValues(string(status)).Add(1) } } - updateWorkspacesDetail := func() { + updateWorkspaceDetails := func() { ws, err := db.GetWorkspaces(ctx, database.GetWorkspacesParams{ Deleted: false, WithSummary: false, }) if err != nil { + if errors.Is(err, sql.ErrNoRows) { + // clear all series if there are no database entries + workspaceDetails.Reset() + } + logger.Warn(ctx, "failed to load active workspaces", slog.Error(err)) return } - workspacesDetail.Reset() + workspaceDetails.Reset() for _, w := range ws { // TODO: there may be a more elegant/idiomatic way to do this? buildStatus := string(database.ProvisionerJobStatusUnknown) @@ -162,8 +168,8 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R doTick := func() { defer ticker.Reset(duration) - updateWorkspacesByStatus() - updateWorkspacesDetail() + updateWorkspaceStatuses() + updateWorkspaceDetails() } go func() { From c31b49882c398cffc8e5173450a3e2c9d148b3f6 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 26 Mar 2024 12:18:15 +0200 Subject: [PATCH 03/13] Change default refresh rates of Agent and Workspaces calls which seemed inappropriately slow Signed-off-by: Danny Kopping --- coderd/prometheusmetrics/prometheusmetrics.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index 79eafd582b9e9..b5c32737ecabd 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -24,10 +24,12 @@ import ( "github.com/coder/coder/v2/tailnet" ) +const defaultRefreshRate = time.Minute + // ActiveUsers tracks the number of users that have authenticated within the past hour. func ActiveUsers(ctx context.Context, registerer prometheus.Registerer, db database.Store, duration time.Duration) (func(), error) { if duration == 0 { - duration = 5 * time.Minute + duration = defaultRefreshRate } gauge := prometheus.NewGauge(prometheus.GaugeOpts{ @@ -74,7 +76,7 @@ func ActiveUsers(ctx context.Context, registerer prometheus.Registerer, db datab // Workspaces tracks the total number of workspaces with labels on status. func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.Registerer, db database.Store, duration time.Duration) (func(), error) { if duration == 0 { - duration = 5 * time.Minute + duration = defaultRefreshRate } workspaceStatuses := prometheus.NewGaugeVec(prometheus.GaugeOpts{ @@ -193,7 +195,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R // Agents tracks the total number of workspaces with labels on status. func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Registerer, db database.Store, coordinator *atomic.Pointer[tailnet.Coordinator], derpMapFn func() *tailcfg.DERPMap, agentInactiveDisconnectTimeout, duration time.Duration) (func(), error) { if duration == 0 { - duration = 1 * time.Minute + duration = defaultRefreshRate } agentsGauge := NewCachedGaugeVec(prometheus.NewGaugeVec(prometheus.GaugeOpts{ @@ -382,7 +384,7 @@ func Agents(ctx context.Context, logger slog.Logger, registerer prometheus.Regis func AgentStats(ctx context.Context, logger slog.Logger, registerer prometheus.Registerer, db database.Store, initialCreateAfter time.Time, duration time.Duration, aggregateByLabels []string) (func(), error) { if duration == 0 { - duration = 1 * time.Minute + duration = defaultRefreshRate } if len(aggregateByLabels) == 0 { From 2f5a9484263f404114fd721e5b0e3d22e3229a8b Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 26 Mar 2024 12:19:19 +0200 Subject: [PATCH 04/13] Added tests for workspace detail metrics Moderate refactoring Signed-off-by: Danny Kopping --- coderd/database/dbmem/dbmem.go | 10 + coderd/prometheusmetrics/prometheusmetrics.go | 2 +- .../prometheusmetrics_test.go | 272 +++++++++++++++++- testutil/duration.go | 9 +- 4 files changed, 273 insertions(+), 20 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index e9c853bbde63b..bf8759feacf6f 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -403,6 +403,16 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac break } } + + if pj, err := q.GetProvisionerJobByID(ctx, build.JobID); err == nil { + wr.LatestBuildStatus = database.NullProvisionerJobStatus{ProvisionerJobStatus: pj.JobStatus, Valid: true} + } + + wr.LatestBuildTransition = build.Transition + } + + if u, err := q.GetUserByID(ctx, w.OwnerID); err == nil { + wr.Username = u.Username } rows = append(rows, wr) diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index b5c32737ecabd..3e9ca9b1bb01c 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -160,7 +160,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R } } - workspacesDetail.WithLabelValues(buildStatus, w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1) + workspaceDetails.WithLabelValues(buildStatus, w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1) } } diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index fe9ec13181d54..bb138f0b19a57 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -5,12 +5,14 @@ import ( "database/sql" "encoding/json" "fmt" + "math/rand" "os" "reflect" "sync/atomic" "testing" "time" + "github.com/coder/coder/v2/cryptorand" "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" @@ -110,7 +112,7 @@ func TestActiveUsers(t *testing.T) { } } -func TestWorkspaces(t *testing.T) { +func TestWorkspaceStatuses(t *testing.T) { t.Parallel() insertRunning := func(db database.Store) database.ProvisionerJob { @@ -229,33 +231,273 @@ func TestWorkspaces(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() registry := prometheus.NewRegistry() - closeFunc, err := prometheusmetrics.Workspaces(context.Background(), slogtest.Make(t, nil), registry, tc.Database(), time.Millisecond) + closeFunc, err := prometheusmetrics.Workspaces(context.Background(), slogtest.Make(t, nil), registry, tc.Database(), testutil.IntervalFast) require.NoError(t, err) t.Cleanup(closeFunc) require.Eventually(t, func() bool { metrics, err := registry.Gather() assert.NoError(t, err) - if len(metrics) < 1 { - return false - } sum := 0 - for _, metric := range metrics[0].Metric { - count, ok := tc.Status[codersdk.ProvisionerJobStatus(metric.Label[0].GetValue())] - if metric.Gauge.GetValue() == 0 { + for _, m := range metrics { + if m.GetName() != "coderd_api_workspace_latest_build_total" { continue } - if !ok { - t.Fail() - } - if metric.Gauge.GetValue() != float64(count) { - return false + + for _, metric := range m.Metric { + count, ok := tc.Status[codersdk.ProvisionerJobStatus(metric.Label[0].GetValue())] + if metric.Gauge.GetValue() == 0 { + continue + } + if !ok { + t.Fail() + } + if metric.Gauge.GetValue() != float64(count) { + return false + } + sum += int(metric.Gauge.GetValue()) } - sum += int(metric.Gauge.GetValue()) } t.Logf("sum %d == total %d", sum, tc.Total) return sum == tc.Total - }, testutil.WaitShort, testutil.IntervalFast) + }, testutil.WaitSuperShort, testutil.IntervalFast) + }) + } +} + +func TestWorkspaceDetails(t *testing.T) { + t.Parallel() + + templateA := uuid.New() + templateVersionA := uuid.New() + templateB := uuid.New() + templateVersionB := uuid.New() + + insertTemplates := func(db database.Store) { + require.NoError(t, db.InsertTemplate(context.Background(), database.InsertTemplateParams{ + ID: templateA, + Name: "template-a", + Provisioner: database.ProvisionerTypeTerraform, + MaxPortSharingLevel: database.AppSharingLevelAuthenticated, + })) + + require.NoError(t, db.InsertTemplateVersion(context.Background(), database.InsertTemplateVersionParams{ + ID: templateVersionA, + TemplateID: uuid.NullUUID{UUID: templateA}, + Name: "version-1a", + })) + + require.NoError(t, db.InsertTemplate(context.Background(), database.InsertTemplateParams{ + ID: templateB, + Name: "template-b", + Provisioner: database.ProvisionerTypeTerraform, + MaxPortSharingLevel: database.AppSharingLevelAuthenticated, + })) + + require.NoError(t, db.InsertTemplateVersion(context.Background(), database.InsertTemplateVersionParams{ + ID: templateVersionB, + TemplateID: uuid.NullUUID{UUID: templateB}, + Name: "version-1b", + })) + } + + insertUser := func(db database.Store) database.User { + username, err := cryptorand.String(8) + require.NoError(t, err) + + user, err := db.InsertUser(context.Background(), database.InsertUserParams{ + ID: uuid.New(), + Username: username, + LoginType: database.LoginTypeNone, + }) + require.NoError(t, err) + + return user + } + + insertRunning := func(db database.Store) database.ProvisionerJob { + var template, templateVersion uuid.UUID + if rand.Intn(10) > 5 { + template = templateB + templateVersion = templateVersionB + } else { + template = templateA + templateVersion = templateVersionA + } + + workspace, err := db.InsertWorkspace(context.Background(), database.InsertWorkspaceParams{ + ID: uuid.New(), + OwnerID: insertUser(db).ID, + Name: uuid.NewString(), + TemplateID: template, + AutomaticUpdates: database.AutomaticUpdatesNever, + }) + require.NoError(t, err) + + job, err := db.InsertProvisionerJob(context.Background(), database.InsertProvisionerJobParams{ + ID: uuid.New(), + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + Provisioner: database.ProvisionerTypeEcho, + StorageMethod: database.ProvisionerStorageMethodFile, + Type: database.ProvisionerJobTypeWorkspaceBuild, + }) + require.NoError(t, err) + err = db.InsertWorkspaceBuild(context.Background(), database.InsertWorkspaceBuildParams{ + ID: uuid.New(), + WorkspaceID: workspace.ID, + JobID: job.ID, + BuildNumber: 1, + Transition: database.WorkspaceTransitionStart, + Reason: database.BuildReasonInitiator, + TemplateVersionID: templateVersion, + }) + require.NoError(t, err) + // This marks the job as started. + _, err = db.AcquireProvisionerJob(context.Background(), database.AcquireProvisionerJobParams{ + OrganizationID: job.OrganizationID, + StartedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + Types: []database.ProvisionerType{database.ProvisionerTypeEcho}, + }) + require.NoError(t, err) + return job + } + + insertCanceled := func(db database.Store) { + job := insertRunning(db) + err := db.UpdateProvisionerJobWithCancelByID(context.Background(), database.UpdateProvisionerJobWithCancelByIDParams{ + ID: job.ID, + CanceledAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(t, err) + err = db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(t, err) + } + + insertFailed := func(db database.Store) { + job := insertRunning(db) + err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + Error: sql.NullString{ + String: "failed", + Valid: true, + }, + }) + require.NoError(t, err) + } + + insertSuccess := func(db database.Store) { + job := insertRunning(db) + err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(t, err) + } + + for _, tc := range []struct { + Name string + Database func() database.Store + ExpectedSeries int + ExpectedStatuses map[codersdk.ProvisionerJobStatus]int + ExpectedWorkspaces int + }{{ + Name: "None", + Database: func() database.Store { + return dbmem.New() + }, + ExpectedSeries: 0, + ExpectedWorkspaces: 0, + }, { + Name: "Multiple", + Database: func() database.Store { + db := dbmem.New() + insertTemplates(db) + insertCanceled(db) + insertFailed(db) + insertFailed(db) + insertSuccess(db) + insertSuccess(db) + insertSuccess(db) + insertRunning(db) + return db + }, + ExpectedSeries: 7, + ExpectedWorkspaces: 7, + ExpectedStatuses: map[codersdk.ProvisionerJobStatus]int{ + codersdk.ProvisionerJobCanceled: 1, + codersdk.ProvisionerJobFailed: 2, + codersdk.ProvisionerJobSucceeded: 3, + codersdk.ProvisionerJobRunning: 1, + }, + }} { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + registry := prometheus.NewRegistry() + closeFunc, err := prometheusmetrics.Workspaces(context.Background(), slogtest.Make(t, nil), registry, tc.Database(), testutil.IntervalFast) + require.NoError(t, err) + t.Cleanup(closeFunc) + + require.Eventually(t, func() bool { + metrics, err := registry.Gather() + assert.NoError(t, err) + stMap := map[codersdk.ProvisionerJobStatus]int{} + wMap := map[string]struct{}{} + for _, m := range metrics { + if m.GetName() != "coderd_api_workspace_detail" { + continue + } + + for _, metric := range m.Metric { + for _, l := range metric.Label { + if l == nil { + continue + } + + switch l.GetName() { + case "status": + status := codersdk.ProvisionerJobStatus(l.GetValue()) + stMap[status] += int(metric.Gauge.GetValue()) + case "workspace_name": + wMap[l.GetValue()] = struct{}{} + } + } + } + } + + stSum := 0 + for st, count := range stMap { + if tc.ExpectedStatuses[st] != count { + return false + } + + stSum += count + } + + t.Logf("status series = %d, expected == %d", stSum, tc.ExpectedSeries) + t.Logf("workspace series = %d, expected == %d", len(wMap), tc.ExpectedWorkspaces) + return stSum == tc.ExpectedSeries && len(wMap) == tc.ExpectedWorkspaces + }, testutil.WaitSuperShort, testutil.IntervalFast) }) } } diff --git a/testutil/duration.go b/testutil/duration.go index 44ae418eba74f..528ec63986d58 100644 --- a/testutil/duration.go +++ b/testutil/duration.go @@ -9,10 +9,11 @@ import ( // Constants for timing out operations, usable for creating contexts // that timeout or in require.Eventually. const ( - WaitShort = 10 * time.Second - WaitMedium = 15 * time.Second - WaitLong = 25 * time.Second - WaitSuperLong = 60 * time.Second + WaitSuperShort = time.Second + WaitShort = 10 * time.Second + WaitMedium = 15 * time.Second + WaitLong = 25 * time.Second + WaitSuperLong = 60 * time.Second ) // Constants for delaying repeated operations, e.g. in From fc61d377974b4beb6007c26ac4abe6128507a626 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 26 Mar 2024 13:26:20 +0200 Subject: [PATCH 05/13] Refactor common funcs Signed-off-by: Danny Kopping --- .../prometheusmetrics_test.go | 404 +++++++----------- 1 file changed, 163 insertions(+), 241 deletions(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index bb138f0b19a57..4fca60e5bf039 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -115,86 +115,6 @@ func TestActiveUsers(t *testing.T) { func TestWorkspaceStatuses(t *testing.T) { t.Parallel() - insertRunning := func(db database.Store) database.ProvisionerJob { - job, err := db.InsertProvisionerJob(context.Background(), database.InsertProvisionerJobParams{ - ID: uuid.New(), - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - Provisioner: database.ProvisionerTypeEcho, - StorageMethod: database.ProvisionerStorageMethodFile, - Type: database.ProvisionerJobTypeWorkspaceBuild, - }) - require.NoError(t, err) - err = db.InsertWorkspaceBuild(context.Background(), database.InsertWorkspaceBuildParams{ - ID: uuid.New(), - WorkspaceID: uuid.New(), - JobID: job.ID, - BuildNumber: 1, - Transition: database.WorkspaceTransitionStart, - Reason: database.BuildReasonInitiator, - }) - require.NoError(t, err) - // This marks the job as started. - _, err = db.AcquireProvisionerJob(context.Background(), database.AcquireProvisionerJobParams{ - OrganizationID: job.OrganizationID, - StartedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - Types: []database.ProvisionerType{database.ProvisionerTypeEcho}, - }) - require.NoError(t, err) - return job - } - - insertCanceled := func(db database.Store) { - job := insertRunning(db) - err := db.UpdateProvisionerJobWithCancelByID(context.Background(), database.UpdateProvisionerJobWithCancelByIDParams{ - ID: job.ID, - CanceledAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - }) - require.NoError(t, err) - err = db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ - ID: job.ID, - CompletedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - }) - require.NoError(t, err) - } - - insertFailed := func(db database.Store) { - job := insertRunning(db) - err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ - ID: job.ID, - CompletedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - Error: sql.NullString{ - String: "failed", - Valid: true, - }, - }) - require.NoError(t, err) - } - - insertSuccess := func(db database.Store) { - job := insertRunning(db) - err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ - ID: job.ID, - CompletedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - }) - require.NoError(t, err) - } - for _, tc := range []struct { Name string Database func() database.Store @@ -210,13 +130,13 @@ func TestWorkspaceStatuses(t *testing.T) { Name: "Multiple", Database: func() database.Store { db := dbmem.New() - insertCanceled(db) - insertFailed(db) - insertFailed(db) - insertSuccess(db) - insertSuccess(db) - insertSuccess(db) - insertRunning(db) + insertCanceled(t, db) + insertFailed(t, db) + insertFailed(t, db) + insertSuccess(t, db) + insertSuccess(t, db) + insertSuccess(t, db) + insertRunning(t, db) return db }, Total: 7, @@ -268,152 +188,6 @@ func TestWorkspaceStatuses(t *testing.T) { func TestWorkspaceDetails(t *testing.T) { t.Parallel() - templateA := uuid.New() - templateVersionA := uuid.New() - templateB := uuid.New() - templateVersionB := uuid.New() - - insertTemplates := func(db database.Store) { - require.NoError(t, db.InsertTemplate(context.Background(), database.InsertTemplateParams{ - ID: templateA, - Name: "template-a", - Provisioner: database.ProvisionerTypeTerraform, - MaxPortSharingLevel: database.AppSharingLevelAuthenticated, - })) - - require.NoError(t, db.InsertTemplateVersion(context.Background(), database.InsertTemplateVersionParams{ - ID: templateVersionA, - TemplateID: uuid.NullUUID{UUID: templateA}, - Name: "version-1a", - })) - - require.NoError(t, db.InsertTemplate(context.Background(), database.InsertTemplateParams{ - ID: templateB, - Name: "template-b", - Provisioner: database.ProvisionerTypeTerraform, - MaxPortSharingLevel: database.AppSharingLevelAuthenticated, - })) - - require.NoError(t, db.InsertTemplateVersion(context.Background(), database.InsertTemplateVersionParams{ - ID: templateVersionB, - TemplateID: uuid.NullUUID{UUID: templateB}, - Name: "version-1b", - })) - } - - insertUser := func(db database.Store) database.User { - username, err := cryptorand.String(8) - require.NoError(t, err) - - user, err := db.InsertUser(context.Background(), database.InsertUserParams{ - ID: uuid.New(), - Username: username, - LoginType: database.LoginTypeNone, - }) - require.NoError(t, err) - - return user - } - - insertRunning := func(db database.Store) database.ProvisionerJob { - var template, templateVersion uuid.UUID - if rand.Intn(10) > 5 { - template = templateB - templateVersion = templateVersionB - } else { - template = templateA - templateVersion = templateVersionA - } - - workspace, err := db.InsertWorkspace(context.Background(), database.InsertWorkspaceParams{ - ID: uuid.New(), - OwnerID: insertUser(db).ID, - Name: uuid.NewString(), - TemplateID: template, - AutomaticUpdates: database.AutomaticUpdatesNever, - }) - require.NoError(t, err) - - job, err := db.InsertProvisionerJob(context.Background(), database.InsertProvisionerJobParams{ - ID: uuid.New(), - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - Provisioner: database.ProvisionerTypeEcho, - StorageMethod: database.ProvisionerStorageMethodFile, - Type: database.ProvisionerJobTypeWorkspaceBuild, - }) - require.NoError(t, err) - err = db.InsertWorkspaceBuild(context.Background(), database.InsertWorkspaceBuildParams{ - ID: uuid.New(), - WorkspaceID: workspace.ID, - JobID: job.ID, - BuildNumber: 1, - Transition: database.WorkspaceTransitionStart, - Reason: database.BuildReasonInitiator, - TemplateVersionID: templateVersion, - }) - require.NoError(t, err) - // This marks the job as started. - _, err = db.AcquireProvisionerJob(context.Background(), database.AcquireProvisionerJobParams{ - OrganizationID: job.OrganizationID, - StartedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - Types: []database.ProvisionerType{database.ProvisionerTypeEcho}, - }) - require.NoError(t, err) - return job - } - - insertCanceled := func(db database.Store) { - job := insertRunning(db) - err := db.UpdateProvisionerJobWithCancelByID(context.Background(), database.UpdateProvisionerJobWithCancelByIDParams{ - ID: job.ID, - CanceledAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - }) - require.NoError(t, err) - err = db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ - ID: job.ID, - CompletedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - }) - require.NoError(t, err) - } - - insertFailed := func(db database.Store) { - job := insertRunning(db) - err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ - ID: job.ID, - CompletedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - Error: sql.NullString{ - String: "failed", - Valid: true, - }, - }) - require.NoError(t, err) - } - - insertSuccess := func(db database.Store) { - job := insertRunning(db) - err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ - ID: job.ID, - CompletedAt: sql.NullTime{ - Time: dbtime.Now(), - Valid: true, - }, - }) - require.NoError(t, err) - } - for _, tc := range []struct { Name string Database func() database.Store @@ -431,14 +205,14 @@ func TestWorkspaceDetails(t *testing.T) { Name: "Multiple", Database: func() database.Store { db := dbmem.New() - insertTemplates(db) - insertCanceled(db) - insertFailed(db) - insertFailed(db) - insertSuccess(db) - insertSuccess(db) - insertSuccess(db) - insertRunning(db) + insertTemplates(t, db) + insertCanceled(t, db) + insertFailed(t, db) + insertFailed(t, db) + insertSuccess(t, db) + insertSuccess(t, db) + insertSuccess(t, db) + insertRunning(t, db) return db }, ExpectedSeries: 7, @@ -843,3 +617,151 @@ func prepareWorkspaceAndAgent(t *testing.T, client *codersdk.Client, user coders agentClient.SetSessionToken(authToken) return agentClient } + +var ( + templateA = uuid.New() + templateVersionA = uuid.New() + templateB = uuid.New() + templateVersionB = uuid.New() +) + +func insertTemplates(t *testing.T, db database.Store) { + require.NoError(t, db.InsertTemplate(context.Background(), database.InsertTemplateParams{ + ID: templateA, + Name: "template-a", + Provisioner: database.ProvisionerTypeTerraform, + MaxPortSharingLevel: database.AppSharingLevelAuthenticated, + })) + + require.NoError(t, db.InsertTemplateVersion(context.Background(), database.InsertTemplateVersionParams{ + ID: templateVersionA, + TemplateID: uuid.NullUUID{UUID: templateA}, + Name: "version-1a", + })) + + require.NoError(t, db.InsertTemplate(context.Background(), database.InsertTemplateParams{ + ID: templateB, + Name: "template-b", + Provisioner: database.ProvisionerTypeTerraform, + MaxPortSharingLevel: database.AppSharingLevelAuthenticated, + })) + + require.NoError(t, db.InsertTemplateVersion(context.Background(), database.InsertTemplateVersionParams{ + ID: templateVersionB, + TemplateID: uuid.NullUUID{UUID: templateB}, + Name: "version-1b", + })) +} + +func insertUser(t *testing.T, db database.Store) database.User { + username, err := cryptorand.String(8) + require.NoError(t, err) + + user, err := db.InsertUser(context.Background(), database.InsertUserParams{ + ID: uuid.New(), + Username: username, + LoginType: database.LoginTypeNone, + }) + require.NoError(t, err) + + return user +} + +func insertRunning(t *testing.T, db database.Store) database.ProvisionerJob { + var template, templateVersion uuid.UUID + if rand.Intn(10) > 5 { + template = templateB + templateVersion = templateVersionB + } else { + template = templateA + templateVersion = templateVersionA + } + + workspace, err := db.InsertWorkspace(context.Background(), database.InsertWorkspaceParams{ + ID: uuid.New(), + OwnerID: insertUser(t, db).ID, + Name: uuid.NewString(), + TemplateID: template, + AutomaticUpdates: database.AutomaticUpdatesNever, + }) + require.NoError(t, err) + + job, err := db.InsertProvisionerJob(context.Background(), database.InsertProvisionerJobParams{ + ID: uuid.New(), + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + Provisioner: database.ProvisionerTypeEcho, + StorageMethod: database.ProvisionerStorageMethodFile, + Type: database.ProvisionerJobTypeWorkspaceBuild, + }) + require.NoError(t, err) + err = db.InsertWorkspaceBuild(context.Background(), database.InsertWorkspaceBuildParams{ + ID: uuid.New(), + WorkspaceID: workspace.ID, + JobID: job.ID, + BuildNumber: 1, + Transition: database.WorkspaceTransitionStart, + Reason: database.BuildReasonInitiator, + TemplateVersionID: templateVersion, + }) + require.NoError(t, err) + // This marks the job as started. + _, err = db.AcquireProvisionerJob(context.Background(), database.AcquireProvisionerJobParams{ + OrganizationID: job.OrganizationID, + StartedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + Types: []database.ProvisionerType{database.ProvisionerTypeEcho}, + }) + require.NoError(t, err) + return job +} + +func insertCanceled(t *testing.T, db database.Store) { + job := insertRunning(t, db) + err := db.UpdateProvisionerJobWithCancelByID(context.Background(), database.UpdateProvisionerJobWithCancelByIDParams{ + ID: job.ID, + CanceledAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(t, err) + err = db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(t, err) +} + +func insertFailed(t *testing.T, db database.Store) { + job := insertRunning(t, db) + err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + Error: sql.NullString{ + String: "failed", + Valid: true, + }, + }) + require.NoError(t, err) +} + +func insertSuccess(t *testing.T, db database.Store) { + job := insertRunning(t, db) + err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(t, err) +} From 6f9537100d42df720fb37974638855ccfac05f13 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 26 Mar 2024 14:04:49 +0200 Subject: [PATCH 06/13] Remove unnecessary metric from test Use stronger rand Signed-off-by: Danny Kopping --- cli/server_test.go | 6 ------ coderd/prometheusmetrics/prometheusmetrics_test.go | 5 +++-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/cli/server_test.go b/cli/server_test.go index 21fd076787732..d856d9b0338c0 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -973,7 +973,6 @@ func TestServer(t *testing.T) { scanner := bufio.NewScanner(res.Body) hasActiveUsers := false - hasWorkspaces := false for scanner.Scan() { // This metric is manually registered to be tracked in the server. That's // why we test it's tracked here. @@ -981,10 +980,6 @@ func TestServer(t *testing.T) { hasActiveUsers = true continue } - if strings.HasPrefix(scanner.Text(), "coderd_api_workspace_latest_build_total") { - hasWorkspaces = true - continue - } if strings.HasPrefix(scanner.Text(), "coderd_db_query_latencies_seconds") { t.Fatal("db metrics should not be tracked when --prometheus-collect-db-metrics is not enabled") } @@ -992,7 +987,6 @@ func TestServer(t *testing.T) { } require.NoError(t, scanner.Err()) require.True(t, hasActiveUsers) - require.True(t, hasWorkspaces) }) t.Run("DBMetricsEnabled", func(t *testing.T) { diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 4fca60e5bf039..f002dc9ca7387 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -5,7 +5,6 @@ import ( "database/sql" "encoding/json" "fmt" - "math/rand" "os" "reflect" "sync/atomic" @@ -669,7 +668,9 @@ func insertUser(t *testing.T, db database.Store) database.User { func insertRunning(t *testing.T, db database.Store) database.ProvisionerJob { var template, templateVersion uuid.UUID - if rand.Intn(10) > 5 { + rnd, err := cryptorand.Intn(10) + require.NoError(t, err) + if rnd > 5 { template = templateB templateVersion = templateVersionB } else { From f42af07c11ecc2164466981fd6d9aa3574970852 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 26 Mar 2024 15:20:39 +0200 Subject: [PATCH 07/13] Use NoLock variety to fix hung tests Signed-off-by: Danny Kopping --- coderd/database/dbmem/dbmem.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index bf8759feacf6f..ff33b01fe31c5 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -404,14 +404,14 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac } } - if pj, err := q.GetProvisionerJobByID(ctx, build.JobID); err == nil { + if pj, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID); err == nil { wr.LatestBuildStatus = database.NullProvisionerJobStatus{ProvisionerJobStatus: pj.JobStatus, Valid: true} } wr.LatestBuildTransition = build.Transition } - if u, err := q.GetUserByID(ctx, w.OwnerID); err == nil { + if u, err := q.getUserByIDNoLock(w.OwnerID); err == nil { wr.Username = u.Username } From 2cb8ccc3b83bceb8727165d6ee5d0ea0036253a4 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 26 Mar 2024 15:30:10 +0200 Subject: [PATCH 08/13] Fix duration on windows builds Signed-off-by: Danny Kopping --- testutil/duration_windows.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/testutil/duration_windows.go b/testutil/duration_windows.go index 4c3b85e67131c..3033bf086b755 100644 --- a/testutil/duration_windows.go +++ b/testutil/duration_windows.go @@ -7,10 +7,11 @@ import "time" // // Windows durations are adjusted for slow CI workers. const ( - WaitShort = 15 * time.Second - WaitMedium = 20 * time.Second - WaitLong = 35 * time.Second - WaitSuperLong = 120 * time.Second + WaitSuperShort = time.Second + WaitShort = 15 * time.Second + WaitMedium = 20 * time.Second + WaitLong = 35 * time.Second + WaitSuperLong = 120 * time.Second ) // Constants for delaying repeated operations, e.g. in From b118044549d20a76ffe6c1a45a440a64e3f7f9a9 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 27 Mar 2024 11:12:16 +0200 Subject: [PATCH 09/13] Changing join type so that latest_build_status is non-nullable Signed-off-by: Danny Kopping --- coderd/database/dbmem/dbmem.go | 2 +- coderd/database/queries.sql.go | 52 +++++++++---------- coderd/database/queries/workspaces.sql | 2 +- coderd/prometheusmetrics/prometheusmetrics.go | 10 +--- 4 files changed, 29 insertions(+), 37 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index ff33b01fe31c5..f578110459dfc 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -405,7 +405,7 @@ func (q *FakeQuerier) convertToWorkspaceRowsNoLock(ctx context.Context, workspac } if pj, err := q.getProvisionerJobByIDNoLock(ctx, build.JobID); err == nil { - wr.LatestBuildStatus = database.NullProvisionerJobStatus{ProvisionerJobStatus: pj.JobStatus, Valid: true} + wr.LatestBuildStatus = pj.JobStatus } wr.LatestBuildTransition = build.Transition diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index be777dcc330a4..cd6c6468d8c1b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -12201,7 +12201,7 @@ LEFT JOIN LATERAL ( provisioner_jobs.job_status FROM workspace_builds - LEFT JOIN + JOIN provisioner_jobs ON provisioner_jobs.id = workspace_builds.job_id @@ -12501,31 +12501,31 @@ 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"` - DormantAt sql.NullTime `db:"dormant_at" json:"dormant_at"` - DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` - AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` - Favorite bool `db:"favorite" json:"favorite"` - 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"` - Username string `db:"username" json:"username"` - LatestBuildCompletedAt sql.NullTime `db:"latest_build_completed_at" json:"latest_build_completed_at"` - LatestBuildCanceledAt sql.NullTime `db:"latest_build_canceled_at" json:"latest_build_canceled_at"` - LatestBuildError sql.NullString `db:"latest_build_error" json:"latest_build_error"` - LatestBuildTransition WorkspaceTransition `db:"latest_build_transition" json:"latest_build_transition"` - LatestBuildStatus NullProvisionerJobStatus `db:"latest_build_status" json:"latest_build_status"` - 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"` + DormantAt sql.NullTime `db:"dormant_at" json:"dormant_at"` + DeletingAt sql.NullTime `db:"deleting_at" json:"deleting_at"` + AutomaticUpdates AutomaticUpdates `db:"automatic_updates" json:"automatic_updates"` + Favorite bool `db:"favorite" json:"favorite"` + 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"` + Username string `db:"username" json:"username"` + LatestBuildCompletedAt sql.NullTime `db:"latest_build_completed_at" json:"latest_build_completed_at"` + LatestBuildCanceledAt sql.NullTime `db:"latest_build_canceled_at" json:"latest_build_canceled_at"` + LatestBuildError sql.NullString `db:"latest_build_error" json:"latest_build_error"` + LatestBuildTransition WorkspaceTransition `db:"latest_build_transition" json:"latest_build_transition"` + LatestBuildStatus ProvisionerJobStatus `db:"latest_build_status" json:"latest_build_status"` + Count int64 `db:"count" json:"count"` } // build_params is used to filter by build parameters if present. diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 29255d475b410..616e83a2bae16 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -119,7 +119,7 @@ LEFT JOIN LATERAL ( provisioner_jobs.job_status FROM workspace_builds - LEFT JOIN + JOIN provisioner_jobs ON provisioner_jobs.id = workspace_builds.job_id diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index 3e9ca9b1bb01c..50cdcee836d3f 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -152,15 +152,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R workspaceDetails.Reset() for _, w := range ws { - // TODO: there may be a more elegant/idiomatic way to do this? - buildStatus := string(database.ProvisionerJobStatusUnknown) - if val, err := w.LatestBuildStatus.Value(); err == nil { - if status, ok := val.(string); ok { - buildStatus = status - } - } - - workspaceDetails.WithLabelValues(buildStatus, w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1) + workspaceDetails.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1) } } From acd104c807e83f95b505bdda435911426fe4aeba Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 28 Mar 2024 07:13:11 +0200 Subject: [PATCH 10/13] Remove SuperShort duration as extraneous Signed-off-by: Danny Kopping --- coderd/prometheusmetrics/prometheusmetrics_test.go | 6 +++--- testutil/duration.go | 9 ++++----- testutil/duration_windows.go | 9 ++++----- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index f002dc9ca7387..0da57f2567c1b 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -150,7 +150,7 @@ func TestWorkspaceStatuses(t *testing.T) { t.Run(tc.Name, func(t *testing.T) { t.Parallel() registry := prometheus.NewRegistry() - closeFunc, err := prometheusmetrics.Workspaces(context.Background(), slogtest.Make(t, nil), registry, tc.Database(), testutil.IntervalFast) + closeFunc, err := prometheusmetrics.Workspaces(context.Background(), slogtest.Make(t, nil).Leveled(slog.LevelWarn), registry, tc.Database(), testutil.IntervalFast) require.NoError(t, err) t.Cleanup(closeFunc) @@ -179,7 +179,7 @@ func TestWorkspaceStatuses(t *testing.T) { } t.Logf("sum %d == total %d", sum, tc.Total) return sum == tc.Total - }, testutil.WaitSuperShort, testutil.IntervalFast) + }, testutil.WaitShort, testutil.IntervalFast) }) } } @@ -270,7 +270,7 @@ func TestWorkspaceDetails(t *testing.T) { t.Logf("status series = %d, expected == %d", stSum, tc.ExpectedSeries) t.Logf("workspace series = %d, expected == %d", len(wMap), tc.ExpectedWorkspaces) return stSum == tc.ExpectedSeries && len(wMap) == tc.ExpectedWorkspaces - }, testutil.WaitSuperShort, testutil.IntervalFast) + }, testutil.WaitShort, testutil.IntervalFast) }) } } diff --git a/testutil/duration.go b/testutil/duration.go index 528ec63986d58..44ae418eba74f 100644 --- a/testutil/duration.go +++ b/testutil/duration.go @@ -9,11 +9,10 @@ import ( // Constants for timing out operations, usable for creating contexts // that timeout or in require.Eventually. const ( - WaitSuperShort = time.Second - WaitShort = 10 * time.Second - WaitMedium = 15 * time.Second - WaitLong = 25 * time.Second - WaitSuperLong = 60 * time.Second + WaitShort = 10 * time.Second + WaitMedium = 15 * time.Second + WaitLong = 25 * time.Second + WaitSuperLong = 60 * time.Second ) // Constants for delaying repeated operations, e.g. in diff --git a/testutil/duration_windows.go b/testutil/duration_windows.go index 3033bf086b755..4c3b85e67131c 100644 --- a/testutil/duration_windows.go +++ b/testutil/duration_windows.go @@ -7,11 +7,10 @@ import "time" // // Windows durations are adjusted for slow CI workers. const ( - WaitSuperShort = time.Second - WaitShort = 15 * time.Second - WaitMedium = 20 * time.Second - WaitLong = 35 * time.Second - WaitSuperLong = 120 * time.Second + WaitShort = 15 * time.Second + WaitMedium = 20 * time.Second + WaitLong = 35 * time.Second + WaitSuperLong = 120 * time.Second ) // Constants for delaying repeated operations, e.g. in From a333f983c6b0bc401271836aaceedd1feba2560d Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 28 Mar 2024 07:38:58 +0200 Subject: [PATCH 11/13] Remove workspace cardinality; rename for clarity Signed-off-by: Danny Kopping --- coderd/prometheusmetrics/prometheusmetrics.go | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index 50cdcee836d3f..8330902f8c288 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -79,23 +79,22 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R duration = defaultRefreshRate } - workspaceStatuses := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + workspaceStatusTotals := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", Subsystem: "api", Name: "workspace_latest_build_total", Help: "The current number of workspace builds by status.", }, []string{"status"}) - if err := registerer.Register(workspaceStatuses); err != nil { + if err := registerer.Register(workspaceStatusTotals); err != nil { return nil, err } - workspaceDetails := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + workspaceStatuses := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", - Subsystem: "api", - Name: "workspace_detail", - Help: "The current workspace details by template, transition, owner, and status.", - }, []string{"status", "template_name", "template_version", "workspace_name", "workspace_owner", "workspace_transition"}) - if err := registerer.Register(workspaceDetails); err != nil { + Name: "workspace_latest_build_status", + Help: "The current workspace statuses by template, transition, and owner.", + }, []string{"status", "template_name", "template_version", "workspace_owner", "workspace_transition"}) + if err := registerer.Register(workspaceStatuses); err != nil { return nil, err } @@ -107,7 +106,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R if err != nil { if errors.Is(err, sql.ErrNoRows) { // clear all series if there are no database entries - workspaceStatuses.Reset() + workspaceStatusTotals.Reset() } logger.Warn(ctx, "failed to load latest workspace builds", slog.Error(err)) @@ -128,10 +127,10 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R return } - workspaceStatuses.Reset() + workspaceStatusTotals.Reset() for _, job := range jobs { status := codersdk.ProvisionerJobStatus(job.JobStatus) - workspaceStatuses.WithLabelValues(string(status)).Add(1) + workspaceStatusTotals.WithLabelValues(string(status)).Add(1) } } @@ -143,16 +142,16 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R if err != nil { if errors.Is(err, sql.ErrNoRows) { // clear all series if there are no database entries - workspaceDetails.Reset() + workspaceStatuses.Reset() } logger.Warn(ctx, "failed to load active workspaces", slog.Error(err)) return } - workspaceDetails.Reset() + workspaceStatuses.Reset() for _, w := range ws { - workspaceDetails.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.Name, w.Username, string(w.LatestBuildTransition)).Set(1) + workspaceStatuses.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.Username, string(w.LatestBuildTransition)).Add(1) } } From c920508c023a6ea267a0dc5e2f7a45762f9240e3 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 28 Mar 2024 08:13:35 +0200 Subject: [PATCH 12/13] Fixing tests Signed-off-by: Danny Kopping --- .../prometheusmetrics_test.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 0da57f2567c1b..5f4e8566b62eb 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -190,15 +190,13 @@ func TestWorkspaceDetails(t *testing.T) { for _, tc := range []struct { Name string Database func() database.Store - ExpectedSeries int - ExpectedStatuses map[codersdk.ProvisionerJobStatus]int ExpectedWorkspaces int + ExpectedStatuses map[codersdk.ProvisionerJobStatus]int }{{ Name: "None", Database: func() database.Store { return dbmem.New() }, - ExpectedSeries: 0, ExpectedWorkspaces: 0, }, { Name: "Multiple", @@ -214,7 +212,6 @@ func TestWorkspaceDetails(t *testing.T) { insertRunning(t, db) return db }, - ExpectedSeries: 7, ExpectedWorkspaces: 7, ExpectedStatuses: map[codersdk.ProvisionerJobStatus]int{ codersdk.ProvisionerJobCanceled: 1, @@ -234,10 +231,10 @@ func TestWorkspaceDetails(t *testing.T) { require.Eventually(t, func() bool { metrics, err := registry.Gather() assert.NoError(t, err) + stMap := map[codersdk.ProvisionerJobStatus]int{} - wMap := map[string]struct{}{} for _, m := range metrics { - if m.GetName() != "coderd_api_workspace_detail" { + if m.GetName() != "coderd_workspace_latest_build_status" { continue } @@ -247,12 +244,9 @@ func TestWorkspaceDetails(t *testing.T) { continue } - switch l.GetName() { - case "status": + if l.GetName() == "status" { status := codersdk.ProvisionerJobStatus(l.GetValue()) stMap[status] += int(metric.Gauge.GetValue()) - case "workspace_name": - wMap[l.GetValue()] = struct{}{} } } } @@ -267,9 +261,8 @@ func TestWorkspaceDetails(t *testing.T) { stSum += count } - t.Logf("status series = %d, expected == %d", stSum, tc.ExpectedSeries) - t.Logf("workspace series = %d, expected == %d", len(wMap), tc.ExpectedWorkspaces) - return stSum == tc.ExpectedSeries && len(wMap) == tc.ExpectedWorkspaces + t.Logf("status series = %d, expected == %d", stSum, tc.ExpectedWorkspaces) + return stSum == tc.ExpectedWorkspaces }, testutil.WaitShort, testutil.IntervalFast) }) } From 8e6cde9e7a2d94ca724e64bda44458126467dffb Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 28 Mar 2024 13:08:08 +0200 Subject: [PATCH 13/13] Renaming for clarity Signed-off-by: Danny Kopping --- coderd/prometheusmetrics/prometheusmetrics.go | 26 +++++++++---------- .../prometheusmetrics_test.go | 4 +-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index 8330902f8c288..4d3f1d1a042cc 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -79,34 +79,34 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R duration = defaultRefreshRate } - workspaceStatusTotals := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + workspaceLatestBuildTotals := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", Subsystem: "api", Name: "workspace_latest_build_total", Help: "The current number of workspace builds by status.", }, []string{"status"}) - if err := registerer.Register(workspaceStatusTotals); err != nil { + if err := registerer.Register(workspaceLatestBuildTotals); err != nil { return nil, err } - workspaceStatuses := prometheus.NewGaugeVec(prometheus.GaugeOpts{ + workspaceLatestBuildStatuses := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", Name: "workspace_latest_build_status", Help: "The current workspace statuses by template, transition, and owner.", }, []string{"status", "template_name", "template_version", "workspace_owner", "workspace_transition"}) - if err := registerer.Register(workspaceStatuses); err != nil { + if err := registerer.Register(workspaceLatestBuildStatuses); err != nil { return nil, err } ctx, cancelFunc := context.WithCancel(ctx) done := make(chan struct{}) - updateWorkspaceStatuses := func() { + updateWorkspaceTotals := func() { builds, err := db.GetLatestWorkspaceBuilds(ctx) if err != nil { if errors.Is(err, sql.ErrNoRows) { // clear all series if there are no database entries - workspaceStatusTotals.Reset() + workspaceLatestBuildTotals.Reset() } logger.Warn(ctx, "failed to load latest workspace builds", slog.Error(err)) @@ -127,14 +127,14 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R return } - workspaceStatusTotals.Reset() + workspaceLatestBuildTotals.Reset() for _, job := range jobs { status := codersdk.ProvisionerJobStatus(job.JobStatus) - workspaceStatusTotals.WithLabelValues(string(status)).Add(1) + workspaceLatestBuildTotals.WithLabelValues(string(status)).Add(1) } } - updateWorkspaceDetails := func() { + updateWorkspaceStatuses := func() { ws, err := db.GetWorkspaces(ctx, database.GetWorkspacesParams{ Deleted: false, WithSummary: false, @@ -142,16 +142,16 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R if err != nil { if errors.Is(err, sql.ErrNoRows) { // clear all series if there are no database entries - workspaceStatuses.Reset() + workspaceLatestBuildStatuses.Reset() } logger.Warn(ctx, "failed to load active workspaces", slog.Error(err)) return } - workspaceStatuses.Reset() + workspaceLatestBuildStatuses.Reset() for _, w := range ws { - workspaceStatuses.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.Username, string(w.LatestBuildTransition)).Add(1) + workspaceLatestBuildStatuses.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.Username, string(w.LatestBuildTransition)).Add(1) } } @@ -161,8 +161,8 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R doTick := func() { defer ticker.Reset(duration) + updateWorkspaceTotals() updateWorkspaceStatuses() - updateWorkspaceDetails() } go func() { diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 5f4e8566b62eb..0ca7884cfbd8c 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -111,7 +111,7 @@ func TestActiveUsers(t *testing.T) { } } -func TestWorkspaceStatuses(t *testing.T) { +func TestWorkspaceLatestBuildTotals(t *testing.T) { t.Parallel() for _, tc := range []struct { @@ -184,7 +184,7 @@ func TestWorkspaceStatuses(t *testing.T) { } } -func TestWorkspaceDetails(t *testing.T) { +func TestWorkspaceLatestBuildStatuses(t *testing.T) { t.Parallel() for _, tc := range []struct {