From 7367ee54e9e5e747be8834df35e3e012656a751a Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Mon, 17 Mar 2025 11:12:00 +0100 Subject: [PATCH 1/6] reporter: simplify TraceReporter interface This proposed change implements https://github.com/open-telemetry/opentelemetry-ebpf-profiler/issues/384#issuecomment-2721165581 as a first step of https://github.com/open-telemetry/opentelemetry-ebpf-profiler/issues/384/. Signed-off-by: Florian Lehner --- metrics/ids.go | 6 --- metrics/metrics.json | 2 + reporter/base_reporter.go | 18 ++----- reporter/collector_reporter_test.go | 4 +- reporter/iface.go | 20 ++------ tracehandler/metrics.go | 18 ++----- tracehandler/tracehandler.go | 74 +++++++++------------------ tracehandler/tracehandler_test.go | 79 ++++++++--------------------- 8 files changed, 64 insertions(+), 157 deletions(-) diff --git a/metrics/ids.go b/metrics/ids.go index 491c2d4ff..a058e0a6d 100644 --- a/metrics/ids.go +++ b/metrics/ids.go @@ -269,12 +269,6 @@ const ( // Number of failures to unwind because return address was not found with heuristic IDUnwindHotspotErrInvalidRA = 130 - // Number of cache hits in tracehandler trace cache by BPF hash - IDKnownTracesHit = 131 - - // Number of cache misses in tracehandler trace cache by BPF hash - IDKnownTracesMiss = 132 - // Current size of the unwind info array IDUnwindInfoArraySize = 133 diff --git a/metrics/metrics.json b/metrics/metrics.json index 285804981..c9c7463a3 100644 --- a/metrics/metrics.json +++ b/metrics/metrics.json @@ -952,6 +952,7 @@ "id": 130 }, { + "obsolete": true, "description": "Number of cache hits in tracehandler trace cache by BPF hash", "type": "counter", "name": "KnownTracesHit", @@ -959,6 +960,7 @@ "id": 131 }, { + "obsolete": true, "description": "Number of cache misses in tracehandler trace cache by BPF hash", "type": "counter", "name": "KnownTracesMiss", diff --git a/reporter/base_reporter.go b/reporter/base_reporter.go index 71c9c3187..30a1d9b32 100644 --- a/reporter/base_reporter.go +++ b/reporter/base_reporter.go @@ -5,6 +5,7 @@ package reporter // import "go.opentelemetry.io/ebpf-profiler/reporter" import ( "context" + "fmt" "time" lru "github.com/elastic/go-freelru" @@ -63,13 +64,6 @@ func (b *baseReporter) addHostmetadata(metadataMap map[string]string) { } } -// ReportFramesForTrace is a NOP -func (*baseReporter) ReportFramesForTrace(_ *libpf.Trace) {} - -// ReportCountForTrace is a NOP -func (b *baseReporter) ReportCountForTrace(_ libpf.TraceHash, _ uint16, _ *samples.TraceEventMeta) { -} - func (b *baseReporter) ExecutableKnown(fileID libpf.FileID) bool { _, known := b.pdata.Executables.GetAndRefresh(fileID, pdata.ExecutableCacheLifetime) return known @@ -93,13 +87,10 @@ func (b *baseReporter) ExecutableMetadata(args *ExecutableMetadataArgs) { }) } -func (*baseReporter) SupportsReportTraceEvent() bool { return true } - -func (b *baseReporter) ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceEventMeta) { +func (b *baseReporter) ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceEventMeta) error { if meta.Origin != support.TraceOriginSampling && meta.Origin != support.TraceOriginOffCPU { // At the moment only on-CPU and off-CPU traces are reported. - log.Errorf("Skip reporting trace for unexpected %d origin", meta.Origin) - return + return fmt.Errorf("skip reporting trace for unexpected %d origin", meta.Origin) } var extraMeta any @@ -131,7 +122,7 @@ func (b *baseReporter) ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceE events.Timestamps = append(events.Timestamps, uint64(meta.Timestamp)) events.OffTimes = append(events.OffTimes, meta.OffTime) (*traceEventsMap)[meta.Origin][key] = events - return + return nil } (*traceEventsMap)[meta.Origin][key] = &samples.TraceEvents{ @@ -145,6 +136,7 @@ func (b *baseReporter) ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceE OffTimes: []int64{meta.OffTime}, EnvVars: meta.EnvVars, } + return nil } func (b *baseReporter) FrameMetadata(args *FrameMetadataArgs) { diff --git a/reporter/collector_reporter_test.go b/reporter/collector_reporter_test.go index 1701faad0..99c29e265 100644 --- a/reporter/collector_reporter_test.go +++ b/reporter/collector_reporter_test.go @@ -57,7 +57,9 @@ func TestCollectorReporterReportTraceEvent(t *testing.T) { CGroupCacheElements: 1, }, next) require.NoError(t, err) - r.ReportTraceEvent(tt.trace, tt.meta) + if err := r.ReportTraceEvent(tt.trace, tt.meta); err != nil { + t.Fatal(err) + } }) } } diff --git a/reporter/iface.go b/reporter/iface.go index 679a61e6d..695b283ec 100644 --- a/reporter/iface.go +++ b/reporter/iface.go @@ -30,22 +30,10 @@ type Reporter interface { } type TraceReporter interface { - // ReportFramesForTrace accepts a trace with the corresponding frames - // and caches this information before a periodic reporting to the backend. - ReportFramesForTrace(trace *libpf.Trace) - - // ReportCountForTrace accepts a hash of a trace with a corresponding count and - // caches this information before a periodic reporting to the backend. - ReportCountForTrace(traceHash libpf.TraceHash, count uint16, meta *samples.TraceEventMeta) - - // ReportTraceEvent accepts a trace event (trace metadata with frames and counts) - // and caches it for reporting to the backend. It returns true if the event was - // enqueued for reporting, and false if the event was ignored. - ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceEventMeta) - - // SupportsReportTraceEvent returns true if the reporter supports reporting trace events - // via ReportTraceEvent(). - SupportsReportTraceEvent() bool + // ReportTraceEvent accepts a trace event (trace metadata with frames) + // and enqueues it for reporting to the backend. + // If handling the trace event fails it returns an error. + ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceEventMeta) error } // ExecutableOpener is a function that attempts to open an executable. diff --git a/tracehandler/metrics.go b/tracehandler/metrics.go index 055d40df5..50c8f555e 100644 --- a/tracehandler/metrics.go +++ b/tracehandler/metrics.go @@ -9,24 +9,14 @@ func (m *traceHandler) collectMetrics() { metrics.AddSlice([]metrics.Metric{ { ID: metrics.IDTraceCacheHit, - Value: metrics.MetricValue(m.umTraceCacheHit), + Value: metrics.MetricValue(m.traceCacheHit), }, { ID: metrics.IDTraceCacheMiss, - Value: metrics.MetricValue(m.umTraceCacheMiss), - }, - { - ID: metrics.IDKnownTracesHit, - Value: metrics.MetricValue(m.bpfTraceCacheHit), - }, - { - ID: metrics.IDKnownTracesMiss, - Value: metrics.MetricValue(m.bpfTraceCacheMiss), + Value: metrics.MetricValue(m.traceCacheMiss), }, }) - m.umTraceCacheHit = 0 - m.umTraceCacheMiss = 0 - m.bpfTraceCacheHit = 0 - m.bpfTraceCacheMiss = 0 + m.traceCacheHit = 0 + m.traceCacheMiss = 0 } diff --git a/tracehandler/tracehandler.go b/tracehandler/tracehandler.go index ad5883360..b404cb7ee 100644 --- a/tracehandler/tracehandler.go +++ b/tracehandler/tracehandler.go @@ -29,6 +29,10 @@ type Times interface { MonitorInterval() time.Duration } +// Default lifetime of elements in the cache to reduce recurring +// symbolization efforts. +var traceCacheLifetime = 5 * time.Minute + // TraceProcessor is an interface used by traceHandler to convert traces // from a form received from eBPF to the form we wish to dispatch to the // collection agent. @@ -54,21 +58,15 @@ type TraceProcessor interface { // from the eBPF components. type traceHandler struct { // Metrics - umTraceCacheHit uint64 - umTraceCacheMiss uint64 - bpfTraceCacheHit uint64 - bpfTraceCacheMiss uint64 + traceCacheHit uint64 + traceCacheMiss uint64 traceProcessor TraceProcessor - // bpfTraceCache stores mappings from BPF to user-mode hashes. This allows + // traceCache stores mappings from BPF hashes to symbolized traces. 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] - - // umTraceCache is a LRU set that suppresses unnecessary resends of traces - // that we have recently reported to the collector already. - umTraceCache *lru.LRU[libpf.TraceHash, libpf.Void] + traceCache *lru.LRU[host.TraceHash, *libpf.Trace] // reporter instance to use to send out traces. reporter reporter.TraceReporter @@ -79,27 +77,20 @@ 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]( + traceCache, err := lru.New[host.TraceHash, *libpf.Trace]( cacheSize, func(k host.TraceHash) uint32 { return uint32(k) }) if err != nil { return nil, err } + // Do not hold elements indefinitely in the cache. + traceCache.SetLifetime(traceCacheLifetime) - umTraceCache, err := lru.New[libpf.TraceHash, libpf.Void]( - cacheSize, libpf.TraceHash.Hash32) - if err != nil { - return nil, err - } - - t := &traceHandler{ + return &traceHandler{ traceProcessor: traceProcessor, - bpfTraceCache: bpfTraceCache, - umTraceCache: umTraceCache, + traceCache: traceCache, reporter: rep, times: intervals, - } - - return t, nil + }, nil } func (m *traceHandler) HandleTrace(bpfTrace *host.Trace) { @@ -117,40 +108,25 @@ func (m *traceHandler) HandleTrace(bpfTrace *host.Trace) { EnvVars: bpfTrace.EnvVars, } - 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) - if traceKnown { - m.bpfTraceCacheHit++ - meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, postConvHash, 1) - m.reporter.ReportCountForTrace(postConvHash, 1, meta) - return + if trace, exists := m.traceCache.GetAndRefresh(bpfTrace.Hash, + traceCacheLifetime); exists { + m.traceCacheHit++ + // Fast path + meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, trace.Hash, 1) + if err := m.reporter.ReportTraceEvent(trace, meta); err != nil { + log.Errorf("Failed to report trace event: %v", err) } - m.bpfTraceCacheMiss++ } + m.traceCacheMiss++ // 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, umTrace) meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, umTrace.Hash, 1) - if m.reporter.SupportsReportTraceEvent() { - m.reporter.ReportTraceEvent(umTrace, meta) - return + if err := m.reporter.ReportTraceEvent(umTrace, meta); err != nil { + log.Errorf("Failed to report trace event: %v", err) } - m.reporter.ReportCountForTrace(umTrace.Hash, 1, meta) - - // Trace already known to collector by UM hash? - if _, known := m.umTraceCache.Get(umTrace.Hash); known { - m.umTraceCacheHit++ - return - } - m.umTraceCacheMiss++ - - // Nope. Send it now. - m.reporter.ReportFramesForTrace(umTrace) - m.umTraceCache.Add(umTrace.Hash, libpf.Void{}) } // Start starts a goroutine that receives and processes trace updates over diff --git a/tracehandler/tracehandler_test.go b/tracehandler/tracehandler_test.go index 05d618bef..67d7fdf9b 100644 --- a/tracehandler/tracehandler_test.go +++ b/tracehandler/tracehandler_test.go @@ -5,10 +5,10 @@ package tracehandler_test import ( "context" + "maps" "testing" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/ebpf-profiler/host" @@ -52,48 +52,26 @@ type arguments struct { trace *host.Trace } -// reportedCount / reportedTrace hold the information reported from traceHandler -// via the reporter functions (reportCountForTrace / reportFramesForTrace). -type reportedCount struct { - traceHash libpf.TraceHash - count uint16 -} - -type reportedTrace struct { - traceHash libpf.TraceHash -} - type mockReporter struct { - t *testing.T - reportedCounts []reportedCount - reportedTraces []reportedTrace -} - -func (m *mockReporter) ReportFramesForTrace(trace *libpf.Trace) { - m.reportedTraces = append(m.reportedTraces, reportedTrace{traceHash: trace.Hash}) - m.t.Logf("reportFramesForTrace: new trace 0x%x", trace.Hash) -} - -func (m *mockReporter) ReportCountForTrace(traceHash libpf.TraceHash, - count uint16, _ *samples.TraceEventMeta) { - m.reportedCounts = append(m.reportedCounts, reportedCount{ - traceHash: traceHash, - count: count, - }) - m.t.Logf("reportCountForTrace: 0x%x count: %d", traceHash, count) + t *testing.T + reports map[libpf.TraceHash]uint16 } -func (m *mockReporter) SupportsReportTraceEvent() bool { return false } +func (m *mockReporter) ReportTraceEvent(trace *libpf.Trace, _ *samples.TraceEventMeta) error { + if _, exists := m.reports[trace.Hash]; exists { + m.reports[trace.Hash]++ + return nil + } + m.reports[trace.Hash] = 1 -func (m *mockReporter) ReportTraceEvent(_ *libpf.Trace, _ *samples.TraceEventMeta) { + return nil } func TestTraceHandler(t *testing.T) { tests := map[string]struct { input []arguments - expectedCounts []reportedCount - expectedTraces []reportedTrace expireTimeout time.Duration + expectedEvents map[libpf.TraceHash]uint16 }{ // no input simulates a case where no data is provided as input // to the functions of traceHandler. @@ -103,9 +81,8 @@ func TestTraceHandler(t *testing.T) { "single trace": {input: []arguments{ {trace: &host.Trace{Hash: host.TraceHash(0x1234)}}, }, - expectedTraces: []reportedTrace{{traceHash: libpf.NewTraceHash(0x1234, 0x1234)}}, - expectedCounts: []reportedCount{ - {traceHash: libpf.NewTraceHash(0x1234, 0x1234), count: 1}, + expectedEvents: map[libpf.TraceHash]uint16{ + libpf.NewTraceHash(0x1234, 0x1234): 1, }, }, @@ -114,10 +91,8 @@ func TestTraceHandler(t *testing.T) { {trace: &host.Trace{Hash: host.TraceHash(4)}}, {trace: &host.Trace{Hash: host.TraceHash(4)}}, }, - expectedTraces: []reportedTrace{{traceHash: libpf.NewTraceHash(4, 4)}}, - expectedCounts: []reportedCount{ - {traceHash: libpf.NewTraceHash(4, 4), count: 1}, - {traceHash: libpf.NewTraceHash(4, 4), count: 1}, + expectedEvents: map[libpf.TraceHash]uint16{ + libpf.NewTraceHash(4, 4): 2, }, }, } @@ -126,7 +101,10 @@ func TestTraceHandler(t *testing.T) { name := name test := test t.Run(name, func(t *testing.T) { - r := &mockReporter{t: t} + r := &mockReporter{ + t: t, + reports: make(map[libpf.TraceHash]uint16), + } traceChan := make(chan *host.Trace) ctx, cancel := context.WithCancel(context.Background()) @@ -142,23 +120,8 @@ func TestTraceHandler(t *testing.T) { cancel() <-exitNotify - assert.Equal(t, len(test.expectedCounts), len(r.reportedCounts)) - assert.Equal(t, len(test.expectedTraces), len(r.reportedTraces)) - - // Expected and reported traces order should match. - assert.Equal(t, test.expectedTraces, r.reportedTraces) - - for _, expCount := range test.expectedCounts { - // Expected and reported count order doesn't necessarily match. - found := false - for _, repCount := range r.reportedCounts { - if expCount == repCount { - found = true - break - } - } - assert.True(t, found, "Expected count %d for trace 0x%x not found", - expCount.count, expCount.traceHash) + if !maps.Equal(r.reports, test.expectedEvents) { + t.Fatalf("Expected %#v but got %#v", test.expectedEvents, r.reports) } }) } From f118edf9305bedf6f2c3866d83452333652cb302 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Mon, 17 Mar 2025 14:23:35 +0100 Subject: [PATCH 2/6] fixup: expose trace origin error Signed-off-by: Florian Lehner --- reporter/base_reporter.go | 6 +++++- reporter/collector_reporter_test.go | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/reporter/base_reporter.go b/reporter/base_reporter.go index 30a1d9b32..f618e0023 100644 --- a/reporter/base_reporter.go +++ b/reporter/base_reporter.go @@ -5,6 +5,7 @@ package reporter // import "go.opentelemetry.io/ebpf-profiler/reporter" import ( "context" + "errors" "fmt" "time" @@ -43,6 +44,8 @@ type baseReporter struct { hostmetadata *lru.SyncedLRU[string, string] } +var errUnknownOrigin = errors.New("unknown trace origin") + func (b *baseReporter) Stop() { b.runLoop.Stop() } @@ -90,7 +93,8 @@ func (b *baseReporter) ExecutableMetadata(args *ExecutableMetadataArgs) { func (b *baseReporter) ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceEventMeta) error { if meta.Origin != support.TraceOriginSampling && meta.Origin != support.TraceOriginOffCPU { // At the moment only on-CPU and off-CPU traces are reported. - return fmt.Errorf("skip reporting trace for unexpected %d origin", meta.Origin) + return fmt.Errorf("skip reporting trace for %d origin: %w", meta.Origin, + errUnknownOrigin) } var extraMeta any diff --git a/reporter/collector_reporter_test.go b/reporter/collector_reporter_test.go index 99c29e265..d5807d465 100644 --- a/reporter/collector_reporter_test.go +++ b/reporter/collector_reporter_test.go @@ -57,7 +57,8 @@ func TestCollectorReporterReportTraceEvent(t *testing.T) { CGroupCacheElements: 1, }, next) require.NoError(t, err) - if err := r.ReportTraceEvent(tt.trace, tt.meta); err != nil { + if err := r.ReportTraceEvent(tt.trace, tt.meta); err != nil && + !errors.Is(err, errUnknownOrigin) { t.Fatal(err) } }) From 863b7673f8f9aee24195b622e7d1de25cf6c80cd Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Mon, 17 Mar 2025 14:26:12 +0100 Subject: [PATCH 3/6] fixup: add missing return Signed-off-by: Florian Lehner --- tracehandler/tracehandler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tracehandler/tracehandler.go b/tracehandler/tracehandler.go index b404cb7ee..eaca731e5 100644 --- a/tracehandler/tracehandler.go +++ b/tracehandler/tracehandler.go @@ -116,6 +116,7 @@ func (m *traceHandler) HandleTrace(bpfTrace *host.Trace) { if err := m.reporter.ReportTraceEvent(trace, meta); err != nil { log.Errorf("Failed to report trace event: %v", err) } + return } m.traceCacheMiss++ From da54892ba182e460c1177f011f27fcb2d78c69cc Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Mon, 17 Mar 2025 16:50:19 +0100 Subject: [PATCH 4/6] fixup: use SyncedLRU Signed-off-by: Florian Lehner --- tracehandler/tracehandler.go | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/tracehandler/tracehandler.go b/tracehandler/tracehandler.go index eaca731e5..7809aca07 100644 --- a/tracehandler/tracehandler.go +++ b/tracehandler/tracehandler.go @@ -8,6 +8,7 @@ package tracehandler // import "go.opentelemetry.io/ebpf-profiler/tracehandler" import ( "context" "fmt" + "sync" "time" lru "github.com/elastic/go-freelru" @@ -66,7 +67,7 @@ type traceHandler struct { // traceCache stores mappings from BPF hashes to symbolized traces. This allows // avoiding the overhead of re-doing user-mode symbolization of traces that // we have recently seen already. - traceCache *lru.LRU[host.TraceHash, *libpf.Trace] + traceCache *lru.SyncedLRU[host.TraceHash, *libpf.Trace] // reporter instance to use to send out traces. reporter reporter.TraceReporter @@ -75,9 +76,9 @@ type traceHandler struct { } // newTraceHandler creates a new traceHandler -func newTraceHandler(rep reporter.TraceReporter, traceProcessor TraceProcessor, - intervals Times, cacheSize uint32) (*traceHandler, error) { - traceCache, err := lru.New[host.TraceHash, *libpf.Trace]( +func newTraceHandler(ctx context.Context, rep reporter.TraceReporter, + traceProcessor TraceProcessor, intervals Times, cacheSize uint32) (*traceHandler, error) { + traceCache, err := lru.NewSynced[host.TraceHash, *libpf.Trace]( cacheSize, func(k host.TraceHash) uint32 { return uint32(k) }) if err != nil { return nil, err @@ -85,6 +86,29 @@ func newTraceHandler(rep reporter.TraceReporter, traceProcessor TraceProcessor, // Do not hold elements indefinitely in the cache. traceCache.SetLifetime(traceCacheLifetime) + var wg sync.WaitGroup + wg.Add(1) + + go func() { + wg.Done() + jitter := 0.2 + ticker := time.NewTicker(libpf.AddJitter(traceCacheLifetime, jitter)) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + traceCache.PurgeExpired() + ticker.Reset(libpf.AddJitter(traceCacheLifetime, jitter)) + } + } + }() + + // Wait to make sure the purge routine did start. + wg.Wait() + return &traceHandler{ traceProcessor: traceProcessor, traceCache: traceCache, @@ -138,7 +162,7 @@ func Start(ctx context.Context, rep reporter.TraceReporter, traceProcessor Trace traceInChan <-chan *host.Trace, intervals Times, cacheSize uint32, ) (workerExited <-chan libpf.Void, err error) { handler, err := - newTraceHandler(rep, traceProcessor, intervals, cacheSize) + newTraceHandler(ctx, rep, traceProcessor, intervals, cacheSize) if err != nil { return nil, fmt.Errorf("failed to create traceHandler: %v", err) } From c3c73270d7b32b37d83c48a2d3935848f4c4eaed Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Thu, 20 Mar 2025 09:28:31 +0100 Subject: [PATCH 5/6] fixup: drop jitter Signed-off-by: Florian Lehner --- tracehandler/tracehandler.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tracehandler/tracehandler.go b/tracehandler/tracehandler.go index 7809aca07..d064bc1f4 100644 --- a/tracehandler/tracehandler.go +++ b/tracehandler/tracehandler.go @@ -91,8 +91,7 @@ func newTraceHandler(ctx context.Context, rep reporter.TraceReporter, go func() { wg.Done() - jitter := 0.2 - ticker := time.NewTicker(libpf.AddJitter(traceCacheLifetime, jitter)) + ticker := time.NewTicker(traceCacheLifetime) defer ticker.Stop() for { @@ -101,7 +100,6 @@ func newTraceHandler(ctx context.Context, rep reporter.TraceReporter, return case <-ticker.C: traceCache.PurgeExpired() - ticker.Reset(libpf.AddJitter(traceCacheLifetime, jitter)) } } }() From 5425e426236ff215c2bfb9b43d28b1e187126a09 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Thu, 20 Mar 2025 09:31:26 +0100 Subject: [PATCH 6/6] fixup: drop pointer type in LRU Signed-off-by: Florian Lehner --- tracehandler/tracehandler.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tracehandler/tracehandler.go b/tracehandler/tracehandler.go index d064bc1f4..58775c483 100644 --- a/tracehandler/tracehandler.go +++ b/tracehandler/tracehandler.go @@ -67,7 +67,7 @@ type traceHandler struct { // traceCache stores mappings from BPF hashes to symbolized traces. This allows // avoiding the overhead of re-doing user-mode symbolization of traces that // we have recently seen already. - traceCache *lru.SyncedLRU[host.TraceHash, *libpf.Trace] + traceCache *lru.SyncedLRU[host.TraceHash, libpf.Trace] // reporter instance to use to send out traces. reporter reporter.TraceReporter @@ -78,7 +78,7 @@ type traceHandler struct { // newTraceHandler creates a new traceHandler func newTraceHandler(ctx context.Context, rep reporter.TraceReporter, traceProcessor TraceProcessor, intervals Times, cacheSize uint32) (*traceHandler, error) { - traceCache, err := lru.NewSynced[host.TraceHash, *libpf.Trace]( + traceCache, err := lru.NewSynced[host.TraceHash, libpf.Trace]( cacheSize, func(k host.TraceHash) uint32 { return uint32(k) }) if err != nil { return nil, err @@ -135,7 +135,7 @@ func (m *traceHandler) HandleTrace(bpfTrace *host.Trace) { m.traceCacheHit++ // Fast path meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, trace.Hash, 1) - if err := m.reporter.ReportTraceEvent(trace, meta); err != nil { + if err := m.reporter.ReportTraceEvent(&trace, meta); err != nil { log.Errorf("Failed to report trace event: %v", err) } return @@ -144,7 +144,7 @@ func (m *traceHandler) HandleTrace(bpfTrace *host.Trace) { // Slow path: convert trace. umTrace := m.traceProcessor.ConvertTrace(bpfTrace) - m.traceCache.Add(bpfTrace.Hash, umTrace) + m.traceCache.Add(bpfTrace.Hash, *umTrace) meta.APMServiceName = m.traceProcessor.MaybeNotifyAPMAgent(bpfTrace, umTrace.Hash, 1) if err := m.reporter.ReportTraceEvent(umTrace, meta); err != nil {