diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index d5cc334f5ff7f..a4759c8636097 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2178,17 +2178,6 @@ func (q *querier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, work return q.db.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspaceID) } -func (q *querier) GetLatestWorkspaceBuilds(ctx context.Context) ([]database.WorkspaceBuild, error) { - // This function is a system function until we implement a join for workspace builds. - // This is because we need to query for all related workspaces to the returned builds. - // This is a very inefficient method of fetching the latest workspace builds. - // We should just join the rbac properties. - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err - } - return q.db.GetLatestWorkspaceBuilds(ctx) -} - func (q *querier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) { // This function is a system function until we implement a join for workspace builds. if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 82b7b47c892b2..322f33404a824 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -4033,12 +4033,6 @@ func (s *MethodTestSuite) TestSystemFunctions() { LoginType: l.LoginType, }).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(l) })) - s.Run("GetLatestWorkspaceBuilds", s.Subtest(func(db database.Store, check *expects) { - dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) - dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) - dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{}) - check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead) - })) s.Run("GetActiveUserCount", s.Subtest(func(db database.Store, check *expects) { check.Args(false).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(int64(0)) })) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index e0606f9e40665..cb61df85db007 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -943,13 +943,6 @@ func (m queryMetricsStore) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Cont return build, err } -func (m queryMetricsStore) GetLatestWorkspaceBuilds(ctx context.Context) ([]database.WorkspaceBuild, error) { - start := time.Now() - builds, err := m.s.GetLatestWorkspaceBuilds(ctx) - m.queryLatencies.WithLabelValues("GetLatestWorkspaceBuilds").Observe(time.Since(start).Seconds()) - return builds, err -} - func (m queryMetricsStore) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) { start := time.Now() builds, err := m.s.GetLatestWorkspaceBuildsByWorkspaceIDs(ctx, ids) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 22807f0e3569d..0966df6acfba4 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1966,21 +1966,6 @@ func (mr *MockStoreMockRecorder) GetLatestWorkspaceBuildByWorkspaceID(ctx, works return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestWorkspaceBuildByWorkspaceID", reflect.TypeOf((*MockStore)(nil).GetLatestWorkspaceBuildByWorkspaceID), ctx, workspaceID) } -// GetLatestWorkspaceBuilds mocks base method. -func (m *MockStore) GetLatestWorkspaceBuilds(ctx context.Context) ([]database.WorkspaceBuild, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetLatestWorkspaceBuilds", ctx) - ret0, _ := ret[0].([]database.WorkspaceBuild) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetLatestWorkspaceBuilds indicates an expected call of GetLatestWorkspaceBuilds. -func (mr *MockStoreMockRecorder) GetLatestWorkspaceBuilds(ctx any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestWorkspaceBuilds", reflect.TypeOf((*MockStore)(nil).GetLatestWorkspaceBuilds), ctx) -} - // GetLatestWorkspaceBuildsByWorkspaceIDs mocks base method. func (m *MockStore) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index a0f265e9658ce..042bee9d3701b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -213,7 +213,6 @@ type sqlcQuerier interface { GetLatestCryptoKeyByFeature(ctx context.Context, feature CryptoKeyFeature) (CryptoKey, error) GetLatestWorkspaceAppStatusesByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAppStatus, error) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) - GetLatestWorkspaceBuilds(ctx context.Context) ([]WorkspaceBuild, error) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceBuild, error) GetLicenseByID(ctx context.Context, id int32) (License, error) GetLicenses(ctx context.Context) ([]License, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 74cefd09359b0..eef7b8b6819d0 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -18664,65 +18664,6 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, w return i, err } -const getLatestWorkspaceBuilds = `-- name: GetLatestWorkspaceBuilds :many -SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline, wb.reason, wb.daily_cost, wb.max_deadline, wb.template_version_preset_id, wb.has_ai_task, wb.ai_task_sidebar_app_id, wb.initiator_by_avatar_url, wb.initiator_by_username, wb.initiator_by_name -FROM ( - SELECT - workspace_id, MAX(build_number) as max_build_number - FROM - workspace_build_with_user AS workspace_builds - GROUP BY - workspace_id -) m -JOIN - workspace_build_with_user AS wb -ON m.workspace_id = wb.workspace_id AND m.max_build_number = wb.build_number -` - -func (q *sqlQuerier) GetLatestWorkspaceBuilds(ctx context.Context) ([]WorkspaceBuild, error) { - rows, err := q.db.QueryContext(ctx, getLatestWorkspaceBuilds) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceBuild - for rows.Next() { - var i WorkspaceBuild - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.UpdatedAt, - &i.WorkspaceID, - &i.TemplateVersionID, - &i.BuildNumber, - &i.Transition, - &i.InitiatorID, - &i.ProvisionerState, - &i.JobID, - &i.Deadline, - &i.Reason, - &i.DailyCost, - &i.MaxDeadline, - &i.TemplateVersionPresetID, - &i.HasAITask, - &i.AITaskSidebarAppID, - &i.InitiatorByAvatarUrl, - &i.InitiatorByUsername, - &i.InitiatorByName, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const getLatestWorkspaceBuildsByWorkspaceIDs = `-- name: GetLatestWorkspaceBuildsByWorkspaceIDs :many SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline, wb.reason, wb.daily_cost, wb.max_deadline, wb.template_version_preset_id, wb.has_ai_task, wb.ai_task_sidebar_app_id, wb.initiator_by_avatar_url, wb.initiator_by_username, wb.initiator_by_name FROM ( diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index be76b6642df1f..be7bec5fa08f2 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -91,20 +91,6 @@ JOIN workspace_build_with_user AS wb ON m.workspace_id = wb.workspace_id AND m.max_build_number = wb.build_number; --- name: GetLatestWorkspaceBuilds :many -SELECT wb.* -FROM ( - SELECT - workspace_id, MAX(build_number) as max_build_number - FROM - workspace_build_with_user AS workspace_builds - GROUP BY - workspace_id -) m -JOIN - workspace_build_with_user AS wb -ON m.workspace_id = wb.workspace_id AND m.max_build_number = wb.build_number; - -- name: InsertWorkspaceBuild :exec INSERT INTO workspace_builds ( diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index 4fd2cfda607ed..cda274145e159 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -150,7 +150,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R Namespace: "coderd", Subsystem: "api", Name: "workspace_latest_build", - Help: "The current number of workspace builds by status.", + Help: "The current number of workspace builds by status for all non-deleted workspaces.", }, []string{"status"}) if err := registerer.Register(workspaceLatestBuildTotals); err != nil { return nil, err @@ -159,7 +159,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R workspaceLatestBuildStatuses := prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: "coderd", Name: "workspace_latest_build_status", - Help: "The current workspace statuses by template, transition, and owner.", + Help: "The current workspace statuses by template, transition, and owner for all non-deleted workspaces.", }, []string{"status", "template_name", "template_version", "workspace_owner", "workspace_transition"}) if err := registerer.Register(workspaceLatestBuildStatuses); err != nil { return nil, err @@ -168,59 +168,37 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R ctx, cancelFunc := context.WithCancel(ctx) done := make(chan struct{}) - 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 - workspaceLatestBuildTotals.Reset() - } else { - logger.Warn(ctx, "failed to load latest workspace builds", slog.Error(err)) - } - return - } - jobIDs := make([]uuid.UUID, 0, len(builds)) - for _, build := range builds { - jobIDs = append(jobIDs, build.JobID) - } - 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 - } - - workspaceLatestBuildTotals.Reset() - for _, job := range jobs { - status := codersdk.ProvisionerJobStatus(job.JobStatus) - workspaceLatestBuildTotals.WithLabelValues(string(status)).Add(1) - // TODO: deprecated: remove in the future - workspaceLatestBuildTotalsDeprecated.WithLabelValues(string(status)).Add(1) - } - } - - updateWorkspaceStatuses := func() { + updateWorkspaceMetrics := 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 + workspaceLatestBuildTotals.Reset() workspaceLatestBuildStatuses.Reset() + } else { + logger.Warn(ctx, "failed to load active workspaces for metrics", slog.Error(err)) } - - logger.Warn(ctx, "failed to load active workspaces", slog.Error(err)) return } + workspaceLatestBuildTotals.Reset() workspaceLatestBuildStatuses.Reset() + for _, w := range ws { - workspaceLatestBuildStatuses.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.OwnerUsername, string(w.LatestBuildTransition)).Add(1) + status := string(w.LatestBuildStatus) + workspaceLatestBuildTotals.WithLabelValues(status).Add(1) + // TODO: deprecated: remove in the future + workspaceLatestBuildTotalsDeprecated.WithLabelValues(status).Add(1) + + workspaceLatestBuildStatuses.WithLabelValues( + status, + w.TemplateName, + w.TemplateVersionName.String, + w.OwnerUsername, + string(w.LatestBuildTransition), + ).Add(1) } } @@ -230,8 +208,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R doTick := func() { defer ticker.Reset(duration) - updateWorkspaceTotals() - updateWorkspaceStatuses() + updateWorkspaceMetrics() } go func() { diff --git a/coderd/prometheusmetrics/prometheusmetrics_test.go b/coderd/prometheusmetrics/prometheusmetrics_test.go index 473dbf46bd958..28046c1dff3fb 100644 --- a/coderd/prometheusmetrics/prometheusmetrics_test.go +++ b/coderd/prometheusmetrics/prometheusmetrics_test.go @@ -247,6 +247,32 @@ func TestWorkspaceLatestBuildTotals(t *testing.T) { codersdk.ProvisionerJobSucceeded: 3, codersdk.ProvisionerJobRunning: 1, }, + }, { + Name: "MultipleWithDeleted", + Database: func() database.Store { + db, _ := dbtestutil.NewDB(t) + u := dbgen.User(t, db, database.User{}) + org := dbgen.Organization(t, db, database.Organization{}) + insertCanceled(t, db, u, org) + insertFailed(t, db, u, org) + insertSuccess(t, db, u, org) + insertRunning(t, db, u, org) + + // Verify that deleted workspaces/builds are NOT counted in metrics. + n, err := cryptorand.Intn(5) + require.NoError(t, err) + for range 1 + n { + insertDeleted(t, db, u, org) + } + return db + }, + Total: 4, // Only non-deleted workspaces should be counted + Status: map[codersdk.ProvisionerJobStatus]int{ + codersdk.ProvisionerJobCanceled: 1, + codersdk.ProvisionerJobFailed: 1, + codersdk.ProvisionerJobSucceeded: 1, + codersdk.ProvisionerJobRunning: 1, + }, }} { t.Run(tc.Name, func(t *testing.T) { t.Parallel() @@ -323,6 +349,33 @@ func TestWorkspaceLatestBuildStatuses(t *testing.T) { codersdk.ProvisionerJobSucceeded: 3, codersdk.ProvisionerJobRunning: 1, }, + }, { + Name: "MultipleWithDeleted", + Database: func() database.Store { + db, _ := dbtestutil.NewDB(t) + u := dbgen.User(t, db, database.User{}) + org := dbgen.Organization(t, db, database.Organization{}) + insertTemplates(t, db, u, org) + insertCanceled(t, db, u, org) + insertFailed(t, db, u, org) + insertSuccess(t, db, u, org) + insertRunning(t, db, u, org) + + // Verify that deleted workspaces/builds are NOT counted in metrics. + n, err := cryptorand.Intn(5) + require.NoError(t, err) + for range 1 + n { + insertDeleted(t, db, u, org) + } + return db + }, + ExpectedWorkspaces: 4, // Only non-deleted workspaces should be counted + ExpectedStatuses: map[codersdk.ProvisionerJobStatus]int{ + codersdk.ProvisionerJobCanceled: 1, + codersdk.ProvisionerJobFailed: 1, + codersdk.ProvisionerJobSucceeded: 1, + codersdk.ProvisionerJobRunning: 1, + }, }} { t.Run(tc.Name, func(t *testing.T) { t.Parallel() @@ -907,3 +960,24 @@ func insertSuccess(t *testing.T, db database.Store, u database.User, org databas }) require.NoError(t, err) } + +func insertDeleted(t *testing.T, db database.Store, u database.User, org database.Organization) { + job := insertRunning(t, db, u, org) + err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{ + Time: dbtime.Now(), + Valid: true, + }, + }) + require.NoError(t, err) + + build, err := db.GetWorkspaceBuildByJobID(context.Background(), job.ID) + require.NoError(t, err) + + err = db.UpdateWorkspaceDeletedByID(context.Background(), database.UpdateWorkspaceDeletedByIDParams{ + ID: build.WorkspaceID, + Deleted: true, + }) + require.NoError(t, err) +}