Skip to content

Commit 8ba8b4f

Browse files
EmyrkEmyrk
authored
chore: add profiling labels for pprof analysis (#19232)
PProf labels segment the code into groups for determing the source of cpu/memory profiles. Since the web server and background jobs share a lot of the same code (eg wsbuilder), it helps to know if the load is user induced, or background job based.
1 parent 26458cd commit 8ba8b4f

File tree

8 files changed

+76
-14
lines changed

8 files changed

+76
-14
lines changed

‎cli/server.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import (
5555

5656
"cdr.dev/slog"
5757
"cdr.dev/slog/sloggers/sloghuman"
58+
"github.com/coder/coder/v2/coderd/pproflabel"
5859
"github.com/coder/pretty"
5960
"github.com/coder/quartz"
6061
"github.com/coder/retry"
@@ -1459,14 +1460,14 @@ func newProvisionerDaemon(
14591460
tracer := coderAPI.TracerProvider.Tracer(tracing.TracerName)
14601461
terraformClient, terraformServer := drpcsdk.MemTransportPipe()
14611462
wg.Add(1)
1462-
go func() {
1463+
pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceTerraformProvisioner), func(ctx context.Context) {
14631464
defer wg.Done()
14641465
<-ctx.Done()
14651466
_ = terraformClient.Close()
14661467
_ = terraformServer.Close()
1467-
}()
1468+
})
14681469
wg.Add(1)
1469-
go func() {
1470+
pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceTerraformProvisioner), func(ctx context.Context) {
14701471
defer wg.Done()
14711472
defer cancel()
14721473

@@ -1485,7 +1486,7 @@ func newProvisionerDaemon(
14851486
default:
14861487
}
14871488
}
1488-
}()
1489+
})
14891490

14901491
connector[string(database.ProvisionerTypeTerraform)] = sdkproto.NewDRPCProvisionerClient(terraformClient)
14911492
default:

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"cdr.dev/slog"
2222
"github.com/coder/coder/v2/coderd/files"
23+
"github.com/coder/coder/v2/coderd/pproflabel"
2324

2425
"github.com/coder/coder/v2/coderd/audit"
2526
"github.com/coder/coder/v2/coderd/database"
@@ -107,10 +108,10 @@ func (e *Executor) WithStatsChannel(ch chan<- Stats) *Executor {
107108
// tick from its channel. It will stop when its context is Done, or when
108109
// its channel is closed.
109110
func (e *Executor) Run() {
110-
go func() {
111+
pproflabel.Go(e.ctx, pproflabel.Service(pproflabel.ServiceLifecycles), func(ctx context.Context) {
111112
for {
112113
select {
113-
case <-e.ctx.Done():
114+
case <-ctx.Done():
114115
return
115116
case t, ok := <-e.tick:
116117
if !ok {
@@ -120,15 +121,15 @@ func (e *Executor) Run() {
120121
e.metrics.autobuildExecutionDuration.Observe(stats.Elapsed.Seconds())
121122
if e.statsCh != nil {
122123
select {
123-
case <-e.ctx.Done():
124+
case <-ctx.Done():
124125
return
125126
case e.statsCh <- stats:
126127
}
127128
}
128-
e.log.Debug(e.ctx, "run stats", slog.F("elapsed", stats.Elapsed), slog.F("transitions", stats.Transitions))
129+
e.log.Debug(ctx, "run stats", slog.F("elapsed", stats.Elapsed), slog.F("transitions", stats.Transitions))
129130
}
130131
}
131-
}()
132+
})
132133
}
133134

134135
func (e *Executor) runOnce(t time.Time) Stats {

‎coderd/coderd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,7 @@ func New(options *Options) *API {
852852

853853
r.Use(
854854
httpmw.Recover(api.Logger),
855+
httpmw.WithProfilingLabels,
855856
tracing.StatusWriterMiddleware,
856857
tracing.Middleware(api.TracerProvider),
857858
httpmw.AttachRequestID,

‎coderd/httpmw/pprof.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package httpmw
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"runtime/pprof"
7+
8+
"github.com/coder/coder/v2/coderd/pproflabel"
9+
)
10+
11+
// WithProfilingLabels adds a pprof label to all http request handlers. This is
12+
// primarily used to determine if load is coming from background jobs, or from
13+
// http traffic.
14+
func WithProfilingLabels(next http.Handler) http.Handler {
15+
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
16+
ctx := r.Context()
17+
18+
// Label to differentiate between http and websocket requests. Websocket requests
19+
// are assumed to be long-lived and more resource consuming.
20+
requestType := "http"
21+
if r.Header.Get("Upgrade") == "websocket" {
22+
requestType = "websocket"
23+
}
24+
25+
pprof.Do(ctx, pproflabel.Service(pproflabel.ServiceHTTPServer, "request_type", requestType), func(ctx context.Context) {
26+
r = r.WithContext(ctx)
27+
next.ServeHTTP(rw, r)
28+
})
29+
})
30+
}

‎coderd/pproflabel/pproflabel.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package pproflabel
2+
3+
import (
4+
"context"
5+
"runtime/pprof"
6+
)
7+
8+
// Go is just a convince wrapper to set off a labeled goroutine.
9+
func Go(ctx context.Context, labels pprof.LabelSet, f func(context.Context)) {
10+
go pprof.Do(ctx, labels, f)
11+
}
12+
13+
const (
14+
ServiceTag = "service"
15+
16+
ServiceHTTPServer = "http-api"
17+
ServiceLifecycles = "lifecycle-executor"
18+
ServiceMetricCollector = "metrics-collector"
19+
ServicePrebuildReconciler = "prebuilds-reconciler"
20+
ServiceTerraformProvisioner = "terraform-provisioner"
21+
)
22+
23+
func Service(name string, pairs ...string) pprof.LabelSet {
24+
return pprof.Labels(append([]string{ServiceTag, name}, pairs...)...)
25+
}

‎coderd/prometheusmetrics/insights/metricscollector.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"cdr.dev/slog"
1515

1616
"github.com/coder/coder/v2/coderd/database"
17+
"github.com/coder/coder/v2/coderd/pproflabel"
1718
"github.com/coder/coder/v2/coderd/util/slice"
1819
"github.com/coder/coder/v2/codersdk"
1920
)
@@ -158,7 +159,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) {
158159
})
159160
}
160161

