From 22a87f6cf6143f3e4a30045ddce7a633c1215611 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 10 Mar 2026 12:17:32 -0500 Subject: [PATCH] fix: filter sub-agents from build duration metric (#22732) --- agent/agent_test.go | 56 +++++++++ coderd/database/querier_test.go | 120 ++++++++++++++++++++ coderd/database/queries.sql.go | 2 +- coderd/database/queries/workspacebuilds.sql | 2 +- 4 files changed, 178 insertions(+), 2 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 40d0195a5e..2e8faa3ad5 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -3040,6 +3040,62 @@ func TestAgent_Reconnect(t *testing.T) { closer.Close() } +func TestAgent_ReconnectNoLifecycleReemit(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + logger := testutil.Logger(t) + + fCoordinator := tailnettest.NewFakeCoordinator() + agentID := uuid.New() + statsCh := make(chan *proto.Stats, 50) + derpMap, _ := tailnettest.RunDERPAndSTUN(t) + + client := agenttest.NewClient(t, + logger, + agentID, + agentsdk.Manifest{ + DERPMap: derpMap, + Scripts: []codersdk.WorkspaceAgentScript{{ + Script: "echo hello", + Timeout: 30 * time.Second, + RunOnStart: true, + }}, + }, + statsCh, + fCoordinator, + ) + defer client.Close() + + closer := agent.New(agent.Options{ + Client: client, + Logger: logger.Named("agent"), + }) + defer closer.Close() + + // Wait for the agent to reach Ready state. + require.Eventually(t, func() bool { + return slices.Contains(client.GetLifecycleStates(), codersdk.WorkspaceAgentLifecycleReady) + }, testutil.WaitShort, testutil.IntervalFast) + + statesBefore := slices.Clone(client.GetLifecycleStates()) + + // Disconnect by closing the coordinator response channel. + call1 := testutil.RequireReceive(ctx, t, fCoordinator.CoordinateCalls) + close(call1.Resps) + + // Wait for reconnect. + testutil.RequireReceive(ctx, t, fCoordinator.CoordinateCalls) + + // Wait for a stats report as a deterministic steady-state proof. + testutil.RequireReceive(ctx, t, statsCh) + + statesAfter := client.GetLifecycleStates() + require.Equal(t, statesBefore, statesAfter, + "lifecycle states should not be re-reported after reconnect") + + closer.Close() +} + func TestAgent_WriteVSCodeConfigs(t *testing.T) { t.Parallel() logger := testutil.Logger(t) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 7b2420302c..07fbc2a4d1 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -9116,3 +9116,123 @@ func TestGetChatMessagesForPromptByChatID(t *testing.T) { require.Contains(t, gotIDs, postUser.ID) }) } + +func TestGetWorkspaceBuildMetricsByResourceID(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + ctx := context.Background() + + org := dbgen.Organization(t, db, database.Organization{}) + user := dbgen.User(t, db, database.User{}) + tmpl := dbgen.Template(t, db, database.Template{ + OrganizationID: org.ID, + CreatedBy: user.ID, + }) + tv := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + OrganizationID: org.ID, + TemplateID: uuid.NullUUID{UUID: tmpl.ID, Valid: true}, + CreatedBy: user.ID, + }) + ws := dbgen.Workspace(t, db, database.WorkspaceTable{ + OrganizationID: org.ID, + TemplateID: tmpl.ID, + OwnerID: user.ID, + AutomaticUpdates: database.AutomaticUpdatesNever, + }) + job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + OrganizationID: org.ID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + }) + _ = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: ws.ID, + TemplateVersionID: tv.ID, + JobID: job.ID, + InitiatorID: user.ID, + }) + resource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{ + JobID: job.ID, + }) + + parentReadyAt := dbtime.Now() + parentStartedAt := parentReadyAt.Add(-time.Second) + _ = dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{ + ResourceID: resource.ID, + StartedAt: sql.NullTime{Time: parentStartedAt, Valid: true}, + ReadyAt: sql.NullTime{Time: parentReadyAt, Valid: true}, + LifecycleState: database.WorkspaceAgentLifecycleStateReady, + }) + + row, err := db.GetWorkspaceBuildMetricsByResourceID(ctx, resource.ID) + require.NoError(t, err) + require.True(t, row.AllAgentsReady) + require.True(t, parentReadyAt.Equal(row.LastAgentReadyAt)) + require.Equal(t, "success", row.WorstStatus) + }) + + t.Run("SubAgentExcluded", func(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + ctx := context.Background() + + org := dbgen.Organization(t, db, database.Organization{}) + user := dbgen.User(t, db, database.User{}) + tmpl := dbgen.Template(t, db, database.Template{ + OrganizationID: org.ID, + CreatedBy: user.ID, + }) + tv := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + OrganizationID: org.ID, + TemplateID: uuid.NullUUID{UUID: tmpl.ID, Valid: true}, + CreatedBy: user.ID, + }) + ws := dbgen.Workspace(t, db, database.WorkspaceTable{ + OrganizationID: org.ID, + TemplateID: tmpl.ID, + OwnerID: user.ID, + AutomaticUpdates: database.AutomaticUpdatesNever, + }) + job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + OrganizationID: org.ID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + }) + _ = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: ws.ID, + TemplateVersionID: tv.ID, + JobID: job.ID, + InitiatorID: user.ID, + }) + resource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{ + JobID: job.ID, + }) + + parentReadyAt := dbtime.Now() + parentStartedAt := parentReadyAt.Add(-time.Second) + parentAgent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{ + ResourceID: resource.ID, + StartedAt: sql.NullTime{Time: parentStartedAt, Valid: true}, + ReadyAt: sql.NullTime{Time: parentReadyAt, Valid: true}, + LifecycleState: database.WorkspaceAgentLifecycleStateReady, + }) + + // Sub-agent with ready_at 1 hour later should be excluded. + subAgentReadyAt := parentReadyAt.Add(time.Hour) + subAgentStartedAt := subAgentReadyAt.Add(-time.Second) + _ = dbgen.WorkspaceSubAgent(t, db, parentAgent, database.WorkspaceAgent{ + StartedAt: sql.NullTime{Time: subAgentStartedAt, Valid: true}, + ReadyAt: sql.NullTime{Time: subAgentReadyAt, Valid: true}, + LifecycleState: database.WorkspaceAgentLifecycleStateReady, + }) + + row, err := db.GetWorkspaceBuildMetricsByResourceID(ctx, resource.ID) + require.NoError(t, err) + require.True(t, row.AllAgentsReady) + // LastAgentReadyAt should be the parent's, not the sub-agent's. + require.True(t, parentReadyAt.Equal(row.LastAgentReadyAt)) + require.Equal(t, "success", row.WorstStatus) + }) +} diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0552b7958b..590dd81cbe 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -23848,7 +23848,7 @@ JOIN workspaces w ON wb.workspace_id = w.id JOIN templates t ON w.template_id = t.id JOIN organizations o ON t.organization_id = o.id JOIN workspace_resources wr ON wr.job_id = wb.job_id -JOIN workspace_agents wa ON wa.resource_id = wr.id +JOIN workspace_agents wa ON wa.resource_id = wr.id AND wa.parent_id IS NULL WHERE wb.job_id = (SELECT job_id FROM workspace_resources WHERE workspace_resources.id = $1) GROUP BY wb.created_at, wb.transition, t.name, o.name, w.owner_id ` diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index d74136deb6..775e9da0ab 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -268,7 +268,7 @@ JOIN workspaces w ON wb.workspace_id = w.id JOIN templates t ON w.template_id = t.id JOIN organizations o ON t.organization_id = o.id JOIN workspace_resources wr ON wr.job_id = wb.job_id -JOIN workspace_agents wa ON wa.resource_id = wr.id +JOIN workspace_agents wa ON wa.resource_id = wr.id AND wa.parent_id IS NULL WHERE wb.job_id = (SELECT job_id FROM workspace_resources WHERE workspace_resources.id = $1) GROUP BY wb.created_at, wb.transition, t.name, o.name, w.owner_id;