Skip to content

Commit 353f5de

Browse files
ssncferreirassncferreira
authored
fix(coderd): fix logic for reporting prebuilt workspace duration metric (coder#19641)
## Description When creating a prebuilt workspace, both `flags.IsPrebuild` and `flags.IsFirstBuild` are true. Previously, the logic rejected cases with multiple flags, so `coderd_workspace_creation_duration_seconds` wasn’t updated for prebuilt creations. This is the only valid scenario where two flags can be true. ## Changes * Fix logic to update `coderd_workspace_creation_duration_seconds` metric for prebuilt workspaces. * Add prebuild helper functions to coderdenttest (other prebuild tests can reuse this). * Update workspace's provisionerdmetric tests to include this metric. Follow-up: coder#19503 Related to: coder#19528
1 parent 02ecf32 commit 353f5de

File tree

3 files changed

+185
-96
lines changed

3 files changed

+185
-96
lines changed

‎coderd/provisionerdserver/metrics.go

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -100,25 +100,14 @@ func (m *Metrics) Register(reg prometheus.Registerer) error {
100100
return reg.Register(m.workspaceClaimTimings)
101101
}
102102

103-
func (f WorkspaceTimingFlags) count() int {
104-
count := 0
105-
if f.IsPrebuild {
106-
count++
107-
}
108-
if f.IsClaim {
109-
count++
110-
}
111-
if f.IsFirstBuild {
112-
count++
113-
}
114-
return count
115-
}
116-
117-
// getWorkspaceTimingType returns the type of the workspace build:
118-
// - isPrebuild: if the workspace build corresponds to the creation of a prebuilt workspace
119-
// - isClaim: if the workspace build corresponds to the claim of a prebuilt workspace
120-
// - isWorkspaceFirstBuild: if the workspace build corresponds to the creation of a regular workspace
121-
// (not created from the prebuild pool)
103+
// getWorkspaceTimingType classifies a workspace build:
104+
// - PrebuildCreation: creation of a prebuilt workspace
105+
// - PrebuildClaim: claim of an existing prebuilt workspace
106+
// - WorkspaceCreation: first build of a regular (non-prebuilt) workspace
107+
//
108+
// Note: order matters. Creating a prebuilt workspace is also a first build
109+
// (IsPrebuild && IsFirstBuild). We check IsPrebuild before IsFirstBuild so
110+
// prebuilds take precedence. This is the only case where two flags can be true.
122111
func getWorkspaceTimingType(flags WorkspaceTimingFlags) WorkspaceTimingType {
123112
switch {
124113
case flags.IsPrebuild:
@@ -149,14 +138,6 @@ func (m *Metrics) UpdateWorkspaceTimingsMetrics(
149138
"isClaim", flags.IsClaim,
150139
"isWorkspaceFirstBuild", flags.IsFirstBuild)
151140

152-
if flags.count() > 1 {
153-
m.logger.Warn(ctx, "invalid workspace timing flags",
154-
"isPrebuild", flags.IsPrebuild,
155-
"isClaim", flags.IsClaim,
156-
"isWorkspaceFirstBuild", flags.IsFirstBuild)
157-
return
158-
}
159-
160141
workspaceTimingType := getWorkspaceTimingType(flags)
161142
switch workspaceTimingType {
162143
case WorkspaceCreation:

‎enterprise/coderd/coderdenttest/coderdenttest.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"crypto/ed25519"
66
"crypto/rand"
77
"crypto/tls"
8+
"database/sql"
89
"io"
910
"net/http"
1011
"os/exec"
@@ -23,10 +24,13 @@ import (
2324
"github.com/coder/coder/v2/coderd/coderdtest"
2425
"github.com/coder/coder/v2/coderd/database"
2526
"github.com/coder/coder/v2/coderd/database/pubsub"
27+
"github.com/coder/coder/v2/coderd/prebuilds"
28+
"github.com/coder/coder/v2/coderd/util/ptr"
2629
"github.com/coder/coder/v2/codersdk"
2730
"github.com/coder/coder/v2/codersdk/drpcsdk"
2831
"github.com/coder/coder/v2/enterprise/coderd"
2932
"github.com/coder/coder/v2/enterprise/coderd/license"
33+
entprebuilds "github.com/coder/coder/v2/enterprise/coderd/prebuilds"
3034
"github.com/coder/coder/v2/enterprise/dbcrypt"
3135
"github.com/coder/coder/v2/provisioner/echo"
3236
"github.com/coder/coder/v2/provisioner/terraform"
@@ -446,3 +450,98 @@ func newExternalProvisionerDaemon(t testing.TB, client *codersdk.Client, org uui
446450

447451
return closer
448452
}
453+
454+
func GetRunningPrebuilds(
455+
ctx context.Context,
456+
t *testing.T,
457+
db database.Store,
458+
desiredPrebuilds int,
459+
) []database.GetRunningPrebuiltWorkspacesRow {
460+
t.Helper()
461+
462+
var runningPrebuilds []database.GetRunningPrebuiltWorkspacesRow
463+
testutil.Eventually(ctx, t, func(context.Context) bool {
464+
prebuiltWorkspaces, err := db.GetRunningPrebuiltWorkspaces(ctx)
465+
assert.NoError(t, err, "failed to get running prebuilds")
466+
467+
for _, prebuild := range prebuiltWorkspaces {
468+
runningPrebuilds = append(runningPrebuilds, prebuild)
469+
470+
agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, prebuild.ID)
471+
assert.NoError(t, err, "failed to get agents")
472+
473+
// Manually mark all agents as ready since tests don't have real agent processes
474+
// that would normally report their lifecycle state. Prebuilt workspaces are only
475+
// eligible for claiming when their agents reach the "ready" state.
476+
for _, agent := range agents {
477+
err = db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{
478+
ID: agent.ID,
479+
LifecycleState: database.WorkspaceAgentLifecycleStateReady,
480+
StartedAt: sql.NullTime{Time: time.Now().Add(time.Hour), Valid: true},
481+
ReadyAt: sql.NullTime{Time: time.Now().Add(-1 * time.Hour), Valid: true},
482+
})
483+
assert.NoError(t, err, "failed to update agent")
484+
}
485+
}
486+
487+
t.Logf("found %d running prebuilds so far, want %d", len(runningPrebuilds), desiredPrebuilds)
488+
return len(runningPrebuilds) == desiredPrebuilds
489+
}, testutil.IntervalSlow, "found %d running prebuilds, expected %d", len(runningPrebuilds), desiredPrebuilds)
490+
491+
return runningPrebuilds
492+
}
493+
494+
func MustRunReconciliationLoopForPreset(
495+
ctx context.Context,
496+
t *testing.T,
497+
db database.Store,
498+
reconciler *entprebuilds.StoreReconciler,
499+
preset codersdk.Preset,
500+
) []*prebuilds.ReconciliationActions {
501+
t.Helper()
502+
503+
state, err := reconciler.SnapshotState(ctx, db)
504+
require.NoError(t, err)
505+
ps, err := state.FilterByPreset(preset.ID)
506+
require.NoError(t, err)
507+
require.NotNil(t, ps)
508+
actions, err := reconciler.CalculateActions(ctx, *ps)
509+
require.NoError(t, err)
510+
require.NotNil(t, actions)
511+
require.NoError(t, reconciler.ReconcilePreset(ctx, *ps))
512+
513+
return actions
514+
}
515+
516+
func MustClaimPrebuild(
517+
ctx context.Context,
518+
t *testing.T,
519+
client *codersdk.Client,
520+
userClient *codersdk.Client,
521+
username string,
522+
version codersdk.TemplateVersion,
523+
presetID uuid.UUID,
524+
autostartSchedule ...string,
525+
) codersdk.Workspace {
526+
t.Helper()
527+
528+
var startSchedule string
529+
if len(autostartSchedule) > 0 {
530+
startSchedule = autostartSchedule[0]
531+
}
532+
533+
workspaceName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-")
534+
userWorkspace, err := userClient.CreateUserWorkspace(ctx, username, codersdk.CreateWorkspaceRequest{
535+
TemplateVersionID: version.ID,
536+
Name: workspaceName,
537+
TemplateVersionPresetID: presetID,
538+
AutostartSchedule: ptr.Ref(startSchedule),
539+
})
540+
require.NoError(t, err)
541+
build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID)
542+
require.Equal(t, build.Job.Status, codersdk.ProvisionerJobSucceeded)
543+
workspace := coderdtest.MustWorkspace(t, client, userWorkspace.ID)
544+
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)
545+
546+
return workspace
547+
}

‎enterprise/coderd/workspaces_test.go

Lines changed: 78 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2879,113 +2879,122 @@ func TestWorkspaceProvisionerdServerMetrics(t *testing.T) {
28792879
t.Parallel()
28802880

28812881
// Setup
2882-
log := testutil.Logger(t)
2882+
clock := quartz.NewMock(t)
2883+
ctx := testutil.Context(t, testutil.WaitSuperLong)
2884+
db, pb := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
2885+
logger := testutil.Logger(t)
28832886
reg := prometheus.NewRegistry()
2884-
provisionerdserverMetrics := provisionerdserver.NewMetrics(log)
2887+
provisionerdserverMetrics := provisionerdserver.NewMetrics(logger)
28852888
err := provisionerdserverMetrics.Register(reg)
28862889
require.NoError(t, err)
2887-
client, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{
2890+
client, _, api, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{
28882891
Options: &coderdtest.Options{
2892+
Database: db,
2893+
Pubsub: pb,
28892894
IncludeProvisionerDaemon: true,
2895+
Clock: clock,
28902896
ProvisionerdServerMetrics: provisionerdserverMetrics,
28912897
},
2892-
LicenseOptions: &coderdenttest.LicenseOptions{
2893-
Features: license.Features{
2894-
codersdk.FeatureWorkspacePrebuilds: 1,
2895-
},
2896-
},
28972898
})
28982899

2899-
// Given: a template and a template version with a preset without prebuild instances
2900-
presetNoPrebuildID := uuid.New()
2901-
versionNoPrebuild := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
2902-
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, versionNoPrebuild.ID)
2903-
templateNoPrebuild := coderdtest.CreateTemplate(t, client, owner.OrganizationID, versionNoPrebuild.ID)
2904-
presetNoPrebuild := dbgen.Preset(t, db, database.InsertPresetParams{
2905-
ID: presetNoPrebuildID,
2906-
TemplateVersionID: versionNoPrebuild.ID,
2907-
})
2900+
// Setup Prebuild reconciler
2901+
cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
2902+
reconciler := prebuilds.NewStoreReconciler(
2903+
db, pb, cache,
2904+
codersdk.PrebuildsConfig{},
2905+
logger,
2906+
clock,
2907+
prometheus.NewRegistry(),
2908+
notifications.NewNoopEnqueuer(),
2909+
api.AGPL.BuildUsageChecker,
2910+
)
2911+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
2912+
api.AGPL.PrebuildsClaimer.Store(&claimer)
2913+
2914+
organizationName, err := client.Organization(ctx, owner.OrganizationID)
2915+
require.NoError(t, err)
2916+
userClient, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleMember())
29082917