161-
go func() {
162+
pproflabel.Go(ctx, pproflabel.Service(pproflabel.ServiceMetricCollector), func(ctx context.Context) {
162163
defer close(done)
163164
defer ticker.Stop()
164165
for {
@@ -170,7 +171,7 @@ func (mc *MetricsCollector) Run(ctx context.Context) (func(), error) {
170171
doTick()
171172
}
172173
}
173-
}()
174+
})
174175
return func() {
175176
closeFunc()
176177
<-done

‎enterprise/coderd/coderd.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,20 @@ import (
1212
"sync"
1313
"time"
1414

15-
"github.com/coder/quartz"
16-
1715
"github.com/coder/coder/v2/buildinfo"
1816
"github.com/coder/coder/v2/coderd/appearance"
1917
"github.com/coder/coder/v2/coderd/database"
2018
"github.com/coder/coder/v2/coderd/entitlements"
2119
"github.com/coder/coder/v2/coderd/idpsync"
2220
agplportsharing "github.com/coder/coder/v2/coderd/portsharing"
21+
"github.com/coder/coder/v2/coderd/pproflabel"
2322
agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds"
2423
"github.com/coder/coder/v2/coderd/rbac/policy"
2524
"github.com/coder/coder/v2/coderd/wsbuilder"
2625
"github.com/coder/coder/v2/enterprise/coderd/connectionlog"
2726
"github.com/coder/coder/v2/enterprise/coderd/enidpsync"
2827
"github.com/coder/coder/v2/enterprise/coderd/portsharing"
28+
"github.com/coder/quartz"
2929

3030
"golang.org/x/xerrors"
3131
"tailscale.com/tailcfg"
@@ -903,7 +903,9 @@ func (api *API) updateEntitlements(ctx context.Context) error {
903903
}
904904

905905
api.AGPL.PrebuildsReconciler.Store(&reconciler)
906-
go reconciler.Run(context.Background())
906+
// TODO: Should this context be the api.ctx context? To cancel when
907+
// the API (and entire app) is closed via shutdown?
908+
pproflabel.Go(context.Background(), pproflabel.Service(pproflabel.ServicePrebuildReconciler), reconciler.Run)
907909

908910
api.AGPL.PrebuildsClaimer.Store(&claimer)
909911
}

‎enterprise/wsproxy/wsproxy.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
333333
r.Use(
334334
// TODO: @emyrk Should we standardize these in some other package?
335335
httpmw.Recover(s.Logger),
336+
httpmw.WithProfilingLabels,
336337
tracing.StatusWriterMiddleware,
337338
tracing.Middleware(s.TracerProvider),
338339
httpmw.AttachRequestID,

0 commit comments

Comments
 (0)