usm: Refactor istio monitor to use new uprobe attacher

pkg/network/usm/ebpf_ssl.go

@@ -466,10 +466,15 @@ func newSSLProgramProtocolFactory(m *manager.Manager) protocols.ProtocolFactory
 			return nil, fmt.Errorf("error initializing nodejs monitor: %w", err)
 		}
 
+		istio, err := newIstioMonitor(c, m)
+		if err != nil {
+			return nil, fmt.Errorf("error initializing istio monitor: %w", err)
+		}
+
 		return &sslProgram{
 			cfg:           c,
 			watcher:       watcher,
-			istioMonitor:  newIstioMonitor(c, m),
+			istioMonitor:  istio,
 			nodeJSMonitor: nodejs,
 		}, nil
 	}

pkg/network/usm/istio.go

@@ -9,24 +9,22 @@ package usm
 
 import (
 	"fmt"
-	"os"
 	"strings"
-	"sync"
-	"time"
 
 	manager "github.com/DataDog/ebpf-manager"
 
+	"github.com/DataDog/datadog-agent/pkg/ebpf/uprobes"
 	"github.com/DataDog/datadog-agent/pkg/network/config"
 	"github.com/DataDog/datadog-agent/pkg/network/usm/consts"
-	"github.com/DataDog/datadog-agent/pkg/network/usm/utils"
 	"github.com/DataDog/datadog-agent/pkg/process/monitor"
-	"github.com/DataDog/datadog-agent/pkg/util/kernel"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 )
 
 const (
 	istioSslReadRetprobe  = "istio_uretprobe__SSL_read"
 	istioSslWriteRetprobe = "istio_uretprobe__SSL_write"
+
+	istioAttacherName = "istio"
 )
 
 var istioProbes = []manager.ProbesSelector{
@@ -83,64 +81,41 @@ var istioProbes = []manager.ProbesSelector{
 // because the Envoy binary embedded in the Istio containers have debug symbols
 // whereas the "vanilla" Envoy images are distributed without them.
 type istioMonitor struct {
-	registry *utils.FileRegistry
-	procRoot string
-	envoyCmd string
-
-	// `utils.FileRegistry` callbacks
-	registerCB   func(utils.FilePath) error
-	unregisterCB func(utils.FilePath) error
-
-	// Termination
-	wg   sync.WaitGroup
-	done chan struct{}
+	attacher       *uprobes.UprobeAttacher
+	envoyCmd       string
+	processMonitor *monitor.ProcessMonitor
 }
 
-// Validate that istioMonitor implements the Attacher interface.
-var _ utils.Attacher = &istioMonitor{}
-
-func newIstioMonitor(c *config.Config, mgr *manager.Manager) *istioMonitor {
+func newIstioMonitor(c *config.Config, mgr *manager.Manager) (*istioMonitor, error) {
 	if !c.EnableIstioMonitoring {
-		return nil
+		return nil, nil
 	}
 
-	procRoot := kernel.ProcFSRoot()
-	return &istioMonitor{
-		registry: utils.NewFileRegistry(consts.USMModuleName, "istio"),
-		procRoot: procRoot,
+	m := &istioMonitor{
 		envoyCmd: c.EnvoyPath,
-		done:     make(chan struct{}),
-
-		// Callbacks
-		registerCB:   addHooks(mgr, procRoot, istioProbes),
-		unregisterCB: removeHooks(mgr, istioProbes),
+		attacher: nil,
 	}
-}
-
-// DetachPID detaches a given pid from the eBPF program
-func (m *istioMonitor) DetachPID(pid uint32) error {
-	return m.registry.Unregister(pid)
-}
 
-var (
-	// ErrNoEnvoyPath is returned when no envoy path is found for a given PID
-	ErrNoEnvoyPath = fmt.Errorf("no envoy path found for PID")
-)
-
-// AttachPID attaches a given pid to the eBPF program
-func (m *istioMonitor) AttachPID(pid uint32) error {
-	path := m.getEnvoyPath(pid)
-	if path == "" {
-		return ErrNoEnvoyPath
+	attachCfg := uprobes.AttacherConfig{
+		ProcRoot: c.ProcRoot,
+		Rules: []*uprobes.AttachRule{{
+			Targets:          uprobes.AttachToExecutable,
+			ProbesSelector:   istioProbes,
+			ExecutableFilter: m.isIstioBinary,
+		}},
+		EbpfConfig:                     &c.Config,
+		ExcludeTargets:                 uprobes.ExcludeSelf | uprobes.ExcludeInternal | uprobes.ExcludeBuildkit | uprobes.ExcludeContainerdTmp,
+		EnablePeriodicScanNewProcesses: true,
+	}
+	attacher, err := uprobes.NewUprobeAttacher(consts.USMModuleName, istioAttacherName, attachCfg, mgr, nil, &uprobes.NativeBinaryInspector{}, m.processMonitor)
+	if err != nil {
+		return nil, fmt.Errorf("Cannot create uprobe attacher: %w", err)
 	}
 
-	return m.registry.Register(
-		path,
-		pid,
-		m.registerCB,
-		m.unregisterCB,
-		utils.IgnoreCB,
-	)
+	m.attacher = attacher
+	m.processMonitor = monitor.GetProcessMonitor()
+
+	return m, nil
 }
 
 // Start the istioMonitor
@@ -149,49 +124,16 @@ func (m *istioMonitor) Start() {
 		return
 	}
 
-	processMonitor := monitor.GetProcessMonitor()
-
-	// Subscribe to process events
-	doneExec := processMonitor.SubscribeExec(m.handleProcessExec)
-	doneExit := processMonitor.SubscribeExit(m.handleProcessExit)
-
-	// Attach to existing processes
-	m.sync()
-
-	m.wg.Add(1)
-	go func() {
-		// This ticker is responsible for controlling the rate at which
-		// we scrape the whole procFS again in order to ensure that we
-		// terminate any dangling uprobes and register new processes
-		// missed by the process monitor stream
-		processSync := time.NewTicker(scanTerminatedProcessesInterval)
-
-		defer func() {
-			processSync.Stop()
-			// Execute process monitor callback termination functions
-			doneExec()
-			doneExit()
-			// Stopping the process monitor (if we're the last instance)
-			processMonitor.Stop()
-			// Cleaning up all active hooks
-			m.registry.Clear()
-			// marking we're finished.
-			m.wg.Done()
-		}()
+	if m.attacher == nil {
+		log.Error("istio monitoring is enabled but the attacher is nil")
+		return
+	}
 
-		for {
-			select {
-			case <-m.done:
-				return
-			case <-processSync.C:
-				m.sync()
-				m.registry.Log()
-			}
-		}
-	}()
+	if err := m.attacher.Start(); err != nil {
+		log.Errorf("Cannot start istio attacher: %s", err)
+	}
 
-	utils.AddAttacher(consts.USMModuleName, "istio", m)
-	log.Info("Istio monitoring enabled")
+	log.Info("istio monitoring enabled")
 }
 
 // Stop the istioMonitor.
@@ -200,62 +142,11 @@ func (m *istioMonitor) Stop() {
 		return
 	}
 
-	close(m.done)
-	m.wg.Wait()
-}
-
-// sync state of istioMonitor with the current state of procFS
-// the purpose of this method is two-fold:
-// 1) register processes for which we missed exec events (targeted mostly at startup)
-// 2) unregister processes for which we missed exit events
-func (m *istioMonitor) sync() {
-	deletionCandidates := m.registry.GetRegisteredProcesses()
-
-	_ = kernel.WithAllProcs(m.procRoot, func(pid int) error {
-		if _, ok := deletionCandidates[uint32(pid)]; ok {
-			// We have previously hooked into this process and it remains active,
-			// so we remove it from the deletionCandidates list, and move on to the next PID
-			delete(deletionCandidates, uint32(pid))
-			return nil
-		}
-
-		// This is a new PID so we attempt to attach SSL probes to it
-		_ = m.AttachPID(uint32(pid))
-		return nil
-	})
-
-	// At this point all entries from deletionCandidates are no longer alive, so
-	// we should detach our SSL probes from them
-	for pid := range deletionCandidates {
-		m.handleProcessExit(pid)
-	}
-}
-
-func (m *istioMonitor) handleProcessExit(pid uint32) {
-	// We avoid filtering PIDs here because it's cheaper to simply do a registry lookup
-	// instead of fetching a process name in order to determine whether it is an
-	// envoy process or not (which at the very minimum involves syscalls)
-	_ = m.DetachPID(pid)
-}
-
-func (m *istioMonitor) handleProcessExec(pid uint32) {
-	_ = m.AttachPID(pid)
+	m.attacher.Stop()
 }
 
-// getEnvoyPath returns the executable path of the envoy binary for a given PID.
-// It constructs the path to the symbolic link for the executable file of the process with the given PID,
-// then resolves this symlink to determine the actual path of the binary.
-//
-// If the resolved path contains the expected envoy command substring (as defined by m.envoyCmd),
-// the function returns this path. If the PID does not correspond to an envoy process or if an error
-// occurs during resolution, it returns an empty string.
-func (m *istioMonitor) getEnvoyPath(pid uint32) string {
-	exePath := fmt.Sprintf("%s/%d/exe", m.procRoot, pid)
-
-	envoyPath, err := os.Readlink(exePath)
-	if err != nil || !strings.Contains(envoyPath, m.envoyCmd) {
-		return ""
-	}
-
-	return envoyPath
+// isIstioBinary checks whether the given file is an istioBinary, based on the expected envoy
+// command substring (as defined by m.envoyCmd).
+func (m *istioMonitor) isIstioBinary(path string, _ *uprobes.ProcInfo) bool {
+	return strings.Contains(path, m.envoyCmd)
 }