Identify the PHP-FPM master by parentage, not cmdline

pkg/node/addon/discovery/discovery.go

@@ -82,6 +82,8 @@ func findParentProcess(name string) (int32, bool, error) {
 			continue
 		}
 
+		// Node has no master/worker split, so the first match is the
+		// process to attach to; no PPID disambiguation is needed here.
 		return p.Pid, true, nil
 	}
 

pkg/php/extension/discovery/discovery.go

@@ -5,8 +5,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"log/slog"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/cenkalti/backoff/v4"
@@ -65,27 +65,64 @@ func GetPathFromProcess(ctx context.Context, processName, extensionPath string,
 	}
 }
 
-// Helper function to find the parent process
+// processInfo is the subset of *process.Process that findMasterProcess needs,
+// so master identification can be tested with a mock process list.
+type processInfo interface {
+	pid() int32
+	name() (string, error)
+	ppid() (int32, error)
+}
+
+type gopsutilProcess struct {
+	p *process.Process
+}
+
+func (g gopsutilProcess) pid() int32            { return g.p.Pid }
+func (g gopsutilProcess) name() (string, error) { return g.p.Name() }
+func (g gopsutilProcess) ppid() (int32, error)  { return g.p.Ppid() }
+
 func findParentProcess(name string) (int32, bool, error) {
 	processes, err := process.Processes()
 	if err != nil {
 		return 0, false, fmt.Errorf("failed to get process list: %w", err)
 	}
 
+	infos := make([]processInfo, 0, len(processes))
+	for _, p := range processes {
+		infos = append(infos, gopsutilProcess{p: p})
+	}
+
+	return findMasterProcess(name, infos)
+}
+
+// findMasterProcess returns the php-fpm process that has no php-fpm parent:
+// workers carry the master's PID as their PPID, the master does not. This
+// avoids matching "master process" in a command line, which any php-fpm
+// invocation containing those words would satisfy.
+func findMasterProcess(name string, processes []processInfo) (int32, bool, error) {
+	names := make(map[int32]string, len(processes))
 	for _, p := range processes {
-		n, err := p.Name()
+		n, err := p.name()
 		if err != nil {
-			// The process may have exited while we were looking at it.
+			continue
+		}
+		names[p.pid()] = n
+	}
+
+	for _, p := range processes {
+		if names[p.pid()] != name {
 			continue
 		}
 
-		if n != name {
+		ppid, err := p.ppid()
+		if err != nil {
+			slog.Warn("failed to read parent PID during discovery, skipping process",
+				"process", name, "pid", p.pid(), "error", err)
 			continue
 		}
 
-		cmdline, _ := p.Cmdline()
-		if strings.Contains(cmdline, "master process") {
-			return p.Pid, true, nil
+		if names[ppid] != name {
+			return p.pid(), true, nil
 		}
 	}
 

pkg/php/extension/discovery/discovery_test.go

@@ -2,12 +2,26 @@ package discovery
 
 import (
 	"context"
+	"errors"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
+// mockProcess is a test stand-in for a real process.
+type mockProcess struct {
+	mockPid     int32
+	mockName    string
+	mockNameErr error
+	mockPpid    int32
+	mockPpidErr error
+}
+
+func (m mockProcess) pid() int32            { return m.mockPid }
+func (m mockProcess) name() (string, error) { return m.mockName, m.mockNameErr }
+func (m mockProcess) ppid() (int32, error)  { return m.mockPpid, m.mockPpidErr }
+
 func TestGetPathFromProcess_NotRunning(t *testing.T) {
 	// A runtime which is not present must be reported as not found, so that the
 	// sidecar can skip it instead of failing.
@@ -22,3 +36,90 @@ func TestGetPathFromProcess_ContextCancelled(t *testing.T) {
 	_, err := GetPathFromProcess(ctx, "compass-does-not-exist-4f2b", "/compass.so", time.Minute)
 	assert.ErrorIs(t, err, context.Canceled)
 }
+
+func TestFindMasterProcess(t *testing.T) {
+	const name = "php-fpm"
+
+	tests := []struct {
+		name      string
+		processes []processInfo
+		wantPid   int32
+		wantFound bool
+	}{
+		{
+			name: "master with init parent and workers",
+			processes: []processInfo{
+				mockProcess{mockPid: 10, mockName: name, mockPpid: 1},
+				mockProcess{mockPid: 11, mockName: name, mockPpid: 10},
+				mockProcess{mockPid: 12, mockName: name, mockPpid: 10},
+			},
+			wantPid:   10,
+			wantFound: true,
+		},
+		{
+			name: "master parent absent from list",
+			processes: []processInfo{
+				mockProcess{mockPid: 20, mockName: name, mockPpid: 99},
+				mockProcess{mockPid: 21, mockName: name, mockPpid: 20},
+			},
+			wantPid:   20,
+			wantFound: true,
+		},
+		{
+			name: "worker whose master parent is also present is not chosen",
+			processes: []processInfo{
+				mockProcess{mockPid: 30, mockName: name, mockPpid: 1},
+				mockProcess{mockPid: 31, mockName: name, mockPpid: 30},
+			},
+			wantPid:   30,
+			wantFound: true,
+		},
+		{
+			name: "cmdline containing 'master process' is no longer a false positive",
+			processes: []processInfo{
+				mockProcess{mockPid: 40, mockName: name, mockPpid: 1},
+				mockProcess{mockPid: 41, mockName: name, mockPpid: 40},
+			},
+			wantPid:   40,
+			wantFound: true,
+		},
+		{
+			name: "no php-fpm process",
+			processes: []processInfo{
+				mockProcess{mockPid: 50, mockName: "nginx", mockPpid: 1},
+				mockProcess{mockPid: 51, mockName: "node", mockPpid: 1},
+			},
+			wantFound: false,
+		},
+		{
+			name: "process whose ppid errors is skipped, master still found",
+			processes: []processInfo{
+				mockProcess{mockPid: 60, mockName: name, mockPpidErr: errors.New("permission denied")},
+				mockProcess{mockPid: 61, mockName: name, mockPpid: 1},
+				mockProcess{mockPid: 62, mockName: name, mockPpid: 61},
+			},
+			wantPid:   61,
+			wantFound: true,
+		},
+		{
+			name: "process whose name errors is ignored",
+			processes: []processInfo{
+				mockProcess{mockPid: 70, mockNameErr: errors.New("gone")},
+				mockProcess{mockPid: 71, mockName: name, mockPpid: 1},
+			},
+			wantPid:   71,
+			wantFound: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			pid, found, err := findMasterProcess(name, tt.processes)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.wantFound, found)
+			if tt.wantFound {
+				assert.Equal(t, tt.wantPid, pid)
+			}
+		})
+	}
+}