fixup: apply feedback

Signed-off-by: Florian Lehner <[email protected]>

cli_flags.go

@@ -12,6 +12,7 @@ import (
 	"github.com/peterbourgon/ff/v3"
 
 	"go.opentelemetry.io/ebpf-profiler/internal/controller"
+	"go.opentelemetry.io/ebpf-profiler/support"
 	"go.opentelemetry.io/ebpf-profiler/tracer"
 )
 
@@ -24,7 +25,7 @@ const (
 	defaultProbabilisticThreshold = tracer.ProbabilisticThresholdMax
 	defaultProbabilisticInterval  = 1 * time.Minute
 	defaultArgSendErrorFrames     = false
-	defaultOffCPUThreshold        = tracer.OffCPUThresholdMax
+	defaultOffCPUThreshold        = support.OffCPUThresholdMax
 
 	// This is the X in 2^(n + x) where n is the default hardcoded map size value
 	defaultArgMapScaleFactor = 0
@@ -66,7 +67,7 @@ var (
 		"off-cpu profiling: Every time an off-cpu entry point is hit, a random number between "+
 		"0 and %d is chosen. If the given threshold is greater than this random number, the "+
 		"off-cpu trace is collected and reported.",
-		tracer.OffCPUThresholdMax-1, tracer.OffCPUThresholdMax-1)
+		support.OffCPUThresholdMax-1, support.OffCPUThresholdMax-1)
 )
 
 // Package-scope variable, so that conditionally compiled other components can refer

host/host.go

