fix(coderd/prometheusmetrics)!: filter deleted wsbuilds to reduce db load (#19197)

This change removes the `GetLatestWorkspaceBuilds` query which includes
all workspaces for all time (including deleted). This allows us to also
stop using `GetProvisionerJobsByIDs` for said builds as the job status
is included in `GetWorkspaces` called separately.

**BREAKING CHANGE**: The `coderd_api_workspace_latest_build` Prometheus
metric no longer includes builds belonging to deleted workspaces, as
such, this metric will show fewer statuses.

Fixes coder/internal#717

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 {

coderd/database/dbauthz/dbauthz_test.go

@@ -4035,12 +4035,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))
 	}))

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)

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()

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)

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 (

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 (

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() {

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)
+}