diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a64005ab53..eeb1eb3666 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -63,7 +63,10 @@ jobs: run: sudo apt-get install llvm libelf-dev - name: Install golangci-lint - run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.42.0 + run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.44.2 + + - name: Install gofumpt + run: go install mvdan.cc/gofumpt@latest - name: Check out code into the Go module directory uses: actions/checkout@v2 diff --git a/.golangci.yml b/.golangci.yml index f5edd089ed..2b5a160080 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,8 +3,42 @@ run: skip-dirs: - internal/pprof - internal/go + +linters: + enable: + - depguard + - godot + - gofumpt + - goimports + - revive + - whitespace + +issues: + exclude-rules: + - path: _test.go + linters: + - errcheck + linters-settings: + depguard: + list-type: blacklist + include-go-root: true + packages-with-error-message: + - sync/atomic: "Use go.uber.org/atomic instead of sync/atomic" + - github.com/stretchr/testify/assert: "Use github.com/stretchr/testify/require instead of github.com/stretchr/testify/assert" + - github.com/go-kit/kit/log: "Use github.com/go-kit/log instead of github.com/go-kit/kit/log" + - github.com/pkg/errors: "Use fmt.Errorf instead" errcheck: exclude: ./.errcheck_excludes.txt + goimports: + local-prefixes: github.com/parca-dev/parca-agent + gofumpt: + extra-rules: true misspell: locale: US + revive: + rules: + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unexported-return + - name: unexported-return + severity: warning + disabled: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 406bebc4ba..ffa1e15d9e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -122,6 +122,8 @@ This is roughly what the contribution workflow should look like: - Create a topic branch from where you want to base your work (usually master). - Make commits of logical units. - Make sure the tests pass, and add any new tests as appropriate. +- Make sure the code is properly formatted. (`make format` could be useful here.) +- Make sure the code is properly linted. (`make lint` could be useful here.) - Make sure your commit messages follow the commit guidelines (see below). - Push your changes to a topic branch in your fork of the repository. - Submit a pull request to the original repository. diff --git a/Makefile b/Makefile index 000c3be8bd..b58ff61b1b 100644 --- a/Makefile +++ b/Makefile @@ -227,14 +227,15 @@ README.md: $(CMD_EMBEDMD) $(OUT_DIR)/help.txt deploy/manifests $(CMD_EMBEDMD) -w README.md .PHONY: format -format: go-fmt check-license +format: go/fmt check-license -.PHONY: c-fmt -c-fmt: +.PHONY: c/fmt +c/fmt: clang-format -i --style=GNU $(BPF_SRC) -.PHONY: go-fmt -go-fmt: +.PHONY: go/fmt +go/fmt: + gofumpt -l -w $(shell go list ./... | grep -E -v "internal/pprof|internal/go" | sed 's#^github.com/parca-dev/parca-agent/##') go fmt $(shell go list ./... | grep -E -v "internal/pprof|internal/go") .PHONY: check-license diff --git a/cmd/debug-info/main.go b/cmd/debug-info/main.go index 35f3e81a11..1717c77019 100644 --- a/cmd/debug-info/main.go +++ b/cmd/debug-info/main.go @@ -114,7 +114,7 @@ func main() { if err := os.RemoveAll(flags.Extract.OutputDir); err != nil { return fmt.Errorf("failed to clean output dir, %s: %w", flags.Extract.OutputDir, err) } - if err := os.MkdirAll(flags.Extract.OutputDir, 0755); err != nil { + if err := os.MkdirAll(flags.Extract.OutputDir, 0o755); err != nil { return fmt.Errorf("failed to create output dir, %s: %w", flags.Extract.OutputDir, err) } buildIDFiles := map[string]string{} diff --git a/cmd/parca-agent/main.go b/cmd/parca-agent/main.go index b8b5679db9..04560aedd9 100644 --- a/cmd/parca-agent/main.go +++ b/cmd/parca-agent/main.go @@ -204,7 +204,6 @@ func main() { Error: profiler.LastError(), Link: fmt.Sprintf("/query?%s", q.Encode()), }) - } } diff --git a/env.sh b/env.sh index 0d76232b0d..56199d14a1 100755 --- a/env.sh +++ b/env.sh @@ -10,3 +10,5 @@ go install github.com/google/go-jsonnet/cmd/jsonnetfmt@latest go install github.com/jsonnet-bundler/jsonnet-bundler/cmd/jb@latest go install github.com/campoy/embedmd@latest + +go install mvdan.cc/gofumpt@latest diff --git a/pkg/agent/profile_listener.go b/pkg/agent/profile_listener.go index 8469ccbea8..427d7e794a 100644 --- a/pkg/agent/profile_listener.go +++ b/pkg/agent/profile_listener.go @@ -25,15 +25,15 @@ import ( "google.golang.org/grpc" ) -type ProfileListener struct { +type profileListener struct { next profilestorepb.ProfileStoreServiceClient observers []*observer omtx *sync.Mutex logger log.Logger } -func NewProfileListener(logger log.Logger, next profilestorepb.ProfileStoreServiceClient) *ProfileListener { - return &ProfileListener{ +func NewProfileListener(logger log.Logger, next profilestorepb.ProfileStoreServiceClient) *profileListener { + return &profileListener{ next: next, observers: []*observer{}, omtx: &sync.Mutex{}, @@ -41,8 +41,8 @@ func NewProfileListener(logger log.Logger, next profilestorepb.ProfileStoreServi } } -func (l *ProfileListener) WriteRaw(ctx context.Context, r *profilestorepb.WriteRawRequest, opts ...grpc.CallOption) (*profilestorepb.WriteRawResponse, error) { - l.ObserveProfile(r) +func (l *profileListener) WriteRaw(ctx context.Context, r *profilestorepb.WriteRawRequest, opts ...grpc.CallOption) (*profilestorepb.WriteRawResponse, error) { + l.observeProfile(r) return l.next.WriteRaw(ctx, r, opts...) } @@ -50,7 +50,7 @@ type observer struct { f func(*profilestorepb.WriteRawRequest) } -func (l *ProfileListener) ObserveProfile(r *profilestorepb.WriteRawRequest) { +func (l *profileListener) observeProfile(r *profilestorepb.WriteRawRequest) { l.omtx.Lock() defer l.omtx.Unlock() @@ -59,7 +59,7 @@ func (l *ProfileListener) ObserveProfile(r *profilestorepb.WriteRawRequest) { } } -func (l *ProfileListener) Observe(f func(*profilestorepb.WriteRawRequest)) *observer { +func (l *profileListener) observe(f func(*profilestorepb.WriteRawRequest)) *observer { l.omtx.Lock() defer l.omtx.Unlock() @@ -70,7 +70,7 @@ func (l *ProfileListener) Observe(f func(*profilestorepb.WriteRawRequest)) *obse return o } -func (l *ProfileListener) RemoveObserver(o *observer) { +func (l *profileListener) removeObserver(o *observer) { l.omtx.Lock() defer l.omtx.Unlock() @@ -87,12 +87,11 @@ func (l *ProfileListener) RemoveObserver(o *observer) { } } -func (l *ProfileListener) NextMatchingProfile(ctx context.Context, matchers []*labels.Matcher) (*profile.Profile, error) { +func (l *profileListener) NextMatchingProfile(ctx context.Context, matchers []*labels.Matcher) (*profile.Profile, error) { pCh := make(chan []byte) defer close(pCh) - o := l.Observe(func(r *profilestorepb.WriteRawRequest) { - + o := l.observe(func(r *profilestorepb.WriteRawRequest) { var searchedSeries *profilestorepb.RawProfileSeries for _, series := range r.Series { @@ -116,7 +115,7 @@ func (l *ProfileListener) NextMatchingProfile(ctx context.Context, matchers []*l pCh <- searchedSeries.Samples[len(searchedSeries.Samples)-1].RawProfile } }) - defer l.RemoveObserver(o) + defer l.removeObserver(o) select { case p := <-pCh: diff --git a/pkg/agent/write_client.go b/pkg/agent/write_client.go index bf4fa572db..7216ab3874 100644 --- a/pkg/agent/write_client.go +++ b/pkg/agent/write_client.go @@ -15,7 +15,6 @@ package agent import ( "context" - "sync" "time" @@ -90,7 +89,7 @@ func (b *Batcher) batchLoop(ctx context.Context) error { return nil } -func isEqualLabel(a *profilestorepb.LabelSet, b *profilestorepb.LabelSet) bool { +func isEqualLabel(a, b *profilestorepb.LabelSet) bool { if len(a.Labels) != len(b.Labels) { return false } diff --git a/pkg/agent/write_client_test.go b/pkg/agent/write_client_test.go index 3b57974ed6..fac64628d5 100644 --- a/pkg/agent/write_client_test.go +++ b/pkg/agent/write_client_test.go @@ -23,7 +23,7 @@ import ( "github.com/stretchr/testify/require" ) -func isEqualSample(a []*profilestorepb.RawSample, b []*profilestorepb.RawSample) bool { +func isEqualSample(a, b []*profilestorepb.RawSample) bool { if len(a) != len(b) { return false } @@ -37,7 +37,7 @@ func isEqualSample(a []*profilestorepb.RawSample, b []*profilestorepb.RawSample) return ret } -func compareProfileSeries(a []*profilestorepb.RawProfileSeries, b []*profilestorepb.RawProfileSeries) bool { +func compareProfileSeries(a, b []*profilestorepb.RawProfileSeries) bool { if len(a) != len(b) { return false } @@ -70,7 +70,7 @@ func TestWriteClient(t *testing.T) { ctx := context.Background() - samples1 := []*profilestorepb.RawSample{{RawProfile: []byte{11, 04, 96}}} + samples1 := []*profilestorepb.RawSample{{RawProfile: []byte{11, 0o4, 96}}} samples2 := []*profilestorepb.RawSample{{RawProfile: []byte{15, 11, 95}}} t.Run("insertFirstProfile", func(t *testing.T) { @@ -78,7 +78,8 @@ func TestWriteClient(t *testing.T) { Series: []*profilestorepb.RawProfileSeries{{ Labels: &labelset1, Samples: samples1, - }}}) + }}, + }) series := []*profilestorepb.RawProfileSeries{{ Labels: &labelset1, @@ -94,14 +95,15 @@ func TestWriteClient(t *testing.T) { Series: []*profilestorepb.RawProfileSeries{{ Labels: &labelset2, Samples: samples2, - }}}) + }}, + }) series := []*profilestorepb.RawProfileSeries{ - &profilestorepb.RawProfileSeries{ + { Labels: &labelset1, Samples: samples1, }, - &profilestorepb.RawProfileSeries{ + { Labels: &labelset2, Samples: samples2, }, @@ -116,14 +118,15 @@ func TestWriteClient(t *testing.T) { Series: []*profilestorepb.RawProfileSeries{{ Labels: &labelset1, Samples: samples2, - }}}) + }}, + }) series := []*profilestorepb.RawProfileSeries{ - &profilestorepb.RawProfileSeries{ + { Labels: &labelset1, Samples: append(samples1, samples2...), }, - &profilestorepb.RawProfileSeries{ + { Labels: &labelset2, Samples: samples2, }, diff --git a/pkg/buildid/buildid.go b/pkg/buildid/buildid.go index af4b39e53c..d15ffa8cb3 100644 --- a/pkg/buildid/buildid.go +++ b/pkg/buildid/buildid.go @@ -21,8 +21,8 @@ import ( "os" "github.com/cespare/xxhash/v2" - gobuildid "github.com/parca-dev/parca-agent/internal/go/buildid" + gobuildid "github.com/parca-dev/parca-agent/internal/go/buildid" "github.com/parca-dev/parca-agent/internal/pprof/elfexec" ) diff --git a/pkg/containerutils/containerd/containerd.go b/pkg/containerutils/containerd/containerd.go index ffdbe48d76..f7d37dd590 100644 --- a/pkg/containerutils/containerd/containerd.go +++ b/pkg/containerutils/containerd/containerd.go @@ -27,23 +27,23 @@ import ( ) const ( - DEFAULT_SOCKET_PATH = "/run/containerd/containerd.sock" - DEFAULT_K3S_SOCKET_PATH = "/run/k3s/containerd/containerd.sock" - DEFAULT_TIMEOUT = 2 * time.Second + DefaultSocketPath = "/run/containerd/containerd.sock" + DefaultK3SSocketPath = "/run/k3s/containerd/containerd.sock" + DefaultTimeout = 2 * time.Second ) -type ContainerdClient struct { +type Client struct { conn *grpc.ClientConn client pb.RuntimeServiceClient } -func NewContainerdClient(path string) (*ContainerdClient, error) { +func NewContainerdClient(path string) (*Client, error) { conn, err := grpc.Dial( path, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { var d net.Dialer - ctx, cancel := context.WithTimeout(ctx, DEFAULT_TIMEOUT) + ctx, cancel := context.WithTimeout(ctx, DefaultTimeout) defer cancel() d.LocalAddr = nil // if you have a local addr, add it here @@ -56,13 +56,13 @@ func NewContainerdClient(path string) (*ContainerdClient, error) { } client := pb.NewRuntimeServiceClient(conn) - return &ContainerdClient{ + return &Client{ conn: conn, client: client, }, nil } -func (c *ContainerdClient) Close() error { +func (c *Client) Close() error { if c.conn != nil { return c.conn.Close() } @@ -70,7 +70,7 @@ func (c *ContainerdClient) Close() error { return nil } -func (c *ContainerdClient) PidFromContainerId(containerID string) (int, error) { +func (c *Client) PIDFromContainerID(containerID string) (int, error) { if !strings.HasPrefix(containerID, "containerd://") { return -1, fmt.Errorf("Invalid CRI %s, it should be containerd", containerID) } diff --git a/pkg/containerutils/containerutils.go b/pkg/containerutils/containerutils.go index 43eef36996..07da8560ce 100644 --- a/pkg/containerutils/containerutils.go +++ b/pkg/containerutils/containerutils.go @@ -76,7 +76,7 @@ import "C" type CRIClient interface { Close() error - PidFromContainerId(containerID string) (int, error) + PIDFromContainerID(containerID string) (int, error) } func CgroupPathV2AddMountpoint(path string) (string, error) { diff --git a/pkg/containerutils/crio/crio.go b/pkg/containerutils/crio/crio.go index 3e4f762f58..823796906f 100644 --- a/pkg/containerutils/crio/crio.go +++ b/pkg/containerutils/crio/crio.go @@ -28,23 +28,23 @@ import ( ) const ( - DEFAULT_SOCKET_PATH = "/run/crio/crio.sock" - DEFAULT_TIMEOUT = 2 * time.Second + DefaultSocketPath = "/run/crio/crio.sock" + DefaultTimeout = 2 * time.Second ) -type CrioClient struct { +type Client struct { logger log.Logger conn *grpc.ClientConn client pb.RuntimeServiceClient } -func NewCrioClient(logger log.Logger, path string) (*CrioClient, error) { +func NewCrioClient(logger log.Logger, path string) (*Client, error) { conn, err := grpc.Dial( path, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { var d net.Dialer - ctx, cancel := context.WithTimeout(ctx, DEFAULT_TIMEOUT) + ctx, cancel := context.WithTimeout(ctx, DefaultTimeout) defer cancel() d.LocalAddr = nil // if you have a local addr, add it here @@ -57,14 +57,14 @@ func NewCrioClient(logger log.Logger, path string) (*CrioClient, error) { } client := pb.NewRuntimeServiceClient(conn) - return &CrioClient{ + return &Client{ logger: logger, conn: conn, client: client, }, nil } -func (c *CrioClient) Close() error { +func (c *Client) Close() error { if c.conn != nil { return c.conn.Close() } @@ -76,7 +76,7 @@ type containerInfo struct { PID int `json:"pid"` } -func (c *CrioClient) PidFromContainerId(containerID string) (int, error) { +func (c *Client) PIDFromContainerID(containerID string) (int, error) { if !strings.HasPrefix(containerID, "cri-o://") { return -1, fmt.Errorf("Invalid CRI %s, it should be cri-o", containerID) } diff --git a/pkg/containerutils/docker/docker.go b/pkg/containerutils/docker/docker.go index e3034dbfde..9dec9acb4a 100644 --- a/pkg/containerutils/docker/docker.go +++ b/pkg/containerutils/docker/docker.go @@ -24,31 +24,31 @@ import ( ) const ( - DEFAULT_SOCKET_PATH = "/var/run/docker.sock" - DEFAULT_TIMEOUT = 2 * time.Second + DefaultSocketPath = "/var/run/docker.sock" + DefaultTimeout = 2 * time.Second ) -type DockerClient struct { +type Client struct { client *client.Client } -func NewDockerClient(path string) (*DockerClient, error) { +func NewDockerClient(path string) (*Client, error) { cli, err := client.NewClientWithOpts( client.WithAPIVersionNegotiation(), client.WithDialContext(func(ctx context.Context, network, addr string) (net.Conn, error) { - return net.DialTimeout("unix", path, DEFAULT_TIMEOUT) + return net.DialTimeout("unix", path, DefaultTimeout) }), ) if err != nil { return nil, err } - return &DockerClient{ + return &Client{ client: cli, }, nil } -func (c *DockerClient) Close() error { +func (c *Client) Close() error { if c.client != nil { return c.client.Close() } @@ -56,21 +56,21 @@ func (c *DockerClient) Close() error { return nil } -func (c *DockerClient) PidFromContainerId(containerID string) (int, error) { +func (c *Client) PIDFromContainerID(containerID string) (int, error) { if !strings.HasPrefix(containerID, "docker://") { return -1, fmt.Errorf("Invalid CRI %s, it should be docker", containerID) } containerID = strings.TrimPrefix(containerID, "docker://") - containerJson, err := c.client.ContainerInspect(context.Background(), containerID) + containerJSON, err := c.client.ContainerInspect(context.Background(), containerID) if err != nil { return -1, err } - if containerJson.State == nil { + if containerJSON.State == nil { return -1, fmt.Errorf("Container state is nil") } - return containerJson.State.Pid, nil + return containerJSON.State.Pid, nil } diff --git a/pkg/debuginfo/debuginfo.go b/pkg/debuginfo/debuginfo.go index f544812a73..20ac7c0d14 100644 --- a/pkg/debuginfo/debuginfo.go +++ b/pkg/debuginfo/debuginfo.go @@ -48,6 +48,7 @@ type NoopClient struct{} func (c *NoopClient) Exists(ctx context.Context, buildID string) (bool, error) { return true, nil } + func (c *NoopClient) Upload(ctx context.Context, buildID string, f io.Reader) (uint64, error) { return 0, nil } @@ -213,11 +214,11 @@ func (di *Extractor) findDebugInfo(buildID string, buildIDFile maps.BuildIDFile) return err } if !info.IsDir() { - debugBuildId, err := buildid.BuildID(path) + debugBuildID, err := buildid.BuildID(path) if err != nil { return fmt.Errorf("failed to extract elf build ID, %w", err) } - if debugBuildId == buildID { + if debugBuildID == buildID { found = true file = path } @@ -238,9 +239,9 @@ func (di *Extractor) findDebugInfo(buildID string, buildIDFile maps.BuildIDFile) return file, nil } -func (di *Extractor) extract(ctx context.Context, buildID string, file string) (string, error) { +func (di *Extractor) extract(ctx context.Context, buildID, file string) (string, error) { tmpDir := path.Join(di.tmpDir, buildID) - if err := os.MkdirAll(tmpDir, 0755); err != nil { + if err := os.MkdirAll(tmpDir, 0o755); err != nil { return "", fmt.Errorf("failed to create temp dir for debug information extraction: %w", err) } @@ -316,7 +317,7 @@ func (di *Extractor) run(cmd *exec.Cmd) error { return nil } -func (di *Extractor) strip(ctx context.Context, tmpDir string, file string, outFile string, toRemove []string) *exec.Cmd { +func (di *Extractor) strip(ctx context.Context, tmpDir, file, outFile string, toRemove []string) *exec.Cmd { level.Debug(di.logger).Log("msg", "using eu-strip", "file", file) // Extract debug symbols. // If we have DWARF symbols, they are enough for us to symbolize the profiles. @@ -335,7 +336,7 @@ func (di *Extractor) strip(ctx context.Context, tmpDir string, file string, outF return exec.CommandContext(ctx, "eu-strip", args...) } -func (di *Extractor) objcopy(ctx context.Context, file string, outFile string, toRemove []string) *exec.Cmd { +func (di *Extractor) objcopy(ctx context.Context, file, outFile string, toRemove []string) *exec.Cmd { level.Debug(di.logger).Log("msg", "using objcopy", "file", file) // Go binaries has a special case. They use ".gopclntab" section to symbolize addresses. // We need to keep ".note.go.buildid", ".symtab" and ".gopclntab", @@ -352,7 +353,7 @@ func (di *Extractor) objcopy(ctx context.Context, file string, outFile string, t return exec.CommandContext(ctx, "objcopy", args...) } -func (di *Extractor) uploadDebugInfo(ctx context.Context, buildID string, file string) error { +func (di *Extractor) uploadDebugInfo(ctx context.Context, buildID, file string) error { f, err := os.Open(file) if err != nil { return fmt.Errorf("failed to open temp file for debug information: %w", err) diff --git a/pkg/discovery/discovery.go b/pkg/discovery/discovery.go index 588c37f7b2..592ea186f7 100644 --- a/pkg/discovery/discovery.go +++ b/pkg/discovery/discovery.go @@ -17,6 +17,7 @@ import ( "context" "github.com/go-kit/log" + "github.com/parca-dev/parca-agent/pkg/target" ) diff --git a/pkg/discovery/discovery_manager.go b/pkg/discovery/discovery_manager.go index 2e897eeb96..5f0eaf2f8b 100644 --- a/pkg/discovery/discovery_manager.go +++ b/pkg/discovery/discovery_manager.go @@ -22,9 +22,10 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" - "github.com/parca-dev/parca-agent/pkg/target" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + + "github.com/parca-dev/parca-agent/pkg/target" ) type metrics struct { @@ -128,7 +129,7 @@ type Manager struct { triggerSend chan struct{} } -// Run starts the background processing +// Run starts the background processing. func (m *Manager) Run() error { go m.sender() for range m.ctx.Done() { @@ -292,15 +293,11 @@ func (m *Manager) allGroups() map[string][]*target.Group { // registerProviders returns a number of failed SD config. func (m *Manager) registerProviders(cfgs Configs, setName string) int { - var ( - failed int - //added bool - ) + var failed int add := func(cfg Config) { for _, p := range m.providers { if reflect.DeepEqual(cfg, p.config) { p.subs = append(p.subs, setName) - //added = true return } } diff --git a/pkg/discovery/kubernetes.go b/pkg/discovery/kubernetes.go index 4878e0b3ab..85fdd94d6c 100644 --- a/pkg/discovery/kubernetes.go +++ b/pkg/discovery/kubernetes.go @@ -18,12 +18,12 @@ import ( "fmt" "github.com/go-kit/log" - "github.com/parca-dev/parca-agent/pkg/target" "github.com/prometheus/common/model" v1 "k8s.io/api/core/v1" "github.com/parca-dev/parca-agent/pkg/agent" "github.com/parca-dev/parca-agent/pkg/k8s" + "github.com/parca-dev/parca-agent/pkg/target" ) type PodConfig struct { @@ -38,14 +38,14 @@ type PodDiscoverer struct { podInformer *k8s.PodInformer createdChan chan *v1.Pod deletedChan chan string - k8sClient *k8s.K8sClient + k8sClient *k8s.Client } func (c *PodConfig) Name() string { return c.nodeName } -func NewPodConfig(podLabel string, socketPath string, nodeName string) *PodConfig { +func NewPodConfig(podLabel, socketPath, nodeName string) *PodConfig { return &PodConfig{ podLabelSelector: podLabel, socketPath: socketPath, @@ -116,10 +116,9 @@ func buildPod(pod *v1.Pod, containers []*k8s.ContainerDefinition) *target.Group tg.Labels["pod"] = model.LabelValue(pod.ObjectMeta.Name) for _, container := range containers { - tg.Targets = append(tg.Targets, model.LabelSet{ "container": model.LabelValue(container.ContainerName), - "containerid": model.LabelValue(container.ContainerId), + "containerid": model.LabelValue(container.ContainerID), agent.CgroupPathLabelName: model.LabelValue(container.PerfEventCgroupPath()), }) } diff --git a/pkg/discovery/systemd.go b/pkg/discovery/systemd.go index 636bec1a04..c6fc5e43e9 100644 --- a/pkg/discovery/systemd.go +++ b/pkg/discovery/systemd.go @@ -28,9 +28,10 @@ import ( "github.com/containerd/cgroups" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/prometheus/common/model" + "github.com/parca-dev/parca-agent/pkg/agent" "github.com/parca-dev/parca-agent/pkg/target" - "github.com/prometheus/common/model" ) type SystemdConfig struct { @@ -52,7 +53,6 @@ func (c *SystemdConfig) Name() string { } func NewSystemdConfig(systemdUnits []string, systemdCgroupPath string) *SystemdConfig { - units := map[string]struct{}{} for _, unit := range systemdUnits { @@ -87,7 +87,6 @@ func (c *SystemdDiscoverer) Run(ctx context.Context, up chan<- []*target.Group) var targetGroups []*target.Group for unit := range c.units { - labelset, err := c.ReconcileUnit(ctx, unit) if err != nil { return err @@ -127,9 +126,8 @@ func (c *SystemdDiscoverer) ReconcileUnit(ctx context.Context, unit string) (mod delete(c.unitProfilers, unit) c.mtx.Unlock() - //TODO(brancz): cleanup cgroup os.Remove(fmt.Sprintf("/sys/fs/cgroup/perf_event/system.slice/%s/") + // TODO(brancz): cleanup cgroup os.Remove(fmt.Sprintf("/sys/fs/cgroup/perf_event/system.slice/%s/") return model.LabelSet{agent.CgroupPathLabelName: model.LabelValue(fmt.Sprintf("/sys/fs/cgroup/perf_event/system.slice/%s/", unit))}, nil - } if err != nil { return nil, err @@ -163,7 +161,6 @@ func (c *SystemdDiscoverer) ReconcileUnit(ctx context.Context, unit string) (mod return model.LabelSet{ agent.CgroupPathLabelName: model.LabelValue(fmt.Sprintf("/sys/fs/cgroup/perf_event/system.slice/%s/", unit)), }, nil - } level.Debug(c.logger).Log("msg", "adding systemd unit profiler") diff --git a/pkg/discovery/systemd_test.go b/pkg/discovery/systemd_test.go index 507dad7512..0a46731b37 100644 --- a/pkg/discovery/systemd_test.go +++ b/pkg/discovery/systemd_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/go-kit/log" + "github.com/parca-dev/parca-agent/pkg/agent" ) diff --git a/pkg/k8s/k8s.go b/pkg/k8s/k8s.go index c7a6c4fcaf..1cc68e811a 100644 --- a/pkg/k8s/k8s.go +++ b/pkg/k8s/k8s.go @@ -37,7 +37,7 @@ import ( const KubeConfigEnv = "KUBECONFIG" -type K8sClient struct { +type Client struct { logger log.Logger clientset *kubernetes.Clientset nodeName string @@ -45,7 +45,7 @@ type K8sClient struct { criClient containerutils.CRIClient } -func NewK8sClient(logger log.Logger, nodeName, socketPath string) (*K8sClient, error) { +func NewK8sClient(logger log.Logger, nodeName, socketPath string) (*Client, error) { var ( config *rest.Config err error @@ -82,7 +82,7 @@ func NewK8sClient(logger log.Logger, nodeName, socketPath string) (*K8sClient, e return nil, fmt.Errorf("create CRI client: %w", err) } - return &K8sClient{ + return &Client{ logger: logger, clientset: clientset, nodeName: nodeName, @@ -91,7 +91,7 @@ func NewK8sClient(logger log.Logger, nodeName, socketPath string) (*K8sClient, e }, nil } -func (c *K8sClient) Clientset() kubernetes.Interface { +func (c *Client) Clientset() kubernetes.Interface { return c.clientset } @@ -107,22 +107,22 @@ func newCRIClient(logger log.Logger, node *v1.Node, socketPath string) (containe switch criType { case "docker": if socketPath == "" { - socketPath = docker.DEFAULT_SOCKET_PATH + socketPath = docker.DefaultSocketPath } return docker.NewDockerClient(socketPath) case "containerd": if socketPath == "" { - if _, err := os.Stat(containerd.DEFAULT_SOCKET_PATH); err == nil { - socketPath = containerd.DEFAULT_SOCKET_PATH + if _, err := os.Stat(containerd.DefaultSocketPath); err == nil { + socketPath = containerd.DefaultSocketPath } - if _, err := os.Stat(containerd.DEFAULT_K3S_SOCKET_PATH); err == nil { - socketPath = containerd.DEFAULT_K3S_SOCKET_PATH + if _, err := os.Stat(containerd.DefaultK3SSocketPath); err == nil { + socketPath = containerd.DefaultK3SSocketPath } } return containerd.NewContainerdClient(socketPath) case "cri-o": if socketPath == "" { - socketPath = crio.DEFAULT_SOCKET_PATH + socketPath = crio.DefaultSocketPath } return crio.NewCrioClient(logger, socketPath) default: @@ -130,15 +130,15 @@ func newCRIClient(logger log.Logger, node *v1.Node, socketPath string) (containe } } -func (k *K8sClient) CloseCRI() { - k.criClient.Close() +func (c *Client) CloseCRI() { + c.criClient.Close() } type ContainerDefinition struct { NodeName string - ContainerId string + ContainerID string CgroupPath string - CgroupId uint64 + CgroupID uint64 Mntns uint64 Namespace string PodName string @@ -165,7 +165,7 @@ func (c *ContainerDefinition) Labels() []*profilestorepb.Label { Value: c.ContainerName, }, { Name: "containerid", - Value: c.ContainerId, + Value: c.ContainerID, }} } @@ -189,7 +189,7 @@ func (c *ContainerDefinition) PerfEventCgroupPath() string { // PodToContainers return a list of the containers of a given Pod. // Containers that are not running or don't have an ID are not considered. -func (k *K8sClient) PodToContainers(pod *v1.Pod) []*ContainerDefinition { +func (c *Client) PodToContainers(pod *v1.Pod) []*ContainerDefinition { containers := []*ContainerDefinition{} for _, s := range pod.Status.ContainerStatuses { @@ -200,29 +200,29 @@ func (k *K8sClient) PodToContainers(pod *v1.Pod) []*ContainerDefinition { continue } - pid, err := k.criClient.PidFromContainerId(s.ContainerID) + pid, err := c.criClient.PIDFromContainerID(s.ContainerID) if err != nil { - level.Warn(k.logger).Log("msg", "skipping pod, cannot find pid", "namespace", pod.GetNamespace(), "pod", pod.GetName(), "err", err) + level.Warn(c.logger).Log("msg", "skipping pod, cannot find pid", "namespace", pod.GetNamespace(), "pod", pod.GetName(), "err", err) continue } cgroupPathV1, cgroupPathV2, err := containerutils.GetCgroupPaths(pid) if err != nil { - level.Warn(k.logger).Log("msg", "skipping pod, cannot find cgroup path", "namespace", pod.GetNamespace(), "pod", pod.GetName(), "err", err) + level.Warn(c.logger).Log("msg", "skipping pod, cannot find cgroup path", "namespace", pod.GetNamespace(), "pod", pod.GetName(), "err", err) continue } cgroupPathV2WithMountpoint, _ := containerutils.CgroupPathV2AddMountpoint(cgroupPathV2) - cgroupId, _ := containerutils.GetCgroupID(cgroupPathV2WithMountpoint) + cgroupID, _ := containerutils.GetCgroupID(cgroupPathV2WithMountpoint) mntns, err := containerutils.GetMntNs(pid) if err != nil { - level.Warn(k.logger).Log("msg", "skipping pod, cannot find mnt namespace", "namespace", pod.GetNamespace(), "pod", pod.GetName(), "err", err) + level.Warn(c.logger).Log("msg", "skipping pod, cannot find mnt namespace", "namespace", pod.GetNamespace(), "pod", pod.GetName(), "err", err) continue } containerDef := &ContainerDefinition{ - NodeName: k.nodeName, - ContainerId: s.ContainerID, + NodeName: c.nodeName, + ContainerID: s.ContainerID, CgroupPath: cgroupPathV2WithMountpoint, - CgroupId: cgroupId, + CgroupID: cgroupID, Mntns: mntns, Namespace: pod.GetNamespace(), PodName: pod.GetName(), @@ -240,17 +240,17 @@ func (k *K8sClient) PodToContainers(pod *v1.Pod) []*ContainerDefinition { // ListContainers return a list of the current containers that are // running in the node. -func (k *K8sClient) ListContainers() (arr []*ContainerDefinition, err error) { +func (c *Client) ListContainers() (arr []*ContainerDefinition, err error) { // List pods - pods, err := k.clientset.CoreV1().Pods("").List(context.TODO(), metav1.ListOptions{ - FieldSelector: k.fieldSelector, + pods, err := c.clientset.CoreV1().Pods("").List(context.TODO(), metav1.ListOptions{ + FieldSelector: c.fieldSelector, }) if err != nil { return nil, err } for _, pod := range pods.Items { - containers := k.PodToContainers(&pod) + containers := c.PodToContainers(&pod) arr = append(arr, containers...) } return arr, nil diff --git a/pkg/k8s/podinformer.go b/pkg/k8s/podinformer.go index 4e99459f09..8e42797423 100644 --- a/pkg/k8s/podinformer.go +++ b/pkg/k8s/podinformer.go @@ -28,6 +28,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" + // Registers default auth client. _ "k8s.io/client-go/plugin/pkg/client/auth" ) @@ -60,7 +61,7 @@ func NewPodInformer(logger log.Logger, node, podLabelSelector string, clientset queue.Add(key) } }, - UpdateFunc: func(old interface{}, new interface{}) { + UpdateFunc: func(old, new interface{}) { key, err := cache.MetaNamespaceKeyFunc(new) if err == nil { queue.Add(key) @@ -118,7 +119,7 @@ func (p *PodInformer) processNextItem() bool { return true } -// notifyChans passes the event to the channels configured by the user +// notifyChans passes the event to the channels configured by the user. func (p *PodInformer) notifyChans(key string) error { obj, exists, err := p.indexer.GetByKey(key) if err != nil { diff --git a/pkg/ksym/ksym.go b/pkg/ksym/ksym.go index ead420dae3..7822245fed 100644 --- a/pkg/ksym/ksym.go +++ b/pkg/ksym/ksym.go @@ -26,14 +26,13 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/parca-dev/parca-agent/pkg/hash" ) -var ( - FunctionNotFoundError = errors.New("kernel function not found") -) +var ErrFunctionNotFound = errors.New("kernel function not found") -type KsymCache struct { +type Cache struct { logger log.Logger fs fs.FS lastHash uint64 @@ -47,8 +46,8 @@ type realfs struct{} func (f *realfs) Open(name string) (fs.File, error) { return os.Open(name) } -func NewKsymCache(logger log.Logger) *KsymCache { - return &KsymCache{ +func NewKsymCache(logger log.Logger) *Cache { + return &Cache{ logger: logger, fs: &realfs{}, fastCache: make(map[uint64]string), @@ -57,7 +56,7 @@ func NewKsymCache(logger log.Logger) *KsymCache { } } -func (c *KsymCache) Resolve(addrs map[uint64]struct{}) (map[uint64]string, error) { +func (c *Cache) Resolve(addrs map[uint64]struct{}) (map[uint64]string, error) { c.mtx.RLock() lastCacheInvalidation := c.lastCacheInvalidation lastHash := c.lastHash @@ -134,7 +133,7 @@ func unsafeString(b []byte) string { // ksym reads /proc/kallsyms and resolved the addresses to their respective // function names. The addrs parameter must be sorted as /proc/kallsyms is // sorted. -func (c *KsymCache) ksym(addrs []uint64) ([]string, error) { +func (c *Cache) ksym(addrs []uint64) ([]string, error) { fd, err := c.fs.Open("/proc/kallsyms") if err != nil { return nil, err @@ -188,6 +187,6 @@ func (c *KsymCache) ksym(addrs []uint64) ([]string, error) { return res, nil } -func (c *KsymCache) kallsymsHash() (uint64, error) { +func (c *Cache) kallsymsHash() (uint64, error) { return hash.File(c.fs, "/proc/kallsyms") } diff --git a/pkg/ksym/ksym_test.go b/pkg/ksym/ksym_test.go index cb52f3321f..f931331622 100644 --- a/pkg/ksym/ksym_test.go +++ b/pkg/ksym/ksym_test.go @@ -26,7 +26,7 @@ import ( ) func TestKsym(t *testing.T) { - c := &KsymCache{ + c := &Cache{ logger: log.NewNopLogger(), fs: testutil.NewFakeFS(map[string][]byte{ "/proc/kallsyms": []byte(` @@ -65,8 +65,8 @@ ffffffff8f6d1780 b xfrm_napi_dev addr3 := uint64(0xffffffff8f6d1480) + 1 syms, err := c.Resolve(map[uint64]struct{}{ - addr1: struct{}{}, - addr2: struct{}{}, + addr1: {}, + addr2: {}, }) require.NoError(t, err) require.Equal(t, map[uint64]string{ @@ -80,9 +80,9 @@ ffffffff8f6d1780 b xfrm_napi_dev }, c.fastCache) syms, err = c.Resolve(map[uint64]struct{}{ - addr1: struct{}{}, - addr2: struct{}{}, - addr3: struct{}{}, + addr1: {}, + addr2: {}, + addr3: {}, }) require.NoError(t, err) require.Equal(t, map[uint64]string{ @@ -100,8 +100,8 @@ ffffffff8f6d1780 b xfrm_napi_dev // Second time should be served from cache. c.fs = testutil.NewErrorFS(errors.New("not served from cache")) syms, err = c.Resolve(map[uint64]struct{}{ - addr1: struct{}{}, - addr2: struct{}{}, + addr1: {}, + addr2: {}, }) require.NoError(t, err) diff --git a/pkg/perf/perf.go b/pkg/perf/perf.go index 7ce84514f3..8de1617f86 100644 --- a/pkg/perf/perf.go +++ b/pkg/perf/perf.go @@ -29,64 +29,61 @@ import ( "github.com/parca-dev/parca-agent/pkg/hash" ) -type PerfCache struct { +type Cache struct { fs fs.FS logger log.Logger - cache map[uint32]*PerfMap + cache map[uint32]*Map pidMapHash map[uint32]uint64 nsPid map[uint32]uint32 } -type PerfMapAddr struct { +type MapAddr struct { Start uint64 End uint64 Symbol string } -type PerfMap struct { - addrs []PerfMapAddr +type Map struct { + addrs []MapAddr } type realfs struct{} -var ( - NoSymbolFound = errors.New("no symbol found") -) +var ErrNoSymbolFound = errors.New("no symbol found") func (f *realfs) Open(name string) (fs.File, error) { return os.Open(name) } -func PerfReadMap(fs fs.FS, fileName string) (PerfMap, error) { +func ReadMap(fs fs.FS, fileName string) (Map, error) { fd, err := fs.Open(fileName) if err != nil { - return PerfMap{}, err + return Map{}, err } defer fd.Close() s := bufio.NewScanner(fd) - addrs := make([]PerfMapAddr, 0) + addrs := make([]MapAddr, 0) for s.Scan() { l := strings.SplitN(s.Text(), " ", 3) if len(l) < 3 { - return PerfMap{}, fmt.Errorf("splitting failed: %v", l) - + return Map{}, fmt.Errorf("splitting failed: %v", l) } // Some runtimes that produce perf maps optionally start memory // addresses with "0x". start, err := strconv.ParseUint(strings.TrimPrefix(l[0], "0x"), 16, 64) if err != nil { - return PerfMap{}, fmt.Errorf("parsing start failed on %v: %w", l, err) + return Map{}, fmt.Errorf("parsing start failed on %v: %w", l, err) } size, err := strconv.ParseUint(l[1], 16, 64) if err != nil { - return PerfMap{}, fmt.Errorf("parsing end failed on %v: %w", l, err) + return Map{}, fmt.Errorf("parsing end failed on %v: %w", l, err) } if start+size < start { - return PerfMap{}, fmt.Errorf("overflowed mapping: %v", l) + return Map{}, fmt.Errorf("overflowed mapping: %v", l) } - addrs = append(addrs, PerfMapAddr{start, start + size, l[2]}) + addrs = append(addrs, MapAddr{start, start + size, l[2]}) } // Sorted by end address to allow binary search during look-up. End to find // the (closest) address _before_ the end. This could be an inlined instruction @@ -94,32 +91,32 @@ func PerfReadMap(fs fs.FS, fileName string) (PerfMap, error) { sort.Slice(addrs, func(i, j int) bool { return addrs[i].End < addrs[j].End }) - return PerfMap{addrs: addrs}, s.Err() + return Map{addrs: addrs}, s.Err() } -func (p *PerfMap) Lookup(addr uint64) (string, error) { +func (p *Map) Lookup(addr uint64) (string, error) { idx := sort.Search(len(p.addrs), func(i int) bool { return addr < p.addrs[i].End }) if idx == len(p.addrs) || p.addrs[idx].Start > addr { - return "", NoSymbolFound + return "", ErrNoSymbolFound } return p.addrs[idx].Symbol, nil } -func NewPerfCache(logger log.Logger) *PerfCache { - return &PerfCache{ +func NewPerfCache(logger log.Logger) *Cache { + return &Cache{ fs: &realfs{}, logger: logger, - cache: map[uint32]*PerfMap{}, + cache: map[uint32]*Map{}, nsPid: map[uint32]uint32{}, pidMapHash: map[uint32]uint64{}, } } -// CacheForPid returns the PerfMap for the given pid if it exists. -func (p *PerfCache) CacheForPid(pid uint32) (*PerfMap, error) { +// CacheForPid returns the Map for the given pid if it exists. +func (p *Cache) CacheForPid(pid uint32) (*Map, error) { // NOTE(zecke): There are various limitations and things to note. // 1st) The input file is "tainted" and under control by the user. By all // means it could be an infinitely large. @@ -146,7 +143,7 @@ func (p *PerfCache) CacheForPid(pid uint32) (*PerfMap, error) { return p.cache[pid], nil } - m, err := PerfReadMap(p.fs, perfFile) + m, err := ReadMap(p.fs, perfFile) if err != nil { return nil, err } @@ -191,7 +188,6 @@ func extractPidsFromLine(line string) ([]uint32, error) { pids := make([]uint32, 0, len(pidStrings)) for _, pidStr := range pidStrings { - pid, err := strconv.ParseUint(pidStr, 10, 32) if err != nil { return nil, fmt.Errorf("parsing pid failed on %v: %w", pidStr, err) diff --git a/pkg/perf/perf_test.go b/pkg/perf/perf_test.go index 9e58a819c2..91dd3ea318 100644 --- a/pkg/perf/perf_test.go +++ b/pkg/perf/perf_test.go @@ -36,11 +36,11 @@ func TestPerfMapParse(t *testing.T) { "/tmp/perf-123.map": mustReadFile("testdata/nodejs-perf-map"), }) - res, err := PerfReadMap(fs, "/tmp/perf-123.map") + res, err := ReadMap(fs, "/tmp/perf-123.map") require.NoError(t, err) require.Len(t, res.addrs, 28) // Check for 4edd3cca B0 LazyCompile:~Timeout internal/timers.js:55 - require.Equal(t, res.addrs[12], PerfMapAddr{0x4edd4f12, 0x4edd4f47, "LazyCompile:~remove internal/linkedlist.js:15"}) + require.Equal(t, res.addrs[12], MapAddr{0x4edd4f12, 0x4edd4f47, "LazyCompile:~remove internal/linkedlist.js:15"}) // Look-up a symbol. sym, err := res.Lookup(0x4edd4f12 + 4) @@ -48,7 +48,7 @@ func TestPerfMapParse(t *testing.T) { require.Equal(t, sym, "LazyCompile:~remove internal/linkedlist.js:15") _, err = res.Lookup(0xFFFFFFFF) - require.ErrorIs(t, err, NoSymbolFound) + require.ErrorIs(t, err, ErrNoSymbolFound) } func TestPerfMapParseErlangPerfMap(t *testing.T) { @@ -56,7 +56,7 @@ func TestPerfMapParseErlangPerfMap(t *testing.T) { "/tmp/perf-123.map": mustReadFile("testdata/erlang-perf-map"), }) - _, err := PerfReadMap(fs, "/tmp/perf-123.map") + _, err := ReadMap(fs, "/tmp/perf-123.map") require.NoError(t, err) } @@ -67,7 +67,7 @@ func BenchmarkPerfMapParse(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := PerfReadMap(fs, "/tmp/perf-123.map") + _, err := ReadMap(fs, "/tmp/perf-123.map") require.NoError(b, err) } } diff --git a/pkg/profiler/profiler.go b/pkg/profiler/profiler.go index c1af352b5c..3753930755 100644 --- a/pkg/profiler/profiler.go +++ b/pkg/profiler/profiler.go @@ -27,7 +27,7 @@ import ( "time" "unsafe" - "C" + "C" //nolint:typecheck bpf "github.com/aquasecurity/libbpfgo" "github.com/go-kit/log" @@ -35,6 +35,8 @@ import ( "github.com/google/pprof/profile" "github.com/parca-dev/parca-agent/pkg/agent" profilestorepb "github.com/parca-dev/parca/gen/proto/go/parca/profilestore/v1alpha1" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/common/model" "golang.org/x/sys/unix" @@ -44,10 +46,6 @@ import ( "github.com/parca-dev/parca-agent/pkg/maps" "github.com/parca-dev/parca-agent/pkg/perf" ) -import ( - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" -) //go:embed parca-agent.bpf.o var bpfObj []byte @@ -60,7 +58,7 @@ const ( type CgroupProfiler struct { logger log.Logger missingStacks *prometheus.CounterVec - ksymCache *ksym.KsymCache + ksymCache *ksym.Cache target model.LabelSet profilingDuration time.Duration cancel func() @@ -73,13 +71,13 @@ type CgroupProfiler struct { lastProfileTakenAt time.Time lastError error - perfCache *perf.PerfCache + perfCache *perf.Cache } func NewCgroupProfiler( logger log.Logger, reg prometheus.Registerer, - ksymCache *ksym.KsymCache, + ksymCache *ksym.Cache, writeClient profilestorepb.ProfileStoreServiceClient, debugInfoClient debuginfo.Client, target model.LabelSet, @@ -139,10 +137,9 @@ func (p *CgroupProfiler) Stop() { } func (p *CgroupProfiler) Labels() model.LabelSet { - labels := - model.LabelSet{ - "__name__": "parca_agent_cpu", - } + labels := model.LabelSet{ + "__name__": "parca_agent_cpu", + } for labelname, labelvalue := range p.target { if !strings.HasPrefix(string(labelname), "__") { @@ -273,7 +270,7 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, now time.Time, counts, samples := map[[doubleStackDepth]uint64]*profile.Sample{} // TODO(brancz): What was this for? - //has_collision := false + // has_collision := false it := counts.Iterator() byteOrder := byteorder.GetHostByteOrder() @@ -393,7 +390,7 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, now time.Time, counts, // Does this addr point to JITed code? if perfMap != nil { - // TODO(zecke): Log errors other than perf.NoSymbolFound + // TODO(zecke): Log errors other than perf.ErrNoSymbolFound jitFunction, ok := userFunctions[key] if !ok { if sym, err := perfMap.Lookup(addr); err == nil { @@ -479,7 +476,8 @@ func (p *CgroupProfiler) profileLoop(ctx context.Context, now time.Time, counts, for key, value := range labels { labeloldformat = append(labeloldformat, - &profilestorepb.Label{Name: string(key), + &profilestorepb.Label{ + Name: string(key), Value: string(value), }) } diff --git a/pkg/target/manager.go b/pkg/target/manager.go index adf76191a9..89a2c615ea 100644 --- a/pkg/target/manager.go +++ b/pkg/target/manager.go @@ -34,7 +34,7 @@ type Manager struct { logger log.Logger reg prometheus.Registerer externalLabels model.LabelSet - ksymCache *ksym.KsymCache + ksymCache *ksym.Cache writeClient profilestorepb.ProfileStoreServiceClient debugInfoClient debuginfo.Client profilingDuration time.Duration @@ -45,7 +45,7 @@ func NewManager( logger log.Logger, reg prometheus.Registerer, externalLabels model.LabelSet, - ksymCache *ksym.KsymCache, + ksymCache *ksym.Cache, writeClient profilestorepb.ProfileStoreServiceClient, debugInfoClient debuginfo.Client, profilingDuration time.Duration, @@ -87,7 +87,6 @@ func (m *Manager) reconcileTargets(ctx context.Context, targetSets map[string][] level.Debug(m.logger).Log("msg", "reconciling targets") for name, targetSet := range targetSets { - pp, found := m.profilerPools[name] if !found { pp = NewProfilerPool(ctx, m.externalLabels, m.logger, m.reg, m.ksymCache, m.writeClient, m.debugInfoClient, m.profilingDuration, m.tmp) diff --git a/pkg/target/profiler_pool.go b/pkg/target/profiler_pool.go index 3f981508f9..cf04f57e3e 100644 --- a/pkg/target/profiler_pool.go +++ b/pkg/target/profiler_pool.go @@ -21,7 +21,6 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" - "github.com/parca-dev/parca-agent/pkg/profiler" profilestorepb "github.com/parca-dev/parca/gen/proto/go/parca/profilestore/v1alpha1" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/model" @@ -29,6 +28,7 @@ import ( "github.com/parca-dev/parca-agent/pkg/debuginfo" "github.com/parca-dev/parca-agent/pkg/ksym" + "github.com/parca-dev/parca-agent/pkg/profiler" ) type Target struct { @@ -50,7 +50,7 @@ type ProfilerPool struct { externalLabels model.LabelSet logger log.Logger reg prometheus.Registerer - ksymCache *ksym.KsymCache + ksymCache *ksym.Cache writeClient profilestorepb.ProfileStoreServiceClient debugInfoClient debuginfo.Client profilingDuration time.Duration @@ -62,7 +62,7 @@ func NewProfilerPool( externalLabels model.LabelSet, logger log.Logger, reg prometheus.Registerer, - ksymCache *ksym.KsymCache, + ksymCache *ksym.Cache, writeClient profilestorepb.ProfileStoreServiceClient, debugInfoClient debuginfo.Client, profilingDuration time.Duration, @@ -104,7 +104,6 @@ func (pp *ProfilerPool) Sync(tg []*Group) { for _, newTargetGroup := range tg { for _, t := range newTargetGroup.Targets { - target := &Target{labelSet: model.LabelSet{}} for labelName, labelValue := range t { @@ -124,7 +123,7 @@ func (pp *ProfilerPool) Sync(tg []*Group) { } } - //add new targets and profile them + // Add new targets and profile them. for _, newTarget := range newTargets { h := labelsetToLabels(newTarget.labelSet).Hash()