diff --git a/collector/internal/controller.go b/collector/internal/controller.go index b5597e29e..a402dad59 100644 --- a/collector/internal/controller.go +++ b/collector/internal/controller.go @@ -36,9 +36,7 @@ func NewController(cfg *controller.Config, GRPCConnectionTimeout: intervals.GRPCConnectionTimeout(), ReportInterval: intervals.ReportInterval(), ExecutablesCacheElements: 16384, - // Next step: Calculate FramesCacheElements from numCores and samplingRate. - FramesCacheElements: 131072, - SamplesPerSecond: cfg.SamplesPerSecond, + SamplesPerSecond: cfg.SamplesPerSecond, }, nextConsumer) if err != nil { return nil, err diff --git a/host/host.go b/host/host.go index 192830690..75c96a04d 100644 --- a/host/host.go +++ b/host/host.go @@ -63,4 +63,5 @@ type Trace struct { CPU int EnvVars map[string]string CustomLabels map[string]string + KernelFrames libpf.Frames } diff --git a/interpreter/dotnet/dotnet.go b/interpreter/dotnet/dotnet.go index fe703c392..9b169acc3 100644 --- a/interpreter/dotnet/dotnet.go +++ b/interpreter/dotnet/dotnet.go @@ -107,7 +107,6 @@ import ( log "github.com/sirupsen/logrus" "go.opentelemetry.io/ebpf-profiler/interpreter" - "go.opentelemetry.io/ebpf-profiler/libpf" ) const ( @@ -125,9 +124,6 @@ var ( // regex for the core language runtime dotnetRegex = regexp.MustCompile(`/(\d+)\.(\d+).(\d+)/libcoreclr.so$`) - // The FileID used for Dotnet stub frames. Same FileID as in other interpreters. - stubsFileID = libpf.NewStubFileID(libpf.DotnetFrame) - // compiler check to make sure the needed interfaces are satisfied _ interpreter.Data = &dotnetData{} _ interpreter.Instance = &dotnetInstance{} diff --git a/interpreter/dotnet/instance.go b/interpreter/dotnet/instance.go index 57ce777db..9a17f9233 100644 --- a/interpreter/dotnet/instance.go +++ b/interpreter/dotnet/instance.go @@ -5,7 +5,6 @@ package dotnet // import "go.opentelemetry.io/ebpf-profiler/interpreter/dotnet" import ( "fmt" - "hash/fnv" "path" "slices" "strings" @@ -134,8 +133,6 @@ type dotnetInstance struct { // bias is the ELF DSO load bias bias libpf.Address - codeTypeMethodIDs [codeReadyToRun]libpf.AddressOrLineno - // Internal class instance pointers codeRangeListPtr libpf.Address precodeStubManagerPtr libpf.Address @@ -159,24 +156,11 @@ type dotnetInstance struct { addrToMethod *freelru.LRU[libpf.Address, *dotnetMethod] } -// calculateAndSymbolizeStubID calculates a stub LineID, and symbolizes it if needed -func (i *dotnetInstance) insertAndSymbolizeStubFrame(symbolReporter reporter.SymbolReporter, - trace *libpf.Trace, codeType uint) { - name := "[stub: " + codeName[codeType] + "]" - lineID := i.codeTypeMethodIDs[codeType] - if lineID == 0 { - h := fnv.New128a() - _, _ = h.Write([]byte(name)) - nameHash := h.Sum(nil) - lineID = libpf.AddressOrLineno(npsr.Uint64(nameHash, 0)) - i.codeTypeMethodIDs[codeType] = lineID - } - - frameID := libpf.NewFrameID(stubsFileID, lineID) - trace.AppendFrameID(libpf.DotnetFrame, frameID) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: libpf.Intern(name), +// appendStubFrame appends a stub frame of given type +func (i *dotnetInstance) appendStubFrame(frames *libpf.Frames, codeType uint) { + frames.Append(&libpf.Frame{ + Type: libpf.DotnetFrame, + FunctionName: libpf.Intern("[stub: " + codeName[codeType] + "]"), }) } @@ -736,8 +720,7 @@ func (i *dotnetInstance) GetAndResetMetrics() ([]metrics.Metric, error) { }, nil } -func (i *dotnetInstance) Symbolize(symbolReporter reporter.SymbolReporter, - frame *host.Frame, trace *libpf.Trace) error { +func (i *dotnetInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { if !frame.Type.IsInterpType(libpf.Dotnet) { return interpreter.ErrMismatchInterpreterType } @@ -761,25 +744,25 @@ func (i *dotnetInstance) Symbolize(symbolReporter reporter.SymbolReporter, // PC is executing: // - on non-leaf frames, it is the return address // - on leaf frames, it is the address after the CALL machine opcode - lineID := libpf.AddressOrLineno(pcOffset) - frameID := libpf.NewFrameID(module.fileID, lineID) - trace.AppendFrameID(libpf.DotnetFrame, frameID) - if !symbolReporter.FrameKnown(frameID) { - methodName := module.resolveR2RMethodName(pcOffset) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: methodName, - SourceFile: module.simpleName, - }) - } + frames.Append(&libpf.Frame{ + Type: libpf.DotnetFrame, + FileID: module.fileID, + AddressOrLineno: libpf.AddressOrLineno(pcOffset), + FunctionName: module.resolveR2RMethodName(pcOffset), + SourceFile: module.simpleName, + }) case codeJIT: // JITted frame in anonymous mapping method, err := i.getMethod(codeHeaderPtr) if err != nil { return err } + if method.index == 0 || method.classification == mcDynamic { + i.appendStubFrame(frames, codeDynamic) + break + } + ilOffset := method.mapPCOffsetToILOffset(pcOffset, frame.ReturnAddress) - fileID := method.module.fileID // The Line ID format is: // 4 bits Set to 0xf to indicate JIT frame. @@ -788,25 +771,18 @@ func (i *dotnetInstance) Symbolize(symbolReporter reporter.SymbolReporter, // pointing to CALL instruction if the debug info was accurate. lineID := libpf.AddressOrLineno(0xf0000000+method.index)<<32 + libpf.AddressOrLineno(ilOffset) - - if method.index == 0 || method.classification == mcDynamic { - i.insertAndSymbolizeStubFrame(symbolReporter, trace, codeDynamic) - } else { - frameID := libpf.NewFrameID(fileID, lineID) - trace.AppendFrameID(libpf.DotnetFrame, frameID) - if !symbolReporter.FrameKnown(frameID) { - methodName := method.module.resolveMethodName(method.index) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - SourceFile: method.module.simpleName, - FunctionName: methodName, - FunctionOffset: ilOffset, - }) - } - } + methodName := method.module.resolveMethodName(method.index) + frames.Append(&libpf.Frame{ + Type: libpf.DotnetFrame, + FileID: method.module.fileID, + AddressOrLineno: lineID, + SourceFile: method.module.simpleName, + FunctionName: methodName, + FunctionOffset: ilOffset, + }) default: // Stub code - i.insertAndSymbolizeStubFrame(symbolReporter, trace, frameType) + i.appendStubFrame(frames, frameType) } sfCounter.ReportSuccess() diff --git a/interpreter/go/go.go b/interpreter/go/go.go index 088b49f96..b98cee49e 100644 --- a/interpreter/go/go.go +++ b/interpreter/go/go.go @@ -5,7 +5,6 @@ package golang // import "go.opentelemetry.io/ebpf-profiler/interpreter/go" import ( "fmt" - "hash/fnv" "sync/atomic" "go.opentelemetry.io/ebpf-profiler/host" @@ -14,7 +13,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/metrics" "go.opentelemetry.io/ebpf-profiler/nativeunwind/elfunwindinfo" "go.opentelemetry.io/ebpf-profiler/remotememory" - "go.opentelemetry.io/ebpf-profiler/reporter" "go.opentelemetry.io/ebpf-profiler/successfailurecounter" ) @@ -94,8 +92,7 @@ func (g *goInstance) Detach(_ interpreter.EbpfHandler, _ libpf.PID) error { return nil } -func (g *goInstance) Symbolize(symbolReporter reporter.SymbolReporter, frame *host.Frame, - trace *libpf.Trace) error { +func (g *goInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { if !frame.Type.IsInterpType(libpf.Native) { return interpreter.ErrMismatchInterpreterType } @@ -107,27 +104,14 @@ func (g *goInstance) Symbolize(symbolReporter reporter.SymbolReporter, frame *ho return fmt.Errorf("failed to symbolize 0x%x", frame.Lineno) } - // The fnv hash Write() method calls cannot fail, so it's safe to ignore the errors. - h := fnv.New128a() - _, _ = h.Write([]byte(frame.File.StringNoQuotes())) - _, _ = h.Write([]byte(fn)) - _, _ = h.Write([]byte(sourceFile)) - fileID, err := libpf.FileIDFromBytes(h.Sum(nil)) - if err != nil { - return fmt.Errorf("failed to create a file ID: %v", err) - } - - frameID := libpf.NewFrameID(fileID, libpf.AddressOrLineno(lineNo)) - - trace.AppendFrameID(libpf.GoFrame, frameID) - - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: libpf.Intern(fn), - SourceFile: libpf.Intern(sourceFile), - SourceLine: libpf.SourceLineno(lineNo), + frames.Append(&libpf.Frame{ + Type: libpf.GoFrame, + //TODO: File: convert the frame.File (host.FileID) to libpf.FileID here + AddressOrLineno: frame.Lineno, + FunctionName: libpf.Intern(fn), + SourceFile: libpf.Intern(sourceFile), + SourceLine: libpf.SourceLineno(lineNo), }) - sfCounter.ReportSuccess() return nil } diff --git a/interpreter/go/go_test.go b/interpreter/go/go_test.go index 77a8f9b21..071174636 100644 --- a/interpreter/go/go_test.go +++ b/interpreter/go/go_test.go @@ -12,53 +12,9 @@ import ( "go.opentelemetry.io/ebpf-profiler/libpf/pfelf" "go.opentelemetry.io/ebpf-profiler/process" "go.opentelemetry.io/ebpf-profiler/remotememory" - "go.opentelemetry.io/ebpf-profiler/reporter" "go.opentelemetry.io/ebpf-profiler/util" ) -// mockReporter implements reporter.SymbolReporter for testing -type mockReporter struct { - b *testing.B - frameMetadata map[libpf.FrameID]*reporter.FrameMetadataArgs -} - -func newMockReporter(b *testing.B) *mockReporter { - return &mockReporter{ - b: b, - frameMetadata: make(map[libpf.FrameID]*reporter.FrameMetadataArgs), - } -} - -func (m *mockReporter) CompareFunctionName(fn string) { - if len(m.frameMetadata) != 1 { - m.b.Fatalf("Expected a single entry but got %d", len(m.frameMetadata)) - } - for _, v := range m.frameMetadata { - // The returned anonymous function has the suffic 'func1'. - // Therefore check only for a matching prefix. - if !strings.HasPrefix(v.FunctionName.String(), fn) { - m.b.Fatalf("Expected '%s()' but got '%s()'", fn, v.FunctionName) - } - } -} - -func (m *mockReporter) FrameMetadata(args *reporter.FrameMetadataArgs) { - m.frameMetadata[args.FrameID] = args -} - -func (m *mockReporter) ExecutableMetadata(args *reporter.ExecutableMetadataArgs) { - // Not used in this test -} - -func (m *mockReporter) FrameKnown(frameID libpf.FrameID) bool { - _, exists := m.frameMetadata[frameID] - return exists -} - -func (m *mockReporter) ExecutableKnown(fileID libpf.FileID) bool { - return false -} - func BenchmarkGolang(b *testing.B) { pc, _, _, ok := runtime.Caller(1) if !ok { @@ -80,7 +36,6 @@ func BenchmarkGolang(b *testing.B) { } loaderInfo := interpreter.NewLoaderInfo(hostFileID, elfRef, []util.Range{}) rm := remotememory.NewProcessVirtualMemory(libpfPID) - symReporter := newMockReporter(b) b.ResetTimer() b.ReportAllocs() @@ -95,16 +50,26 @@ func BenchmarkGolang(b *testing.B) { b.Fatalf("Failed to create instance: %v", err) } - trace := libpf.Trace{} + frames := libpf.Frames{} - if err := gI.Symbolize(symReporter, &host.Frame{ + if err := gI.Symbolize(&host.Frame{ File: hostFileID, Lineno: libpf.AddressOrLineno(pc), Type: libpf.FrameType(libpf.Native), - }, &trace); err != nil { + }, &frames); err != nil { b.Fatalf("Failed to symbolize 0x%x: %v", pc, err) } - symReporter.CompareFunctionName(fn.Name()) + if len(frames) != 1 { + b.Fatalf("Expected a single entry but got %d", len(frames)) + } + for _, uniqueFrame := range frames { + f := uniqueFrame.Value() + // The returned anonymous function has the suffic 'func1'. + // Therefore check only for a matching prefix. + if !strings.HasPrefix(f.FunctionName.String(), fn.Name()) { + b.Fatalf("Expected '%s()' but got '%s()'", fn, f.FunctionName) + } + } } } diff --git a/interpreter/hotspot/data.go b/interpreter/hotspot/data.go index eb814fbfb..6826ffc7f 100644 --- a/interpreter/hotspot/data.go +++ b/interpreter/hotspot/data.go @@ -361,23 +361,21 @@ func (d *hotspotData) Attach(_ interpreter.EbpfHandler, _ libpf.PID, bias libpf. } // In total there are about 100 to 200 intrinsics. We don't expect to encounter // everyone single one. So we use a small cache size here than LruFunctionCacheSize. - addrToStubNameID, err := - freelru.New[libpf.Address, libpf.AddressOrLineno](128, - libpf.Address.Hash32) + addrToStubName, err := freelru.New[libpf.Address, libpf.String](128, libpf.Address.Hash32) if err != nil { return nil, err } return &hotspotInstance{ - d: d, - rm: rm, - bias: bias, - addrToSymbol: addrToSymbol, - addrToMethod: addrToMethod, - addrToJITInfo: addrToJITInfo, - addrToStubNameID: addrToStubNameID, - prefixes: libpf.Set[lpm.Prefix]{}, - stubs: map[libpf.Address]StubRoutine{}, + d: d, + rm: rm, + bias: bias, + addrToSymbol: addrToSymbol, + addrToMethod: addrToMethod, + addrToJITInfo: addrToJITInfo, + addrToStubName: addrToStubName, + prefixes: libpf.Set[lpm.Prefix]{}, + stubs: map[libpf.Address]StubRoutine{}, }, nil } diff --git a/interpreter/hotspot/hotspot.go b/interpreter/hotspot/hotspot.go index efc3a1377..fbee12621 100644 --- a/interpreter/hotspot/hotspot.go +++ b/interpreter/hotspot/hotspot.go @@ -111,7 +111,6 @@ import ( log "github.com/sirupsen/logrus" "go.opentelemetry.io/ebpf-profiler/interpreter" - "go.opentelemetry.io/ebpf-profiler/libpf" ) var ( @@ -122,9 +121,6 @@ var ( hiddenClassRegex = regexp.MustCompile(`\+0x[0-9a-f]{16}`) hiddenClassMask = "+" - // The FileID used for intrinsic stub frames - hotspotStubsFileID = libpf.NewStubFileID(libpf.HotSpotFrame) - _ interpreter.Data = &hotspotData{} _ interpreter.Instance = &hotspotInstance{} ) diff --git a/interpreter/hotspot/instance.go b/interpreter/hotspot/instance.go index f68f0b83e..94bb565ba 100644 --- a/interpreter/hotspot/instance.go +++ b/interpreter/hotspot/instance.go @@ -80,8 +80,8 @@ type hotspotInstance struct { // the needed data from it. addrToJITInfo *freelru.LRU[libpf.Address, *hotspotJITInfo] - // addrToStubNameID maps a stub name to its unique identifier. - addrToStubNameID *freelru.LRU[libpf.Address, libpf.AddressOrLineno] + // addrToStubName maps a stub address to its name identifier. + addrToStubName *freelru.LRU[libpf.Address, libpf.String] // mainMappingsInserted stores whether the heap areas and proc data are already populated. mainMappingsInserted bool @@ -97,7 +97,7 @@ func (d *hotspotInstance) GetAndResetMetrics() ([]metrics.Metric, error) { addrToSymbolStats := d.addrToSymbol.ResetMetrics() addrToMethodStats := d.addrToMethod.ResetMetrics() addrToJITInfoStats := d.addrToJITInfo.ResetMetrics() - addrToStubNameIDStats := d.addrToStubNameID.ResetMetrics() + addrToStubNameStats := d.addrToStubName.ResetMetrics() return []metrics.Metric{ { @@ -158,19 +158,19 @@ func (d *hotspotInstance) GetAndResetMetrics() ([]metrics.Metric, error) { }, { ID: metrics.IDHotspotAddrToStubNameIDHit, - Value: metrics.MetricValue(addrToStubNameIDStats.Hits), + Value: metrics.MetricValue(addrToStubNameStats.Hits), }, { ID: metrics.IDHotspotAddrToStubNameIDMiss, - Value: metrics.MetricValue(addrToStubNameIDStats.Misses), + Value: metrics.MetricValue(addrToStubNameStats.Misses), }, { ID: metrics.IDHotspotAddrToStubNameIDAdd, - Value: metrics.MetricValue(addrToStubNameIDStats.Inserts), + Value: metrics.MetricValue(addrToStubNameStats.Inserts), }, { ID: metrics.IDHotspotAddrToStubNameIDDel, - Value: metrics.MetricValue(addrToStubNameIDStats.Removals), + Value: metrics.MetricValue(addrToStubNameStats.Removals), }, }, nil } @@ -231,10 +231,9 @@ func (d *hotspotInstance) getPoolSymbol(addr libpf.Address, ndx uint16) libpf.St return d.getSymbol(cpoolVal &^ 1) } -// getStubNameID read the stub name from the code blob at given address and generates a ID. -func (d *hotspotInstance) getStubNameID(symbolReporter reporter.SymbolReporter, ripOrBci uint32, - addr libpf.Address, _ uint32) libpf.AddressOrLineno { - if value, ok := d.addrToStubNameID.Get(addr); ok { +// getStubName read the stub name from the code blob at given address and generates a ID. +func (d *hotspotInstance) getStubName(ripOrBci uint32, addr libpf.Address) libpf.String { + if value, ok := d.addrToStubName.Get(addr); ok { return value } vms := &d.d.Get().vmStructs @@ -248,18 +247,9 @@ func (d *hotspotInstance) getStubNameID(symbolReporter reporter.SymbolReporter, break } } - - h := fnv.New128a() - _, _ = h.Write([]byte(stubName)) - nameHash := h.Sum(nil) - stubID := libpf.AddressOrLineno(npsr.Uint64(nameHash, 0)) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: libpf.NewFrameID(hotspotStubsFileID, stubID), - FunctionName: libpf.Intern(stubName), - }) - d.addrToStubNameID.Add(addr, stubID) - - return stubID + name := libpf.Intern(stubName) + d.addrToStubName.Add(addr, name) + return name } // getMethod reads and returns the interesting data from "class Method" at given address @@ -852,11 +842,9 @@ func (d *hotspotInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler, } // Symbolize interpreters Hotspot eBPF uwinder given data containing target -// process address and translates it to static IDs expanding any inlined frames -// to multiple new frames. Associated symbolization metadata is extracted and -// queued to be sent to collection agent. -func (d *hotspotInstance) Symbolize(symbolReporter reporter.SymbolReporter, - frame *host.Frame, trace *libpf.Trace) error { +// process address and translates it to decorated frames expanding any inlined +// frames to multiple new frames. +func (d *hotspotInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { if !frame.Type.IsInterpType(libpf.HotSpot) { return interpreter.ErrMismatchInterpreterType } @@ -875,20 +863,23 @@ func (d *hotspotInstance) Symbolize(symbolReporter reporter.SymbolReporter, case support.FrameHotspotStub, support.FrameHotspotVtable: // These are stub frames that may or may not be interesting // to be seen in the trace. - stubID := d.getStubNameID(symbolReporter, ripOrBci, ptr, ptrCheck) - trace.AppendFrame(libpf.HotSpotFrame, hotspotStubsFileID, stubID) + stubName := d.getStubName(ripOrBci, ptr) + frames.Append(&libpf.Frame{ + Type: libpf.HotSpotFrame, + FunctionName: stubName, + }) case support.FrameHotspotInterpreter: method, err1 := d.getMethod(ptr, ptrCheck) if err1 != nil { return err1 } - method.symbolize(symbolReporter, ripOrBci, d, trace) + method.symbolize(ripOrBci, d, frames) case support.FrameHotspotNative: jitinfo, err1 := d.getJITInfo(ptr, ptrCheck) if err1 != nil { return err1 } - err = jitinfo.symbolize(symbolReporter, int32(ripOrBci), d, trace) + err = jitinfo.symbolize(int32(ripOrBci), d, frames) default: return fmt.Errorf("hotspot frame subtype %v is not supported", subtype) } diff --git a/interpreter/hotspot/method.go b/interpreter/hotspot/method.go index 56d64e594..d1f2c72dd 100644 --- a/interpreter/hotspot/method.go +++ b/interpreter/hotspot/method.go @@ -9,7 +9,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/libpf" npsr "go.opentelemetry.io/ebpf-profiler/nopanicslicereader" - "go.opentelemetry.io/ebpf-profiler/reporter" ) // Constants for the JVM internals that have never changed @@ -29,31 +28,27 @@ type hotspotMethod struct { // Symbolize generates symbolization information for given hotspot method and // a Byte Code Index (BCI) -func (m *hotspotMethod) symbolize(symbolReporter reporter.SymbolReporter, bci uint32, - ii *hotspotInstance, trace *libpf.Trace) { +func (m *hotspotMethod) symbolize(bci uint32, ii *hotspotInstance, frames *libpf.Frames) { // Make sure the BCI is within the method range if bci >= uint32(m.bytecodeSize) { bci = 0 } - // Check if this is already known - frameID := libpf.NewFrameID(m.objectID, libpf.AddressOrLineno(bci)) - trace.AppendFrameID(libpf.HotSpotFrame, frameID) - if !symbolReporter.FrameKnown(frameID) { - dec := ii.d.newUnsigned5Decoder(bytes.NewReader(m.lineTable)) - lineNo := dec.mapByteCodeIndexToLine(bci) - functionOffset := uint32(0) - if lineNo > uint32(m.startLineNo) { - functionOffset = lineNo - uint32(m.startLineNo) - } - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: m.methodName, - SourceFile: m.sourceFileName, - SourceLine: libpf.SourceLineno(lineNo), - FunctionOffset: functionOffset, - }) + dec := ii.d.newUnsigned5Decoder(bytes.NewReader(m.lineTable)) + lineNo := dec.mapByteCodeIndexToLine(bci) + functionOffset := uint32(0) + if lineNo > uint32(m.startLineNo) { + functionOffset = lineNo - uint32(m.startLineNo) } + + frames.Append(&libpf.Frame{ + Type: libpf.HotSpotFrame, + AddressOrLineno: libpf.AddressOrLineno(bci), + FunctionName: m.methodName, + SourceFile: m.sourceFileName, + SourceLine: libpf.SourceLineno(lineNo), + FunctionOffset: functionOffset, + }) } // hotspotJITInfo contains symbolization and debug information for one JIT compiled @@ -74,8 +69,8 @@ type hotspotJITInfo struct { // Symbolize parses JIT method inlining data and fills in symbolization information // for each inlined method for given RIP. -func (ji *hotspotJITInfo) symbolize(symbolReporter reporter.SymbolReporter, ripDelta int32, - ii *hotspotInstance, trace *libpf.Trace) error { +func (ji *hotspotJITInfo) symbolize(ripDelta int32, ii *hotspotInstance, + frames *libpf.Frames) error { //nolint:lll // Unfortunately the data structures read here are not well documented in the JVM // source, but for reference implementation you can look: @@ -110,7 +105,7 @@ func (ji *hotspotJITInfo) symbolize(symbolReporter reporter.SymbolReporter, ripD // It is possible that there is no debug info, or no scope information, // for the given RIP. In this case we can provide the method name // from the metadata. - ji.method.symbolize(symbolReporter, 0, ii, trace) + ji.method.symbolize(0, ii, frames) return nil } @@ -153,7 +148,7 @@ func (ji *hotspotJITInfo) symbolize(symbolReporter reporter.SymbolReporter, ripD if err != nil { return err } - method.symbolize(symbolReporter, byteCodeIndex, ii, trace) + method.symbolize(byteCodeIndex, ii, frames) } } return nil diff --git a/interpreter/instancestubs.go b/interpreter/instancestubs.go index 6fb415c60..31ebfc318 100644 --- a/interpreter/instancestubs.go +++ b/interpreter/instancestubs.go @@ -26,7 +26,7 @@ func (is *InstanceStubs) UpdateTSDInfo(EbpfHandler, libpf.PID, tpbase.TSDInfo) e return nil } -func (is *InstanceStubs) Symbolize(reporter.SymbolReporter, *host.Frame, *libpf.Trace) error { +func (is *InstanceStubs) Symbolize(*host.Frame, *libpf.Frames) error { return ErrMismatchInterpreterType } diff --git a/interpreter/nodev8/v8.go b/interpreter/nodev8/v8.go index 282ea40d8..d131b35a0 100644 --- a/interpreter/nodev8/v8.go +++ b/interpreter/nodev8/v8.go @@ -156,7 +156,6 @@ import ( "bytes" "errors" "fmt" - "hash/fnv" "io" "reflect" "regexp" @@ -197,13 +196,6 @@ const ( // The largest possible identifier for V8 frame type (marker) MaxFrameType = 64 - // The base address for Trace frame addressOrLine of native code. - // This is make sure that if same function gets both bytecode based and - // native frames, that we don't end up generating conflicting symbolization - // for them as native frames use real line number, and bytecode uses - // bytecode offset as the line number. - nativeCodeBaseAddress = 0x100000000 - // The maximum fixed table size we accept to read. An arbitrarily selected // value to avoid huge malloc that could cause OOM crash. maximumFixedTableSize = 512 * 1024 @@ -223,9 +215,6 @@ var ( // regex for the interpreter executable or shared library v8Regex = regexp.MustCompile(`^(?:.*/)?(?:node|nsolid)(\d+)?$|^(?:.*/)libnode\.so(\.\d+)?$`) - // The FileID used for V8 stub frames - v8StubsFileID = libpf.NewStubFileID(libpf.V8Frame) - // the source file entry for unknown code blobs unknownSource = &v8Source{fileName: interpreter.UnknownSourceFile} @@ -489,8 +478,8 @@ type v8Data struct { // bytecodeCount is the number of bytecode opcodes bytecodeCount uint8 - // frametypeToID caches frametype's to a hash used as its identifier - frametypeToID [MaxFrameType]libpf.AddressOrLineno + // frametypeToName caches frametype's name + frametypeToName [MaxFrameType]libpf.String } type v8Instance struct { @@ -542,7 +531,6 @@ type v8SFI struct { bytecodePositionTable []byte bytecode []byte funcName libpf.String - funcID libpf.FileID funcStartLine libpf.SourceLineno funcStartPos int funcEndPos int @@ -730,24 +718,14 @@ func isHeapObject(val libpf.Address) bool { return val&HeapObjectTagMask == HeapObjectTag } -// calculateStubID calculates the hash for a given string. -func calculateStubID(name string) libpf.AddressOrLineno { - h := fnv.New128a() - _, _ = h.Write([]byte(name)) - nameHash := h.Sum(nil) - return libpf.AddressOrLineno(npsr.Uint64(nameHash, 0)) -} - // symbolizeMarkerFrame symbolizes and adds to trace a V8 stub frame -func (i *v8Instance) symbolizeMarkerFrame(symbolReporter reporter.SymbolReporter, marker uint64, - trace *libpf.Trace) error { +func (i *v8Instance) symbolizeMarkerFrame(marker uint64, frames *libpf.Frames) error { if marker >= MaxFrameType { return fmt.Errorf("v8 tracer returned invalid marker: %d", marker) } - stubID := i.d.frametypeToID[marker] - frameID := libpf.NewFrameID(v8StubsFileID, stubID) - if stubID == 0 || !symbolReporter.FrameKnown(frameID) { + stubName := i.d.frametypeToName[marker] + if stubName == libpf.NullString { name := "V8::UnknownFrame" frameTypesType := reflect.TypeOf(&i.d.vmStructs.FrameType).Elem() frameTypesValue := reflect.ValueOf(&i.d.vmStructs.FrameType).Elem() @@ -757,15 +735,14 @@ func (i *v8Instance) symbolizeMarkerFrame(symbolReporter reporter.SymbolReporter break } } - stubID = calculateStubID(name) - frameID = libpf.NewFrameID(v8StubsFileID, stubID) - i.d.frametypeToID[marker] = stubID - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: libpf.Intern(name), - }) + stubName = libpf.Intern(name) + i.d.frametypeToName[marker] = stubName } - trace.AppendFrameID(libpf.V8Frame, frameID) + + frames.Append(&libpf.Frame{ + Type: libpf.V8Frame, + FunctionName: stubName, + }) return nil } @@ -1152,17 +1129,6 @@ func (i *v8Instance) getSFI(taggedPtr libpf.Address) (*v8SFI, error) { taggedPtr, sfi.funcName, sfi.funcStartPos, sfi.funcEndPos, sfi.source.fileName, sfi.funcStartLine, len(sfi.source.lineTable)) - // Synthesize function ID hash - h := fnv.New128a() - _, _ = h.Write([]byte(sfi.source.fileName.String())) - _, _ = h.Write([]byte(sfi.funcName.String())) - _, _ = h.Write(sfi.bytecode) - _, _ = h.Write(sfi.bytecodePositionTable) - sfi.funcID, err = libpf.FileIDFromBytes(h.Sum(nil)) - if err != nil { - return nil, fmt.Errorf("failed to create a function object ID: %v", err) - } - i.addrToSFI.Add(taggedPtr, sfi) return sfi, nil } @@ -1507,15 +1473,14 @@ func (sfi *v8SFI) scriptOffsetToLine(position sourcePosition) libpf.SourceLineno return mapPositionToLine(sfi.source.lineTable, scriptOffset-1) } -// symbolize symbolizes the raw frame data -func (i *v8Instance) symbolize(symbolReporter reporter.SymbolReporter, frameID libpf.FrameID, - sfi *v8SFI, lineNo libpf.SourceLineno) { +// appendFrame adds a new frame to frames. +func (i *v8Instance) appendFrame(frames *libpf.Frames, sfi *v8SFI, lineNo libpf.SourceLineno) { funcOffset := uint32(0) if lineNo > sfi.funcStartLine { funcOffset = uint32(lineNo - sfi.funcStartLine) } - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, + frames.Append(&libpf.Frame{ + Type: libpf.V8Frame, FunctionName: sfi.funcName, SourceFile: sfi.source.fileName, SourceLine: lineNo, @@ -1525,43 +1490,30 @@ func (i *v8Instance) symbolize(symbolReporter reporter.SymbolReporter, frameID l var externalFunctionTag = libpf.Intern("") -var externalStubID = calculateStubID(externalFunctionTag.String()) - // generateNativeFrame and conditionally symbolizes a native frame. -func (i *v8Instance) generateNativeFrame(symbolReporter reporter.SymbolReporter, - sourcePos sourcePosition, sfi *v8SFI, trace *libpf.Trace) { +func (i *v8Instance) generateNativeFrame(sourcePos sourcePosition, sfi *v8SFI, + frames *libpf.Frames) { if sourcePos.isExternal() { - frameID := libpf.NewFrameID(v8StubsFileID, externalStubID) - trace.AppendFrameID(libpf.V8Frame, frameID) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, + frames.Append(&libpf.Frame{ + Type: libpf.V8Frame, FunctionName: externalFunctionTag, }) return } lineNo := sfi.scriptOffsetToLine(sourcePos) - addressOrLineno := libpf.AddressOrLineno(lineNo) + nativeCodeBaseAddress - frameID := libpf.NewFrameID(sfi.funcID, addressOrLineno) - trace.AppendFrameID(libpf.V8Frame, frameID) - i.symbolize(symbolReporter, frameID, sfi, lineNo) + i.appendFrame(frames, sfi, lineNo) } -// insertAndSymbolizeBytecodeFrame symbolizes and records to a trace a Bytecode based frame. -func (i *v8Instance) insertAndSymbolizeBytecodeFrame(symbolReporter reporter.SymbolReporter, - sfi *v8SFI, delta uint64, trace *libpf.Trace) { - frameID := libpf.NewFrameID(sfi.funcID, libpf.AddressOrLineno(delta)) - trace.AppendFrameID(libpf.V8Frame, frameID) - if !symbolReporter.FrameKnown(frameID) { - sourcePos := decodePosition(sfi.bytecodePositionTable, delta) - lineNo := sfi.scriptOffsetToLine(sourcePos) - i.symbolize(symbolReporter, frameID, sfi, lineNo) - } +// appendBytecodeFrame symbolizes and records to a trace a Bytecode based frame. +func (i *v8Instance) appendBytecodeFrame(sfi *v8SFI, delta uint64, frames *libpf.Frames) { + sourcePos := decodePosition(sfi.bytecodePositionTable, delta) + lineNo := sfi.scriptOffsetToLine(sourcePos) + i.appendFrame(frames, sfi, lineNo) } // symbolizeSFI symbolizes and records to a trace a SharedFunctionInfo based frame. -func (i *v8Instance) symbolizeSFI(symbolReporter reporter.SymbolReporter, pointer libpf.Address, - delta uint64, trace *libpf.Trace) error { +func (i *v8Instance) symbolizeSFI(pointer libpf.Address, delta uint64, frames *libpf.Frames) error { vms := &i.d.vmStructs sfi, err := i.getSFI(pointer) if err != nil { @@ -1578,9 +1530,9 @@ func (i *v8Instance) symbolizeSFI(symbolReporter reporter.SymbolReporter, pointe bytecodeDelta = 0 } else if bytecodeDelta >= int64(sfi.bytecodeLength) { // Invalid value - bytecodeDelta = nativeCodeBaseAddress - 1 + bytecodeDelta = 0xffffffff } - i.insertAndSymbolizeBytecodeFrame(symbolReporter, sfi, uint64(bytecodeDelta), trace) + i.appendBytecodeFrame(sfi, uint64(bytecodeDelta), frames) return nil } @@ -1671,32 +1623,31 @@ func (i *v8Instance) mapBaselineCodeOffsetToBytecode(code *v8Code, pcDelta uint3 } // symbolizeBaselineCode symbolizes and records to a trace a Baseline Code based frame. -func (i *v8Instance) symbolizeBaselineCode(symbolReporter reporter.SymbolReporter, code *v8Code, - delta uint32, trace *libpf.Trace) { +func (i *v8Instance) symbolizeBaselineCode(code *v8Code, delta uint32, frames *libpf.Frames) { bytecodeDelta, ok := code.codeDeltaToPosition[delta] if !ok { // Decode bytecode delta and memoize it bytecodeDelta = sourcePosition(i.mapBaselineCodeOffsetToBytecode(code, delta)) code.codeDeltaToPosition[delta] = bytecodeDelta } - i.insertAndSymbolizeBytecodeFrame(symbolReporter, code.sfi, uint64(bytecodeDelta), trace) + i.appendBytecodeFrame(code.sfi, uint64(bytecodeDelta), frames) } // symbolizeCode symbolizes and records to a trace a Code based frame. -func (i *v8Instance) symbolizeCode(symbolReporter reporter.SymbolReporter, code *v8Code, - delta uint64, trace *libpf.Trace) error { +func (i *v8Instance) symbolizeCode(code *v8Code, delta uint64, returnAddress bool, + frames *libpf.Frames) error { var err error sfi := code.sfi delta &= support.V8LineDeltaMask // This is a native PC delta and points to the instruction after // the call function. Adjust to get the CALL instruction. - if len(trace.FrameTypes) > 0 && delta > 0 { + if returnAddress && delta > 0 { delta-- } if code.isBaseline { - i.symbolizeBaselineCode(symbolReporter, code, uint32(delta), trace) + i.symbolizeBaselineCode(code, uint32(delta), frames) return nil } @@ -1729,7 +1680,7 @@ func (i *v8Instance) symbolizeCode(symbolReporter reporter.SymbolReporter, code return fmt.Errorf("failed to get inlined SFI: %w", err) } } - i.generateNativeFrame(symbolReporter, sourcePos, inlinedSFI, trace) + i.generateNativeFrame(sourcePos, inlinedSFI, frames) sourcePos = sourcePosition(npsr.Uint64(code.inliningPositions, itemOff)) if sourcePos.inliningID() > inliningID { @@ -1739,13 +1690,12 @@ func (i *v8Instance) symbolizeCode(symbolReporter reporter.SymbolReporter, code sourcePos.inliningID(), inliningID) } } - i.generateNativeFrame(symbolReporter, sourcePos, sfi, trace) + i.generateNativeFrame(sourcePos, sfi, frames) return nil } -func (i *v8Instance) Symbolize(symbolReporter reporter.SymbolReporter, - frame *host.Frame, trace *libpf.Trace) error { +func (i *v8Instance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { if !frame.Type.IsInterpType(libpf.V8) { return interpreter.ErrMismatchInterpreterType } @@ -1764,9 +1714,9 @@ func (i *v8Instance) Symbolize(symbolReporter reporter.SymbolReporter, // This is a stub V8 frame, with deltaOrMarker containing the marker. // Convert the V8 build specific marker ID to a static ID and symbolize // that if needed. - err = i.symbolizeMarkerFrame(symbolReporter, deltaOrMarker, trace) + err = i.symbolizeMarkerFrame(deltaOrMarker, frames) case support.V8FileTypeByteCode, support.V8FileTypeNativeSFI: - err = i.symbolizeSFI(symbolReporter, pointer, deltaOrMarker, trace) + err = i.symbolizeSFI(pointer, deltaOrMarker, frames) case support.V8FileTypeNativeCode, support.V8FileTypeNativeJSFunc: var code *v8Code codeCookie := uint32(deltaOrMarker & support.V8LineCookieMask >> support.V8LineCookieShift) @@ -1776,7 +1726,7 @@ func (i *v8Instance) Symbolize(symbolReporter reporter.SymbolReporter, code, err = i.getCodeFromJSFunc(pointer, codeCookie) } if err == nil { - err = i.symbolizeCode(symbolReporter, code, deltaOrMarker, trace) + err = i.symbolizeCode(code, deltaOrMarker, frame.ReturnAddress, frames) } default: err = fmt.Errorf("unsupported frame type %#x", frameType) diff --git a/interpreter/perl/instance.go b/interpreter/perl/instance.go index fc7ef1f21..7f8379b90 100644 --- a/interpreter/perl/instance.go +++ b/interpreter/perl/instance.go @@ -6,7 +6,6 @@ package perl // import "go.opentelemetry.io/ebpf-profiler/interpreter/perl" import ( "errors" "fmt" - "hash/fnv" "sync" "sync/atomic" "unsafe" @@ -21,7 +20,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/metrics" npsr "go.opentelemetry.io/ebpf-profiler/nopanicslicereader" "go.opentelemetry.io/ebpf-profiler/remotememory" - "go.opentelemetry.io/ebpf-profiler/reporter" "go.opentelemetry.io/ebpf-profiler/successfailurecounter" "go.opentelemetry.io/ebpf-profiler/support" "go.opentelemetry.io/ebpf-profiler/tpbase" @@ -61,7 +59,6 @@ type perlInstance struct { // perlCOP contains information about Perl Control OPS structure type perlCOP struct { - fileID libpf.FileID sourceFileName libpf.String line libpf.AddressOrLineno } @@ -402,30 +399,15 @@ func (i *perlInstance) getCOP(copAddr libpf.Address, funcName libpf.String) ( line := npsr.Uint32(cop, vms.cop.cop_line) - // Synthesize a FileID. - // The fnv hash Write() method calls cannot fail, so it's safe to ignore the errors. - h := fnv.New128a() - _, _ = h.Write([]byte{uint8(libpf.PerlFrame)}) - _, _ = h.Write([]byte(sourceFileName)) - // Unfortunately there is very little information to extract for each function - // from the GV. Use just the function name at this time. - _, _ = h.Write([]byte(funcName.String())) - fileID, err := libpf.FileIDFromBytes(h.Sum(nil)) - if err != nil { - return nil, fmt.Errorf("failed to create a file ID: %v", err) - } - c := &perlCOP{ sourceFileName: libpf.Intern(sourceFileName), - fileID: fileID, line: libpf.AddressOrLineno(line), } i.addrToCOP.Add(key, c) return c, nil } -func (i *perlInstance) Symbolize(symbolReporter reporter.SymbolReporter, - frame *host.Frame, trace *libpf.Trace) error { +func (i *perlInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { if !frame.Type.IsInterpType(libpf.Perl) { return interpreter.ErrMismatchInterpreterType } @@ -448,10 +430,8 @@ func (i *perlInstance) Symbolize(symbolReporter reporter.SymbolReporter, // Since the COP contains all the data without extra work, just always // send the symbolization information. - frameID := libpf.NewFrameID(cop.fileID, cop.line) - trace.AppendFrameID(libpf.PerlFrame, frameID) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, + frames.Append(&libpf.Frame{ + Type: libpf.PerlFrame, FunctionName: functionName, SourceFile: cop.sourceFileName, SourceLine: libpf.SourceLineno(cop.line), diff --git a/interpreter/php/instance.go b/interpreter/php/instance.go index 5da894146..edf0d8ed2 100644 --- a/interpreter/php/instance.go +++ b/interpreter/php/instance.go @@ -6,7 +6,6 @@ package php // import "go.opentelemetry.io/ebpf-profiler/interpreter/php" import ( "errors" "fmt" - "hash/fnv" "sync/atomic" log "github.com/sirupsen/logrus" @@ -19,7 +18,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/metrics" npsr "go.opentelemetry.io/ebpf-profiler/nopanicslicereader" "go.opentelemetry.io/ebpf-profiler/remotememory" - "go.opentelemetry.io/ebpf-profiler/reporter" "go.opentelemetry.io/ebpf-profiler/successfailurecounter" "go.opentelemetry.io/ebpf-profiler/util" ) @@ -44,9 +42,6 @@ type phpFunction struct { // sourceFileName is the extracted filename field sourceFileName libpf.String - // fileID is the synthesized methodID - fileID libpf.FileID - // lineStart is the first source code line for this function lineStart uint32 } @@ -143,7 +138,6 @@ func (i *phpInstance) getFunction(addr libpf.Address, typeInfo uint32) (*phpFunc sourceFileName := "" lineStart := uint32(0) - var lineBytes []byte switch ftype { case ZEND_USER_FUNCTION, ZEND_EVAL_CODE: sourceAddr := npsr.Ptr(fobj, vms.zend_function.op_array_filename) @@ -163,32 +157,18 @@ func (i *phpInstance) getFunction(addr libpf.Address, typeInfo uint32) (*phpFunc } lineStart = npsr.Uint32(fobj, vms.zend_function.op_array_linestart) - //nolint:lll - lineBytes = fobj[vms.zend_function.op_array_linestart : vms.zend_function.op_array_linestart+8] - } - - // The fnv hash Write() method calls cannot fail, so it's safe to ignore the errors. - h := fnv.New128a() - _, _ = h.Write([]byte(sourceFileName)) - _, _ = h.Write([]byte(functionName.String())) - _, _ = h.Write(lineBytes) - fileID, err := libpf.FileIDFromBytes(h.Sum(nil)) - if err != nil { - return nil, fmt.Errorf("failed to create a file ID: %v", err) } pf := &phpFunction{ name: functionName, sourceFileName: libpf.Intern(sourceFileName), - fileID: fileID, lineStart: lineStart, } i.addrToFunction.Add(addr, pf) return pf, nil } -func (i *phpInstance) Symbolize(symbolReporter reporter.SymbolReporter, - frame *host.Frame, trace *libpf.Trace) error { +func (i *phpInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { // With Symbolize() in opcacheInstance there is a dedicated function to symbolize JITTed // PHP frames. But as we also attach phpInstance to PHP processes with JITTed frames, we // use this function to symbolize all PHP frames, as the process to do so is the same. @@ -214,10 +194,9 @@ func (i *phpInstance) Symbolize(symbolReporter reporter.SymbolReporter, if f.lineStart != 0 && libpf.AddressOrLineno(f.lineStart) <= line { funcOff = uint32(line) - f.lineStart } - frameID := libpf.NewFrameID(f.fileID, line) - trace.AppendFrameID(libpf.PHPFrame, frameID) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, + + frames.Append(&libpf.Frame{ + Type: libpf.PHPFrame, FunctionName: f.name, SourceFile: f.sourceFileName, SourceLine: libpf.SourceLineno(line), diff --git a/interpreter/python/python.go b/interpreter/python/python.go index ae10c6f73..8ab517828 100644 --- a/interpreter/python/python.go +++ b/interpreter/python/python.go @@ -9,7 +9,6 @@ import ( "encoding/hex" "errors" "fmt" - "hash/fnv" "io" "reflect" "regexp" @@ -32,7 +31,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/metrics" npsr "go.opentelemetry.io/ebpf-profiler/nopanicslicereader" "go.opentelemetry.io/ebpf-profiler/remotememory" - "go.opentelemetry.io/ebpf-profiler/reporter" "go.opentelemetry.io/ebpf-profiler/successfailurecounter" "go.opentelemetry.io/ebpf-profiler/support" "go.opentelemetry.io/ebpf-profiler/tpbase" @@ -176,11 +174,6 @@ type pythonCodeObject struct { // ebpfChecksum is the simple hash of few PyCodeObject fields sent from eBPF // to verify that the data we extracted from remote process is still valid ebpfChecksum uint32 - - // fileID is a more complete hash of various PyCodeObject fields, which is - // used as the global ID of the PyCodeObject. It is stored as the FileID - // part of the Frame in the DB. - fileID libpf.FileID } // readVarint returns a variable length encoded unsigned integer from a location table entry. @@ -317,23 +310,6 @@ func mapByteCodeIndexToLine(m *pythonCodeObject, bci uint32) uint32 { return lineno } -func (m *pythonCodeObject) symbolize(symbolReporter reporter.SymbolReporter, bci uint32, - getFuncOffset getFuncOffsetFunc, trace *libpf.Trace) { - frameID := libpf.NewFrameID(m.fileID, libpf.AddressOrLineno(bci)) - trace.AppendFrameID(libpf.PythonFrame, frameID) - if !symbolReporter.FrameKnown(frameID) { - functionOffset := getFuncOffset(m, bci) - lineNo := libpf.SourceLineno(m.firstLineNo + functionOffset) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: m.name, - SourceFile: m.sourceFileName, - SourceLine: lineNo, - FunctionOffset: functionOffset, - }) - } -} - // getFuncOffsetFunc provides functionality to return a function offset from a PyCodeObject type getFuncOffsetFunc func(m *pythonCodeObject, bci uint32) uint32 @@ -551,19 +527,6 @@ func (p *pythonInstance) getCodeObject(addr libpf.Address, return nil, fmt.Errorf("failed to read line table: %v", err) } - // The fnv hash Write() method calls cannot fail, so it's safe to ignore the errors. - h := fnv.New128a() - _, _ = h.Write([]byte(sourceFileName)) - _, _ = h.Write([]byte(name)) - _, _ = h.Write(cobj[vms.PyCodeObject.FirstLineno : vms.PyCodeObject.FirstLineno+4]) - _, _ = h.Write(cobj[vms.PyCodeObject.ArgCount : vms.PyCodeObject.ArgCount+4]) - _, _ = h.Write(cobj[vms.PyCodeObject.KwOnlyArgCount : vms.PyCodeObject.KwOnlyArgCount+4]) - _, _ = h.Write(lineTable) - fileID, err := libpf.FileIDFromBytes(h.Sum(nil)) - if err != nil { - return nil, fmt.Errorf("failed to create a file ID: %v", err) - } - pco := &pythonCodeObject{ version: p.d.version, name: libpf.Intern(name), @@ -571,14 +534,12 @@ func (p *pythonInstance) getCodeObject(addr libpf.Address, firstLineNo: firstLineNo, lineTable: lineTable, ebpfChecksum: ebpfChecksum, - fileID: fileID, } p.addrToCodeObject.Add(addr, pco) return pco, nil } -func (p *pythonInstance) Symbolize(symbolReporter reporter.SymbolReporter, - frame *host.Frame, trace *libpf.Trace) error { +func (p *pythonInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { if !frame.Type.IsInterpType(libpf.Python) { return interpreter.ErrMismatchInterpreterType } @@ -592,11 +553,20 @@ func (p *pythonInstance) Symbolize(symbolReporter reporter.SymbolReporter, defer sfCounter.DefaultToFailure() // Extract and symbolize - method, err := p.getCodeObject(ptr, objectID) + m, err := p.getCodeObject(ptr, objectID) if err != nil { return fmt.Errorf("failed to get python object %x: %v", objectID, err) } - method.symbolize(symbolReporter, lastI, p.getFuncOffset, trace) + + functionOffset := p.getFuncOffset(m, lastI) + frames.Append(&libpf.Frame{ + Type: libpf.PythonFrame, + FunctionName: m.name, + SourceFile: m.sourceFileName, + SourceLine: libpf.SourceLineno(m.firstLineNo + functionOffset), + FunctionOffset: functionOffset, + }) + sfCounter.ReportSuccess() return nil } diff --git a/interpreter/ruby/ruby.go b/interpreter/ruby/ruby.go index 3a33fa69f..e3464353c 100644 --- a/interpreter/ruby/ruby.go +++ b/interpreter/ruby/ruby.go @@ -7,7 +7,6 @@ import ( "encoding/binary" "errors" "fmt" - "hash/fnv" "math/bits" "regexp" "runtime" @@ -28,7 +27,6 @@ import ( "go.opentelemetry.io/ebpf-profiler/libpf/pfelf" "go.opentelemetry.io/ebpf-profiler/metrics" "go.opentelemetry.io/ebpf-profiler/remotememory" - "go.opentelemetry.io/ebpf-profiler/reporter" "go.opentelemetry.io/ebpf-profiler/successfailurecounter" "go.opentelemetry.io/ebpf-profiler/support" "go.opentelemetry.io/ebpf-profiler/util" @@ -253,11 +251,11 @@ type rubyIseq struct { // sourceFileName is the extracted filename field sourceFileName libpf.String - // fileID is the synthesized methodID - fileID libpf.FileID + // functionName is the function name for this sequence + functionName libpf.String // line of code in source file for this instruction sequence - line libpf.AddressOrLineno + line libpf.SourceLineno } type rubyInstance struct { @@ -413,13 +411,6 @@ func immBlockRankGet(v uint64, i uint32) uint32 { return uint32(tmp) & 0x7f } -// uint64ToBytes is a helper function to convert an uint64 into its []byte representation. -func uint64ToBytes(val uint64) []byte { - b := make([]byte, 8) - binary.LittleEndian.PutUint64(b, val) - return b -} - // getObsoleteRubyLineNo implements a binary search algorithm to get the line number for a position. // // Implementation according to Ruby: @@ -638,8 +629,7 @@ func (r *rubyInstance) getRubyLineNo(iseqBody libpf.Address, pc uint64) (uint32, return lineNo, nil } -func (r *rubyInstance) Symbolize(symbolReporter reporter.SymbolReporter, - frame *host.Frame, trace *libpf.Trace) error { +func (r *rubyInstance) Symbolize(frame *host.Frame, frames *libpf.Frames) error { if !frame.Type.IsInterpType(libpf.Ruby) { return interpreter.ErrMismatchInterpreterType } @@ -663,62 +653,42 @@ func (r *rubyInstance) Symbolize(symbolReporter reporter.SymbolReporter, pc: uint64(pc), } - if iseq, ok := r.iseqBodyPCToFunction.Get(key); ok && - symbolReporter.FrameKnown(libpf.NewFrameID(iseq.fileID, iseq.line)) { - trace.AppendFrame(libpf.RubyFrame, iseq.fileID, iseq.line) - sfCounter.ReportSuccess() - return nil - } - - lineNo, err := r.getRubyLineNo(iseqBody, uint64(pc)) - if err != nil { - return err - } - - sourceFileNamePtr := r.rm.Ptr(iseqBody + - libpf.Address(vms.iseq_constant_body.location+vms.iseq_location_struct.pathobj)) - sourceFileName, err := r.getStringCached(sourceFileNamePtr, r.readPathObjRealPath) - if err != nil { - return err - } - - funcNamePtr := r.rm.Ptr(iseqBody + - libpf.Address(vms.iseq_constant_body.location+vms.iseq_location_struct.base_label)) - functionName, err := r.getStringCached(funcNamePtr, r.readRubyString) - if err != nil { - return err - } + iseq, ok := r.iseqBodyPCToFunction.Get(key) + if !ok { + lineNo, err := r.getRubyLineNo(iseqBody, uint64(pc)) + if err != nil { + return err + } - pcBytes := uint64ToBytes(uint64(pc)) - iseqBodyBytes := uint64ToBytes(uint64(iseqBody)) + sourceFileNamePtr := r.rm.Ptr(iseqBody + + libpf.Address(vms.iseq_constant_body.location+vms.iseq_location_struct.pathobj)) + sourceFileName, err := r.getStringCached(sourceFileNamePtr, r.readPathObjRealPath) + if err != nil { + return err + } - // The fnv hash Write() method calls cannot fail, so it's safe to ignore the errors. - h := fnv.New128a() - _, _ = h.Write([]byte(sourceFileName.String())) - _, _ = h.Write([]byte(functionName.String())) - _, _ = h.Write(pcBytes) - _, _ = h.Write(iseqBodyBytes) - fileID, err := libpf.FileIDFromBytes(h.Sum(nil)) - if err != nil { - return fmt.Errorf("failed to create a file ID: %v", err) - } + funcNamePtr := r.rm.Ptr(iseqBody + + libpf.Address(vms.iseq_constant_body.location+vms.iseq_location_struct.base_label)) + functionName, err := r.getStringCached(funcNamePtr, r.readRubyString) + if err != nil { + return err + } - iseq := &rubyIseq{ - sourceFileName: sourceFileName, - fileID: fileID, - line: libpf.AddressOrLineno(lineNo), + iseq = &rubyIseq{ + functionName: functionName, + sourceFileName: sourceFileName, + line: libpf.SourceLineno(lineNo), + } + r.iseqBodyPCToFunction.Add(key, iseq) } - r.iseqBodyPCToFunction.Add(key, iseq) // Ruby doesn't provide the information about the function offset for the // particular line. So we report 0 for this to our backend. - frameID := libpf.NewFrameID(fileID, libpf.AddressOrLineno(lineNo)) - trace.AppendFrameID(libpf.RubyFrame, frameID) - symbolReporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: functionName, - SourceFile: sourceFileName, - SourceLine: libpf.SourceLineno(lineNo), + frames.Append(&libpf.Frame{ + Type: libpf.RubyFrame, + FunctionName: iseq.functionName, + SourceFile: iseq.sourceFileName, + SourceLine: iseq.line, }) sfCounter.ReportSuccess() return nil diff --git a/interpreter/types.go b/interpreter/types.go index 2e572b667..f1b944aab 100644 --- a/interpreter/types.go +++ b/interpreter/types.go @@ -147,11 +147,9 @@ type Instance interface { // introspection data has been updated. UpdateTSDInfo(ebpf EbpfHandler, pid libpf.PID, info tpbase.TSDInfo) error - // Symbolize requests symbolization of the given frame, and dispatches this symbolization - // to the collection agent. The frame's contents (frame type, file ID and line number) - // are appended to newTrace. - Symbolize(symbolReporter reporter.SymbolReporter, frame *host.Frame, - trace *libpf.Trace) error + // Symbolize converts one ebpf frame to one or more (if inlining was expanded) libpf.Frame. + // The resulting libpf.Frame values are appended to frames. + Symbolize(ebpfFrame *host.Frame, frames *libpf.Frames) error // GetAndResetMetrics collects the metrics from the Instance and resets // the counters to their initial value. diff --git a/libpf/fileid.go b/libpf/fileid.go index 19bd15272..f58a07a92 100644 --- a/libpf/fileid.go +++ b/libpf/fileid.go @@ -34,11 +34,6 @@ func NewFileID(hi, lo uint64) FileID { return FileID{basehash.New128(hi, lo)} } -// NewStubFileID returns a FrameType specific stub FileID. -func NewStubFileID(typ FrameType) FileID { - return FileID{basehash.New128(0x578b, uint64(0x1d00|typ))} -} - // FileIDFromBytes parses a byte slice into the internal data representation for a file ID. func FileIDFromBytes(b []byte) (FileID, error) { // We need to check for nil since byte slice fields in protobuf messages can be optional. diff --git a/libpf/frameid.go b/libpf/frameid.go deleted file mode 100644 index 9c0e38ac5..000000000 --- a/libpf/frameid.go +++ /dev/null @@ -1,122 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package libpf // import "go.opentelemetry.io/ebpf-profiler/libpf" - -import ( - "encoding/base64" - "encoding/binary" - "fmt" - "net" - - "github.com/zeebo/xxh3" -) - -// FrameID represents a frame as an address in an executable file -// or as a line in a source code file. -type FrameID struct { - // fileID is the fileID of the frame - fileID FileID - - // addressOrLineno is the address or lineno of the frame - addressOrLineno AddressOrLineno -} - -// NewFrameID creates a new FrameID from the fileId and address or line. -func NewFrameID(fileID FileID, addressOrLineno AddressOrLineno) FrameID { - return FrameID{ - fileID: fileID, - addressOrLineno: addressOrLineno, - } -} - -// NewFrameIDFromString creates a new FrameID from its base64 string representation. -func NewFrameIDFromString(frameEncoded string) (FrameID, error) { - var frameID FrameID - - bytes, err := base64.RawURLEncoding.DecodeString(frameEncoded) - if err != nil { - return frameID, fmt.Errorf("failed to decode frameID %v: %v", frameEncoded, err) - } - - return NewFrameIDFromBytes(bytes) -} - -// NewFrameIDFromBytes creates a new FrameID from a byte array of length 24. -func NewFrameIDFromBytes(bytes []byte) (FrameID, error) { - var frameID FrameID - var err error - - if len(bytes) != 24 { - return frameID, fmt.Errorf("unexpected frameID size (expected 24 bytes): %d", - len(bytes)) - } - - if frameID.fileID, err = FileIDFromBytes(bytes[0:16]); err != nil { - return frameID, fmt.Errorf("failed to create fileID from bytes: %v", err) - } - - frameID.addressOrLineno = AddressOrLineno(binary.BigEndian.Uint64(bytes[16:24])) - - return frameID, nil -} - -// Hash32 returns a 32 bits hash of the input. -// It's main purpose is to be used as key for caching. -func (f FrameID) Hash32() uint32 { - return uint32(f.Hash()) -} - -// String returns the base64 encoded representation. -func (f FrameID) String() string { - return base64.RawURLEncoding.EncodeToString(f.Bytes()) -} - -// EncodeTo encodes the frame ID into the base64 encoded representation -// and stores it in the provided destination byte array. -// The length of the destination must be at least EncodedLen(). -func (f FrameID) EncodeTo(dst []byte) { - base64.RawURLEncoding.Encode(dst, f.Bytes()) -} - -// EncodedLen returns the length of the FrameID's base64 representation. -func (FrameID) EncodedLen() int { - // FrameID is 24 bytes long, the base64 representation is one base64 byte per 6 bits. - return ((16 + 8) * 8) / 6 -} - -// Bytes returns the frameid as byte sequence. -func (f FrameID) Bytes() []byte { - // Using frameID := make([byte, 24]) here makes the function ~5% slower. - var frameID [24]byte - - copy(frameID[:], f.fileID.Bytes()) - binary.BigEndian.PutUint64(frameID[16:], uint64(f.addressOrLineno)) - return frameID[:] -} - -// Hash calculates a hash from the frameid. -// xxh3 is 4x faster than fnv. -func (f FrameID) Hash() uint64 { - return xxh3.Hash(f.Bytes()) -} - -// FileID returns the fileID part of the frameID. -func (f FrameID) FileID() FileID { - return f.fileID -} - -// AddressOrLine returns the addressOrLine part of the frameID. -func (f FrameID) AddressOrLine() AddressOrLineno { - return f.addressOrLineno -} - -// AsIP returns the FrameID as a net.IP type to be used -// for the PC range in profiling-symbols-*. -func (f FrameID) AsIP() net.IP { - bytes := f.Bytes() - ip := make([]byte, 16) - copy(ip[:8], bytes[:8]) // first 64bits of FileID - copy(ip[8:], bytes[16:]) // addressOrLine - return ip -} diff --git a/libpf/frameid_test.go b/libpf/frameid_test.go deleted file mode 100644 index d1f7587e3..000000000 --- a/libpf/frameid_test.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package libpf - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -const ( - fileIDLo = 0x77efa716a912a492 - fileIDHi = 0x17445787329fd29a - addressOrLine = 0xe51c -) - -func TestFrameID(t *testing.T) { - var fileID = NewFileID(fileIDLo, fileIDHi) - - tests := []struct { - name string - input string - expected FrameID - bytes []byte - err error - }{ - { - name: "frame base64", - input: "d--nFqkSpJIXRFeHMp_SmgAAAAAAAOUc", - expected: NewFrameID(fileID, addressOrLine), - bytes: []byte{ - 0x77, 0xef, 0xa7, 0x16, 0xa9, 0x12, 0xa4, 0x92, 0x17, 0x44, 0x57, 0x87, - 0x32, 0x9f, 0xd2, 0x9a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe5, 0x1c, - }, - err: nil, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - frameID, err := NewFrameIDFromString(test.input) - assert.Equal(t, test.err, err) - assert.Equal(t, test.expected, frameID) - - // check if the roundtrip back to the input works - assert.Equal(t, test.input, frameID.String()) - - assert.Equal(t, test.bytes, frameID.Bytes()) - - frameID, err = NewFrameIDFromBytes(frameID.Bytes()) - assert.Equal(t, test.err, err) - assert.Equal(t, test.expected, frameID) - - ip := []byte(frameID.AsIP()) - bytes := frameID.Bytes() - assert.Equal(t, bytes[:8], ip[:8]) - assert.Equal(t, bytes[16:], ip[8:]) - }) - } -} diff --git a/libpf/libpf.go b/libpf/libpf.go index b9b6884d9..a234f04ef 100644 --- a/libpf/libpf.go +++ b/libpf/libpf.go @@ -37,15 +37,6 @@ type UnixTime64 uint64 // a native file. type AddressOrLineno uint64 -type FrameMetadata struct { - FileID FileID - AddressOrLine AddressOrLineno - LineNumber SourceLineno - FunctionOffset uint32 - FunctionName string - Filename string -} - // Void allows to use maps as sets without memory allocation for the values. // From the "Go Programming Language": // diff --git a/libpf/trace.go b/libpf/trace.go index dc997eb56..bbae7c8d5 100644 --- a/libpf/trace.go +++ b/libpf/trace.go @@ -3,18 +3,46 @@ package libpf // import "go.opentelemetry.io/ebpf-profiler/libpf" -// Trace represents a stack trace. Each tuple (Files[i], Linenos[i]) represents a -// stack frame via the file ID and line number at the offset i in the trace. The -// information for the most recently called function is at offset 0. +import ( + "unique" +) + +// Frame represents one frame in a stack trace. +type Frame struct { + // Type is the frame type. + Type FrameType + // FunctionOffset is the line offset from function start line for the frame. + FunctionOffset uint32 + // FunctionName is the name of the function for the frame. + FunctionName String + // SourceFile is the source code file name for the frame. + SourceFile String + // SourceLine is the source code level line number of this frame. + SourceLine SourceLineno + + // Calculated executable FileID for the backing mapping file. + FileID FileID + // An address in ELF VA space (native frame) or line number (interpreted frame). + AddressOrLineno AddressOrLineno + + MappingStart Address + MappingEnd Address + MappingFileOffset uint64 +} + +// Frames is a list of interned frames. +type Frames []unique.Handle[Frame] + +// Append interns and appends a frame to the slice of frames. +func (frames *Frames) Append(frame *Frame) { + *frames = append(*frames, unique.Make(*frame)) +} + +// Trace represents a stack trace. type Trace struct { - Files []FileID - Linenos []AddressOrLineno - FrameTypes []FrameType - MappingStart []Address - MappingEnd []Address - MappingFileOffsets []uint64 - Hash TraceHash - CustomLabels map[string]string + Frames Frames + Hash TraceHash + CustomLabels map[string]string } // AppendFrame appends a frame to the columnar frame array without mapping information. @@ -22,18 +50,15 @@ func (trace *Trace) AppendFrame(ty FrameType, file FileID, addrOrLine AddressOrL trace.AppendFrameFull(ty, file, addrOrLine, 0, 0, 0) } -// AppendFrameID appends a frame to the columnar frame array without mapping information. -func (trace *Trace) AppendFrameID(ty FrameType, frameID FrameID) { - trace.AppendFrameFull(ty, frameID.FileID(), frameID.AddressOrLine(), 0, 0, 0) -} - // AppendFrameFull appends a frame with mapping info to the columnar frame array. func (trace *Trace) AppendFrameFull(ty FrameType, file FileID, addrOrLine AddressOrLineno, mappingStart Address, mappingEnd Address, mappingFileOffset uint64) { - trace.FrameTypes = append(trace.FrameTypes, ty) - trace.Files = append(trace.Files, file) - trace.Linenos = append(trace.Linenos, addrOrLine) - trace.MappingStart = append(trace.MappingStart, mappingStart) - trace.MappingEnd = append(trace.MappingEnd, mappingEnd) - trace.MappingFileOffsets = append(trace.MappingFileOffsets, mappingFileOffset) + trace.Frames.Append(&Frame{ + Type: ty, + FileID: file, + AddressOrLineno: addrOrLine, + MappingStart: mappingStart, + MappingEnd: mappingEnd, + MappingFileOffset: mappingFileOffset, + }) } diff --git a/main.go b/main.go index 995dea415..8e027ad6e 100644 --- a/main.go +++ b/main.go @@ -113,9 +113,7 @@ func mainWithExitCode() exitCode { GRPCConnectionTimeout: intervals.GRPCConnectionTimeout(), ReportInterval: intervals.ReportInterval(), ExecutablesCacheElements: 16384, - // Next step: Calculate FramesCacheElements from numCores and samplingRate. - FramesCacheElements: 131072, - SamplesPerSecond: cfg.SamplesPerSecond, + SamplesPerSecond: cfg.SamplesPerSecond, }) if err != nil { log.Error(err) diff --git a/processmanager/manager.go b/processmanager/manager.go index fc0fbe610..78865e071 100644 --- a/processmanager/manager.go +++ b/processmanager/manager.go @@ -192,8 +192,7 @@ func collectInterpreterMetrics(ctx context.Context, pm *ProcessManager, func (pm *ProcessManager) Close() { } -func (pm *ProcessManager) symbolizeFrame(frame int, trace *host.Trace, - newTrace *libpf.Trace) error { +func (pm *ProcessManager) symbolizeFrame(frame int, trace *host.Trace, frames *libpf.Frames) error { pm.mu.Lock() defer pm.mu.Unlock() @@ -202,7 +201,7 @@ func (pm *ProcessManager) symbolizeFrame(frame int, trace *host.Trace, } for _, instance := range pm.interpreters[trace.PID] { - if err := instance.Symbolize(pm.reporter, &trace.Frames[frame], newTrace); err != nil { + if err := instance.Symbolize(&trace.Frames[frame], frames); err != nil { if errors.Is(err, interpreter.ErrMismatchInterpreterType) { // The interpreter type of instance did not match the type of frame. // So continue with the next interpreter instance for this PID. @@ -219,13 +218,12 @@ func (pm *ProcessManager) symbolizeFrame(frame int, trace *host.Trace, func (pm *ProcessManager) ConvertTrace(trace *host.Trace) (newTrace *libpf.Trace) { traceLen := len(trace.Frames) - + kernelFramesLen := len(trace.KernelFrames) newTrace = &libpf.Trace{ - Files: make([]libpf.FileID, 0, traceLen), - Linenos: make([]libpf.AddressOrLineno, 0, traceLen), - FrameTypes: make([]libpf.FrameType, 0, traceLen), + Frames: make(libpf.Frames, kernelFramesLen, kernelFramesLen+traceLen), CustomLabels: trace.CustomLabels, } + copy(newTrace.Frames, trace.KernelFrames) for i := range traceLen { frame := &trace.Frames[i] @@ -274,7 +272,7 @@ func (pm *ProcessManager) ConvertTrace(trace *host.Trace) (newTrace *libpf.Trace // Attempt symbolization of native frames. It is best effort and // provides non-symbolized frames if no native symbolizer is active. - if err := pm.symbolizeFrame(i, trace, newTrace); err == nil { + if err := pm.symbolizeFrame(i, trace, &newTrace.Frames); err == nil { continue } @@ -292,7 +290,7 @@ func (pm *ProcessManager) ConvertTrace(trace *host.Trace) (newTrace *libpf.Trace newTrace.AppendFrameFull(frame.Type, fileID, relativeRIP, mappingStart, mappingEnd, fileOffset) default: - err := pm.symbolizeFrame(i, trace, newTrace) + err := pm.symbolizeFrame(i, trace, &newTrace.Frames) if err != nil { log.Debugf( "symbolization failed for PID %d, frame %d/%d, frame type %d: %v", diff --git a/processmanager/manager_test.go b/processmanager/manager_test.go index 50a85e418..d4dd91ab6 100644 --- a/processmanager/manager_test.go +++ b/processmanager/manager_test.go @@ -254,8 +254,6 @@ func (mockup *ebpfMapsMockup) SupportsLPMTrieBatchOperations() bool { return fal type symbolReporterMockup struct{} -func (s *symbolReporterMockup) ReportFallbackSymbol(_ libpf.FrameID, _ string) {} - func (s *symbolReporterMockup) ExecutableKnown(_ libpf.FileID) bool { return true } @@ -263,13 +261,6 @@ func (s *symbolReporterMockup) ExecutableKnown(_ libpf.FileID) bool { func (s *symbolReporterMockup) ExecutableMetadata(_ *reporter.ExecutableMetadataArgs) { } -func (s *symbolReporterMockup) FrameKnown(_ libpf.FrameID) bool { - return true -} - -func (s *symbolReporterMockup) FrameMetadata(_ *reporter.FrameMetadataArgs) { -} - var _ reporter.SymbolReporter = (*symbolReporterMockup)(nil) func TestInterpreterConvertTrace(t *testing.T) { @@ -303,8 +294,8 @@ func TestInterpreterConvertTrace(t *testing.T) { for name, testcase := range tests { t.Run(name, func(t *testing.T) { mapper := NewMapFileIDMapper() - for i := range testcase.trace.Frames { - mapper.Set(testcase.trace.Frames[i].File, testcase.expect.Files[i]) + for i, f := range testcase.trace.Frames { + mapper.Set(f.File, testcase.expect.Frames[i].Value().FileID) } // For this test do not include interpreters. @@ -329,8 +320,7 @@ func TestInterpreterConvertTrace(t *testing.T) { testcase.expect.Hash = traceutil.HashTrace(testcase.expect) if testcase.expect.Hash == newTrace.Hash { - assert.Equal(t, testcase.expect.Linenos, newTrace.Linenos) - assert.Equal(t, testcase.expect.Files, newTrace.Files) + assert.Equal(t, testcase.expect, newTrace) } }) } @@ -344,18 +334,13 @@ func getExpectedTrace(origTrace *host.Trace, linenos []libpf.AddressOrLineno) *l Hash: libpf.NewTraceHash(uint64(origTrace.Hash), uint64(origTrace.Hash)), } - for _, frame := range origTrace.Frames { - newTrace.Files = append(newTrace.Files, libpf.NewFileID(uint64(frame.File), 0)) - newTrace.FrameTypes = append(newTrace.FrameTypes, frame.Type) - if linenos == nil { - newTrace.Linenos = append(newTrace.Linenos, frame.Lineno) + for i, frame := range origTrace.Frames { + lineno := frame.Lineno + if linenos != nil { + lineno = linenos[i] } + newTrace.AppendFrame(frame.Type, libpf.NewFileID(uint64(frame.File), 0), lineno) } - - if linenos != nil { - newTrace.Linenos = linenos - } - return newTrace } diff --git a/reporter/base_reporter.go b/reporter/base_reporter.go index 24f0ff967..aa380fafc 100644 --- a/reporter/base_reporter.go +++ b/reporter/base_reporter.go @@ -10,7 +10,6 @@ import ( "time" lru "github.com/elastic/go-freelru" - log "github.com/sirupsen/logrus" "go.opentelemetry.io/ebpf-profiler/libpf" "go.opentelemetry.io/ebpf-profiler/libpf/xsync" "go.opentelemetry.io/ebpf-profiler/reporter/internal/pdata" @@ -69,11 +68,6 @@ func (b *baseReporter) ExecutableKnown(fileID libpf.FileID) bool { return known } -func (b *baseReporter) FrameKnown(frameID libpf.FrameID) bool { - _, known := b.pdata.Frames.GetAndRefresh(frameID, pdata.FrameMapLifetime) - return known -} - func (b *baseReporter) ExecutableMetadata(args *ExecutableMetadataArgs) { b.pdata.Executables.Add(args.FileID, samples.ExecInfo{ FileName: args.FileName, @@ -126,28 +120,10 @@ func (b *baseReporter) ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceE return nil } (*eventsTree)[samples.ContainerID(containerID)][meta.Origin][key] = &samples.TraceEvents{ - Files: trace.Files, - Linenos: trace.Linenos, - FrameTypes: trace.FrameTypes, - MappingStarts: trace.MappingStart, - MappingEnds: trace.MappingEnd, - MappingFileOffsets: trace.MappingFileOffsets, - Timestamps: []uint64{uint64(meta.Timestamp)}, - OffTimes: []int64{meta.OffTime}, - EnvVars: meta.EnvVars, + Frames: trace.Frames, + Timestamps: []uint64{uint64(meta.Timestamp)}, + OffTimes: []int64{meta.OffTime}, + EnvVars: meta.EnvVars, } return nil } - -func (b *baseReporter) FrameMetadata(args *FrameMetadataArgs) { - log.Debugf("FrameMetadata [%x] %v+%v at %v:%v", - args.FrameID.FileID(), args.FunctionName, args.FunctionOffset, - args.SourceFile, args.SourceLine) - si := samples.SourceInfo{ - LineNumber: args.SourceLine, - FilePath: args.SourceFile, - FunctionOffset: args.FunctionOffset, - FunctionName: args.FunctionName, - } - b.pdata.Frames.Add(args.FrameID, si) -} diff --git a/reporter/collector_reporter.go b/reporter/collector_reporter.go index 6983bd0ec..050fd354a 100644 --- a/reporter/collector_reporter.go +++ b/reporter/collector_reporter.go @@ -39,7 +39,6 @@ func NewCollector(cfg *Config, nextConsumer xconsumer.Profiles) (*CollectorRepor data, err := pdata.New( cfg.SamplesPerSecond, cfg.ExecutablesCacheElements, - cfg.FramesCacheElements, cfg.ExtraSampleAttrProd, ) if err != nil { diff --git a/reporter/collector_reporter_test.go b/reporter/collector_reporter_test.go index 7a8b2d2e8..f7d231eb9 100644 --- a/reporter/collector_reporter_test.go +++ b/reporter/collector_reporter_test.go @@ -53,7 +53,6 @@ func TestCollectorReporterReportTraceEvent(t *testing.T) { r, err := NewCollector(&Config{ ExecutablesCacheElements: 1, - FramesCacheElements: 1, }, next) require.NoError(t, err) if err := r.ReportTraceEvent(tt.trace, tt.meta); err != nil && diff --git a/reporter/config.go b/reporter/config.go index ae05eb57d..52f41c8d5 100644 --- a/reporter/config.go +++ b/reporter/config.go @@ -28,8 +28,6 @@ type Config struct { DisableTLS bool // ExecutablesCacheElements defines item capacity of the executables cache. ExecutablesCacheElements uint32 - // FramesCacheElements defines the item capacity of the frames cache. - FramesCacheElements uint32 // samplesPerSecond defines the number of samples per second. SamplesPerSecond int diff --git a/reporter/iface.go b/reporter/iface.go index 02267ee29..cf56a5b80 100644 --- a/reporter/iface.go +++ b/reporter/iface.go @@ -59,21 +59,6 @@ type ExecutableMetadataArgs struct { Open ExecutableOpener } -// FrameMetadataArgs collects metadata about a single frame in a trace, for -// reporting it to a SymbolReporter via the FrameMetadata method. -type FrameMetadataArgs struct { - // FrameID is a unique identifier for the frame. - FrameID libpf.FrameID - // FunctionName is the name of the function for the frame. - FunctionName libpf.String - // SourceFile is the source code file name for the frame. - SourceFile libpf.String - // SourceLine is the source code level line number of this frame. - SourceLine libpf.SourceLineno - // FunctionOffset is the line offset from function start line for the frame. - FunctionOffset uint32 -} - type SymbolReporter interface { // ExecutableKnown may be used to query the reporter if the FileID is known. // The callers of ExecutableMetadata can optionally use this method to determine if the data @@ -91,17 +76,6 @@ type SymbolReporter interface { // wish to upload executables should NOT block this function to do so and instead just // open the file and then enqueue the upload in the background. ExecutableMetadata(args *ExecutableMetadataArgs) - - // FrameKnown may be used to query the reporter if the FrameID is known. The interpreter - // modules can optionally use this method to determine if the data is already cached - // and avoid extra work resolving the metadata. If the reporter returns false, - // the interpreter plugin will resolve the frame metadata and submit it to the reporter - // via a subsequent FrameMetadata call. - FrameKnown(frameID libpf.FrameID) bool - - // FrameMetadata accepts metadata associated with a frame and caches this information before - // a periodic reporting to the backend. - FrameMetadata(frameMetadata *FrameMetadataArgs) } type HostMetadataReporter interface { diff --git a/reporter/internal/pdata/generate.go b/reporter/internal/pdata/generate.go index ee2e1b1bc..7d583c70d 100644 --- a/reporter/internal/pdata/generate.go +++ b/reporter/internal/pdata/generate.go @@ -23,8 +23,6 @@ import ( const ( ExecutableCacheLifetime = 1 * time.Hour - FramesCacheLifetime = 1 * time.Hour - FrameMapLifetime = 1 * time.Hour ) // DummyFileID is used as the FileID for a dummy mapping @@ -154,18 +152,19 @@ func (p *Pdata) setProfile( } // Walk every frame of the trace. - for i := range traceInfo.FrameTypes { + for _, uniqueFrame := range traceInfo.Frames { + frame := uniqueFrame.Value() locInfo := locationInfo{ - address: uint64(traceInfo.Linenos[i]), - frameType: traceInfo.FrameTypes[i].String(), + address: uint64(frame.AddressOrLineno), + frameType: frame.Type.String(), } - switch frameKind := traceInfo.FrameTypes[i]; frameKind { + switch frameKind := frame.Type; frameKind { case libpf.NativeFrame: // As native frames are resolved in the backend, we use Mapping to // report these frames. - locationMappingIndex, exists := mappingSet.AddWithCheck(traceInfo.Files[i]) + locationMappingIndex, exists := mappingSet.AddWithCheck(frame.FileID) if !exists { - ei, exists := p.Executables.GetAndRefresh(traceInfo.Files[i], + ei, exists := p.Executables.GetAndRefresh(frame.FileID, ExecutableCacheLifetime) // Next step: Select a proper default value, // if the name of the executable is not known yet. @@ -175,9 +174,9 @@ func (p *Pdata) setProfile( } mapping := dic.MappingTable().AppendEmpty() - mapping.SetMemoryStart(uint64(traceInfo.MappingStarts[i])) - mapping.SetMemoryLimit(uint64(traceInfo.MappingEnds[i])) - mapping.SetFileOffset(traceInfo.MappingFileOffsets[i]) + mapping.SetMemoryStart(uint64(frame.MappingStart)) + mapping.SetMemoryLimit(uint64(frame.MappingEnd)) + mapping.SetFileOffset(frame.MappingFileOffset) mapping.SetFilenameStrindex(stringSet.Add(fileName)) // Once SemConv and its Go package is released with the new @@ -188,7 +187,7 @@ func (p *Pdata) setProfile( ei.GnuBuildID) attrMgr.AppendOptionalString(mapping.AttributeIndices(), semconv.ProcessExecutableBuildIDHtlhashKey, - traceInfo.Files[i].StringNoQuotes()) + frame.FileID.StringNoQuotes()) } locInfo.mappingIndex = locationMappingIndex case libpf.AbortFrame: @@ -199,27 +198,12 @@ func (p *Pdata) setProfile( default: // Store interpreted frame information as a Line message locInfo.hasLine = true - if si, exists := p.Frames.GetAndRefresh( - libpf.NewFrameID(traceInfo.Files[i], traceInfo.Linenos[i]), - FramesCacheLifetime); exists { - locInfo.lineNumber = int64(si.LineNumber) - fi := funcInfo{ - nameIdx: stringSet.Add(si.FunctionName.String()), - fileNameIdx: stringSet.Add(si.FilePath.String()), - } - locInfo.functionIndex = funcSet.Add(fi) - } else { - // At this point, we do not have enough information for the frame. - // Therefore, we report a dummy entry and use the interpreter as filename. - // To differentiate this case from the case where no information about - // the file ID is available at all, we use a different name for reported - // function. - fi := funcInfo{ - nameIdx: stringSet.Add("UNRESOLVED"), - fileNameIdx: stringSet.Add(frameKind.String()), - } - locInfo.functionIndex = funcSet.Add(fi) + locInfo.lineNumber = int64(frame.SourceLine) + fi := funcInfo{ + nameIdx: stringSet.Add(frame.FunctionName.String()), + fileNameIdx: stringSet.Add(frame.SourceFile.String()), } + locInfo.functionIndex = funcSet.Add(fi) // mapping_table[0] is always the dummy mapping locInfo.mappingIndex = 0 } // End frame type switch @@ -273,7 +257,7 @@ func (p *Pdata) setProfile( sample.AttributeIndices().Append(extra...) } - sample.SetLocationsLength(int32(len(traceInfo.FrameTypes))) + sample.SetLocationsLength(int32(len(traceInfo.Frames))) locationIndex += sample.LocationsLength() } // End sample processing diff --git a/reporter/internal/pdata/generate_test.go b/reporter/internal/pdata/generate_test.go index 196ed6335..b26ceae8e 100644 --- a/reporter/internal/pdata/generate_test.go +++ b/reporter/internal/pdata/generate_test.go @@ -2,6 +2,7 @@ package pdata import ( "testing" + "unique" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -94,7 +95,42 @@ func TestGetDummyMappingIndex(t *testing.T) { } } -//nolint:lll +func newTestFrames() libpf.Frames { + fileID := libpf.NewFileID(2, 3) + frames := make(libpf.Frames, 0, 5) + frames.Append(&libpf.Frame{ + Type: libpf.KernelFrame, + FileID: fileID, + AddressOrLineno: 0xef, + FunctionName: libpf.Intern("func1"), + }) + frames.Append(&libpf.Frame{ + Type: libpf.KernelFrame, + FileID: fileID, + AddressOrLineno: 0x1ef, + FunctionName: libpf.Intern("func2"), + }) + frames.Append(&libpf.Frame{ + Type: libpf.KernelFrame, + FileID: fileID, + AddressOrLineno: 0x2ef, + FunctionName: libpf.Intern("func3"), + }) + frames.Append(&libpf.Frame{ + Type: libpf.KernelFrame, + FileID: fileID, + AddressOrLineno: 0x3ef, + FunctionName: libpf.Intern("func4"), + }) + frames.Append(&libpf.Frame{ + Type: libpf.KernelFrame, + FileID: fileID, + AddressOrLineno: 0x4ef, + FunctionName: libpf.Intern("func5"), + }) + return frames +} + func TestFunctionTableOrder(t *testing.T) { for _, tt := range []struct { name string @@ -108,7 +144,6 @@ func TestFunctionTableOrder(t *testing.T) { { name: "no events", executables: map[libpf.FileID]samples.ExecInfo{}, - frames: map[libpf.FileID]map[libpf.AddressOrLineno]samples.SourceInfo{}, events: map[libpf.Origin]samples.KeyToEventMapping{}, wantFunctionTable: []string{}, expectedResourceProfiles: 0, @@ -118,60 +153,10 @@ func TestFunctionTableOrder(t *testing.T) { executables: map[libpf.FileID]samples.ExecInfo{ libpf.NewFileID(2, 3): {}, }, - frames: map[libpf.FileID]map[libpf.AddressOrLineno]samples.SourceInfo{ - libpf.NewFileID(2, 3): { - libpf.AddressOrLineno(0xef): {FunctionName: libpf.Intern("func1")}, - libpf.AddressOrLineno(0x1ef): {FunctionName: libpf.Intern("func2")}, - libpf.AddressOrLineno(0x2ef): {FunctionName: libpf.Intern("func3")}, - libpf.AddressOrLineno(0x3ef): {FunctionName: libpf.Intern("func4")}, - libpf.AddressOrLineno(0x4ef): {FunctionName: libpf.Intern("func5")}, - }, - }, events: map[libpf.Origin]samples.KeyToEventMapping{ support.TraceOriginSampling: map[samples.TraceAndMetaKey]*samples.TraceEvents{ {}: { - Files: []libpf.FileID{ - libpf.NewFileID(2, 3), - libpf.NewFileID(2, 3), - libpf.NewFileID(2, 3), - libpf.NewFileID(2, 3), - libpf.NewFileID(2, 3), - }, - Linenos: []libpf.AddressOrLineno{ - libpf.AddressOrLineno(0xef), - libpf.AddressOrLineno(0x1ef), - libpf.AddressOrLineno(0x2ef), - libpf.AddressOrLineno(0x3ef), - libpf.AddressOrLineno(0x4ef), - }, - FrameTypes: []libpf.FrameType{ - libpf.KernelFrame, - libpf.KernelFrame, - libpf.KernelFrame, - libpf.KernelFrame, - libpf.KernelFrame, - }, - MappingStarts: []libpf.Address{ - libpf.Address(0), - libpf.Address(0), - libpf.Address(0), - libpf.Address(0), - libpf.Address(0), - }, - MappingEnds: []libpf.Address{ - libpf.Address(0), - libpf.Address(0), - libpf.Address(0), - libpf.Address(0), - libpf.Address(0), - }, - MappingFileOffsets: []uint64{ - 0, - 0, - 0, - 0, - 0, - }, + Frames: newTestFrames(), Timestamps: []uint64{1, 2, 3, 4, 5}, }, }, @@ -182,13 +167,8 @@ func TestFunctionTableOrder(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - d, err := New(100, 100, 100, nil) + d, err := New(100, 100, nil) require.NoError(t, err) - for fileID, addrWithSourceInfos := range tt.frames { - for addr, si := range addrWithSourceInfos { - d.Frames.Add(libpf.NewFrameID(fileID, addr), si) - } - } for k, v := range tt.executables { d.Executables.Add(k, v) } @@ -239,7 +219,7 @@ func TestProfileDuration(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - d, err := New(100, 100, 100, nil) + d, err := New(100, 100, nil) require.NoError(t, err) tree := make(samples.TraceEventsTree) @@ -254,7 +234,7 @@ func TestProfileDuration(t *testing.T) { } } func TestGenerate_EmptyTree(t *testing.T) { - d, err := New(100, 100, 100, nil) + d, err := New(100, 100, nil) require.NoError(t, err) tree := make(samples.TraceEventsTree) @@ -263,19 +243,28 @@ func TestGenerate_EmptyTree(t *testing.T) { assert.Equal(t, 0, profiles.ResourceProfiles().Len()) } +func singleFrameTrace(ty libpf.FrameType, fileID libpf.FileID, lineno libpf.AddressOrLineno, + funcName, sourceFile string, sourceLine libpf.SourceLineno) libpf.Frames { + frames := make(libpf.Frames, 1) + frames[0] = unique.Make(libpf.Frame{ + Type: ty, + FileID: fileID, + AddressOrLineno: lineno, + FunctionName: libpf.Intern(funcName), + SourceFile: libpf.Intern(sourceFile), + SourceLine: sourceLine, + }) + return frames +} + func TestGenerate_SingleContainerSingleOrigin(t *testing.T) { - d, err := New(100, 100, 100, nil) + d, err := New(100, 100, nil) require.NoError(t, err) fileID := libpf.NewFileID(1, 2) funcName := "main" filePath := "/bin/test" d.Executables.Add(fileID, samples.ExecInfo{FileName: filePath}) - d.Frames.Add(libpf.NewFrameID(fileID, 0x10), samples.SourceInfo{ - FunctionName: libpf.Intern(funcName), - FilePath: libpf.Intern(filePath), - LineNumber: 42, - }) traceKey := samples.TraceAndMetaKey{ ExecutablePath: filePath, @@ -287,14 +276,9 @@ func TestGenerate_SingleContainerSingleOrigin(t *testing.T) { events := map[libpf.Origin]samples.KeyToEventMapping{ support.TraceOriginSampling: { traceKey: &samples.TraceEvents{ - Files: []libpf.FileID{fileID}, - Linenos: []libpf.AddressOrLineno{0x10}, - FrameTypes: []libpf.FrameType{libpf.GoFrame}, - MappingStarts: []libpf.Address{0}, - MappingEnds: []libpf.Address{0}, - MappingFileOffsets: []uint64{0}, - Timestamps: []uint64{100}, - EnvVars: map[string]string{"FOO": "BAR"}, + Frames: singleFrameTrace(libpf.GoFrame, fileID, 0x10, funcName, filePath, 42), + Timestamps: []uint64{100}, + EnvVars: map[string]string{"FOO": "BAR"}, }, }, } @@ -342,32 +326,24 @@ func TestGenerate_SingleContainerSingleOrigin(t *testing.T) { } func TestGenerate_MultipleOriginsAndContainers(t *testing.T) { - d, err := New(100, 100, 100, nil) + d, err := New(100, 100, nil) require.NoError(t, err) fileID := libpf.NewFileID(5, 6) d.Executables.Add(fileID, samples.ExecInfo{FileName: "/bin/foo"}) - d.Frames.Add(libpf.NewFrameID(fileID, 0x20), samples.SourceInfo{ - FunctionName: libpf.Intern("f"), - FilePath: libpf.Intern("/bin/foo"), - LineNumber: 1, - }) - traceKey := samples.TraceAndMetaKey{ExecutablePath: "/bin/foo"} + frames := singleFrameTrace(libpf.PythonFrame, fileID, 0x20, "f", "/bin/foo", 1) + events1 := map[libpf.Origin]samples.KeyToEventMapping{ support.TraceOriginSampling: { traceKey: &samples.TraceEvents{ - Files: []libpf.FileID{fileID}, - Linenos: []libpf.AddressOrLineno{0x20}, - FrameTypes: []libpf.FrameType{libpf.PythonFrame}, + Frames: frames, Timestamps: []uint64{1, 2}, }, }, support.TraceOriginOffCPU: { traceKey: &samples.TraceEvents{ - Files: []libpf.FileID{fileID}, - Linenos: []libpf.AddressOrLineno{0x20}, - FrameTypes: []libpf.FrameType{libpf.PythonFrame}, + Frames: frames, Timestamps: []uint64{3, 4}, OffTimes: []int64{10, 20}, }, @@ -376,9 +352,7 @@ func TestGenerate_MultipleOriginsAndContainers(t *testing.T) { events2 := map[libpf.Origin]samples.KeyToEventMapping{ support.TraceOriginSampling: { traceKey: &samples.TraceEvents{ - Files: []libpf.FileID{fileID}, - Linenos: []libpf.AddressOrLineno{0x20}, - FrameTypes: []libpf.FrameType{libpf.PythonFrame}, + Frames: frames, Timestamps: []uint64{5}, }, }, @@ -410,26 +384,20 @@ func TestGenerate_MultipleOriginsAndContainers(t *testing.T) { } func TestGenerate_StringAndFunctionTablePopulation(t *testing.T) { - d, err := New(100, 100, 100, nil) + d, err := New(100, 100, nil) require.NoError(t, err) fileID := libpf.NewFileID(7, 8) funcName := "myfunc" filePath := "/bin/bar" d.Executables.Add(fileID, samples.ExecInfo{FileName: filePath}) - d.Frames.Add(libpf.NewFrameID(fileID, 0x30), samples.SourceInfo{ - FunctionName: libpf.Intern(funcName), - FilePath: libpf.Intern(filePath), - LineNumber: 123, - }) traceKey := samples.TraceAndMetaKey{ExecutablePath: filePath} events := map[libpf.Origin]samples.KeyToEventMapping{ support.TraceOriginSampling: { traceKey: &samples.TraceEvents{ - Files: []libpf.FileID{fileID}, - Linenos: []libpf.AddressOrLineno{0x30}, - FrameTypes: []libpf.FrameType{libpf.PythonFrame}, + Frames: singleFrameTrace(libpf.PythonFrame, fileID, 0x30, + funcName, filePath, 123), Timestamps: []uint64{42}, }, }, @@ -458,8 +426,22 @@ func TestGenerate_StringAndFunctionTablePopulation(t *testing.T) { assert.Equal(t, filePath, dic.StringTable().At(int(fn.FilenameStrindex()))) } +func singleFrameNative(fileID libpf.FileID, lineno libpf.AddressOrLineno, + mappingStart, mappingEnd libpf.Address, mappingFileOffset uint64) libpf.Frames { + frames := make(libpf.Frames, 1) + frames[0] = unique.Make(libpf.Frame{ + Type: libpf.NativeFrame, + FileID: fileID, + AddressOrLineno: lineno, + MappingStart: mappingStart, + MappingEnd: mappingEnd, + MappingFileOffset: mappingFileOffset, + }) + return frames +} + func TestGenerate_NativeFrame(t *testing.T) { - d, err := New(100, 100, 100, nil) + d, err := New(100, 100, nil) require.NoError(t, err) fileID := libpf.NewFileID(9, 10) @@ -475,13 +457,8 @@ func TestGenerate_NativeFrame(t *testing.T) { events := map[libpf.Origin]samples.KeyToEventMapping{ support.TraceOriginSampling: { traceKey: &samples.TraceEvents{ - Files: []libpf.FileID{fileID}, - Linenos: []libpf.AddressOrLineno{0x1000}, - FrameTypes: []libpf.FrameType{libpf.NativeFrame}, - MappingStarts: []libpf.Address{0x1000}, - MappingEnds: []libpf.Address{0x2000}, - MappingFileOffsets: []uint64{0x100}, - Timestamps: []uint64{789}, + Frames: singleFrameNative(fileID, 0x1000, 0x1000, 0x2000, 0x100), + Timestamps: []uint64{789}, }, }, } diff --git a/reporter/internal/pdata/pdata.go b/reporter/internal/pdata/pdata.go index 43caefd8d..a8271a512 100644 --- a/reporter/internal/pdata/pdata.go +++ b/reporter/internal/pdata/pdata.go @@ -19,15 +19,12 @@ type Pdata struct { // Executables stores metadata for executables. Executables *lru.SyncedLRU[libpf.FileID, samples.ExecInfo] - // Frames maps frame information to its source location. - Frames *lru.SyncedLRU[libpf.FrameID, samples.SourceInfo] - // ExtraSampleAttrProd is an optional hook point for adding custom // attributes to samples. ExtraSampleAttrProd samples.SampleAttrProducer } -func New(samplesPerSecond int, executablesCacheElements, framesCacheElements uint32, +func New(samplesPerSecond int, executablesCacheElements uint32, extra samples.SampleAttrProducer) (*Pdata, error) { executables, err := lru.NewSynced[libpf.FileID, samples.ExecInfo](executablesCacheElements, libpf.FileID.Hash32) @@ -36,17 +33,9 @@ func New(samplesPerSecond int, executablesCacheElements, framesCacheElements uin } executables.SetLifetime(ExecutableCacheLifetime) // Allow GC to clean stale items. - frames, err := - lru.NewSynced[libpf.FrameID, samples.SourceInfo](framesCacheElements, libpf.FrameID.Hash32) - if err != nil { - return nil, err - } - frames.SetLifetime(FramesCacheLifetime) // Allow GC to clean stale items. - return &Pdata{ samplesPerSecond: samplesPerSecond, Executables: executables, - Frames: frames, ExtraSampleAttrProd: extra, }, nil } @@ -54,5 +43,4 @@ func New(samplesPerSecond int, executablesCacheElements, framesCacheElements uin // Purge purges all the expired data func (p *Pdata) Purge() { p.Executables.PurgeExpired() - p.Frames.PurgeExpired() } diff --git a/reporter/otlp_reporter.go b/reporter/otlp_reporter.go index 54e9f5d11..4b198f222 100644 --- a/reporter/otlp_reporter.go +++ b/reporter/otlp_reporter.go @@ -55,7 +55,6 @@ func NewOTLP(cfg *Config) (*OTLPReporter, error) { data, err := pdata.New( cfg.SamplesPerSecond, cfg.ExecutablesCacheElements, - cfg.FramesCacheElements, cfg.ExtraSampleAttrProd, ) if err != nil { diff --git a/reporter/samples/samples.go b/reporter/samples/samples.go index 7e945918b..834b44971 100644 --- a/reporter/samples/samples.go +++ b/reporter/samples/samples.go @@ -23,15 +23,10 @@ type TraceEventMeta struct { // TraceEvents holds known information about a trace. type TraceEvents struct { - Files []libpf.FileID - Linenos []libpf.AddressOrLineno - FrameTypes []libpf.FrameType - MappingStarts []libpf.Address - MappingEnds []libpf.Address - MappingFileOffsets []uint64 - Timestamps []uint64 // in nanoseconds - OffTimes []int64 // in nanoseconds - EnvVars map[string]string + Frames libpf.Frames + Timestamps []uint64 // in nanoseconds + OffTimes []int64 // in nanoseconds + EnvVars map[string]string } // TraceAndMetaKey is the deduplication key for samples. This **must always** diff --git a/testutils/helpers.go b/testutils/helpers.go index 7d68323a7..dad5f2791 100644 --- a/testutils/helpers.go +++ b/testutils/helpers.go @@ -36,12 +36,6 @@ func (f MockReporter) ExecutableKnown(_ libpf.FileID) bool { func (f MockReporter) ExecutableMetadata(_ *reporter.ExecutableMetadataArgs) { } -func (f MockReporter) FrameKnown(_ libpf.FrameID) bool { - return true -} - -func (f MockReporter) FrameMetadata(_ *reporter.FrameMetadataArgs) {} - func StartTracer(ctx context.Context, t *testing.T, et tracertypes.IncludedTracers, r reporter.SymbolReporter) (chan *host.Trace, *tracer.Tracer) { trc, err := tracer.NewTracer(ctx, &tracer.Config{ diff --git a/tools/coredump/coredump.go b/tools/coredump/coredump.go index f35d81330..18afb05fa 100644 --- a/tools/coredump/coredump.go +++ b/tools/coredump/coredump.go @@ -35,14 +35,12 @@ func sliceBuffer(buf unsafe.Pointer, sz C.int) []byte { // symbolizationCache collects and caches the interpreter manager's symbolization // callbacks to be used for trace stringification. type symbolizationCache struct { - files map[libpf.FileID]string - symbols map[libpf.FrameID]*reporter.FrameMetadataArgs + files map[libpf.FileID]string } func newSymbolizationCache() *symbolizationCache { return &symbolizationCache{ - files: make(map[libpf.FileID]string), - symbols: make(map[libpf.FrameID]*reporter.FrameMetadataArgs), + files: make(map[libpf.FileID]string), } } @@ -55,17 +53,6 @@ func (c *symbolizationCache) ExecutableMetadata(args *reporter.ExecutableMetadat c.files[args.FileID] = args.FileName } -func (c *symbolizationCache) FrameKnown(frameID libpf.FrameID) bool { - _, exists := c.symbols[frameID] - return exists -} - -func (c *symbolizationCache) FrameMetadata(args *reporter.FrameMetadataArgs) { - c.symbols[args.FrameID] = args -} - -func (c *symbolizationCache) ReportFallbackSymbol(libpf.FrameID, string) {} - func generateErrorMap() (map[libpf.AddressOrLineno]string, error) { file, err := os.Open("../errors-codegen/errors.json") if err != nil { @@ -92,35 +79,35 @@ func generateErrorMap() (map[libpf.AddressOrLineno]string, error) { var errorMap xsync.Once[map[libpf.AddressOrLineno]string] -func (c *symbolizationCache) symbolize(ty libpf.FrameType, fileID libpf.FileID, - lineNumber libpf.AddressOrLineno) (string, error) { - if ty.IsError() { +func (c *symbolizationCache) formatFrame(frame *libpf.Frame) (string, error) { + if frame.Type.IsError() { errMap, err := errorMap.GetOrInit(generateErrorMap) if err != nil { return "", fmt.Errorf("unable to construct error map: %v", err) } - errName, ok := (*errMap)[lineNumber] + errName, ok := (*errMap)[frame.AddressOrLineno] if !ok { return "", fmt.Errorf( - "got invalid error code %d. forgot to `make generate`", lineNumber) + "got invalid error code %d. forgot to `make generate`", + frame.AddressOrLineno) } - if ty == libpf.AbortFrame { + if frame.Type == libpf.AbortFrame { return fmt.Sprintf("", errName), nil } return fmt.Sprintf("", errName), nil } - if data, ok := c.symbols[libpf.NewFrameID(fileID, lineNumber)]; ok { + if frame.FunctionName != libpf.NullString { return fmt.Sprintf("%s+%d in %s:%d", - data.FunctionName, data.FunctionOffset, - data.SourceFile, data.SourceLine), nil + frame.FunctionName, frame.FunctionOffset, + frame.SourceFile, frame.SourceLine), nil } - sourceFile, ok := c.files[fileID] + sourceFile, ok := c.files[frame.FileID] if !ok { - sourceFile = fmt.Sprintf("%08x", fileID) + sourceFile = fmt.Sprintf("%08x", frame.FileID) } - return fmt.Sprintf("%s+0x%x", sourceFile, lineNumber), nil + return fmt.Sprintf("%s+0x%x", sourceFile, frame.AddressOrLineno), nil } func ExtractTraces(ctx context.Context, pr process.Process, debug bool, @@ -200,12 +187,13 @@ func ExtractTraces(ctx context.Context, pr process.Process, debug bool, // Symbolize traces with interpreter manager trace := manager.ConvertTrace(&ebpfCtx.trace) tinfo := ThreadInfo{LWP: thread.LWP} - for i := range trace.FrameTypes { - frame, err := symCache.symbolize(trace.FrameTypes[i], trace.Files[i], trace.Linenos[i]) + for _, f := range trace.Frames { + frame := f.Value() + frameText, err := symCache.formatFrame(&frame) if err != nil { return nil, err } - tinfo.Frames = append(tinfo.Frames, frame) + tinfo.Frames = append(tinfo.Frames, frameText) } info = append(info, tinfo) } diff --git a/tracer/ebpf_integration_test.go b/tracer/ebpf_integration_test.go index eb7bb202d..ebfd46194 100644 --- a/tracer/ebpf_integration_test.go +++ b/tracer/ebpf_integration_test.go @@ -74,13 +74,13 @@ func runKernelFrameProbe(t *testing.T, tr *tracer.Tracer) { require.NoError(t, err) } -func validateTrace(t *testing.T, numKernelFrames int, expected, returned *host.Trace) { +func validateTrace(t *testing.T, expected, returned *host.Trace) { t.Helper() - assert.Equal(t, len(expected.Frames), len(returned.Frames)-numKernelFrames) + require.Len(t, returned.Frames, len(expected.Frames)) for i, expFrame := range expected.Frames { - retFrame := returned.Frames[numKernelFrames+i] + retFrame := returned.Frames[i] assert.Equal(t, expFrame.File, retFrame.File) assert.Equal(t, expFrame.Lineno, retFrame.Lineno) assert.Equal(t, expFrame.Type, retFrame.Type) @@ -203,14 +203,9 @@ Loop: trace, ok := traces[testcase.id] require.Truef(t, ok, "trace ID %d not received", testcase.id) - var numKernelFrames int - for _, frame := range trace.Frames { - if frame.Type == support.FrameMarkerKernel { - numKernelFrames++ - } - } + numKernelFrames := len(trace.KernelFrames) + userspaceFrameCount := len(trace.Frames) - userspaceFrameCount := len(trace.Frames) - numKernelFrames assert.Equal(t, len(testcase.userSpaceTrace.Frames), userspaceFrameCount) assert.False(t, !testcase.hasKernelFrames && numKernelFrames > 0, "unexpected kernel frames") @@ -227,7 +222,7 @@ Loop: t.Logf("Received %d user frames and %d kernel frames", userspaceFrameCount, numKernelFrames) - validateTrace(t, numKernelFrames, &testcase.userSpaceTrace, trace) + validateTrace(t, &testcase.userSpaceTrace, trace) }) } } diff --git a/tracer/tracer.go b/tracer/tracer.go index fcca906b6..cc585cc0e 100644 --- a/tracer/tracer.go +++ b/tracer/tracer.go @@ -13,7 +13,6 @@ import ( "math" "math/rand/v2" "strings" - "sync/atomic" "time" "unsafe" @@ -66,9 +65,6 @@ type Intervals interface { // Tracer provides an interface for loading and initializing the eBPF components as // well as for monitoring the output maps for new traces and count updates. type Tracer struct { - fallbackSymbolHit atomic.Uint64 - fallbackSymbolMiss atomic.Uint64 - // ebpfMaps holds the currently loaded eBPF maps. ebpfMaps map[string]*cebpf.Map // ebpfProgs holds the currently loaded eBPF programs. @@ -663,17 +659,15 @@ func loadProgram(ebpfProgs map[string]*cebpf.Program, tailcallMap *cebpf.Map, return nil } -// insertKernelFrames fetches the kernel stack frames for a particular kstackID and populates -// the trace with these kernel frames. It also allocates the memory for the frames of the trace. -// It returns the number of kernel frames for kstackID or an error. -func (t *Tracer) insertKernelFrames(trace *host.Trace, ustackLen uint32, - kstackID int32) (uint32, error) { +// readKernelFrames fetches the kernel stack frames for a particular kstackID and +// returns them as symbolized libpf.Frames. +func (t *Tracer) readKernelFrames(kstackID int32) (libpf.Frames, error) { cKstackID := kstackID kstackVal := make([]uint64, support.PerfMaxStackDepth) if err := t.ebpfMaps["kernel_stackmap"].Lookup(unsafe.Pointer(&cKstackID), unsafe.Pointer(&kstackVal[0])); err != nil { - return 0, fmt.Errorf("failed to lookup kernel frames for stackID %d: %v", kstackID, err) + return nil, fmt.Errorf("failed to lookup kernel frames for stackID %d: %v", kstackID, err) } // The kernel returns absolute addresses in kernel address @@ -684,63 +678,34 @@ func (t *Tracer) insertKernelFrames(trace *host.Trace, ustackLen uint32, kstackLen++ } - trace.Frames = make([]host.Frame, kstackLen+ustackLen) - - var kernelSymbolCacheHit, kernelSymbolCacheMiss uint64 - + frames := make(libpf.Frames, kstackLen) for i := uint32(0); i < kstackLen; i++ { - fileID := libpf.UnknownKernelFileID address := libpf.Address(kstackVal[i]) - kmod, err := t.kernelSymbolizer.GetModuleByAddress(address) - if err == nil { - fileID = kmod.FileID() - address -= kmod.Start() + frame := libpf.Frame{ + Type: libpf.KernelFrame, + FileID: libpf.UnknownKernelFileID, + AddressOrLineno: libpf.AddressOrLineno(address - 1), } - hostFileID := host.FileIDFromLibpf(fileID) - t.processManager.FileIDMapper.Set(hostFileID, fileID) - - trace.Frames[i] = host.Frame{ - File: hostFileID, - Lineno: libpf.AddressOrLineno(address), - Type: libpf.KernelFrame, - - // For all kernel frames, the kernel unwinder will always produce a - // frame in which the RIP is after a call instruction (it hides the - // top frames that leads to the unwinder itself). - ReturnAddress: true, - } - - // Kernel frame PCs need to be adjusted by -1. This duplicates logic done in the trace - // converter. This should be fixed with PF-1042. - frameID := libpf.NewFrameID(fileID, trace.Frames[i].Lineno-1) - if t.reporter.FrameKnown(frameID) { - kernelSymbolCacheHit++ - continue - } - kernelSymbolCacheMiss++ - - if kmod == nil { - continue - } - if funcName, _, err := kmod.LookupSymbolByAddress(libpf.Address(kstackVal[i])); err == nil { - t.reporter.FrameMetadata(&reporter.FrameMetadataArgs{ - FrameID: frameID, - FunctionName: libpf.Intern(funcName), - }) - t.reporter.ExecutableMetadata(&reporter.ExecutableMetadataArgs{ - FileID: kmod.FileID(), - FileName: kmod.Name(), - GnuBuildID: kmod.BuildID(), - Interp: libpf.Kernel, - }) + kmod, err := t.kernelSymbolizer.GetModuleByAddress(address) + if err == nil { + frame.FileID = kmod.FileID() + frame.AddressOrLineno -= libpf.AddressOrLineno(kmod.Start()) + + if funcName, _, err := kmod.LookupSymbolByAddress(address); err == nil { + frame.FunctionName = libpf.Intern(funcName) + t.reporter.ExecutableMetadata(&reporter.ExecutableMetadataArgs{ + FileID: kmod.FileID(), + FileName: kmod.Name(), + GnuBuildID: kmod.BuildID(), + Interp: libpf.Kernel, + }) + } } + frames.Append(&frame) } - t.fallbackSymbolMiss.Add(kernelSymbolCacheMiss) - t.fallbackSymbolHit.Add(kernelSymbolCacheHit) - - return kstackLen, nil + return frames, nil } // enableEvent removes the entry of given eventType from the inhibitEvents map @@ -925,15 +890,11 @@ func (t *Tracer) loadBpfTrace(raw []byte, cpu int) *host.Trace { ptr.Offtime = 0 trace.Hash = host.TraceHash(xxh3.Hash128(raw).Lo) - userFrameOffs := 0 if ptr.Kernel_stack_id >= 0 { - kstackLen, err := t.insertKernelFrames( - trace, ptr.Stack_len, ptr.Kernel_stack_id) - + var err error + trace.KernelFrames, err = t.readKernelFrames(ptr.Kernel_stack_id) if err != nil { log.Errorf("Failed to get kernel stack frames for 0x%x: %v", trace.Hash, err) - } else { - userFrameOffs = int(kstackLen) } } @@ -947,15 +908,10 @@ func (t *Tracer) loadBpfTrace(raw []byte, cpu int) *host.Trace { } } - // If there are no kernel frames, or reading them failed, we are responsible - // for allocating the columnar frame array. - if len(trace.Frames) == 0 { - trace.Frames = make([]host.Frame, ptr.Stack_len) - } - + trace.Frames = make([]host.Frame, ptr.Stack_len) for i := 0; i < int(ptr.Stack_len); i++ { rawFrame := &ptr.Frames[i] - trace.Frames[userFrameOffs+i] = host.Frame{ + trace.Frames[i] = host.Frame{ File: host.FileID(rawFrame.File_id), Lineno: libpf.AddressOrLineno(rawFrame.Addr_or_line), Type: libpf.FrameType(rawFrame.Kind), @@ -1001,17 +957,6 @@ func (t *Tracer) StartMapMonitors(ctx context.Context, traceOutChan chan<- *host metrics.AddSlice(eventMetricCollector()) metrics.AddSlice(traceEventMetricCollector()) metrics.AddSlice(t.eBPFMetricsCollector(translateIDs, previousMetricValue)) - - metrics.AddSlice([]metrics.Metric{ - { - ID: metrics.IDKernelFallbackSymbolLRUHit, - Value: metrics.MetricValue(t.fallbackSymbolHit.Swap(0)), - }, - { - ID: metrics.IDKernelFallbackSymbolLRUMiss, - Value: metrics.MetricValue(t.fallbackSymbolMiss.Swap(0)), - }, - }) }) return nil diff --git a/traceutil/traceutil.go b/traceutil/traceutil.go index 6768b7e13..fead3a987 100644 --- a/traceutil/traceutil.go +++ b/traceutil/traceutil.go @@ -16,11 +16,12 @@ import ( func HashTrace(trace *libpf.Trace) libpf.TraceHash { var buf [24]byte h := fnv.New128a() - for i := uint64(0); i < uint64(len(trace.Files)); i++ { - _, _ = h.Write(trace.Files[i].Bytes()) + for _, uniqueFrame := range trace.Frames { + frame := uniqueFrame.Value() + _, _ = h.Write(frame.FileID.Bytes()) // Using FormatUint() or putting AppendUint() into a function leads // to escaping to heap (allocation). - _, _ = h.Write(strconv.AppendUint(buf[:0], uint64(trace.Linenos[i]), 10)) + _, _ = h.Write(strconv.AppendUint(buf[:0], uint64(frame.AddressOrLineno), 10)) } // make instead of nil avoids a heap allocation traceHash, _ := libpf.TraceHashFromBytes(h.Sum(make([]byte, 0, 16))) diff --git a/traceutil/traceutil_test.go b/traceutil/traceutil_test.go index 0a59d544e..c1bc5d08c 100644 --- a/traceutil/traceutil_test.go +++ b/traceutil/traceutil_test.go @@ -19,6 +19,14 @@ func TestLibpfEBPFFrameMarkerEquality(t *testing.T) { assert.Equal(t, int(libpf.PHPFrame), support.FrameMarkerPHP) } +func newPythonTrace() *libpf.Trace { + trace := &libpf.Trace{} + trace.AppendFrame(libpf.NativeFrame, libpf.NewFileID(0, 0), 0) + trace.AppendFrame(libpf.NativeFrame, libpf.NewFileID(1, 1), 1) + trace.AppendFrame(libpf.NativeFrame, libpf.NewFileID(2, 2), 2) + return trace +} + func TestHashTrace(t *testing.T) { tests := map[string]struct { trace *libpf.Trace @@ -28,18 +36,7 @@ func TestHashTrace(t *testing.T) { trace: &libpf.Trace{}, result: libpf.NewTraceHash(0x6c62272e07bb0142, 0x62b821756295c58d)}, "python trace": { - trace: &libpf.Trace{ - Linenos: []libpf.AddressOrLineno{0, 1, 2}, - Files: []libpf.FileID{ - libpf.NewFileID(0, 0), - libpf.NewFileID(1, 1), - libpf.NewFileID(2, 2), - }, - FrameTypes: []libpf.FrameType{ - libpf.NativeFrame, - libpf.NativeFrame, - libpf.NativeFrame, - }}, + trace: newPythonTrace(), result: libpf.NewTraceHash(0x21c6fe4c62868856, 0xcf510596eab68dc8)}, }