Skip to content

Commit 94664d3

Browse files
authored
chore: Migrate pyroscope components to slog (grafana#6431)
### Pull Request Details Migrate all pyroscope components to use slog ### Issue(s) fixed by this Pull Request Part of: grafana#4813 ### Notes to the Reviewer Before some log lines missed `msg` field and I am not sure that what I used for them is correct ### PR Checklist <!-- Remove items that do not apply. For completed items, change [ ] to [x]. --> - [ ] Documentation added - [x] Tests updated - [ ] Config converters updated
1 parent ed526ef commit 94664d3

39 files changed

Lines changed: 242 additions & 256 deletions

internal/component/pyroscope/ebpf/ebpf_linux.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package ebpf
55
import (
66
"context"
77
"fmt"
8+
"log/slog"
89
"os"
910
"path"
1011
"path/filepath"
@@ -14,9 +15,6 @@ import (
1415
"syscall"
1516
"time"
1617

17-
"github.com/go-kit/log"
18-
"github.com/go-kit/log/level"
19-
"github.com/grafana/alloy/internal/component/pyroscope/ebpf/symb/irsymcache"
2018
"github.com/grafana/pyroscope/lidia"
2119
"github.com/oklog/run"
2220
"github.com/prometheus/client_golang/prometheus"
@@ -38,6 +36,7 @@ import (
3836
alloydiscovery "github.com/grafana/alloy/internal/component/pyroscope/ebpf/discovery"
3937
"github.com/grafana/alloy/internal/component/pyroscope/ebpf/reporter"
4038
rargs "github.com/grafana/alloy/internal/component/pyroscope/ebpf/reporter/args"
39+
"github.com/grafana/alloy/internal/component/pyroscope/ebpf/symb/irsymcache"
4140
"github.com/grafana/alloy/internal/component/pyroscope/write/debuginfo"
4241
"github.com/grafana/alloy/internal/featuregate"
4342
)
@@ -51,7 +50,7 @@ func init() {
5150
Build: func(opts component.Options, args component.Arguments) (component.Component, error) {
5251
arguments := args.(Arguments)
5352

54-
return New(opts.Logger, opts.Registerer, opts.ID, arguments)
53+
return New(opts.SLogger, opts.Registerer, opts.ID, arguments)
5554
},
5655
})
5756
python.NoContinueWithNextUnwinder.Store(true)
@@ -63,7 +62,7 @@ var (
6362
ebpfMetricsErr error // stored for all instances to check
6463
)
6564

66-
func New(logger log.Logger, reg prometheus.Registerer, id string, args Arguments) (*Component, error) {
65+
func New(logger *slog.Logger, reg prometheus.Registerer, id string, args Arguments) (*Component, error) {
6766
// ebpfmetrics.Start writes to package-level globals in the upstream library,
6867
// so it must only be called once. All instances share the same OTel registry.
6968
ebpfMetricsOnce.Do(func() {
@@ -160,7 +159,7 @@ func New(logger log.Logger, reg prometheus.Registerer, id string, args Arguments
160159
}
161160

162161
type Component struct {
163-
logger log.Logger
162+
logger *slog.Logger
164163
args Arguments
165164
dynamicProfilingPolicy bool
166165
argsUpdate chan Arguments
@@ -179,7 +178,7 @@ func (c *Component) Run(ctx context.Context) error {
179178
c.checkTraceFS()
180179

181180
if c.args.LazyMode && len(c.args.Targets) == 0 {
182-
_ = level.Info(c.logger).Log("msg", "lazy mode enabled, waiting for targets to profile")
181+
c.logger.Info("lazy mode enabled, waiting for targets to profile")
183182
if err := c.waitForTargets(ctx); err != nil {
184183
return err
185184
}
@@ -253,14 +252,13 @@ func (c *Component) Update(args component.Arguments) error {
253252
select {
254253
case c.argsUpdate <- newArgs:
255254
default:
256-
_ = level.Debug(c.logger).Log("msg", "dropped args update")
255+
c.logger.Debug("dropped args update")
257256
}
258257
return nil
259258
}
260259

261260
func (c *Component) reportUnhealthy(err error) {
262-
_ = level.Error(c.logger).
263-
Log("msg", "unhealthy", "err", err)
261+
c.logger.Error("unhealthy", "err", err)
264262

265263
c.healthMut.Lock()
266264
defer c.healthMut.Unlock()
@@ -296,15 +294,15 @@ func (c *Component) checkTraceFS() {
296294
if err != nil {
297295
continue
298296
}
299-
level.Debug(c.logger).Log("msg", "found tracefs at "+p)
297+
c.logger.Debug("found tracefs at " + p)
300298
return
301299
}
302300
mountPath := candidates[0]
303301
err := syscall.Mount("tracefs", mountPath, "tracefs", 0, "")
304302
if err != nil {
305-
level.Error(c.logger).Log("msg", "failed to mount tracefs at "+mountPath, "err", err)
303+
c.logger.Error("failed to mount tracefs at "+mountPath, "err", err)
306304
} else {
307-
level.Debug(c.logger).Log("msg", "mounted tracefs at "+mountPath)
305+
c.logger.Debug("mounted tracefs at " + mountPath)
308306
}
309307
}
310308

internal/component/pyroscope/ebpf/ebpf_linux_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func TestReconstructionAfterError(t *testing.T) {
116116
require.NoError(t, err)
117117
invalidCachePath := filepath.Join(f.Name(), "symb.cache")
118118

119-
logger := util.TestLogger(t)
119+
logger := util.TestAlloyLogger(t).Slog()
120120
reg := prometheus.NewRegistry()
121121

122122
args := NewDefaultArguments()

internal/component/pyroscope/ebpf/ebpf_placeholder.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
"github.com/grafana/alloy/internal/component"
99
"github.com/grafana/alloy/internal/featuregate"
10-
"github.com/grafana/alloy/internal/runtime/logging/level"
1110
)
1211

1312
func init() {
@@ -28,7 +27,7 @@ type Component struct {
2827
}
2928

3029
func New(opts component.Options, args Arguments) (component.Component, error) {
31-
level.Warn(opts.Logger).Log("msg", "the pyroscope.ebpf component only works on ARM64 and AMD64 Linux platforms; enabling it otherwise will do nothing")
30+
opts.SLogger.Warn("the pyroscope.ebpf component only works on ARM64 and AMD64 Linux platforms; enabling it otherwise will do nothing")
3231
return &Component{}, nil
3332
}
3433

internal/component/pyroscope/ebpf/reporter/parca/reporter/pyroscope_uploader.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"context"
77
"fmt"
88
"io"
9+
"log/slog"
910
"maps"
1011
"os"
1112
"path/filepath"
@@ -19,9 +20,6 @@ import (
1920
"connectrpc.com/connect"
2021
debuginfov1alpha1 "github.com/grafana/pyroscope/api/gen/proto/go/debuginfo/v1alpha1"
2122

22-
"github.com/go-kit/log"
23-
"github.com/go-kit/log/level"
24-
2523
lru "github.com/elastic/go-freelru"
2624
"github.com/prometheus/client_golang/prometheus"
2725
"golang.org/x/sync/errgroup" //nolint:depguard
@@ -43,7 +41,7 @@ type uploadRequest struct {
4341
}
4442

4543
type PyroscopeSymbolUploader struct {
46-
logger log.Logger
44+
logger *slog.Logger
4745

4846
retry *lru.SyncedLRU[libpf.FileID, struct{}]
4947

@@ -58,7 +56,7 @@ type PyroscopeSymbolUploader struct {
5856
}
5957

6058
func NewPyroscopeSymbolUploader(
61-
logger log.Logger,
59+
logger *slog.Logger,
6260
cacheSize uint32,
6361
stripTextSection bool,
6462
queueSize uint32,
@@ -74,28 +72,28 @@ func NewPyroscopeSymbolUploader(
7472

7573
cacheDirectory := filepath.Join(cacheDir, "symuploader")
7674
if _, err := os.Stat(cacheDirectory); os.IsNotExist(err) {
77-
level.Debug(logger).Log("msg", "creating cache directory", "path", cacheDirectory)
75+
logger.Debug("creating cache directory", "path", cacheDirectory)
7876
if err := os.MkdirAll(cacheDirectory, os.ModePerm); err != nil {
7977
return nil, fmt.Errorf("failed to create cache directory (%s): %s", cacheDirectory, err)
8078
}
8179
}
8280

8381
if err := filepath.Walk(cacheDirectory, func(path string, info os.FileInfo, err error) error {
8482
if err != nil {
85-
level.Warn(logger).Log("msg", "failed to access cached file during walk", "path", path, "err", err)
83+
logger.Warn("failed to access cached file during walk", "path", path, "err", err)
8684
return nil
8785
}
8886

8987
if info == nil {
90-
level.Warn(logger).Log("msg", "filepath.Walk returned nil FileInfo", "path", path)
88+
logger.Warn("filepath.Walk returned nil FileInfo", "path", path)
9189
return nil
9290
}
9391
if info.IsDir() {
9492
return nil
9593
}
9694

9795
if os.Remove(path) != nil {
98-
level.Warn(logger).Log("msg", "failed to remove cached file", "path", path)
96+
logger.Warn("failed to remove cached file", "path", path)
9997
}
10098

10199
return nil
@@ -180,7 +178,7 @@ func (u *PyroscopeSymbolUploader) Run(ctx context.Context) error {
180178
return nil
181179
case req := <-u.queue:
182180
if err := u.attemptUpload(ctx, req.client, req.fileID, req.fileName, req.buildID, req.open); err != nil {
183-
level.Warn(u.logger).Log("msg", "failed to upload", "file_name", req.fileName, "build_id", req.buildID, "err", err)
181+
u.logger.Warn("failed to upload", "file_name", req.fileName, "build_id", req.buildID, "err", err)
184182
}
185183
}
186184
}
@@ -220,7 +218,7 @@ func (u *PyroscopeSymbolUploader) Upload(ctx context.Context, client *debuginfoc
220218
default:
221219
// The queue is full, we can't enqueue the request.
222220
u.inProgressTracker.Remove(fileID)
223-
level.Warn(u.logger).Log("msg", "failed to enqueue upload request, queue is full", "file_name", fileName, "build_id", buildID)
221+
u.logger.Warn("failed to enqueue upload request, queue is full", "file_name", fileName, "build_id", buildID)
224222
}
225223
}
226224

@@ -248,13 +246,13 @@ func (u *PyroscopeSymbolUploader) attemptUpload(ctx context.Context, client *deb
248246
return fmt.Errorf("ShouldInitiateUpload: %w", err)
249247
}
250248

251-
l := log.With(u.logger,
249+
l := u.logger.With(
252250
"file_name", fileName,
253251
"otel_file_id", fileID,
254252
"gnu_build_id", buildID,
255253
)
256254

257-
level.Debug(l).Log("msg", "ShouldInitiateUpload result",
255+
l.Debug("ShouldInitiateUpload result",
258256
"should_initiate_upload", resp.Msg.ShouldInitiateUpload,
259257
"reason", resp.Msg.Reason)
260258

@@ -353,7 +351,7 @@ func (u *PyroscopeSymbolUploader) attemptUpload(ctx context.Context, client *deb
353351
}
354352

355353
u.uploadRequestBytes.Add(float64(fileSize))
356-
level.Debug(l).Log("msg", "upload succeeded", "bytes", fileSize)
354+
l.Debug("upload succeeded", "bytes", fileSize)
357355
u.retry.Add(fileID, struct{}{})
358356
return nil
359357
}
@@ -363,10 +361,10 @@ type Stater interface {
363361
}
364362

365363
// readAtCloserSize attempts to determine the size of the reader.
366-
func readAtCloserSize(logger log.Logger, r process.ReadAtCloser) (int64, error) {
364+
func readAtCloserSize(logger *slog.Logger, r process.ReadAtCloser) (int64, error) {
367365
stater, ok := r.(Stater)
368366
if !ok {
369-
level.Debug(logger).Log("msg", "ReadAtCloser is not a Stater, can't determine size")
367+
logger.Debug("ReadAtCloser is not a Stater, can't determine size")
370368
return 0, nil
371369
}
372370

internal/component/pyroscope/ebpf/reporter/parca/reporter/pyroscope_uploader_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"bytes"
77
"context"
88
"io"
9+
"log/slog"
910
"math/rand"
1011
"net/http"
1112
"net/http/httptest"
@@ -15,7 +16,6 @@ import (
1516
"time"
1617

1718
"connectrpc.com/connect"
18-
"github.com/go-kit/log"
1919
"github.com/gorilla/mux"
2020
"github.com/grafana/alloy/internal/component/pyroscope/write/debuginfoclient"
2121
debuginfov1alpha1 "github.com/grafana/pyroscope/api/gen/proto/go/debuginfo/v1alpha1"
@@ -61,7 +61,7 @@ func newTestUploader(t *testing.T) (*PyroscopeSymbolUploader, prometheus.Counter
6161
t.Helper()
6262
counter := prometheus.NewCounter(prometheus.CounterOpts{Name: "test_upload_bytes"})
6363
u, err := NewPyroscopeSymbolUploader(
64-
log.NewNopLogger(),
64+
slog.New(slog.DiscardHandler),
6565
1024, // cacheSize
6666
false, // stripTextSection
6767
64, // queueSize

internal/component/pyroscope/ebpf/reporter/pprof.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ import (
77
"context"
88
"errors"
99
"fmt"
10+
"log/slog"
1011
"strconv"
1112
"sync"
1213
"time"
1314

14-
"github.com/go-kit/log"
15-
"github.com/go-kit/log/level"
1615
"github.com/google/pprof/profile"
1716
"github.com/grafana/alloy/internal/component/pyroscope/ebpf/discovery"
1817
"github.com/grafana/alloy/internal/component/pyroscope/ebpf/reporter/args"
@@ -45,7 +44,7 @@ type Config struct {
4544
}
4645
type PPROFReporter struct {
4746
cfg *Config
48-
log log.Logger
47+
log *slog.Logger
4948

5049
consumer PPROFConsumer
5150
symbols irsymcache.NativeSymbolResolver
@@ -58,7 +57,7 @@ type PPROFReporter struct {
5857
cancelReporting context.CancelFunc
5958
}
6059

61-
func NewPPROF(log log.Logger,
60+
func NewPPROF(log *slog.Logger,
6261
cfg *Config,
6362
sd discovery.TargetProducer,
6463
symbols irsymcache.NativeSymbolResolver,
@@ -179,7 +178,7 @@ func (p *PPROFReporter) reportProfile(ctx context.Context) {
179178
for _, it := range profiles {
180179
sz += len(it.Raw)
181180
}
182-
_ = level.Debug(p.log).Log("msg", "pprof report successful", "count", len(profiles), "total-size", sz)
181+
p.log.Debug("pprof report successful", "count", len(profiles), "total-size", sz)
183182
}
184183

185184
func (p *PPROFReporter) createProfile(resourceKey samples.ResourceKey, origin libpf.Origin, events map[samples.SampleKey]*samples.TraceEvents) []PPROF {
@@ -313,7 +312,7 @@ func (p *PPROFReporter) createProfile(resourceKey samples.ResourceKey, origin li
313312
buf := bytes.NewBuffer(nil)
314313
_, err := b.Write(buf)
315314
if err != nil {
316-
_ = p.log.Log("err", err)
315+
p.log.Error("failed to encode profile", "err", err)
317316
continue
318317
}
319318
_, ls := b.Target.Labels()

internal/component/pyroscope/ebpf/reporter/pprof_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package reporter
44

55
import (
66
"bytes"
7+
"log/slog"
78
"testing"
89

910
"github.com/google/pprof/profile"
@@ -47,7 +48,7 @@ func newReporter() *PPROFReporter {
4748
},
4849
})
4950
return NewPPROF(
50-
nil,
51+
slog.New(slog.DiscardHandler),
5152
&Config{
5253
SamplesPerSecond: 97,
5354
KernelFrames: true,

internal/component/pyroscope/ebpf/send.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"context"
77
"time"
88

9-
"github.com/go-kit/log/level"
109
"github.com/grafana/alloy/internal/component/pyroscope"
1110
"github.com/grafana/alloy/internal/component/pyroscope/ebpf/reporter"
1211
)
@@ -23,7 +22,7 @@ func (c *Component) sendProfiles(ctx context.Context, ps []reporter.PPROF) {
2322
defer func() {
2423
pool.stop()
2524
cancel()
26-
level.Debug(c.logger).Log("msg", "sent profiles", "duration", time.Since(start), "queued", queued)
25+
c.logger.Debug("sent profiles", "duration", time.Since(start), "queued", queued)
2726
}()
2827
j := 0
2928
for _, p := range ps {
@@ -40,7 +39,7 @@ func (c *Component) sendProfiles(ctx context.Context, ps []reporter.PPROF) {
4039
samples := []*pyroscope.RawSample{{RawProfile: rawProfile}}
4140
err := appender.Append(ctx, p.Labels, samples)
4241
if err != nil {
43-
level.Error(c.logger).Log("msg", "ebpf pprof write", "err", err)
42+
c.logger.Error("ebpf pprof write", "err", err)
4443
}
4544
}
4645
select {
@@ -49,7 +48,7 @@ func (c *Component) sendProfiles(ctx context.Context, ps []reporter.PPROF) {
4948
case <-ctx.Done():
5049
dropped := n - j
5150
c.metrics.pprofsDroppedTotal.Add(float64(dropped))
52-
level.Debug(c.logger).Log("msg", "dropped profiles", "count", dropped)
51+
c.logger.Debug("dropped profiles", "count", dropped)
5352
return
5453
}
5554
j++

internal/component/pyroscope/ebpf/send_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func TestSendProfilesConcurrently(t *testing.T) {
5959
reg := prometheus.NewRegistry()
6060
c := new(Component)
6161
c.metrics = newMetrics(reg)
62-
c.logger = util.TestLogger(t)
62+
c.logger = util.TestAlloyLogger(t).Slog()
6363
c.args.CollectInterval = td.collectionInterval
6464
successes := atomic.Uint32{}
6565
failures := atomic.Uint32{}

0 commit comments

Comments
 (0)