From a70924aa71ecbe77ab818419ef9876a0a9965897 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Aug 2025 12:49:13 -0500 Subject: [PATCH 1/3] chore: remove agent pprof label, rename server -> system --- cli/server.go | 4 ++-- coderd/autobuild/lifecycle_executor.go | 2 +- coderd/coderd.go | 9 ++++++++- coderd/httpmw/pprof.go | 15 ++++++++++++++- coderd/pproflabel/pproflabel.go | 18 +++++++++++------- .../insights/metricscollector.go | 2 +- coderd/workspaceagentsrpc.go | 19 ++++++++----------- enterprise/coderd/coderd.go | 2 +- 8 files changed, 46 insertions(+), 25 deletions(-) diff --git a/cli/server.go b/cli/server.go index f9e744761b22e..6f9d4fc068ceb 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1460,14 +1460,14 @@ func newProvisionerDaemon( tracer := coderAPI.TracerProvider.Tracer(tracing.TracerName) terraformClient, terraformServer := drpcsdk.MemTransportPipe() wg.Add(1) - pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceTerraformProvisioner), func(ctx context.Context) { + pproflabel.Go(ctx, pproflabel.Service(pproflabel.SystemTerraformProvisioner), func(ctx context.Context) { defer wg.Done() <-ctx.Done() _ = terraformClient.Close() _ = terraformServer.Close() }) wg.Add(1) - pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceTerraformProvisioner), func(ctx context.Context) { + pproflabel.Go(ctx, pproflabel.Service(pproflabel.SystemTerraformProvisioner), func(ctx context.Context) { defer wg.Done() defer cancel() diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 16072e6517125..e8c94e618e7e1 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -108,7 +108,7 @@ func (e *Executor) WithStatsChannel(ch chan<- Stats) *Executor { // tick from its channel. It will stop when its context is Done, or when // its channel is closed. func (e *Executor) Run() { - pproflabel.Go(e.ctx, pproflabel.Service(pproflabel.ServiceLifecycles), func(ctx context.Context) { + pproflabel.Go(e.ctx, pproflabel.Service(pproflabel.SystemLifecycles), func(ctx context.Context) { for { select { case <-ctx.Done(): diff --git a/coderd/coderd.go b/coderd/coderd.go index 928fa21a95242..419f660b483dd 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -14,6 +14,7 @@ import ( "net/url" "path/filepath" "regexp" + "runtime/pprof" "strings" "sync" "sync/atomic" @@ -1340,7 +1341,13 @@ func New(options *Options) *API { ).Get("/connection", api.workspaceAgentConnectionGeneric) r.Route("/me", func(r chi.Router) { r.Use(workspaceAgentInfo) - r.Get("/rpc", api.workspaceAgentRPC) + r.Group(func(r chi.Router) { + r.Use( + // Override the request_type for agent rpc traffic. + httpmw.WithStaticProfilingLabels(pprof.Labels("request_type", "agent_rpc")), + ) + r.Get("/rpc", api.workspaceAgentRPC) + }) r.Patch("/logs", api.patchWorkspaceAgentLogs) r.Patch("/app-status", api.patchWorkspaceAgentAppStatus) // Deprecated: Required to support legacy agents diff --git a/coderd/httpmw/pprof.go b/coderd/httpmw/pprof.go index eee3e9c9fdbe1..349a38a6648ad 100644 --- a/coderd/httpmw/pprof.go +++ b/coderd/httpmw/pprof.go @@ -22,9 +22,22 @@ func WithProfilingLabels(next http.Handler) http.Handler { requestType = "websocket" } - pprof.Do(ctx, pproflabel.Service(pproflabel.ServiceHTTPServer, "request_type", requestType), func(ctx context.Context) { + pprof.Do(ctx, pproflabel.Service(pproflabel.SystemHTTPServer, "request_type", requestType), func(ctx context.Context) { r = r.WithContext(ctx) next.ServeHTTP(rw, r) }) }) } + +func WithStaticProfilingLabels(labels pprof.LabelSet) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + pprof.Do(ctx, labels, func(ctx context.Context) { + r = r.WithContext(ctx) + next.ServeHTTP(rw, r) + }) + }) + } +} diff --git a/coderd/pproflabel/pproflabel.go b/coderd/pproflabel/pproflabel.go index cd803b0f1baea..e9eaecf31ebf3 100644 --- a/coderd/pproflabel/pproflabel.go +++ b/coderd/pproflabel/pproflabel.go @@ -10,16 +10,20 @@ func Go(ctx context.Context, labels pprof.LabelSet, f func(context.Context)) { go pprof.Do(ctx, labels, f) } +func Do(ctx context.Context, labels pprof.LabelSet, f func(context.Context)) { + pprof.Do(ctx, labels, f) +} + const ( - ServiceTag = "service" + SystemTag = "service" - ServiceHTTPServer = "http-api" - ServiceLifecycles = "lifecycle-executor" - ServiceMetricCollector = "metrics-collector" - ServicePrebuildReconciler = "prebuilds-reconciler" - ServiceTerraformProvisioner = "terraform-provisioner" + SystemHTTPServer = "http-api" + SystemLifecycles = "lifecycle-executor" + SystemMetricCollector = "metrics-collector" + SystemPrebuildReconciler = "prebuilds-reconciler" + SystemTerraformProvisioner = "terraform-provisioner" ) func Service(name string, pairs ...string) pprof.LabelSet { - return pprof.Labels(append([]string{ServiceTag, name}, pairs...)...) + return pprof.Labels(append([]string{SystemTag, name}, pairs...)...) } diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index a095968526ca8..d9a9670f4e64d 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -159,7 +159,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { }) } - pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceMetricCollector), func(ctx context.Context) { + pproflabel.Go(ctx, pproflabel.Service(pproflabel.SystemMetricCollector), func(ctx context.Context) { defer close(done) defer ticker.Stop() for { diff --git a/coderd/workspaceagentsrpc.go b/coderd/workspaceagentsrpc.go index 0806118f2a832..8dacbe9812ca9 100644 --- a/coderd/workspaceagentsrpc.go +++ b/coderd/workspaceagentsrpc.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "net/http" - "runtime/pprof" "sync" "sync/atomic" "time" @@ -348,16 +347,14 @@ func (m *agentConnectionMonitor) init() { func (m *agentConnectionMonitor) start(ctx context.Context) { ctx, m.cancel = context.WithCancel(ctx) m.wg.Add(2) - go pprof.Do(ctx, pprof.Labels("agent", m.workspaceAgent.ID.String()), - func(ctx context.Context) { - defer m.wg.Done() - m.sendPings(ctx) - }) - go pprof.Do(ctx, pprof.Labels("agent", m.workspaceAgent.ID.String()), - func(ctx context.Context) { - defer m.wg.Done() - m.monitor(ctx) - }) + go func(ctx context.Context) { + defer m.wg.Done() + m.sendPings(ctx) + }(ctx) + go func(ctx context.Context) { + defer m.wg.Done() + m.monitor(ctx) + }(ctx) } func (m *agentConnectionMonitor) monitor(ctx context.Context) { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 40569ead70658..0ef504d1a70d7 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -905,7 +905,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { api.AGPL.PrebuildsReconciler.Store(&reconciler) // TODO: Should this context be the api.ctx context? To cancel when // the API (and entire app) is closed via shutdown? - pproflabel.Go(context.Background(), pproflabel.Service(pproflabel.ServicePrebuildReconciler), reconciler.Run) + pproflabel.Go(context.Background(), pproflabel.Service(pproflabel.SystemPrebuildReconciler), reconciler.Run) api.AGPL.PrebuildsClaimer.Store(&claimer) } From 9db543aa32d97262b32d7f57c331a22964bbd36e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Aug 2025 13:30:44 -0500 Subject: [PATCH 2/3] coder_ tag --- cli/server.go | 4 ++-- coderd/autobuild/lifecycle_executor.go | 2 +- coderd/httpmw/pprof.go | 2 +- coderd/pproflabel/pproflabel.go | 16 +++++++++------- .../insights/metricscollector.go | 2 +- enterprise/coderd/coderd.go | 2 +- 6 files changed, 15 insertions(+), 13 deletions(-) diff --git a/cli/server.go b/cli/server.go index 6f9d4fc068ceb..f9e744761b22e 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1460,14 +1460,14 @@ func newProvisionerDaemon( tracer := coderAPI.TracerProvider.Tracer(tracing.TracerName) terraformClient, terraformServer := drpcsdk.MemTransportPipe() wg.Add(1) - pproflabel.Go(ctx, pproflabel.Service(pproflabel.SystemTerraformProvisioner), func(ctx context.Context) { + pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceTerraformProvisioner), func(ctx context.Context) { defer wg.Done() <-ctx.Done() _ = terraformClient.Close() _ = terraformServer.Close() }) wg.Add(1) - pproflabel.Go(ctx, pproflabel.Service(pproflabel.SystemTerraformProvisioner), func(ctx context.Context) { + pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceTerraformProvisioner), func(ctx context.Context) { defer wg.Done() defer cancel() diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index e8c94e618e7e1..16072e6517125 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -108,7 +108,7 @@ func (e *Executor) WithStatsChannel(ch chan<- Stats) *Executor { // tick from its channel. It will stop when its context is Done, or when // its channel is closed. func (e *Executor) Run() { - pproflabel.Go(e.ctx, pproflabel.Service(pproflabel.SystemLifecycles), func(ctx context.Context) { + pproflabel.Go(e.ctx, pproflabel.Service(pproflabel.ServiceLifecycles), func(ctx context.Context) { for { select { case <-ctx.Done(): diff --git a/coderd/httpmw/pprof.go b/coderd/httpmw/pprof.go index 349a38a6648ad..e03058c806b83 100644 --- a/coderd/httpmw/pprof.go +++ b/coderd/httpmw/pprof.go @@ -22,7 +22,7 @@ func WithProfilingLabels(next http.Handler) http.Handler { requestType = "websocket" } - pprof.Do(ctx, pproflabel.Service(pproflabel.SystemHTTPServer, "request_type", requestType), func(ctx context.Context) { + pprof.Do(ctx, pproflabel.Service(pproflabel.ServiceHTTPServer, "request_type", requestType), func(ctx context.Context) { r = r.WithContext(ctx) next.ServeHTTP(rw, r) }) diff --git a/coderd/pproflabel/pproflabel.go b/coderd/pproflabel/pproflabel.go index e9eaecf31ebf3..2f9a019b05583 100644 --- a/coderd/pproflabel/pproflabel.go +++ b/coderd/pproflabel/pproflabel.go @@ -15,15 +15,17 @@ func Do(ctx context.Context, labels pprof.LabelSet, f func(context.Context)) { } const ( - SystemTag = "service" + // ServiceTag should not collide with the pyroscope built-in tag "service". + // Use `coder_` to avoid collisions. + ServiceTag = "coder_service" - SystemHTTPServer = "http-api" - SystemLifecycles = "lifecycle-executor" - SystemMetricCollector = "metrics-collector" - SystemPrebuildReconciler = "prebuilds-reconciler" - SystemTerraformProvisioner = "terraform-provisioner" + ServiceHTTPServer = "http-api" + ServiceLifecycles = "lifecycle-executor" + ServiceMetricCollector = "metrics-collector" + ServicePrebuildReconciler = "prebuilds-reconciler" + ServiceTerraformProvisioner = "terraform-provisioner" ) func Service(name string, pairs ...string) pprof.LabelSet { - return pprof.Labels(append([]string{SystemTag, name}, pairs...)...) + return pprof.Labels(append([]string{ServiceTag, name}, pairs...)...) } diff --git a/coderd/prometheusmetrics/insights/metricscollector.go b/coderd/prometheusmetrics/insights/metricscollector.go index d9a9670f4e64d..a095968526ca8 100644 --- a/coderd/prometheusmetrics/insights/metricscollector.go +++ b/coderd/prometheusmetrics/insights/metricscollector.go @@ -159,7 +159,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) { }) } - pproflabel.Go(ctx, pproflabel.Service(pproflabel.SystemMetricCollector), func(ctx context.Context) { + pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceMetricCollector), func(ctx context.Context) { defer close(done) defer ticker.Stop() for { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 0ef504d1a70d7..40569ead70658 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -905,7 +905,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { api.AGPL.PrebuildsReconciler.Store(&reconciler) // TODO: Should this context be the api.ctx context? To cancel when // the API (and entire app) is closed via shutdown? - pproflabel.Go(context.Background(), pproflabel.Service(pproflabel.SystemPrebuildReconciler), reconciler.Run) + pproflabel.Go(context.Background(), pproflabel.Service(pproflabel.ServicePrebuildReconciler), reconciler.Run) api.AGPL.PrebuildsClaimer.Store(&claimer) } From a73de2f84faf193723976ed888d5979129bf79e7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 7 Aug 2025 13:35:48 -0500 Subject: [PATCH 3/3] use constant --- coderd/coderd.go | 3 ++- coderd/httpmw/pprof.go | 2 +- coderd/pproflabel/pproflabel.go | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 419f660b483dd..78ae849fd1894 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -21,6 +21,7 @@ import ( "time" "github.com/coder/coder/v2/coderd/oauth2provider" + "github.com/coder/coder/v2/coderd/pproflabel" "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/wsbuilder" @@ -1344,7 +1345,7 @@ func New(options *Options) *API { r.Group(func(r chi.Router) { r.Use( // Override the request_type for agent rpc traffic. - httpmw.WithStaticProfilingLabels(pprof.Labels("request_type", "agent_rpc")), + httpmw.WithStaticProfilingLabels(pprof.Labels(pproflabel.RequestTypeTag, "agent-rpc")), ) r.Get("/rpc", api.workspaceAgentRPC) }) diff --git a/coderd/httpmw/pprof.go b/coderd/httpmw/pprof.go index e03058c806b83..4c51c1ebe552e 100644 --- a/coderd/httpmw/pprof.go +++ b/coderd/httpmw/pprof.go @@ -22,7 +22,7 @@ func WithProfilingLabels(next http.Handler) http.Handler { requestType = "websocket" } - pprof.Do(ctx, pproflabel.Service(pproflabel.ServiceHTTPServer, "request_type", requestType), func(ctx context.Context) { + pprof.Do(ctx, pproflabel.Service(pproflabel.ServiceHTTPServer, pproflabel.RequestTypeTag, requestType), func(ctx context.Context) { r = r.WithContext(ctx) next.ServeHTTP(rw, r) }) diff --git a/coderd/pproflabel/pproflabel.go b/coderd/pproflabel/pproflabel.go index 2f9a019b05583..2bfd071dcdc39 100644 --- a/coderd/pproflabel/pproflabel.go +++ b/coderd/pproflabel/pproflabel.go @@ -24,6 +24,8 @@ const ( ServiceMetricCollector = "metrics-collector" ServicePrebuildReconciler = "prebuilds-reconciler" ServiceTerraformProvisioner = "terraform-provisioner" + + RequestTypeTag = "coder_request_type" ) func Service(name string, pairs ...string) pprof.LabelSet {