[USM] Handle Istio binary path

pkg/config/setup/system_probe.go

@@ -51,6 +51,9 @@ const (
 	defaultZypperReposDirSuffix = "/zypp/repos.d"
 
 	defaultOffsetThreshold = 400
+
+	// defaultEnvoyPath is the default path for envoy binary
+	defaultEnvoyPath = "/bin/envoy"
 )
 
 var (
@@ -237,6 +240,7 @@ func InitSystemProbeConfig(cfg pkgconfigmodel.Config) {
 	cfg.BindEnv(join(smNS, "enable_postgres_monitoring"))
 	cfg.BindEnv(join(smNS, "enable_redis_monitoring"))
 	cfg.BindEnvAndSetDefault(join(smNS, "tls", "istio", "enabled"), false)
+	cfg.BindEnvAndSetDefault(join(smNS, "tls", "istio", "envoy_path"), defaultEnvoyPath)
 	cfg.BindEnv(join(smNS, "tls", "nodejs", "enabled"))
 	cfg.BindEnvAndSetDefault(join(smjtNS, "enabled"), false)
 	cfg.BindEnvAndSetDefault(join(smjtNS, "debug"), false)

pkg/network/config/config.go

@@ -96,6 +96,9 @@ type Config struct {
 	// EnableIstioMonitoring specifies whether USM should monitor Istio traffic
 	EnableIstioMonitoring bool
 
+	// EnvoyPath specifies the envoy path to be used for Istio monitoring
+	EnvoyPath string
+
 	// EnableNodeJSMonitoring specifies whether USM should monitor NodeJS TLS traffic
 	EnableNodeJSMonitoring bool
 
@@ -364,6 +367,7 @@ func New() *Config {
 		EnableRedisMonitoring:     cfg.GetBool(join(smNS, "enable_redis_monitoring")),
 		EnableNativeTLSMonitoring: cfg.GetBool(join(smNS, "tls", "native", "enabled")),
 		EnableIstioMonitoring:     cfg.GetBool(join(smNS, "tls", "istio", "enabled")),
+		EnvoyPath:                 cfg.GetString(join(smNS, "tls", "istio", "envoy_path")),
 		EnableNodeJSMonitoring:    cfg.GetBool(join(smNS, "tls", "nodejs", "enabled")),
 		MaxUSMConcurrentRequests:  uint32(cfg.GetInt(join(smNS, "max_concurrent_requests"))),
 		MaxHTTPStatsBuffered:      cfg.GetInt(join(smNS, "max_http_stats_buffered")),

pkg/network/config/config_test.go

@@ -1399,6 +1399,33 @@ service_monitoring_config:
 	})
 }
 
+func TestEnvoyPathConfig(t *testing.T) {
+	t.Run("default value", func(t *testing.T) {
+		aconfig.ResetSystemProbeConfig(t)
+		cfg := New()
+		assert.EqualValues(t, cfg.EnvoyPath, "/bin/envoy")
+	})
+
+	t.Run("via yaml", func(t *testing.T) {
+		aconfig.ResetSystemProbeConfig(t)
+		cfg := configurationFromYAML(t, `
+service_monitoring_config:
+  tls:
+    istio:
+      envoy_path: "/test/envoy"
+`)
+		assert.EqualValues(t, "/test/envoy", cfg.EnvoyPath)
+	})
+
+	t.Run("value set through env var", func(t *testing.T) {
+		aconfig.ResetSystemProbeConfig(t)
+		t.Setenv("DD_SERVICE_MONITORING_CONFIG_TLS_ISTIO_ENVOY_PATH", "/test/envoy")
+
+		cfg := New()
+		assert.EqualValues(t, "/test/envoy", cfg.EnvoyPath)
+	})
+}
+
 func TestNodeJSMonitoring(t *testing.T) {
 	t.Run("default value", func(t *testing.T) {
 		aconfig.ResetSystemProbeConfig(t)

pkg/network/usm/ebpf_gotls.go

@@ -319,21 +319,8 @@ func (p *goTLSProgram) AttachPID(pid uint32) error {
 	pidAsStr := strconv.FormatUint(uint64(pid), 10)
 	exePath := filepath.Join(p.procRoot, pidAsStr, "exe")
 
-	binPath, err := os.Readlink(exePath)
+	binPath, err := utils.ResolveSymlink(exePath)
 	if err != nil {
-		// We receive the Exec event, /proc could be slow to update
-		end := time.Now().Add(10 * time.Millisecond)
-		for end.After(time.Now()) {
-			binPath, err = os.Readlink(exePath)
-			if err == nil {
-				break
-			}
-			time.Sleep(time.Millisecond)
-		}
-	}
-	if err != nil {
-		// we can't access to the binary path here (pid probably ended already)
-		// there are not much we can do, and we don't want to flood the logs
 		return err
 	}
 

pkg/network/usm/istio.go

@@ -8,9 +8,8 @@
 package usm
 
 import (
-	"bytes"
 	"fmt"
-	"os"
+	"strings"
 	"sync"
 	"time"
 
@@ -19,8 +18,6 @@ import (
 	"github.com/DataDog/datadog-agent/pkg/process/monitor"
 	"github.com/DataDog/datadog-agent/pkg/util/kernel"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
-	ddsync "github.com/DataDog/datadog-agent/pkg/util/sync"
-
 	manager "github.com/DataDog/ebpf-manager"
 )
 
@@ -76,16 +73,6 @@ var istioProbes = []manager.ProbesSelector{
 	},
 }
 
-// envoyCmd represents the search term used for determining
-// whether or not a given PID represents an Envoy process.
-// The search is done over the /proc/<pid>/cmdline file.
-var envoyCmd = []byte("/bin/envoy")
-
-// readBufferPool is used for reading /proc/<pid>/cmdline files.
-// We use a pointer to a slice to avoid allocations when casting
-// values to the empty interface during Put() calls.
-var readBufferPool = ddsync.NewSlicePool[byte](128, 128)
-
 // istioMonitor essentially scans for Envoy processes and attaches SSL uprobes
 // to them.
 //
@@ -95,6 +82,7 @@ var readBufferPool = ddsync.NewSlicePool[byte](128, 128)
 type istioMonitor struct {
 	registry *utils.FileRegistry
 	procRoot string
+	envoyCmd string
 
 	// `utils.FileRegistry` callbacks
 	registerCB   func(utils.FilePath) error
@@ -117,6 +105,7 @@ func newIstioMonitor(c *config.Config, mgr *manager.Manager) *istioMonitor {
 	return &istioMonitor{
 		registry: utils.NewFileRegistry("istio"),
 		procRoot: procRoot,
+		envoyCmd: c.EnvoyPath,
 		done:     make(chan struct{}),
 
 		// Callbacks
@@ -250,46 +239,19 @@ func (m *istioMonitor) handleProcessExec(pid uint32) {
 }
 
 // getEnvoyPath returns the executable path of the envoy binary for a given PID.
-// In case the PID doesn't represent an envoy process, an empty string is returned.
+// 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.
 //
-// TODO:
-// refine process detection heuristic so we can remove the number of false
-// positives. A common case that is likely worth optimizing for is filtering
-// out "vanilla" envoy processes, and selecting only envoy processes that are
-// running inside istio containers. Based on a quick inspection I made, it
-// seems that we could also search for "istio" in the cmdline string in addition
-// to "envoy", since the command line arguments look more or less the following:
-//
-// /usr/local/bin/envoy -cetc/istio/proxy/envoy-rev.json ...
+// 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 {
-	cmdlinePath := fmt.Sprintf("%s/%d/cmdline", m.procRoot, pid)
-
-	f, err := os.Open(cmdlinePath)
-	if err != nil {
-		// This can happen often in the context of ephemeral processes
-		return ""
-	}
-	defer f.Close()
-
-	// From here on we shouldn't allocate for the common case
-	// (eg., a process is *not* envoy)
-	bufferPtr := readBufferPool.Get()
-	defer func() {
-		readBufferPool.Put(bufferPtr)
-	}()
-
-	buffer := *bufferPtr
-	n, _ := f.Read(buffer)
-	if n == 0 {
-		return ""
-	}
+	exePath := fmt.Sprintf("%s/%d/exe", m.procRoot, pid)
 
-	buffer = buffer[:n]
-	i := bytes.Index(buffer, envoyCmd)
-	if i < 0 {
+	envoyPath, err := utils.ResolveSymlink(exePath)
+	if err != nil || !strings.Contains(envoyPath, m.envoyCmd) {
 		return ""
 	}
 
-	executable := buffer[:i+len(envoyCmd)]
-	return string(executable)
+	return envoyPath
 }