Don't exit if parsing of /proc/PID/maps fails

libpf/convenience_test.go

@@ -11,42 +11,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestHexTo(t *testing.T) {
-	tests := map[string]struct {
-		result uint64
-	}{
-		"0":      {result: 0},
-		"FFFFFF": {result: 16777215},
-		"42":     {result: 66},
-	}
-
-	for name, testcase := range tests {
-		name := name
-		testcase := testcase
-		t.Run(name, func(t *testing.T) {
-			assert.Equal(t, testcase.result, util.HexToUint64(name))
-		})
-	}
-}
-
-func TestDecTo(t *testing.T) {
-	tests := map[string]struct {
-		result uint64
-	}{
-		"0":   {result: 0},
-		"123": {result: 123},
-		"42":  {result: 42},
-	}
-
-	for name, testcase := range tests {
-		name := name
-		testcase := testcase
-		t.Run(name, func(t *testing.T) {
-			assert.Equal(t, testcase.result, util.DecToUint64(name))
-		})
-	}
-}
-
 func TestIsValidString(t *testing.T) {
 	tests := map[string]struct {
 		input    []byte

metrics/metrics.json

@@ -1978,5 +1978,12 @@
     "name": "TraceEventReadError",
     "field": "agent.errors.trace_event_read_error",
     "id": 274
+  },
+  {
+    "description": "Number of parsing errors seen during processing /proc/<PID>/maps",
+    "type": "counter",
+    "name": "ErrProcParse",
+    "field": "agent.errors.proc_parse",
+    "id": 275
   }
 ]

process/coredump.go

@@ -258,8 +258,8 @@ func (cd *CoredumpProcess) GetMachineData() MachineData {
 }
 
 // GetMappings implements the Process interface
-func (cd *CoredumpProcess) GetMappings() ([]Mapping, error) {
-	return cd.mappings, nil
+func (cd *CoredumpProcess) GetMappings() ([]Mapping, uint32, error) {
+	return cd.mappings, 0, nil
 }
 
 // GetThreadInfo implements the Process interface

process/process.go

@@ -11,16 +11,17 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strconv"
 	"strings"
 	"sync"
 
+	log "github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 
 	"go.opentelemetry.io/ebpf-profiler/libpf"
 	"go.opentelemetry.io/ebpf-profiler/libpf/pfelf"
 	"go.opentelemetry.io/ebpf-profiler/remotememory"
 	"go.opentelemetry.io/ebpf-profiler/stringutil"
-	"go.opentelemetry.io/ebpf-profiler/util"
 )
 
 // systemProcess provides an implementation of the Process interface for a
@@ -79,12 +80,13 @@ func trimMappingPath(path string) string {
 	return path
 }
 
