Skip to content

Commit b2de7cd

Browse files
dhartuniandhartunian
committed
*: standardize pprof labeling code
This commit replaces usages of `pprof.Do` with our internal implementation that wraps the `SetProfilerLabels` utility. This enables us to have better standardization around profiler label application and easier tracking of all usages. A linter is added to enforce this usage. Epic: CRDB-55080 Part of: CRDB-55923 Release note: None
1 parent 4d1a96a commit b2de7cd

File tree

19 files changed

+83
-41
lines changed

19 files changed

+83
-41
lines changed

‎pkg/crosscluster/logical/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ go_library(
101101
"//pkg/util/log/logcrash",
102102
"//pkg/util/metamorphic",
103103
"//pkg/util/metric",
104+
"//pkg/util/pprofutil",
104105
"//pkg/util/protoutil",
105106
"//pkg/util/randutil",
106107
"//pkg/util/retry",

‎pkg/crosscluster/logical/logical_replication_writer_processor.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"fmt"
1111
"hash/fnv"
1212
"regexp"
13-
"runtime/pprof"
1413
"slices"
1514
"strings"
1615
"time"
@@ -45,6 +44,7 @@ import (
4544
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
4645
"github.com/cockroachdb/cockroach/pkg/util/log"
4746
"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
47+
"github.com/cockroachdb/cockroach/pkg/util/pprofutil"
4848
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
4949
"github.com/cockroachdb/cockroach/pkg/util/span"
5050
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -349,12 +349,12 @@ func (lrw *logicalReplicationWriterProcessor) Start(ctx context.Context) {
349349
})
350350
lrw.workerGroup.GoCtx(func(ctx context.Context) error {
351351
defer close(lrw.checkpointCh)
352-
pprof.Do(ctx, pprof.Labels("proc", fmt.Sprintf("%d", lrw.ProcessorID)), func(ctx context.Context) {
352+
pprofutil.Do(ctx, func(ctx context.Context) {
353353
if err := lrw.consumeEvents(ctx); err != nil {
354354
log.Dev.Infof(lrw.Ctx(), "consumer completed. Error: %s", err)
355355
lrw.sendError(errors.Wrap(err, "consume events"))
356356
}
357-
})
357+
}, "proc", fmt.Sprintf("%d", lrw.ProcessorID))
358358
return nil
359359
})
360360
}

