Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
146 changes: 146 additions & 0 deletions interpreter/multi.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// 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)
}
}

if len(instances) == 0 {
// If all interpreters failed with errors, return the combined error
if len(errs) > 0 {
return nil, errors.Join(errs...)
}
// All interpreters returned nil instances without error (e.g., not ready yet)
// This is a valid state - return nil, nil
return nil, nil
}
Comment thread
gnurizen marked this conversation as resolved.

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

// We got at least one valid instance, log any errors that occurred
if len(errs) > 0 {
log.Errorf("Errors occurred while attaching interpreters: %v", errs)
}
Comment thread
gnurizen marked this conversation as resolved.
Outdated

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...)

@christos68k christos68k Aug 19, 2025

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.

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 its possible ii is a multi-instance
Comment thread
gnurizen marked this conversation as resolved.
Outdated
// 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