Move process Info creation into NewManager (#1943)

The Manager manages probes for a process. Have the manager handle the
process info creation and coordination instead of extraneously passing
it from the Instrumentation.

Author: MrAlias

instrumentation.go

@@ -53,9 +53,7 @@ const (
 // Instrumentation manages and controls all OpenTelemetry Go
 // auto-instrumentation.
 type Instrumentation struct {
-	target   *process.Info
-	analyzer *process.Analyzer
-	manager  *instrumentation.Manager
+	manager *instrumentation.Manager
 
 	stopMu  sync.Mutex
 	stop    context.CancelFunc
@@ -97,42 +95,17 @@ func NewInstrumentation(ctx context.Context, opts ...InstrumentationOption) (*In
 	}
 
 	cp := convertConfigProvider(c.cp)
-	mngr, err := instrumentation.NewManager(c.logger, ctrl, cp, p...)
+	mngr, err := instrumentation.NewManager(c.logger, ctrl, c.pid, cp, p...)
 	if err != nil {
 		return nil, err
 	}
 
-	pa := process.NewAnalyzer(c.logger, c.pid)
-	pi, err := pa.Analyze(mngr.GetRelevantFuncs())
-	if err != nil {
-		return nil, err
-	}
-
-	alloc, err := process.Allocate(c.logger, c.pid)
-	if err != nil {
-		return nil, err
-	}
-	pi.Allocation = alloc
-
-	c.logger.Info(
-		"target process analysis completed",
-		"pid", pi.ID,
-		"go_version", pi.GoVersion,
-		"dependencies", pi.Modules,
-		"total_functions_found", len(pi.Functions),
-	)
-	mngr.FilterUnusedProbes(pi)
-
-	return &Instrumentation{
-		target:   pi,
-		analyzer: pa,
-		manager:  mngr,
-	}, nil
+	return &Instrumentation{manager: mngr}, nil
 }
 
 // Load loads and attaches the relevant probes to the target process.
 func (i *Instrumentation) Load(ctx context.Context) error {
-	return i.manager.Load(ctx, i.target)
+	return i.manager.Load(ctx)
 }
 
 // Run starts the instrumentation. It must be called after [Instrumentation.Load].

internal/pkg/instrumentation/manager.go

@@ -55,19 +55,43 @@ type Manager struct {
 }
 
 // NewManager returns a new [Manager].
-func NewManager(logger *slog.Logger, otelController *opentelemetry.Controller, cp ConfigProvider, probes ...probe.Probe) (*Manager, error) {
+func NewManager(logger *slog.Logger, otelController *opentelemetry.Controller, pid process.ID, cp ConfigProvider, probes ...probe.Probe) (*Manager, error) {
 	m := &Manager{
 		logger:         logger,
 		probes:         make(map[probe.ID]probe.Probe),
 		otelController: otelController,
 		cp:             cp,
 	}
 
-	err := m.registerProbes(probes)
+	funcs := make(map[string]any)
+	for _, p := range probes {
+		if err := m.registerProbe(p); err != nil {
+			return nil, err
+		}
+
+		for _, s := range p.Manifest().Symbols {
+			funcs[s.Symbol] = nil
+		}
+	}
+
+	pa := process.NewAnalyzer(logger, pid)
+
+	var err error
+	m.proc, err = pa.Analyze(funcs)
 	if err != nil {
 		return nil, err
 	}
 
+	alloc, err := process.Allocate(logger, pid)
+	if err != nil {
+		return nil, err
+	}
+	m.proc.Allocation = alloc
+
+	m.logger.Info("loaded process info", "process", m.proc)
+
+	m.filterUnusedProbes()
+
 	return m, nil
 }
 
@@ -103,23 +127,11 @@ func (m *Manager) registerProbe(p probe.Probe) error {
 	return nil
 }
 
-// GetRelevantFuncs returns the instrumented functions for all managed probes.
-func (m *Manager) GetRelevantFuncs() map[string]interface{} {
-	funcsMap := make(map[string]interface{})
-	for _, i := range m.probes {
-		for _, s := range i.Manifest().Symbols {
-			funcsMap[s.Symbol] = nil
-		}
-	}
-
-	return funcsMap
-}
-
-// FilterUnusedProbes filterers probes whose functions are already instrumented
+// filterUnusedProbes filterers probes whose functions are already instrumented
 // out of the Manager.
-func (m *Manager) FilterUnusedProbes(target *process.Info) {
+func (m *Manager) filterUnusedProbes() {
 	existingFuncMap := make(map[string]interface{})
-	for _, f := range target.Functions {
+	for _, f := range m.proc.Functions {
 		existingFuncMap[f.Name] = nil
 	}
 
@@ -237,14 +249,14 @@ func (m *Manager) ConfigLoop(ctx context.Context) {
 	}
 }
 
-func (m *Manager) Load(ctx context.Context, target *process.Info) error {
+func (m *Manager) Load(ctx context.Context) error {
 	if len(m.probes) == 0 {
 		return errors.New("no instrumentation for target process")
 	}
 	if m.cp == nil {
 		return errors.New("no config provider set")
 	}
-	if target == nil {
+	if m.proc == nil {
 		return errors.New("target details not set - load is called on non-initialized instrumentation")
 	}
 	m.stateMu.Lock()
@@ -255,12 +267,11 @@ func (m *Manager) Load(ctx context.Context, target *process.Info) error {
 	}
 
 	m.currentConfig = m.cp.InitialConfig(ctx)
-	err := m.loadProbes(target)
+	err := m.loadProbes()
 	if err != nil {
 		return err
 	}
 
-	m.proc = target
 	m.state = managerStateLoaded
 
 	return nil
@@ -330,7 +341,7 @@ func (m *Manager) Stop() error {
 	defer m.probeMu.Unlock()
 
 	m.logger.Debug("Shutting down all probes")
-	err := m.cleanup(m.proc)
+	err := m.cleanup()
 
 	// Wait for all probes to stop.
 	m.runningProbesWG.Wait()
@@ -339,30 +350,30 @@ func (m *Manager) Stop() error {
 	return err
 }
 
-func (m *Manager) loadProbes(target *process.Info) error {
+func (m *Manager) loadProbes() error {
 	// Remove resource limits for kernels <5.11.
 	if err := rlimitRemoveMemlock(); err != nil {
 		return err
 	}
 
-	exe, err := openExecutable(target.ID.ExePath())
+	exe, err := openExecutable(m.proc.ID.ExePath())
 	if err != nil {
 		return err
 	}
 	m.exe = exe
 
-	if err := m.mount(target); err != nil {
+	if err := m.mount(); err != nil {
 		return err
 	}
 
 	// Load probes
 	for name, i := range m.probes {
 		if isProbeEnabled(name, m.currentConfig) {
 			m.logger.Info("loading probe", "name", name)
-			err := i.Load(exe, target, m.currentConfig.SamplingConfig)
+			err := i.Load(exe, m.proc, m.currentConfig.SamplingConfig)
 			if err != nil {
 				m.logger.Error("error while loading probes, cleaning up", "error", err, "name", name)
-				return errors.Join(err, m.cleanup(target))
+				return errors.Join(err, m.cleanup())
 			}
 		}
 	}
@@ -371,16 +382,16 @@ func (m *Manager) loadProbes(target *process.Info) error {
 	return nil
 }
 
-func (m *Manager) mount(target *process.Info) error {
-	if target.Allocation != nil {
-		m.logger.Debug("Mounting bpffs", "allocation", target.Allocation)
+func (m *Manager) mount() error {
+	if m.proc.Allocation != nil {
+		m.logger.Debug("Mounting bpffs", "allocation", m.proc.Allocation)
 	} else {
 		m.logger.Debug("Mounting bpffs")
 	}
-	return bpffsMount(target)
+	return bpffsMount(m.proc)
 }
 
-func (m *Manager) cleanup(target *process.Info) error {
+func (m *Manager) cleanup() error {
 	ctx := context.Background()
 	err := m.cp.Shutdown(context.Background())
 	for _, i := range m.probes {
@@ -394,14 +405,5 @@ func (m *Manager) cleanup(target *process.Info) error {
 	}
 
 	m.logger.Debug("Cleaning bpffs")
-	return errors.Join(err, bpffsCleanup(target))
-}
-
-func (m *Manager) registerProbes(probes []probe.Probe) error {
-	for _, p := range probes {
-		if err := m.registerProbe(p); err != nil {
-			return err
-		}
-	}
-	return nil
+	return errors.Join(err, bpffsCleanup(m.proc))
 }

internal/pkg/instrumentation/manager_load_test.go

@@ -22,8 +22,11 @@ import (
 	grpcServer "go.opentelemetry.io/auto/internal/pkg/instrumentation/bpf/google.golang.org/grpc/server"
 	httpClient "go.opentelemetry.io/auto/internal/pkg/instrumentation/bpf/net/http/client"
 	httpServer "go.opentelemetry.io/auto/internal/pkg/instrumentation/bpf/net/http/server"
+	"go.opentelemetry.io/auto/internal/pkg/instrumentation/probe"
 	"go.opentelemetry.io/auto/internal/pkg/instrumentation/testutils"
 	"go.opentelemetry.io/auto/internal/pkg/instrumentation/utils"
+	"go.opentelemetry.io/auto/internal/pkg/process"
+	"go.opentelemetry.io/auto/internal/pkg/process/binary"
 )
 
 func TestLoadProbes(t *testing.T) {
@@ -51,10 +54,9 @@ func TestLoadProbes(t *testing.T) {
 	}
 }
 
-func fakeManager(t *testing.T) *Manager {
+func fakeManager(t *testing.T, fnNames ...string) *Manager {
 	logger := slog.Default()
-	m, err := NewManager(
-		logger, nil, NewNoopConfigProvider(nil),
+	probes := []probe.Probe{
 		grpcClient.New(logger, ""),
 		grpcServer.New(logger, ""),
 		httpServer.New(logger, ""),
@@ -64,9 +66,27 @@ func fakeManager(t *testing.T) *Manager {
 		kafkaConsumer.New(logger, ""),
 		autosdk.New(logger),
 		otelTraceGlobal.New(logger),
-	)
-	assert.NoError(t, err)
-	assert.NotNil(t, m)
+	}
+	ver := semver.New(1, 20, 0, "", "")
+	var fn []*binary.Func
+	for _, name := range fnNames {
+		fn = append(fn, &binary.Func{Name: name})
+	}
+	m := &Manager{
+		logger: slog.Default(),
+		cp:     NewNoopConfigProvider(nil),
+		probes: make(map[probe.ID]probe.Probe),
+		proc: &process.Info{
+			ID:        1,
+			Functions: fn,
+			GoVersion: ver,
+			Modules:   map[string]*semver.Version{},
+		},
+	}
+	for _, p := range probes {
+		m.probes[p.Manifest().ID] = p
+	}
+	m.filterUnusedProbes()
 
 	return m
 }