2909-
// Given: a template and a template version with a preset with a prebuild instance
2910-
presetPrebuildID := uuid.New()
2911-
versionPrebuild := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
2912-
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, versionPrebuild.ID)
2918+
// Setup template and template version with a preset with 1 prebuild instance
2919+
versionPrebuild := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithAgentAndPresetsWithPrebuilds(1))
2920+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, versionPrebuild.ID)
29132921
templatePrebuild := coderdtest.CreateTemplate(t, client, owner.OrganizationID, versionPrebuild.ID)
2914-
presetPrebuild := dbgen.Preset(t, db, database.InsertPresetParams{
2915-
ID: presetPrebuildID,
2916-
TemplateVersionID: versionPrebuild.ID,
2917-
DesiredInstances: sql.NullInt32{Int32: 1, Valid: true},
2918-
})
2919-
// Given: a prebuild workspace
2920-
wb := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
2921-
OwnerID: database.PrebuildsSystemUserID,
2922-
TemplateID: templatePrebuild.ID,
2923-
}).Seed(database.WorkspaceBuild{
2924-
TemplateVersionID: versionPrebuild.ID,
2925-
TemplateVersionPresetID: uuid.NullUUID{
2926-
UUID: presetPrebuildID,
2927-
Valid: true,
2928-
},
2929-
}).WithAgent(func(agent []*proto.Agent) []*proto.Agent {
2930-
return agent
2931-
}).Do()
2922+
presetsPrebuild, err := client.TemplateVersionPresets(ctx, versionPrebuild.ID)
2923+
require.NoError(t, err)
2924+
require.Len(t, presetsPrebuild, 1)
29322925

