From f2eb6d5af0df74fe6a64b45e6b45f9f93257942b Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 10 Mar 2026 20:10:08 -0500 Subject: [PATCH] fix: prevent emitting build duration metric for devcontainer subagents (#22929) --- coderd/agentapi/lifecycle.go | 9 +++-- coderd/agentapi/lifecycle_test.go | 58 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/coderd/agentapi/lifecycle.go b/coderd/agentapi/lifecycle.go index 61d9d5e37c..d821d6eb3f 100644 --- a/coderd/agentapi/lifecycle.go +++ b/coderd/agentapi/lifecycle.go @@ -134,9 +134,12 @@ func (a *LifecycleAPI) UpdateLifecycle(ctx context.Context, req *agentproto.Upda case database.WorkspaceAgentLifecycleStateReady, database.WorkspaceAgentLifecycleStateStartTimeout, database.WorkspaceAgentLifecycleStateStartError: - a.emitMetricsOnce.Do(func() { - a.emitBuildDurationMetric(ctx, workspaceAgent.ResourceID) - }) + // Only emit metrics for the parent agent, this metric is not intended to measure devcontainer durations. + if !workspaceAgent.ParentID.Valid { + a.emitMetricsOnce.Do(func() { + a.emitBuildDurationMetric(ctx, workspaceAgent.ResourceID) + }) + } } return req.Lifecycle, nil diff --git a/coderd/agentapi/lifecycle_test.go b/coderd/agentapi/lifecycle_test.go index 2457af8a22..afb8c8878f 100644 --- a/coderd/agentapi/lifecycle_test.go +++ b/coderd/agentapi/lifecycle_test.go @@ -582,6 +582,64 @@ func TestUpdateLifecycle(t *testing.T) { require.Equal(t, uint64(1), got.GetSampleCount()) require.Equal(t, expectedDuration, got.GetSampleSum()) }) + + t.Run("SubAgentDoesNotEmitMetric", func(t *testing.T) { + t.Parallel() + parentID := uuid.New() + subAgent := database.WorkspaceAgent{ + ID: uuid.New(), + ParentID: uuid.NullUUID{UUID: parentID, Valid: true}, + LifecycleState: database.WorkspaceAgentLifecycleStateStarting, + StartedAt: sql.NullTime{Valid: true, Time: someTime}, + ReadyAt: sql.NullTime{Valid: false}, + } + lifecycle := &agentproto.Lifecycle{ + State: agentproto.Lifecycle_READY, + ChangedAt: timestamppb.New(now), + } + dbM := dbmock.NewMockStore(gomock.NewController(t)) + dbM.EXPECT().UpdateWorkspaceAgentLifecycleStateByID(gomock.Any(), database.UpdateWorkspaceAgentLifecycleStateByIDParams{ + ID: subAgent.ID, + LifecycleState: database.WorkspaceAgentLifecycleStateReady, + StartedAt: subAgent.StartedAt, + ReadyAt: sql.NullTime{ + Time: now, + Valid: true, + }, + }).Return(nil) + // GetWorkspaceBuildMetricsByResourceID should NOT be called + // because sub-agents should be skipped before querying. + reg := prometheus.NewRegistry() + metrics := agentapi.NewLifecycleMetrics(reg) + api := &agentapi.LifecycleAPI{ + AgentFn: func(ctx context.Context) (database.WorkspaceAgent, error) { + return subAgent, nil + }, + WorkspaceID: workspaceID, + Database: dbM, + Log: testutil.Logger(t), + Metrics: metrics, + PublishWorkspaceUpdateFn: nil, + } + resp, err := api.UpdateLifecycle(context.Background(), &agentproto.UpdateLifecycleRequest{ + Lifecycle: lifecycle, + }) + require.NoError(t, err) + require.Equal(t, lifecycle, resp) + + // We don't expect the metric to be emitted for sub-agents, by default this will fail anyway but it doesn't hurt + // to document the test explicitly. + dbM.EXPECT().GetWorkspaceBuildMetricsByResourceID(gomock.Any(), gomock.Any()).Times(0) + + // If we were emitting the metric we would have failed by now since it would include a call to the database that we're not expecting. + pm, err := reg.Gather() + require.NoError(t, err) + for _, m := range pm { + if m.GetName() == fullMetricName { + t.Fatal("metric should not be emitted for sub-agent") + } + } + }) } func TestUpdateStartup(t *testing.T) {