fix(ebpf): ignore pods with empty containerID (#1437)

* add logger to spies

* ignore pods without containers

* fix

* linter

* fix build

* do not use unknown for module+offset

* fix kernel symbol resolution

go.mod

@@ -31,6 +31,7 @@ require (
 	github.com/hashicorp/golang-lru v0.5.4
 	github.com/iancoleman/strcase v0.2.0
 	github.com/imdario/mergo v0.3.12
+	github.com/johejo/go-content-encoding v0.0.0-20220721183050-9ea4a7717479
 	github.com/josephspurrier/goversioninfo v1.2.0
 	github.com/jsonnet-bundler/jsonnet-bundler v0.4.0
 	github.com/kardianos/service v1.2.0
@@ -121,7 +122,6 @@ require (
 	github.com/jinzhu/inflection v1.0.0 // indirect
 	github.com/jinzhu/now v1.1.3 // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
-	github.com/johejo/go-content-encoding v0.0.0-20220721183050-9ea4a7717479 // indirect
 	github.com/joho/godotenv v1.3.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/jpillora/backoff v1.0.0 // indirect

go.sum

@@ -495,8 +495,6 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI
 github.com/kisielk/godepgraph v0.0.0-20190626013829-57a7e4a651a9 h1:ZkWH0x1yafBo+Y2WdGGdszlJrMreMXWl7/dqpEkwsIk=
 github.com/kisielk/godepgraph v0.0.0-20190626013829-57a7e4a651a9/go.mod h1:Gb5YEgxqiSSVrXKWQxDcKoCM94NO5QAwOwTaVmIUAMI=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
-github.com/klauspost/compress v1.13.6 h1:P76CopJELS0TiO2mebmnzgWaajssP/EszplttgQxcgc=
-github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk=
 github.com/klauspost/compress v1.15.9 h1:wKRjX6JRtDdrE9qwa4b/Cip7ACOshUI4smpCQanqjSY=
 github.com/klauspost/compress v1.15.9/go.mod h1:PhcZ0MbTNciWF3rruxRgKxI5NkcHHrHUDtV4Yw2GlzU=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
@@ -1017,7 +1015,6 @@ golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20210420072515-93ed5bcd2bfe/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20210514084401-e8d321eab015/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

pkg/agent/clib/logging.go

@@ -3,8 +3,7 @@ package main
 import (
 	"C"
 	"fmt"
-
-	"github.com/pyroscope-io/pyroscope/pkg/agent"
+	logger2 "github.com/pyroscope-io/pyroscope/pkg/agent/log"
 )
 
 //export TestLogger
@@ -21,7 +20,7 @@ func SetLoggerLevel(l int) int {
 	return 0
 }
 
-var logger agent.Logger
+var logger logger2.Logger
 var level int
 
 func init() {

pkg/agent/debugspy/debugspy.go

@@ -13,9 +13,9 @@ type DebugSpy struct {
 	pid int
 }
 
-func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
+func Start(params spy.InitParams) (spy.Spy, error) {
 	return &DebugSpy{
-		pid: pid,
+		pid: params.Pid,
 	}, nil
 }
 

pkg/agent/dotnetspy/dotnetspy.go

@@ -16,8 +16,8 @@ func init() {
 	spy.RegisterSpy("dotnetspy", Start)
 }
 
-func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
-	s := newSession(pid)
+func Start(params spy.InitParams) (spy.Spy, error) {
+	s := newSession(params.Pid)
 	_ = s.start()
 	return &DotnetSpy{session: s}, nil
 }

pkg/agent/ebpfspy/ebpfspy_linux.go

@@ -24,8 +24,8 @@ func NewEBPFSpy(s *Session) *EbpfSpy {
 	}
 }
 
-func Start(pid int, _ spy.ProfileType, sampleRate uint32, _ bool) (spy.Spy, error) {
-	s := NewSession(pid, sampleRate, 256, sd.NoopServiceDiscovery{}, false)
+func Start(params spy.InitParams) (spy.Spy, error) {
+	s := NewSession(params.Logger, params.Pid, params.SampleRate, 256, sd.NoopServiceDiscovery{}, false)
 	err := s.Start()
 	if err != nil {
 		return nil, err

pkg/agent/ebpfspy/sd/k8s.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"context"
 	"fmt"
+	"github.com/pyroscope-io/pyroscope/pkg/agent/log"
 	"github.com/pyroscope-io/pyroscope/pkg/agent/spy"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
@@ -16,6 +17,7 @@ import (
 )
 
 type K8SServiceDiscovery struct {
+	logger             log.Logger
 	cs                 *kubernetes.Clientset
 	nodeName           string
 	containerID2Labels map[string]*spy.Labels
@@ -25,7 +27,7 @@ type K8SServiceDiscovery struct {
 var knownContainerIDPrefixes = []string{"docker://", "containerd://"}
 var knownRuntimes = []string{"docker://", "containerd://"}
 
-func NewK8ServiceDiscovery(ctx context.Context, nodeName string) (ServiceDiscovery, error) {
+func NewK8ServiceDiscovery(ctx context.Context, logger log.Logger, nodeName string) (ServiceDiscovery, error) {
 	config, err := rest.InClusterConfig()
 	if err != nil {
 		return nil, err
@@ -47,6 +49,7 @@ func NewK8ServiceDiscovery(ctx context.Context, nodeName string) (ServiceDiscove
 	}
 
 	return &K8SServiceDiscovery{
+		logger:             logger,
 		cs:                 clientset,
 		nodeName:           nodeName,
 		containerID2Labels: map[string]*spy.Labels{},
@@ -63,9 +66,14 @@ func (sd *K8SServiceDiscovery) Refresh(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
+	sd.logger.Debugf("K8SServiceDiscovery#Refresh pods %v", pods)
 
 	for _, pod := range pods.Items {
 		for _, status := range pod.Status.ContainerStatuses {
+			if status.ContainerID == "" {
+				sd.logger.Debugf("Unknown containerID for pod %v, status %v", pod, status)
+				continue
+			}
 			cid, err := getContainerIDFromK8S(status.ContainerID)
 			if err != nil {
 				return err

pkg/agent/ebpfspy/session_linux.go

@@ -11,8 +11,10 @@ import (
 	"context"
 	_ "embed"
 	"encoding/binary"
+	"fmt"
 	"github.com/pyroscope-io/pyroscope/pkg/agent/ebpfspy/cpuonline"
 	"github.com/pyroscope-io/pyroscope/pkg/agent/ebpfspy/sd"
+	"github.com/pyroscope-io/pyroscope/pkg/agent/log"
 	"github.com/pyroscope-io/pyroscope/pkg/agent/spy"
 	"golang.org/x/sys/unix"
 	"sync"
@@ -28,6 +30,7 @@ import (
 import "C"
 
 type Session struct {
+	logger           log.Logger
 	pid              int
 	sampleRate       uint32
 	symbolCacheSize  int
@@ -52,8 +55,9 @@ type Session struct {
 
 const btf = "should not be used" // canary to detect we got relocations
 
-func NewSession(pid int, sampleRate uint32, symbolCacheSize int, serviceDiscovery sd.ServiceDiscovery, onlyServices bool) *Session {
+func NewSession(logger log.Logger, pid int, sampleRate uint32, symbolCacheSize int, serviceDiscovery sd.ServiceDiscovery, onlyServices bool) *Session {
 	return &Session{
+		logger:           logger,
 		pid:              pid,
 		sampleRate:       sampleRate,
 		symbolCacheSize:  symbolCacheSize,
@@ -101,6 +105,7 @@ func (s *Session) Start() error {
 }
 
 func (s *Session) Reset(cb func(labels *spy.Labels, name []byte, value uint64, pid uint32) error) error {
+	s.logger.Debugf("ebpf session reset")
 	s.modMutex.Lock()
 	defer s.modMutex.Unlock()
 
@@ -159,7 +164,7 @@ func (s *Session) Reset(cb func(labels *spy.Labels, name []byte, value uint64, p
 		buf.Write([]byte(it.comm))
 		buf.Write([]byte{';'})
 		s.walkStack(buf, it.uStack, it.pid, true)
-		s.walkStack(buf, it.kStack, it.pid, false)
+		s.walkStack(buf, it.kStack, 0, false)
 
 		err = cb(it.labels, buf.Bytes(), uint64(it.count), it.pid)
 		if err != nil {
@@ -264,12 +269,16 @@ func (s *Session) walkStack(line *bytes.Buffer, stack []byte, pid uint32, usersp
 		if ip == 0 {
 			break
 		}
-		sym, _, _ := s.symCache.bccResolve(pid, ip, s.roundNumber)
+		sym, offset, module := s.symCache.bccResolve(pid, ip, s.roundNumber)
 		if !userspace && sym == "" {
 			continue
 		}
 		if sym == "" {
-			sym = symbolUnknown
+			if module != "" {
+				sym = fmt.Sprintf("%s+0x%x", module, offset)
+			} else {
+				sym = "[unknown]"
+			}
 		}
 		stackFrames = append(stackFrames, sym+";")
 	}

pkg/agent/gospy/gospy.go

@@ -58,16 +58,16 @@ func stopCPUProfile(hz uint32) {
 	custom_pprof.StopCPUProfile()
 }
 
-func Start(_ int, profileType spy.ProfileType, sampleRate uint32, disableGCRuns bool) (spy.Spy, error) {
+func Start(params spy.InitParams) (spy.Spy, error) {
 	s := &GoSpy{
 		stopCh:        make(chan struct{}),
 		buf:           &bytes.Buffer{},
-		profileType:   profileType,
-		disableGCRuns: disableGCRuns,
-		sampleRate:    sampleRate,
+		profileType:   params.ProfileType,
+		disableGCRuns: params.DisableGCRuns,
+		sampleRate:    params.SampleRate,
 	}
 	if s.profileType == spy.ProfileCPU {
-		if err := startCPUProfile(s.buf, sampleRate); err != nil {
+		if err := startCPUProfile(s.buf, params.SampleRate); err != nil {
 			return nil, err
 		}
 	}
@@ -81,7 +81,8 @@ func (s *GoSpy) Stop() error {
 }
 
 // TODO: this is not the most elegant solution as it creates global state
-//   the idea here is that we can reuse heap profiles
+//
+//	the idea here is that we can reuse heap profiles
 var (
 	lastProfileMutex     sync.Mutex
 	lastProfile          *tree.Profile

pkg/agent/gospy/gospy_test.go

@@ -15,7 +15,8 @@ var _ = Describe("analytics", func() {
 	testing.WithConfig(func(cfg **config.Config) {
 		Describe("NewSession", func() {
 			It("works as expected", func(done Done) {
-				s, err := Start(0, spy.ProfileCPU, 100, false)
+				params := spy.InitParams{ProfileType: spy.ProfileCPU, SampleRate: 100}
+				s, err := Start(params)
 				Expect(err).ToNot(HaveOccurred())
 				go func() {
 					s := time.Now()

pkg/agent/logger.go → pkg/agent/log/logger.go

@@ -1,4 +1,4 @@
-package agent
+package log
 
 // Logger is an interface that library users can use
 // It is based on logrus, but much smaller — That's because we don't want library users to have to implement

pkg/agent/phpspy/phpspy.go

@@ -33,7 +33,7 @@ type PhpSpy struct {
 	pid int
 }
 
-func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
+func Start(params spy.InitParams) (spy.Spy, error) {
 	dataBuf := make([]byte, bufferLength)
 	dataPtr := unsafe.Pointer(&dataBuf[0])
 
@@ -44,7 +44,7 @@ func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
 	// TODO: handle this better
 	time.Sleep(1 * time.Second)
 
-	r := C.phpspy_init(C.int(pid), errorPtr, C.int(bufferLength))
+	r := C.phpspy_init(C.int(params.Pid), errorPtr, C.int(bufferLength))
 
 	if r < 0 {
 		return nil, errors.New(string(errorBuf[:-r]))
@@ -55,7 +55,7 @@ func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
 		dataBuf:  dataBuf,
 		errorBuf: errorBuf,
 		errorPtr: errorPtr,
-		pid:      pid,
+		pid:      params.Pid,
 	}, nil
 }
 

pkg/agent/profiler/profiler.go

@@ -1,10 +1,12 @@
 // Package profiler is a public API golang apps should use to send data to pyroscope server. It is intentionally separate from the rest of the code.
-//   The idea is that this API won't change much over time, while all the other code will.
+//
+//	The idea is that this API won't change much over time, while all the other code will.
 package profiler
 
 import (
 	"context"
 	"fmt"
+	"github.com/pyroscope-io/pyroscope/pkg/agent/log"
 	"os"
 	"runtime/pprof"
 	"time"
@@ -31,7 +33,7 @@ type Config struct {
 	ServerAddress   string // e.g http://pyroscope.services.internal:4040
 	AuthToken       string // specify this token when using pyroscope cloud
 	SampleRate      uint32
-	Logger          agent.Logger
+	Logger          log.Logger
 	ProfileTypes    []ProfileType
 	DisableGCRuns   bool // this will disable automatic runtime.GC runs
 }
@@ -49,7 +51,7 @@ func Start(cfg Config) (*Profiler, error) {
 		cfg.SampleRate = types.DefaultSampleRate
 	}
 	if cfg.Logger == nil {
-		cfg.Logger = &agent.NoopLogger{}
+		cfg.Logger = &log.NoopLogger{}
 	}
 
 	// Override the address to use when the environment variable is defined.

pkg/agent/pyspy/pyspy.go

@@ -37,7 +37,7 @@ type PySpy struct {
 	pid int
 }
 
-func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
+func Start(params spy.InitParams) (spy.Spy, error) {
 	dataBuf := make([]byte, bufferLength)
 	dataPtr := unsafe.Pointer(&dataBuf[0])
 
@@ -52,7 +52,7 @@ func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
 	if Blocking {
 		blocking = 1
 	}
-	r := C.pyspy_init(C.int(pid), C.int(blocking), errorPtr, C.int(bufferLength))
+	r := C.pyspy_init(C.int(params.Pid), C.int(blocking), errorPtr, C.int(bufferLength))
 
 	if r < 0 {
 		return nil, errors.New(string(errorBuf[:-r]))
@@ -63,7 +63,7 @@ func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
 		dataBuf:  dataBuf,
 		errorBuf: errorBuf,
 		errorPtr: errorPtr,
-		pid:      pid,
+		pid:      params.Pid,
 	}, nil
 }
 

pkg/agent/rbspy/rbspy.go

@@ -36,7 +36,7 @@ type RbSpy struct {
 	pid int
 }
 
-func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
+func Start(params spy.InitParams) (spy.Spy, error) {
 	dataBuf := make([]byte, bufferLength)
 	dataPtr := unsafe.Pointer(&dataBuf[0])
 
@@ -51,7 +51,7 @@ func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
 	if Blocking {
 		blocking = 1
 	}
-	r := C.rbspy_init(C.int(pid), C.int(blocking), errorPtr, C.int(bufferLength))
+	r := C.rbspy_init(C.int(params.Pid), C.int(blocking), errorPtr, C.int(bufferLength))
 
 	if r < 0 {
 		return nil, errors.New(string(errorBuf[:-r]))
@@ -62,7 +62,7 @@ func Start(pid int, _ spy.ProfileType, _ uint32, _ bool) (spy.Spy, error) {
 		dataBuf:  dataBuf,
 		errorBuf: errorBuf,
 		errorPtr: errorPtr,
-		pid:      pid,
+		pid:      params.Pid,
 	}, nil
 }