2933-
// Mark the prebuilt workspace's agent as ready so the prebuild can be claimed
2934-
// nolint:gocritic
2935-
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitLong))
2936-
agent, err := db.GetWorkspaceAgentAndLatestBuildByAuthToken(ctx, uuid.MustParse(wb.AgentToken))
2926+
// Setup template and template version with a preset without prebuild instances
2927+
versionNoPrebuild := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithAgentAndPresetsWithPrebuilds(0))
2928+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, versionNoPrebuild.ID)
2929+
templateNoPrebuild := coderdtest.CreateTemplate(t, client, owner.OrganizationID, versionNoPrebuild.ID)
2930+
presetsNoPrebuild, err := client.TemplateVersionPresets(ctx, versionNoPrebuild.ID)
29372931
require.NoError(t, err)
2938-
err = db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{
2939-
ID: agent.WorkspaceAgent.ID,
2940-
LifecycleState: database.WorkspaceAgentLifecycleStateReady,
2932+
require.Len(t, presetsNoPrebuild, 1)
2933+
2934+
// Given: no histogram value for prebuilt workspaces creation
2935+
prebuildCreationMetric := promhelp.MetricValue(t, reg, "coderd_workspace_creation_duration_seconds", prometheus.Labels{
2936+
"organization_name": organizationName.Name,
2937+
"template_name": templatePrebuild.Name,
2938+
"preset_name": presetsPrebuild[0].Name,
2939+
"type": "prebuild",
29412940
})
2942-
require.NoError(t, err)
2941+
require.Nil(t, prebuildCreationMetric)
29432942

2944-
organizationName, err := client.Organization(ctx, owner.OrganizationID)
2945-
require.NoError(t, err)
2946-
user, err := client.User(ctx, "testUser")
2947-
require.NoError(t, err)
2943+
// Given: reconciliation loop runs and starts prebuilt workspace
2944+
coderdenttest.MustRunReconciliationLoopForPreset(ctx, t, db, reconciler, presetsPrebuild[0])
2945+
runningPrebuilds := coderdenttest.GetRunningPrebuilds(ctx, t, db, 1)
2946+
require.Len(t, runningPrebuilds, 1)
2947+
2948+
// Then: the histogram value for prebuilt workspace creation should be updated
2949+
prebuildCreationHistogram := promhelp.HistogramValue(t, reg, "coderd_workspace_creation_duration_seconds", prometheus.Labels{
2950+
"organization_name": organizationName.Name,
2951+
"template_name": templatePrebuild.Name,
2952+
"preset_name": presetsPrebuild[0].Name,
2953+
"type": "prebuild",
2954+
})
2955+
require.NotNil(t, prebuildCreationHistogram)
2956+
require.Equal(t, uint64(1), prebuildCreationHistogram.GetSampleCount())
2957+
2958+
// Given: a running prebuilt workspace, ready to be claimed
2959+
prebuild := coderdtest.MustWorkspace(t, client, runningPrebuilds[0].ID)
2960+
require.Equal(t, codersdk.WorkspaceTransitionStart, prebuild.LatestBuild.Transition)
2961+
require.Nil(t, prebuild.DormantAt)
2962+
require.Nil(t, prebuild.DeletingAt)
29482963

29492964
// Given: no histogram value for prebuilt workspaces claim
2950-
prebuiltWorkspaceHistogramMetric := promhelp.MetricValue(t, reg, "coderd_prebuilt_workspace_claim_duration_seconds", prometheus.Labels{
2965+
prebuildClaimMetric := promhelp.MetricValue(t, reg, "coderd_prebuilt_workspace_claim_duration_seconds", prometheus.Labels{
29512966
"organization_name": organizationName.Name,
29522967
"template_name": templatePrebuild.Name,
2953-
"preset_name": presetPrebuild.Name,
2968+
"preset_name": presetsPrebuild[0].Name,
29542969
})
2955-
require.Nil(t, prebuiltWorkspaceHistogramMetric)
2970+
require.Nil(t, prebuildClaimMetric)
29562971

29572972
// Given: the prebuilt workspace is claimed by a user
2958-
claimedWorkspace, err := client.CreateUserWorkspace(ctx, user.ID.String(), codersdk.CreateWorkspaceRequest{
2959-
TemplateVersionID: versionPrebuild.ID,
2960-
TemplateVersionPresetID: presetPrebuildID,
2961-
Name: coderdtest.RandomUsername(t),
2962-
})
2963-
require.NoError(t, err)
2964-
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, claimedWorkspace.LatestBuild.ID)
2965-
require.Equal(t, wb.Workspace.ID, claimedWorkspace.ID)
2973+
workspace := coderdenttest.MustClaimPrebuild(ctx, t, client, userClient, user.Username, versionPrebuild, presetsPrebuild[0].ID)
2974+
require.Equal(t, prebuild.ID, workspace.ID)
29662975

29672976
// Then: the histogram value for prebuilt workspace claim should be updated
2968-
prebuiltWorkspaceHistogram := promhelp.HistogramValue(t, reg, "coderd_prebuilt_workspace_claim_duration_seconds", prometheus.Labels{
2977+
prebuildClaimHistogram := promhelp.HistogramValue(t, reg, "coderd_prebuilt_workspace_claim_duration_seconds", prometheus.Labels{
29692978
"organization_name": organizationName.Name,
29702979
"template_name": templatePrebuild.Name,
2971-
"preset_name": presetPrebuild.Name,
2980+
"preset_name": presetsPrebuild[0].Name,
29722981
})
2973-
require.NotNil(t, prebuiltWorkspaceHistogram)
2974-
require.Equal(t, uint64(1), prebuiltWorkspaceHistogram.GetSampleCount())
2982+
require.NotNil(t, prebuildClaimHistogram)
2983+
require.Equal(t, uint64(1), prebuildClaimHistogram.GetSampleCount())
29752984

29762985
// Given: no histogram value for regular workspaces creation
29772986
regularWorkspaceHistogramMetric := promhelp.MetricValue(t, reg, "coderd_workspace_creation_duration_seconds", prometheus.Labels{
29782987
"organization_name": organizationName.Name,
29792988
"template_name": templateNoPrebuild.Name,
2980-
"preset_name": presetNoPrebuild.Name,
2989+
"preset_name": presetsNoPrebuild[0].Name,
29812990
"type": "regular",
29822991
})
29832992
require.Nil(t, regularWorkspaceHistogramMetric)
29842993

29852994
// Given: a user creates a regular workspace (without prebuild pool)
29862995
regularWorkspace, err := client.CreateUserWorkspace(ctx, user.ID.String(), codersdk.CreateWorkspaceRequest{
29872996
TemplateVersionID: versionNoPrebuild.ID,
2988-
TemplateVersionPresetID: presetNoPrebuildID,
2997+
TemplateVersionPresetID: presetsNoPrebuild[0].ID,
29892998
Name: coderdtest.RandomUsername(t),
29902999
})
29913000
require.NoError(t, err)
@@ -2995,7 +3004,7 @@ func TestWorkspaceProvisionerdServerMetrics(t *testing.T) {
29953004
regularWorkspaceHistogram := promhelp.HistogramValue(t, reg, "coderd_workspace_creation_duration_seconds", prometheus.Labels{
29963005
"organization_name": organizationName.Name,
29973006
"template_name": templateNoPrebuild.Name,
2998-
"preset_name": presetNoPrebuild.Name,
3007+
"preset_name": presetsNoPrebuild[0].Name,
29993008
"type": "regular",
30003009
})
30013010
require.NotNil(t, regularWorkspaceHistogram)

0 commit comments

Comments
 (0)