@@ -54,8 +54,8 @@ type Trace struct {
 	KTime            times.KTime
 	PID              libpf.PID
 	TID              libpf.PID
-	Origin           int
-	OffTime          uint64 // Time a task was off-cpu.
+	Origin           libpf.Origin
+	OffTime          uint64 // Time a task was off-cpu in nanoseconds.
 	APMTraceID       libpf.APMTraceID
 	APMTransactionID libpf.APMTransactionID
 	CPU              int

internal/controller/controller.go

@@ -13,6 +13,7 @@ import (
 	"go.opentelemetry.io/ebpf-profiler/internal/helpers"
 	"go.opentelemetry.io/ebpf-profiler/metrics"
 	"go.opentelemetry.io/ebpf-profiler/reporter"
+	"go.opentelemetry.io/ebpf-profiler/support"
 	"go.opentelemetry.io/ebpf-profiler/times"
 	"go.opentelemetry.io/ebpf-profiler/tracehandler"
 	"go.opentelemetry.io/ebpf-profiler/tracer"
@@ -129,7 +130,7 @@ func (c *Controller) Start(ctx context.Context) error {
 	}
 	log.Info("Attached tracer program")
 
-	if c.config.OffCPUThreshold < tracer.OffCPUThresholdMax {
+	if c.config.OffCPUThreshold < support.OffCPUThresholdMax {
 		if err := trc.StartOffCPUProfiling(); err != nil {
 			return fmt.Errorf("failed to start off-cpu profiling: %v", err)
 		}

libpf/libpf.go

@@ -70,3 +70,6 @@ type Void struct{}
 // source line numbers associated with offsets in native code, or for source line numbers in
 // interpreted code.
 type SourceLineno uint64
+
+// Origin determines the source of a trace.
+type Origin int

reporter/base_reporter.go

@@ -36,7 +36,7 @@ type baseReporter struct {
 	cgroupv2ID *lru.SyncedLRU[libpf.PID, string]
 
 	// traceEvents stores reported trace events (trace metadata with frames and counts)
-	traceEvents xsync.RWMutex[map[int]samples.KeyToEventMapping]
+	traceEvents xsync.RWMutex[map[libpf.Origin]samples.KeyToEventMapping]
 
 	// hostmetadata stores metadata that is sent out with every request.
 	hostmetadata *lru.SyncedLRU[string, string]

reporter/collector_reporter.go

@@ -60,8 +60,8 @@ func NewCollector(cfg *Config, nextConsumer consumerprofiles.Profiles) (*Collect
 		return nil, err
 	}
 
-	originsMap := make(map[int]samples.KeyToEventMapping, 2)
-	for _, origin := range []int{support.TraceOriginSampling,
+	originsMap := make(map[libpf.Origin]samples.KeyToEventMapping, 2)
+	for _, origin := range []libpf.Origin{support.TraceOriginSampling,
 		support.TraceOriginOffCPU} {
 		originsMap[origin] = make(samples.KeyToEventMapping)
 	}

reporter/internal/pdata/generate.go

@@ -25,11 +25,12 @@ const (
 
 // Generate generates a pdata request out of internal profiles data, to be
 // exported.
-func (p Pdata) Generate(events map[int]samples.KeyToEventMapping) pprofile.Profiles {
+func (p Pdata) Generate(events map[libpf.Origin]samples.KeyToEventMapping) pprofile.Profiles {
 	profiles := pprofile.NewProfiles()
 	rp := profiles.ResourceProfiles().AppendEmpty()
 	sp := rp.ScopeProfiles().AppendEmpty()
-	for _, origin := range []int{support.TraceOriginSampling, support.TraceOriginOffCPU} {
+	for _, origin := range []libpf.Origin{support.TraceOriginSampling,
+		support.TraceOriginOffCPU} {
 		prof := sp.Profiles().AppendEmpty()
 		prof.SetProfileID(pprofile.ProfileID(mkProfileID()))
 		p.setProfile(origin, events[origin], prof)
@@ -50,7 +51,7 @@ func mkProfileID() []byte {
 // setProfile sets the data an OTLP profile with all collected samples up to
 // this moment.
 func (p *Pdata) setProfile(
-	origin int,
+	origin libpf.Origin,
 	events map[samples.TraceAndMetaKey]*samples.TraceEvents,
 	profile pprofile.Profile,
 ) {

reporter/internal/samples/samples.go

@@ -12,7 +12,7 @@ type TraceEventMeta struct {
 	APMServiceName string
 	PID, TID       libpf.PID
 	CPU            int
-	Origin         int
+	Origin         libpf.Origin
 	OffTime        uint64
 }
 

reporter/otlp_reporter.go

@@ -87,8 +87,8 @@ func NewOTLP(cfg *Config) (*OTLPReporter, error) {
 		return nil, err
 	}
 
-	originsMap := make(map[int]samples.KeyToEventMapping, 2)
-	for _, origin := range []int{support.TraceOriginSampling,
+	originsMap := make(map[libpf.Origin]samples.KeyToEventMapping, 2)
+	for _, origin := range []libpf.Origin{support.TraceOriginSampling,
 		support.TraceOriginOffCPU} {
 		originsMap[origin] = make(samples.KeyToEventMapping)
 	}
@@ -170,9 +170,9 @@ func (r *OTLPReporter) Start(ctx context.Context) error {
 func (r *OTLPReporter) reportOTLPProfile(ctx context.Context) error {
 	traceEvents := r.traceEvents.WLock()
 	events := maps.Clone(*traceEvents)
-	originsMap := make(map[int]samples.KeyToEventMapping, 2)
+	originsMap := make(map[libpf.Origin]samples.KeyToEventMapping, 2)
 	clear(*traceEvents)
-	for _, origin := range []int{support.TraceOriginSampling,
+	for _, origin := range []libpf.Origin{support.TraceOriginSampling,
 		support.TraceOriginOffCPU} {
 		originsMap[origin] = make(samples.KeyToEventMapping)
 	}

support/ebpf/native_stack_trace.ebpf.c

@@ -634,6 +634,11 @@ int native_tracer_entry(struct bpf_perf_event_data *ctx) {
   u64 id = bpf_get_current_pid_tgid();
   u32 pid = id >> 32;
   u32 tid = id & 0xFFFFFFFF;
+
+  if (pid == 0) {
+    return 0;
+  }
+
   u64 ts = bpf_ktime_get_ns();
   return collect_trace((struct pt_regs*) &ctx->regs, TRACE_SAMPLING, pid, tid, ts, 0);
 }

support/ebpf/off_cpu.ebpf.c

@@ -10,8 +10,8 @@ bpf_map_def SEC("maps") kprobe_progs = {
   .max_entries = NUM_TRACER_PROGS,
 };
 
-// profile_off_cpu communicates scheduler tasks.
-bpf_map_def SEC("maps") profile_off_cpu = {
+// sched_times keeps track of sched_switch call times.
+	bpf_map_def SEC("maps") sched_times = {
   .type = BPF_MAP_TYPE_LRU_PERCPU_HASH,
   .key_size = sizeof(u64),   // pid_tgid
   .value_size = sizeof(u64), // time in ns
@@ -42,7 +42,7 @@ int tracepoint__sched_switch(void *ctx) {
 
   u64 ts = bpf_ktime_get_ns();
 
-  if (bpf_map_update_elem(&profile_off_cpu, &pid_tgid, &ts, BPF_ANY)<0){
+  if (bpf_map_update_elem(&sched_times, &pid_tgid, &ts, BPF_ANY)<0){
     DEBUG_PRINT("Failed to record sched_switch event entry");
 	  return 0;
   }
@@ -73,7 +73,7 @@ int finish_task_switch(struct pt_regs *ctx) {
 
   u64 ts = bpf_ktime_get_ns();
 
-  u64 *start_ts = bpf_map_lookup_elem(&profile_off_cpu, &pid_tgid);
+  u64 *start_ts = bpf_map_lookup_elem(&sched_times, &pid_tgid);
   if (!start_ts){
     // There is no information from the sched/sched_switch entry hook.
     return 0;

support/ebpf/tracemgmt.h

@@ -23,7 +23,6 @@
         return func_name(ctx); \
     }
 
-
 // increment_metric increments the value of the given metricID by 1
 static inline __attribute__((__always_inline__))
 void increment_metric(u32 metricID) {
@@ -666,10 +665,6 @@ static inline ErrorCode get_usermode_regs(struct pt_regs *ctx,
 static inline
 int collect_trace(struct pt_regs *ctx, TraceOrigin origin, u32 pid, u32 tid,
   u64 trace_timestamp, u64 off_cpu_time) {
-  if (pid == 0) {
-    return 0;
-  }
-
   // The trace is reused on each call to this function so we have to reset the
   // variables used to maintain state.
   DEBUG_PRINT("Resetting CPU record");

tracer/tracer.go

@@ -65,12 +65,6 @@ const (
 	probProfilingDisable = -1
 )
 
-const (
-	// OffCpuThresholdMax defines the upper bound for the off-cpu profiling
-	// threshold.
-	OffCPUThresholdMax = support.OffCPUThresholdMax
-)
-
 // Intervals is a subset of config.IntervalsAndTimers.
 type Intervals interface {
 	MonitorInterval() time.Duration
@@ -496,7 +490,7 @@ func initializeMapsAndPrograms(kernelSymbols *libpf.SymbolMap, cfg *Config) (
 		return nil, nil, fmt.Errorf("failed to load perf eBPF programs: %v", err)
 	}
 
-	if cfg.OffCPUThreshold < OffCPUThresholdMax {
+	if cfg.OffCPUThreshold < support.OffCPUThresholdMax {
 		if err = loadKProbeUnwinders(coll, ebpfProgs, ebpfMaps["kprobe_progs"], tailCallProgs,
 			cfg.BPFVerifierLogLevel, ebpfMaps["perf_progs"].FD()); err != nil {
 			return nil, nil, fmt.Errorf("failed to load kprobe eBPF programs: %v", err)
@@ -575,7 +569,7 @@ func loadAllMaps(coll *cebpf.CollectionSpec, ebpfMaps map[string]*cebpf.Map,
 	return nil
 }
 
-// loadPerfUnwinders just satisfies the proof of concept and loads all eBPF programs
+// loadPerfUnwinders loads all perf eBPF Programs and their tail call targets.
 func loadPerfUnwinders(coll *cebpf.CollectionSpec, ebpfProgs map[string]*cebpf.Program,
 	tailcallMap *cebpf.Map, tailCallProgs []progLoaderHelper,
 	bpfVerifierLogLevel uint32) error {
@@ -643,7 +637,7 @@ func progArrayReferences(perfTailCallMapFD int, insns asm.Instructions) []int {
 
 // loadKProbeUnwinders reuses large parts of loadPerfUnwinders. By default all eBPF programs
 // are written as perf event eBPF programs. loadKProbeUnwinders dynamically rewrites the
-// specification of these programs to krpobe eBPF programs and adjusts tail call maps.
+// specification of these programs to kprobe eBPF programs and adjusts tail call maps.
 func loadKProbeUnwinders(coll *cebpf.CollectionSpec, ebpfProgs map[string]*cebpf.Program,
 	tailcallMap *cebpf.Map, tailCallProgs []progLoaderHelper,
 	bpfVerifierLogLevel uint32, perfTailCallMapFD int) error {
@@ -980,7 +974,7 @@ func (t *Tracer) loadBpfTrace(raw []byte, cpu int) *host.Trace {
 		APMTransactionID: *(*libpf.APMTransactionID)(unsafe.Pointer(&ptr.apm_transaction_id)),
 		PID:              pid,
 		TID:              libpf.PID(ptr.tid),
-		Origin:           int(ptr.origin),
+		Origin:           libpf.Origin(ptr.origin),
 		OffTime:          uint64(ptr.offtime),
 		KTime:            times.KTime(ptr.ktime),
 		CPU:              cpu,