-func parseMappings(mapsFile io.Reader) ([]Mapping, error) {
+func parseMappings(mapsFile io.Reader) ([]Mapping, uint32, error) {
+	numParseErrors := uint32(0)
 	mappings := make([]Mapping, 0, 32)
 	scanner := bufio.NewScanner(mapsFile)
 	scanBuf := bufPool.Get().(*[]byte)
 	if scanBuf == nil {
-		return mappings, errors.New("failed to get memory from sync pool")
+		return mappings, 0, errors.New("failed to get memory from sync pool")
 	}
 	defer func() {
 		// Reset memory and return it for reuse.
@@ -101,14 +103,17 @@ func parseMappings(mapsFile io.Reader) ([]Mapping, error) {
 
 		line := stringutil.ByteSlice2String(scanner.Bytes())
 		if stringutil.FieldsN(line, fields[:]) < 5 {
+			numParseErrors++
 			continue
 		}
 		if stringutil.SplitN(fields[0], "-", addrs[:]) < 2 {
+			numParseErrors++
 			continue
 		}
 
 		mapsFlags := fields[1]
 		if len(mapsFlags) < 3 {
+			numParseErrors++
 			continue
 		}
 		flags := elf.ProgFlag(0)
@@ -126,12 +131,31 @@ func parseMappings(mapsFile io.Reader) ([]Mapping, error) {
 		if flags&(elf.PF_R|elf.PF_X) == 0 {
 			continue
 		}
-		inode := util.DecToUint64(fields[4])
+		inode, err := strconv.ParseUint(fields[4], 10, 64)
+		if err != nil {
+			log.Debugf("inode: failed to convert %s to uint64: %v", fields[4], err)
+			numParseErrors++
+			continue
+		}
+
 		path := fields[5]
 		if stringutil.SplitN(fields[3], ":", devs[:]) < 2 {
+			numParseErrors++
 			continue
 		}
-		device := util.HexToUint64(devs[0])<<8 + util.HexToUint64(devs[1])
+		major, err := strconv.ParseUint(devs[0], 16, 64)
+		if err != nil {
+			log.Debugf("major device: failed to convert %s to uint64: %v", devs[0], err)
+			numParseErrors++
+			continue
+		}
+		minor, err := strconv.ParseUint(devs[1], 16, 64)
+		if err != nil {
+			log.Debugf("minor device: failed to convert %s to uint64: %v", devs[1], err)
+			numParseErrors++
+			continue
+		}
+		device := major<<8 + minor
 
 		if inode == 0 {
 			if path == "[vdso]" {
@@ -149,33 +173,53 @@ func parseMappings(mapsFile io.Reader) ([]Mapping, error) {
 			path = strings.Clone(path)
 		}
 
-		vaddr := util.HexToUint64(addrs[0])
+		vaddr, err := strconv.ParseUint(addrs[0], 16, 64)
+		if err != nil {
+			log.Debugf("vaddr: failed to convert %s to uint64: %v", addrs[0], err)
+			numParseErrors++
+			continue
+		}
+		vend, err := strconv.ParseUint(addrs[1], 16, 64)
+		if err != nil {
+			log.Debugf("vend: failed to convert %s to uint64: %v", addrs[1], err)
+			numParseErrors++
+			continue
+		}
+		length := vend - vaddr
+
+		fileOffset, err := strconv.ParseUint(fields[2], 16, 64)
+		if err != nil {
+			log.Debugf("fileOffset: failed to convert %s to uint64: %v", fields[2], err)
+			numParseErrors++
+			continue
+		}
+
 		mappings = append(mappings, Mapping{
 			Vaddr:      vaddr,
-			Length:     util.HexToUint64(addrs[1]) - vaddr,
+			Length:     length,
 			Flags:      flags,
-			FileOffset: util.HexToUint64(fields[2]),
+			FileOffset: fileOffset,
 			Device:     device,
 			Inode:      inode,
 			Path:       path,
 		})
 	}
-	return mappings, scanner.Err()
+	return mappings, numParseErrors, scanner.Err()
 }
 
 // GetMappings will process the mappings file from proc. Additionally,
 // a reverse map from mapping filename to a Mapping node is built to allow
 // OpenELF opening ELF files using the corresponding proc map_files entry.
 // WARNING: This implementation does not support calling GetMappings
 // concurrently with itself, or with OpenELF.
-func (sp *systemProcess) GetMappings() ([]Mapping, error) {
+func (sp *systemProcess) GetMappings() ([]Mapping, uint32, error) {
 	mapsFile, err := os.Open(fmt.Sprintf("/proc/%d/maps", sp.pid))
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 	defer mapsFile.Close()
 
-	mappings, err := parseMappings(mapsFile)
+	mappings, numParseErrors, err := parseMappings(mapsFile)
 	if err == nil {
 		fileToMapping := make(map[string]*Mapping, len(mappings))
 		for idx := range mappings {
@@ -189,7 +233,7 @@ func (sp *systemProcess) GetMappings() ([]Mapping, error) {
 		}
 		sp.fileToMapping = fileToMapping
 	}
-	return mappings, err
+	return mappings, numParseErrors, err
 }
 
 func (sp *systemProcess) GetThreads() ([]ThreadInfo, error) {

process/process_test.go

@@ -22,11 +22,16 @@ var testMappings = `55fe82710000-55fe8273c000 r--p 00000000 fd:01 1068432
 55fe82836000-55fe8283d000 r--p 00125000 fd:01 1068432                    /tmp/usr_bin_seahorse
 55fe8283d000-55fe8283e000 rw-p 0012c000 fd:01 1068432                    /tmp/usr_bin_seahorse
 7f63c8c3e000-7f63c8de0000 r-xp 00085000 08:01 1048922                    /tmp/usr_lib_x86_64-linux-gnu_libcrypto.so.1.1
-7f63c8ebf000-7f63c8fef000 r-xp 0001c000 1fd:01 1075944                   /tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0`
+7f63c8ebf000-7f63c8fef000 r-xp 0001c000 1fd:01 1075944                   /tmp/usr_lib_x86_64-linux-gnu_libopensc.so.6.0.0
+7f63c8eef000-7f63c8fdf000 r-xp 0001c000 1fd:01
+7f63c8eef000-7f63c8fdf000 r-xp 0001c000 1fd.01 1075944
+7f63c8eef000-7f63c8fdf000 r- 0001c000 1fd:01 1075944
+7f63c8eef000 r-xp 0001c000 1fd:01 1075944`
 
 func TestParseMappings(t *testing.T) {
-	mappings, err := parseMappings(strings.NewReader(testMappings))
+	mappings, numParseErrors, err := parseMappings(strings.NewReader(testMappings))
 	require.NoError(t, err)
+	require.Equal(t, uint32(4), numParseErrors)
 	assert.NotNil(t, mappings)
 
 	expected := []Mapping{
@@ -101,7 +106,8 @@ func TestNewPIDOfSelf(t *testing.T) {
 	pr := New(libpf.PID(os.Getpid()))
 	assert.NotNil(t, pr)
 
-	mappings, err := pr.GetMappings()
+	mappings, numParseErrors, err := pr.GetMappings()
 	require.NoError(t, err)
+	require.Equal(t, uint32(0), numParseErrors)
 	assert.NotEmpty(t, mappings)
 }

process/types.go

@@ -101,7 +101,7 @@ type Process interface {
 	GetMachineData() MachineData
 
 	// GetMappings reads and parses process memory mappings
-	GetMappings() ([]Mapping, error)
+	GetMappings() ([]Mapping, uint32, error)
 
 	// GetThreads reads the process thread states
 	GetThreads() ([]ThreadInfo, error)

processmanager/manager.go

@@ -174,6 +174,8 @@ func collectInterpreterMetrics(ctx context.Context, pm *ProcessManager,
 			metrics.MetricValue(pm.mappingStats.maxProcParseUsec.Swap(0))
 		summary[metrics.IDTotalProcParseUsec] =
 			metrics.MetricValue(pm.mappingStats.totalProcParseUsec.Swap(0))
+		summary[metrics.IDErrProcParse] =
+			metrics.MetricValue(pm.mappingStats.numProcParseErrors.Swap(0))
 
 		mapsMetrics := pm.ebpf.CollectMetrics()
 		for _, metric := range mapsMetrics {

processmanager/manager_test.go

@@ -48,8 +48,8 @@ func (d *dummyProcess) GetMachineData() process.MachineData {
 	return process.MachineData{}
 }
 
-func (d *dummyProcess) GetMappings() ([]process.Mapping, error) {
-	return nil, errors.New("not implemented")
+func (d *dummyProcess) GetMappings() ([]process.Mapping, uint32, error) {
+	return nil, 0, errors.New("not implemented")
 }
 
 func (d *dummyProcess) GetThreads() ([]process.ThreadInfo, error) {

processmanager/processinfo.go

@@ -600,8 +600,9 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) {
 
 	pm.mappingStats.numProcAttempts.Add(1)
 	start := time.Now()
-	mappings, err := pr.GetMappings()
+	mappings, numParseErrors, err := pr.GetMappings()
 	elapsed := time.Since(start)
+	pm.mappingStats.numProcParseErrors.Add(numParseErrors)
 
 	if err != nil {
 		if os.IsPermission(err) {
@@ -622,7 +623,7 @@ func (pm *ProcessManager) SynchronizeProcess(pr process.Process) {
 			pm.mappingStats.errProcNotExist.Add(1)
 		} else if e, ok := err.(*os.PathError); ok && e.Err == syscall.ESRCH {
 			// If the process exits while reading its /proc/$PID/maps, the kernel will
-			// return ESRCH. Handle it as if the process did not exists.
+			// return ESRCH. Handle it as if the process did not exist.
 			pm.mappingStats.errProcESRCH.Add(1)
 		}
 		return

processmanager/types.go

@@ -78,6 +78,7 @@ type ProcessManager struct {
 		numProcAttempts    atomic.Uint32
 		maxProcParseUsec   atomic.Uint32
 		totalProcParseUsec atomic.Uint32
+		numProcParseErrors atomic.Uint32
 	}
 
 	// elfInfoCache provides a cache to quickly retrieve the ELF info and fileID for a particular

util/util.go

@@ -5,36 +5,13 @@ package util // import "go.opentelemetry.io/ebpf-profiler/util"
 
 import (
 	"math/bits"
-	"strconv"
 	"sync/atomic"
 	"unicode"
 	"unicode/utf8"
 
-	"github.com/sirupsen/logrus"
-
 	"go.opentelemetry.io/ebpf-profiler/libpf/hash"
 )
 
-// HexToUint64 is a convenience function to extract a hex string to a uint64 and
-// not worry about errors. Essentially a "mustConvertHexToUint64".
-func HexToUint64(str string) uint64 {
-	v, err := strconv.ParseUint(str, 16, 64)
-	if err != nil {
-		logrus.Fatalf("Failure to hex-convert %s to uint64: %v", str, err)
-	}
-	return v
-}
-
-// DecToUint64 is a convenience function to extract a decimal string to a uint64
-// and not worry about errors. Essentially a "mustConvertDecToUint64".
-func DecToUint64(str string) uint64 {
-	v, err := strconv.ParseUint(str, 10, 64)
-	if err != nil {
-		logrus.Fatalf("Failure to dec-convert %s to uint64: %v", str, err)
-	}
-	return v
-}
-
 // IsValidString checks if string is UTF-8-encoded and only contains expected characters.
 func IsValidString(s string) bool {
 	if s == "" {