Skip to content

Commit

Permalink
Fixed DEBUG-3205, DEBUG-3230, DEBUG-3454, DEBUG-3211 + Improved Explo…
Browse files Browse the repository at this point in the history
…ration Testing
  • Loading branch information
GreenMatan committed Feb 23, 2025
1 parent e4b9544 commit 82a2c11
Show file tree
Hide file tree
Showing 7 changed files with 365 additions and 102 deletions.
12 changes: 7 additions & 5 deletions pkg/dynamicinstrumentation/diconfig/binary_inspection.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ import (
// inspectGoBinaries goes through each service and populates information about the binary
// and the relevant parameters, and their types
// configEvent maps service names to info about the service and their configurations
func inspectGoBinaries(configEvent ditypes.DIProcs) error {
var err error
func inspectGoBinaries(configEvent ditypes.DIProcs) bool {
var inspectedAtLeastOneBinary bool
for i := range configEvent {
err = AnalyzeBinary(configEvent[i])
err := AnalyzeBinary(configEvent[i])
if err != nil {
return fmt.Errorf("inspection of PID %d (path=%s) failed: %w", configEvent[i].PID, configEvent[i].BinaryPath, err)
log.Info("inspection of PID %d (path=%s) failed: %w", configEvent[i].PID, configEvent[i].BinaryPath, err)
} else {
inspectedAtLeastOneBinary = true
}
}
return nil
return inspectedAtLeastOneBinary
}

// AnalyzeBinary reads the binary associated with the specified process and parses
Expand Down
13 changes: 8 additions & 5 deletions pkg/dynamicinstrumentation/diconfig/config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,17 @@ func (cm *RCConfigManager) readConfigs(r *ringbuf.Reader, procInfo *ditypes.Proc

func applyConfigUpdate(procInfo *ditypes.ProcessInfo, probe *ditypes.Probe) {
log.Tracef("Applying config update: %v\n", probe)
err := AnalyzeBinary(procInfo)
if err != nil {
log.Errorf("couldn't inspect binary: %v\n", err)
return

if procInfo.TypeMap == nil {
err := AnalyzeBinary(procInfo)
if err != nil {
log.Errorf("couldn't inspect binary: %v\n", err)
return
}
}

generateCompileAttach:
err = codegen.GenerateBPFParamsCode(procInfo, probe)
err := codegen.GenerateBPFParamsCode(procInfo, probe)
if err != nil {
log.Info("Couldn't generate BPF programs", err)
if !probe.InstrumentationInfo.AttemptedRebuild {
Expand Down
10 changes: 10 additions & 0 deletions pkg/dynamicinstrumentation/diconfig/location_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ func GenerateLocationExpression(limitsInfo *ditypes.InstrumentationInfo, param *
}
slicePointer := elementParam.ParameterPieces[0]
sliceLength := elementParam.ParameterPieces[1]

if slicePointer == nil || sliceLength == nil {
continue
}

sliceLength.LocationExpressions = append(sliceLength.LocationExpressions,
ditypes.PrintStatement("%s", "Reading the length of slice"),
)
Expand All @@ -188,6 +193,11 @@ func GenerateLocationExpression(limitsInfo *ditypes.InstrumentationInfo, param *
// Generate and collect the location expressions for collecting an individual
// element of this slice
sliceElementType := slicePointer.ParameterPieces[0]

if sliceElementType == nil {
continue
}

sliceIdentifier := randomLabel()
labelName := randomLabel()

Expand Down
37 changes: 23 additions & 14 deletions pkg/dynamicinstrumentation/diconfig/mem_config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
type ReaderConfigManager struct {
sync.Mutex
ConfigWriter *ConfigWriter
procTracker *proctracker.ProcessTracker
ProcTracker *proctracker.ProcessTracker

callback configUpdateCallback
configs configsByService
Expand All @@ -40,8 +40,8 @@ func NewReaderConfigManager() (*ReaderConfigManager, error) {
state: ditypes.NewDIProcs(),
}

cm.procTracker = proctracker.NewProcessTracker(cm.updateProcessInfo)
err := cm.procTracker.Start()
cm.ProcTracker = proctracker.NewProcessTracker(cm.updateProcessInfo)
err := cm.ProcTracker.Start()
if err != nil {
return nil, err
}
Expand All @@ -63,7 +63,7 @@ func (cm *ReaderConfigManager) GetProcInfos() ditypes.DIProcs {
// Stop causes the ReaderConfigManager to stop processing data
func (cm *ReaderConfigManager) Stop() {
cm.ConfigWriter.Stop()
cm.procTracker.Stop()
cm.ProcTracker.Stop()
}

func (cm *ReaderConfigManager) update() error {
Expand All @@ -80,9 +80,9 @@ func (cm *ReaderConfigManager) update() error {
}

if !reflect.DeepEqual(cm.state, updatedState) {
err := inspectGoBinaries(updatedState)
if err != nil {
return err
atLeastOneBinaryAnalyzed := inspectGoBinaries(updatedState)
if !atLeastOneBinaryAnalyzed {
return fmt.Errorf("failed to inspect all tracked go binaries.")
}

for pid, procInfo := range cm.state {
Expand Down Expand Up @@ -159,20 +159,18 @@ func (r *ConfigWriter) Write(p []byte) (n int, e error) {
return 0, nil
}

func (r *ConfigWriter) WriteSync(p []byte) error {
return r.parseRawConfigBytesAndTriggerCallback(p)
}

// Start initiates the ConfigWriter to start processing data
func (r *ConfigWriter) Start() error {
go func() {
configUpdateLoop:
for {
select {
case rawConfigBytes := <-r.updateChannel:
conf := map[string]map[string]rcConfig{}
err := json.Unmarshal(rawConfigBytes, &conf)
if err != nil {
log.Errorf("invalid config read from reader: %v", err)
continue
}
r.configCallback(conf)
r.parseRawConfigBytesAndTriggerCallback(rawConfigBytes)
case <-r.stopChannel:
break configUpdateLoop
}
Expand All @@ -181,6 +179,17 @@ func (r *ConfigWriter) Start() error {
return nil
}

func (r *ConfigWriter) parseRawConfigBytesAndTriggerCallback(rawConfigBytes []byte) error {
conf := map[string]map[string]rcConfig{}
err := json.Unmarshal(rawConfigBytes, &conf)
if err != nil {
log.Errorf("invalid config read from reader: %v", err)
return fmt.Errorf("invalid config read from reader: %v", err)
}
r.configCallback(conf)
return nil
}

// Stop causes the ConfigWriter to stop processing data
func (r *ConfigWriter) Stop() {
r.stopChannel <- true
Expand Down
6 changes: 6 additions & 0 deletions pkg/dynamicinstrumentation/ditypes/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"io"
"strconv"
"strings"
"sync"

"github.com/DataDog/datadog-agent/pkg/util/log"

Expand Down Expand Up @@ -138,6 +139,7 @@ type ProcessInfo struct {
ProbesByID ProbesByID
InstrumentationUprobes map[ProbeID]*link.Link
InstrumentationObjects map[ProbeID]*ebpf.Collection
mu sync.RWMutex
}

// SetupConfigUprobe sets the configuration probe for the process
Expand Down Expand Up @@ -172,12 +174,16 @@ func (pi *ProcessInfo) CloseConfigUprobe() error {
// SetUprobeLink associates the uprobe link with the specified probe
// in the tracked process
func (pi *ProcessInfo) SetUprobeLink(probeID ProbeID, l *link.Link) {
pi.mu.Lock()
defer pi.mu.Unlock()
pi.InstrumentationUprobes[probeID] = l
}

// CloseUprobeLink closes the probe and deletes the link for the probe
// in the tracked process
func (pi *ProcessInfo) CloseUprobeLink(probeID ProbeID) error {
pi.mu.Lock()
defer pi.mu.Unlock()
if l, ok := pi.InstrumentationUprobes[probeID]; ok {
err := (*l).Close()
delete(pi.InstrumentationUprobes, probeID)
Expand Down
6 changes: 6 additions & 0 deletions pkg/dynamicinstrumentation/proctracker/proctracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ func (pt *ProcessTracker) Stop() {
}
}

func (pt *ProcessTracker) Test_HandleProcessStart(pid uint32) {
exePath := filepath.Join(pt.procRoot, strconv.FormatUint(uint64(pid), 10), "exe")

pt.inspectBinary(exePath, pid)
}

func (pt *ProcessTracker) handleProcessStart(pid uint32) {
exePath := filepath.Join(pt.procRoot, strconv.FormatUint(uint64(pid), 10), "exe")

Expand Down
Loading

0 comments on commit 82a2c11

Please sign in to comment.