Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions interpreter/golabels/integrationtests/golabels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func Test_Golabels(t *testing.T) {

enabledTracers, _ := tracertypes.Parse("")
enabledTracers.Enable(tracertypes.Labels)
enabledTracers.Enable(tracertypes.GoTracer)

trc, err := tracer.NewTracer(ctx, &tracer.Config{
Reporter: &mockReporter{},
Expand Down
143 changes: 143 additions & 0 deletions interpreter/multi.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package interpreter // import "go.opentelemetry.io/ebpf-profiler/interpreter"

import (
"errors"

log "github.com/sirupsen/logrus"
"go.opentelemetry.io/ebpf-profiler/host"
"go.opentelemetry.io/ebpf-profiler/libpf"
"go.opentelemetry.io/ebpf-profiler/metrics"
"go.opentelemetry.io/ebpf-profiler/process"
"go.opentelemetry.io/ebpf-profiler/remotememory"
"go.opentelemetry.io/ebpf-profiler/reporter"
"go.opentelemetry.io/ebpf-profiler/tpbase"
)

// MultiData implements the Data interface for multiple interpreters.
type MultiData struct {
interpreters []Data
}

// NewMultiData creates a new MultiData instance from multiple Data instances.
func NewMultiData(interpreters []Data) *MultiData {
return &MultiData{
interpreters: interpreters,
}
}

// Attach attaches all interpreters and returns a MultiInstance.
func (m *MultiData) Attach(ebpf EbpfHandler, pid libpf.PID, bias libpf.Address,
rm remotememory.RemoteMemory) (Instance, error) {
var instances []Instance
var errs []error

for _, data := range m.interpreters {
instance, err := data.Attach(ebpf, pid, bias, rm)
if err != nil {
errs = append(errs, err)
continue
}
if instance != nil {
instances = append(instances, instance)
}
}

err := errors.Join(errs...)
if len(instances) == 0 {
// Either all interpreters returned nil instances without error (e.g., not ready yet)
// in which case return nil, nil (valid state) otherwise return combined error.
return nil, err
}

// We got at least one valid instance, log any errors that occurred
if err != nil {
log.Errorf("Errors occurred while attaching interpreters: %v", err)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're potentially swallowing/hiding an error here (if len(instances) > 0 && len(errs) > 0)), maybe we should log the latter case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch

return NewMultiInstance(instances), nil
}

// Unload unloads all interpreters.
func (m *MultiData) Unload(ebpf EbpfHandler) {
for _, data := range m.interpreters {
data.Unload(ebpf)
}
}

// MultiInstance implements the Instance interface for multiple interpreters.
type MultiInstance struct {
instances []Instance
}

// NewMultiInstance creates a new MultiInstance from multiple Instance instances.
func NewMultiInstance(instances []Instance) *MultiInstance {
return &MultiInstance{
instances: instances,
}
}

// Detach detaches all interpreter instances.
func (m *MultiInstance) Detach(ebpf EbpfHandler, pid libpf.PID) error {
var errs []error
for _, instance := range m.instances {
if err := instance.Detach(ebpf, pid); err != nil {
errs = append(errs, err)
}
}
return errors.Join(errs...)
}

// SynchronizeMappings synchronizes mappings for all interpreter instances.
func (m *MultiInstance) SynchronizeMappings(ebpf EbpfHandler,
symbolReporter reporter.SymbolReporter, pr process.Process, mappings []process.Mapping) error {
var errs []error
for _, instance := range m.instances {
if err := instance.SynchronizeMappings(ebpf, symbolReporter, pr, mappings); err != nil {
errs = append(errs, err)
}
}
return errors.Join(errs...)
}

// UpdateTSDInfo updates TSD info for all interpreter instances.
func (m *MultiInstance) UpdateTSDInfo(ebpf EbpfHandler, pid libpf.PID, info tpbase.TSDInfo) error {
var errs []error
for _, instance := range m.instances {
if err := instance.UpdateTSDInfo(ebpf, pid, info); err != nil {
errs = append(errs, err)
}
}
return errors.Join(errs...)
}

// Symbolize tries to symbolize the frame with each interpreter instance until one succeeds.
func (m *MultiInstance) Symbolize(ebpfFrame *host.Frame, frames *libpf.Frames) error {
// Try each interpreter in order
for _, instance := range m.instances {
err := instance.Symbolize(ebpfFrame, frames)
if err != ErrMismatchInterpreterType {
return err
}
}
return ErrMismatchInterpreterType
}

// GetAndResetMetrics collects metrics from all interpreter instances.
func (m *MultiInstance) GetAndResetMetrics() ([]metrics.Metric, error) {
var allMetrics []metrics.Metric
var errs []error

for _, instance := range m.instances {
metrics, err := instance.GetAndResetMetrics()
if err != nil {
errs = append(errs, err)
continue
}
allMetrics = append(allMetrics, metrics...)
}

return allMetrics, errors.Join(errs...)
Copy link
Copy Markdown
Member

@christos68k christos68k Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the existing caller assumption that no useful metrics are returned if error != nil, so we should probably also update the caller to process the metrics regardless of error.

}
22 changes: 18 additions & 4 deletions processmanager/execinfomanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,11 @@ type executableInfoManagerState struct {

// detectAndLoadInterpData attempts to detect the given executable as an interpreter. If detection
// succeeds, it then loads additional per-interpreter data into the BPF maps and returns the
// interpreter data.
// interpreter data. If multiple loaders recognize the executable, it returns a MultiData instance.
func (state *executableInfoManagerState) detectAndLoadInterpData(
loaderInfo *interpreter.LoaderInfo) interpreter.Data {
var interpreterDatas []interpreter.Data //nolint:prealloc

// Ask all interpreter loaders whether they want to handle this executable.
for _, loader := range state.interpreterLoaders {
data, err := loader(state.ebpf, loaderInfo)
Expand All @@ -336,18 +338,30 @@ func (state *executableInfoManagerState) detectAndLoadInterpData(
log.Errorf("Failed to load %v (%#016x): %v",
loaderInfo.FileName(), loaderInfo.FileID(), err)
}
return nil
// Continue checking other loaders even if one fails
continue
}
if data == nil {
continue
}

log.Debugf("Interpreter data %v for %v (%#016x)",
data, loaderInfo.FileName(), loaderInfo.FileID())
return data
interpreterDatas = append(interpreterDatas, data)
}

return nil
// Return based on how many interpreters matched
switch len(interpreterDatas) {
case 0:
return nil
case 1:
return interpreterDatas[0]
default:
// Multiple interpreters matched, create a MultiData
log.Debugf("Multiple interpreters (%d) matched for %v (%#016x)",
len(interpreterDatas), loaderInfo.FileName(), loaderInfo.FileID())
return interpreter.NewMultiData(interpreterDatas)
}
}

// loadDeltas converts the sdtypes.StackDelta to StackDeltaEBPF and passes that to
Expand Down
14 changes: 6 additions & 8 deletions processmanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,18 @@ func metricSummaryToSlice(summary metrics.Summary) []metrics.Metric {
return result
}

// updateMetricSummary gets the metrics from the provided interpreter instance and updaates the
// updateMetricSummary gets the metrics from the provided interpreter instance and updates the
// provided summary by aggregating the new metrics into the summary.
// The caller is responsible to hold the lock on the interpreter.Instance to avoid race conditions.
func updateMetricSummary(ii interpreter.Instance, summary metrics.Summary) error {
instanceMetrics, err := ii.GetAndResetMetrics()
if err != nil {
return err
}

// Update metrics even if there was an error, because it's possible ii is a MultiInstance
// and some of the instances may have returned metrics.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// and some of the instances may have returned metrics.
// and some of the underlying instances may have returned metrics.

for _, metric := range instanceMetrics {
summary[metric.ID] += metric.Value
}

return nil
return err
}

// collectInterpreterMetrics starts a goroutine that periodically fetches and reports interpreter
Expand All @@ -145,8 +143,8 @@ func collectInterpreterMetrics(ctx context.Context, pm *ProcessManager,
summary := make(map[metrics.MetricID]metrics.MetricValue)

for pid := range pm.interpreters {
for addr := range pm.interpreters[pid] {
if err := updateMetricSummary(pm.interpreters[pid][addr], summary); err != nil {
for addr, ii := range pm.interpreters[pid] {
if err := updateMetricSummary(ii, summary); err != nil {
log.Errorf("Failed to get/reset metrics for PID %d at 0x%x: %v",
pid, addr, err)
}
Expand Down
Loading