‎pkg/crosscluster/logical/offline_initial_scan_processor.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package logical
88
import (
99
"context"
1010
"fmt"
11-
"runtime/pprof"
1211
"time"
1312

1413
"github.com/cockroachdb/cockroach/pkg/backup"
@@ -31,6 +30,7 @@ import (
3130
"github.com/cockroachdb/cockroach/pkg/util/hlc"
3231
"github.com/cockroachdb/cockroach/pkg/util/log"
3332
"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
33+
"github.com/cockroachdb/cockroach/pkg/util/pprofutil"
3434
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
3535
"github.com/cockroachdb/cockroach/pkg/util/span"
3636
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -225,7 +225,7 @@ func (o *offlineInitialScanProcessor) Start(ctx context.Context) {
225225
o.workerGroup.GoCtx(func(ctx context.Context) error {
226226
defer close(o.checkpointCh)
227227
defer close(o.rangeStatsCh)
228-
pprof.Do(ctx, pprof.Labels("proc", fmt.Sprintf("%d", o.ProcessorID)), func(ctx context.Context) {
228+
pprofutil.Do(ctx, func(ctx context.Context) {
229229
for event := range o.subscription.Events() {
230230
if err := o.handleEvent(ctx, event); err != nil {
231231
log.Dev.Infof(o.Ctx(), "consumer completed. Error: %s", err)
@@ -235,7 +235,7 @@ func (o *offlineInitialScanProcessor) Start(ctx context.Context) {
235235
if err := o.subscription.Err(); err != nil {
236236
o.sendError(errors.Wrap(err, "subscription"))
237237
}
238-
})
238+
}, "proc", fmt.Sprintf("%d", o.ProcessorID))
239239
return nil
240240
})
241241
}

‎pkg/kv/kvclient/rangefeed/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ go_library(
2626
"//pkg/util/limit",
2727
"//pkg/util/log",
2828
"//pkg/util/mon",
29+
"//pkg/util/pprofutil",
2930
"//pkg/util/retry",
3031
"//pkg/util/span",
3132
"//pkg/util/stop",

‎pkg/kv/kvclient/rangefeed/rangefeed.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package rangefeed
88
import (
99
"context"
1010
"fmt"
11-
"runtime/pprof"
1211
"strings"
1312
"sync"
1413
"sync/atomic"
@@ -23,6 +22,7 @@ import (
2322
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
2423
"github.com/cockroachdb/cockroach/pkg/util/hlc"
2524
"github.com/cockroachdb/cockroach/pkg/util/log"
25+
"github.com/cockroachdb/cockroach/pkg/util/pprofutil"
2626
"github.com/cockroachdb/cockroach/pkg/util/retry"
2727
"github.com/cockroachdb/cockroach/pkg/util/span"
2828
"github.com/cockroachdb/cockroach/pkg/util/stop"
@@ -241,9 +241,9 @@ func (f *RangeFeed) start(
241241
// pprof.Do function does exactly what we do here, but it also results in
242242
// pprof.Do function showing up in the stack traces -- so, just set and reset
243243
// labels manually.
244-
defer pprof.SetGoroutineLabels(ctx)
245-
ctx = pprof.WithLabels(ctx, pprof.Labels(append(f.extraPProfLabels, "rangefeed", f.name)...))
246-
pprof.SetGoroutineLabels(ctx)
244+
ctx, reset := pprofutil.SetProfilerLabels(ctx, append(f.extraPProfLabels, "rangefeed", f.name)...)
245+
defer reset()
246+
247247
if f.invoker != nil {
248248
_ = f.invoker(func() error {
249249
f.run(ctx, frontier, resumeFromFrontier)

‎pkg/kv/kvserver/raft_transport.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"context"
1010
"math"
1111
"net"
12-
"runtime/pprof"
1312
"sync/atomic"
1413
"time"
1514

@@ -29,6 +28,7 @@ import (
2928
"github.com/cockroachdb/cockroach/pkg/storage"
3029
"github.com/cockroachdb/cockroach/pkg/util/hlc"
3130
"github.com/cockroachdb/cockroach/pkg/util/log"
31+
"github.com/cockroachdb/cockroach/pkg/util/pprofutil"
3232
"github.com/cockroachdb/cockroach/pkg/util/stop"
3333
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
3434
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -931,7 +931,7 @@ func (t *RaftTransport) startProcessNewQueue(
931931
}
932932
go func(ctx context.Context) {
933933
defer hdl.Activate(ctx).Release(ctx)
934-
pprof.Do(ctx, pprof.Labels("remote_node_id", toNodeID.String()), worker)
934+
pprofutil.Do(ctx, worker, "remote_node_id", toNodeID.String())
935935
}(ctx)
936936
return true
937937
}

‎pkg/kv/kvserver/replica_send.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package kvserver
88
import (
99
"context"
1010
"reflect"
11-
"runtime/pprof"
1211
"runtime/trace"
1312
"time"
1413

@@ -30,6 +29,7 @@ import (
3029
"github.com/cockroachdb/cockroach/pkg/util/grunning"
3130
"github.com/cockroachdb/cockroach/pkg/util/hlc"
3231
"github.com/cockroachdb/cockroach/pkg/util/log"
32+
"github.com/cockroachdb/cockroach/pkg/util/pprofutil"
3333
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
3434
"github.com/cockroachdb/cockroach/pkg/util/tracing"
3535
"github.com/cockroachdb/errors"
@@ -131,16 +131,13 @@ func (r *Replica) SendWithWriteBytes(
131131
defer r.MeasureReqCPUNanos(ctx, startCPU)
132132

133133
if r.store.cfg.Settings.CPUProfileType() == cluster.CPUProfileWithLabels {
134-
defer pprof.SetGoroutineLabels(ctx)
135-
// Note: the defer statement captured the previous context.
136-
var lbls pprof.LabelSet
134+
var reset func()
137135
if tenantIDOrZero.IsSet() {
138-
lbls = pprof.Labels("range_str", r.rangeStr.ID(), "tenant_id", tenantIDOrZero.String())
136+
ctx, reset = pprofutil.SetProfilerLabels(ctx, "range_str", r.rangeStr.ID(), "tenant_id", tenantIDOrZero.String())
139137
} else {
140-
lbls = pprof.Labels("range_str", r.rangeStr.ID())
138+
ctx, reset = pprofutil.SetProfilerLabels(ctx, "range_str", r.rangeStr.ID())
141139
}
142-
ctx = pprof.WithLabels(ctx, lbls)
143-
pprof.SetGoroutineLabels(ctx)
140+
defer reset()
144141
}
145142
if trace.IsEnabled() {
146143
defer trace.StartRegion(ctx, r.rangeStr.String() /* cheap */).End()

‎pkg/kv/kvserver/storeliveness/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ go_library(
3232
"//pkg/util/log",
3333
"//pkg/util/metamorphic",
3434
"//pkg/util/metric",
35+
"//pkg/util/pprofutil",
3536
"//pkg/util/protoutil",
3637
"//pkg/util/stop",
3738
"//pkg/util/syncutil",

‎pkg/kv/kvserver/storeliveness/transport.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package storeliveness
77

88
import (
99
"context"
10-
"runtime/pprof"
1110
"time"
1211

1312
slpb "github.com/cockroachdb/cockroach/pkg/kv/kvserver/storeliveness/storelivenesspb"
@@ -19,6 +18,7 @@ import (
1918
"github.com/cockroachdb/cockroach/pkg/util/hlc"
2019
"github.com/cockroachdb/cockroach/pkg/util/log"
2120
"github.com/cockroachdb/cockroach/pkg/util/metamorphic"
21+
"github.com/cockroachdb/cockroach/pkg/util/pprofutil"
2222
"github.com/cockroachdb/cockroach/pkg/util/stop"
2323
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
2424
"github.com/cockroachdb/cockroach/pkg/util/taskpacer"
@@ -570,7 +570,7 @@ func (t *Transport) startProcessNewQueue(
570570
err := t.stopper.RunAsyncTask(
571571
ctx, "storeliveness.Transport: sending messages",
572572
func(ctx context.Context) {
573-
pprof.Do(ctx, pprof.Labels("remote_node_id", toNodeID.String()), worker)
573+
pprofutil.Do(ctx, worker, "remote_node_id", toNodeID.String())
574574
},
575575
)
576576
if err != nil {

‎pkg/kv/kvserver/txnwait/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ go_library(
2121
"//pkg/util/hlc",
2222
"//pkg/util/log",
2323
"//pkg/util/metric",
24+
"//pkg/util/pprofutil",
2425
"//pkg/util/retry",
2526
"//pkg/util/stop",
2627
"//pkg/util/syncutil",

0 commit comments

Comments
 (0)