From cab17234306cf4d98b9b8a24b027d2ad4da22b64 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Tue, 4 Feb 2025 11:30:46 +0100 Subject: [PATCH 1/2] host: drop 64 bit TraceHash A 64 bit TraceHash was used to differentiate between hashes on a host vs hashes on a global context. The later one are 128 bit hashes. Signed-off-by: Florian Lehner --- host/host.go | 7 +------ processmanager/manager_test.go | 2 +- tracehandler/tracehandler.go | 6 +++--- tracehandler/tracehandler_test.go | 18 +++++++++--------- tracer/tracer.go | 3 ++- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/host/host.go b/host/host.go index d04c8c381..963b5a542 100644 --- a/host/host.go +++ b/host/host.go @@ -12,11 +12,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/times" ) -// TraceHash is used for unique identifiers for traces, and is required to be 64-bits -// due to the constraints imposed by the eBPF maps, unlike the larger TraceHash used -// outside the host agent. -type TraceHash uint64 - // FileID is used for unique identifiers for files, and is required to be 64-bits // due to the constraints imposed by the eBPF maps, unlike the larger FileID used // outside the host agent. @@ -51,7 +46,7 @@ type Trace struct { ProcessName string ExecutablePath string Frames []Frame - Hash TraceHash + Hash libpf.TraceHash KTime times.KTime PID libpf.PID TID libpf.PID diff --git a/processmanager/manager_test.go b/processmanager/manager_test.go index 76589b35f..4f7c17c58 100644 --- a/processmanager/manager_test.go +++ b/processmanager/manager_test.go @@ -337,7 +337,7 @@ func TestInterpreterConvertTrace(t *testing.T) { // trace for tests below. func getExpectedTrace(origTrace *host.Trace, linenos []libpf.AddressOrLineno) *libpf.Trace { newTrace := &libpf.Trace{ - Hash: libpf.NewTraceHash(uint64(origTrace.Hash), uint64(origTrace.Hash)), + Hash: origTrace.Hash, } for _, frame := range origTrace.Frames { diff --git a/tracehandler/tracehandler.go b/tracehandler/tracehandler.go index 474ae9321..05c4c5493 100644 --- a/tracehandler/tracehandler.go +++ b/tracehandler/tracehandler.go @@ -68,7 +68,7 @@ type traceHandler struct { // bpfTraceCache stores mappings from BPF to user-mode hashes. This allows // avoiding the overhead of re-doing user-mode symbolization of traces that // we have recently seen already. - bpfTraceCache *lru.LRU[host.TraceHash, libpf.TraceHash] + bpfTraceCache *lru.LRU[libpf.TraceHash, libpf.TraceHash] // umTraceCache is a LRU set that suppresses unnecessary resends of traces // that we have recently reported to the collector already. @@ -87,8 +87,8 @@ type traceHandler struct { // newTraceHandler creates a new traceHandler func newTraceHandler(rep reporter.TraceReporter, traceProcessor TraceProcessor, intervals Times, cacheSize uint32) (*traceHandler, error) { - bpfTraceCache, err := lru.New[host.TraceHash, libpf.TraceHash]( - cacheSize, func(k host.TraceHash) uint32 { return uint32(k) }) + bpfTraceCache, err := lru.New[libpf.TraceHash, libpf.TraceHash]( + cacheSize, func(k libpf.TraceHash) uint32 { return k.Hash32() }) if err != nil { return nil, err } diff --git a/tracehandler/tracehandler_test.go b/tracehandler/tracehandler_test.go index 05d618bef..90029ea42 100644 --- a/tracehandler/tracehandler_test.go +++ b/tracehandler/tracehandler_test.go @@ -36,7 +36,7 @@ var _ tracehandler.TraceProcessor = (*fakeTraceProcessor)(nil) func (f *fakeTraceProcessor) ConvertTrace(trace *host.Trace) *libpf.Trace { var newTrace libpf.Trace - newTrace.Hash = libpf.NewTraceHash(uint64(trace.Hash), uint64(trace.Hash)) + newTrace.Hash = trace.Hash return &newTrace } @@ -101,23 +101,23 @@ func TestTraceHandler(t *testing.T) { // simulates a single trace being received. "single trace": {input: []arguments{ - {trace: &host.Trace{Hash: host.TraceHash(0x1234)}}, + {trace: &host.Trace{Hash: libpf.NewTraceHash(0x12, 0x34)}}, }, - expectedTraces: []reportedTrace{{traceHash: libpf.NewTraceHash(0x1234, 0x1234)}}, + expectedTraces: []reportedTrace{{traceHash: libpf.NewTraceHash(0x12, 0x34)}}, expectedCounts: []reportedCount{ - {traceHash: libpf.NewTraceHash(0x1234, 0x1234), count: 1}, + {traceHash: libpf.NewTraceHash(0x12, 0x34), count: 1}, }, }, // double trace simulates a case where the same trace is encountered in quick succession. "double trace": {input: []arguments{ - {trace: &host.Trace{Hash: host.TraceHash(4)}}, - {trace: &host.Trace{Hash: host.TraceHash(4)}}, + {trace: &host.Trace{Hash: libpf.NewTraceHash(0x56, 0x78)}}, + {trace: &host.Trace{Hash: libpf.NewTraceHash(0x56, 0x78)}}, }, - expectedTraces: []reportedTrace{{traceHash: libpf.NewTraceHash(4, 4)}}, + expectedTraces: []reportedTrace{{traceHash: libpf.NewTraceHash(0x56, 0x78)}}, expectedCounts: []reportedCount{ - {traceHash: libpf.NewTraceHash(4, 4), count: 1}, - {traceHash: libpf.NewTraceHash(4, 4), count: 1}, + {traceHash: libpf.NewTraceHash(0x56, 0x78), count: 1}, + {traceHash: libpf.NewTraceHash(0x56, 0x78), count: 1}, }, }, } diff --git a/tracer/tracer.go b/tracer/tracer.go index c9f3ad415..fc1e4bb03 100644 --- a/tracer/tracer.go +++ b/tracer/tracer.go @@ -1007,7 +1007,8 @@ func (t *Tracer) loadBpfTrace(raw []byte, cpu int) *host.Trace { ptr.ktime = 0 ptr.origin = 0 ptr.offtime = 0 - trace.Hash = host.TraceHash(xxh3.Hash128(raw).Lo) + tmpHash := xxh3.Hash128(raw) + trace.Hash = libpf.NewTraceHash(tmpHash.Hi, tmpHash.Lo) userFrameOffs := 0 if ptr.kernel_stack_id >= 0 { From 0e3ee465532386cb592c63a5abc927792ed1b189 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Tue, 4 Feb 2025 11:38:55 +0100 Subject: [PATCH 2/2] tracehandler: remodel traceCache bpfTraceCache used to be essential for the eBPF to user space communication at the time when only hashes and their respective count were reported. As this workflow got refactores, this is no longer required. Signed-off-by: Florian Lehner --- tracehandler/tracehandler.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tracehandler/tracehandler.go b/tracehandler/tracehandler.go index 05c4c5493..85c46d152 100644 --- a/tracehandler/tracehandler.go +++ b/tracehandler/tracehandler.go @@ -65,10 +65,9 @@ type traceHandler struct { traceProcessor TraceProcessor - // bpfTraceCache stores mappings from BPF to user-mode hashes. This allows - // avoiding the overhead of re-doing user-mode symbolization of traces that - // we have recently seen already. - bpfTraceCache *lru.LRU[libpf.TraceHash, libpf.TraceHash] + // traceCache avoids the overhead of re-doing user-mode symbolization of traces + // that we have recently seen already. + traceCache *lru.LRU[libpf.TraceHash, libpf.Void] // umTraceCache is a LRU set that suppresses unnecessary resends of traces // that we have recently reported to the collector already. @@ -87,7 +86,7 @@ type traceHandler struct { // newTraceHandler creates a new traceHandler func newTraceHandler(rep reporter.TraceReporter, traceProcessor TraceProcessor, intervals Times, cacheSize uint32) (*traceHandler, error) { - bpfTraceCache, err := lru.New[libpf.TraceHash, libpf.TraceHash]( + traceCache, err := lru.New[libpf.TraceHash, libpf.Void]( cacheSize, func(k libpf.TraceHash) uint32 { return k.Hash32() }) if err != nil { return nil, err @@ -107,7 +106,7 @@ func newTraceHandler(rep reporter.TraceReporter, traceProcessor TraceProcessor, t := &traceHandler{ traceProcessor: traceProcessor, - bpfTraceCache: bpfTraceCache, + traceCache: traceCache, umTraceCache: umTraceCache, reporter: rep, times: intervals, @@ -133,11 +132,11 @@ func (m *traceHandler) HandleTrace(bpfTrace *host.Trace) { if !m.reporter.SupportsReportTraceEvent() { // Fast path: if the trace is already known remotely, we just send a counter update. - postConvHash, traceKnown := m.bpfTraceCache.Get(bpfTrace.Hash) + _, traceKnown := m.traceCache.Get(bpfTrace.Hash) if traceKnown { m.bpfTraceCacheHit++ - meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, postConvHash, 1) - m.reporter.ReportCountForTrace(postConvHash, 1, meta) + meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, bpfTrace.Hash, 1) + m.reporter.ReportCountForTrace(bpfTrace.Hash, 1, meta) return } m.bpfTraceCacheMiss++ @@ -146,7 +145,7 @@ func (m *traceHandler) HandleTrace(bpfTrace *host.Trace) { // Slow path: convert trace. umTrace := m.traceProcessor.ConvertTrace(bpfTrace) log.Debugf("Trace hash remap 0x%x -> 0x%x", bpfTrace.Hash, umTrace.Hash) - m.bpfTraceCache.Add(bpfTrace.Hash, umTrace.Hash) + m.traceCache.Add(bpfTrace.Hash, libpf.Void{}) meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, umTrace.Hash, 1) if m.reporter.SupportsReportTraceEvent() {