From 8ae6afbd735f20ad4c0150a8cdb946dc02a58736 Mon Sep 17 00:00:00 2001 From: mayankpande88 Date: Thu, 10 Sep 2026 18:23:39 +0530 Subject: [PATCH] perf(ebpf): cache uprobe symbol lookups by binary identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attaching the Node.js probes read the full ELF symbol table on every pid. node is a large statically linked binary with V8 embedded, and readSymbols loads both .symtab and .dynsym — allocating a Go string per symbol name — to locate six libuv functions. Each found symbol is then disassembled to find its RET instructions for uretprobes. None of that is per-process. The address and return offsets are pure functions of (binary, symbol name), but nothing cached them: instrumentNodejs is gated on a per-Process field, so every new pid repeated the whole thing. Node cluster mode runs many workers off one binary, and pod churn repeats it again. A customer CPU profile showed the cost: Process.instrumentNodejs -> AttachNodejsProbes -> ELFFile.GetSymbol -> readSymbols 17.5% of a core debug/elf.getSymbols64 11.2% The negative case is worse than the positive one. Any executable reaching this path that does not export uv__io_poll was re-parsed for every pid, forever, to reach the same conclusion. LookupSymbols caches ProbeTarget{address, returnOffsets} keyed by file identity — dev+inode+size+mtime — not by path. Paths here are per-process (/proc//root/...), so a path-keyed cache would miss every time, which is precisely the case being fixed; pods from one image share the read-only overlay layer, so dev+inode collapses them onto one entry. Absent symbols are cached too, so a non-Node binary is parsed once and never again. Caching is per (binary, symbol) rather than one map per binary. Keying only by binary and storing whichever names the first caller asked for would hand a later caller that map and read its own symbols as absent — reachable, since the Node.js and Go-TLS probes can target the same executable. A partial hit resolves the remaining names in a single parse. Attach behaviour is unchanged, including stopping at the first missing libuv callback. ./ebpftracer is not excluded from CI tests, so the new cases gate this. --- ebpftracer/nodejs.go | 36 ++++--- ebpftracer/symbol_cache.go | 184 ++++++++++++++++++++++++++++++++ ebpftracer/symbol_cache_test.go | 105 ++++++++++++++++++ 3 files changed, 313 insertions(+), 12 deletions(-) create mode 100644 ebpftracer/symbol_cache.go create mode 100644 ebpftracer/symbol_cache_test.go diff --git a/ebpftracer/nodejs.go b/ebpftracer/nodejs.go index 30079587..4eec3f2e 100644 --- a/ebpftracer/nodejs.go +++ b/ebpftracer/nodejs.go @@ -2,6 +2,7 @@ package ebpftracer import ( "bufio" + "fmt" "os" "strings" @@ -11,6 +12,16 @@ import ( "k8s.io/klog/v2" ) +const nodejsPollSymbol = "uv__io_poll" + +// libuv I/O callbacks. Not every libuv build exports all of them; the attach +// loop stops at the first one missing, matching the previous behaviour. +var nodejsCallbackSymbols = []string{"uv__stream_io", "uv__async_io", "uv__poll_io", "uv__server_io", "uv__udp_io"} + +// Every symbol resolved in one pass over the ELF symbol table, so a cache miss +// costs a single parse rather than one per symbol. +var nodejsProbeSymbols = append([]string{nodejsPollSymbol}, nodejsCallbackSymbols...) + func (t *Tracer) AttachNodejsProbes(pid uint32, exe string) []link.Link { log := func(libPath, msg string, err error) { if err != nil { @@ -44,24 +55,25 @@ func (t *Tracer) attachNodejsUprobes(libPath string, pid uint32) ([]link.Link, e if err != nil { return nil, err } - ef, err := OpenELFFile(libPath) + + // Resolved once per binary rather than once per pid — see LookupSymbols. + targets, err := LookupSymbols(libPath, nodejsProbeSymbols) if err != nil { return nil, err } - defer ef.Close() - s, err := ef.GetSymbol("uv__io_poll") - if err != nil { - return nil, err + poll := targets[nodejsPollSymbol] + if !poll.Found { + return nil, fmt.Errorf("symbol %s not found", nodejsPollSymbol) } - l, err := s.AttachUprobe(exe, t.uprobes["uv_io_poll_enter"], pid) + l, err := attachUprobeAt(exe, t.uprobes["uv_io_poll_enter"], pid, poll.Address) if err != nil { return nil, err } var links []link.Link links = append(links, l) - ls, err := s.AttachUretprobes(exe, t.uprobes["uv_io_poll_exit"], pid) + ls, err := attachUretprobesAt(exe, t.uprobes["uv_io_poll_exit"], pid, poll) links = append(links, ls...) if err != nil { for _, l := range links { @@ -70,17 +82,17 @@ func (t *Tracer) attachNodejsUprobes(libPath string, pid uint32) ([]link.Link, e return nil, err } - for _, cb := range []string{"uv__stream_io", "uv__async_io", "uv__poll_io", "uv__server_io", "uv__udp_io"} { - s, err = ef.GetSymbol(cb) - if err != nil { + for _, cb := range nodejsCallbackSymbols { + target := targets[cb] + if !target.Found { break } - l, err = s.AttachUprobe(exe, t.uprobes["uv_io_cb_enter"], pid) + l, err = attachUprobeAt(exe, t.uprobes["uv_io_cb_enter"], pid, target.Address) if err != nil { break } links = append(links, l) - ls, err = s.AttachUretprobes(exe, t.uprobes["uv_io_cb_exit"], pid) + ls, err = attachUretprobesAt(exe, t.uprobes["uv_io_cb_exit"], pid, target) links = append(links, ls...) if err != nil { break diff --git a/ebpftracer/symbol_cache.go b/ebpftracer/symbol_cache.go new file mode 100644 index 00000000..7d2b915b --- /dev/null +++ b/ebpftracer/symbol_cache.go @@ -0,0 +1,184 @@ +package ebpftracer + +import ( + "fmt" + "os" + "sync" + "syscall" + + "github.com/cilium/ebpf" + "github.com/cilium/ebpf/link" + lru "github.com/hashicorp/golang-lru/v2" +) + +// ProbeTarget is everything uprobe attachment needs from a symbol: the file +// offset to attach at, and the offsets of the RET instructions inside the +// function for uretprobes. +// +// Both are pure functions of (binary, symbol name) — nothing about them is +// per-process — so they can be resolved once and reused for every process +// running that binary. +type ProbeTarget struct { + Address uint64 + ReturnOffsets []int + Found bool +} + +// binaryKey identifies a file by identity rather than by path. +// +// Paths here are per-process (/proc//root/usr/bin/node), so keying the +// cache on the path string would miss for every new pid — exactly the case the +// cache exists to eliminate. Pods from the same image share the read-only +// overlay layer, so dev+inode collapses them onto one entry. Size and mtime +// guard against inode reuse after a delete. +type binaryKey struct { + dev, ino, size uint64 + mtimeNsec int64 +} + +// symbolKey caches one symbol at a time rather than one map per binary. +// +// Keying only by binary and storing the map the first caller happened to ask +// for is wrong: a later caller wanting different symbols from the same file +// would be handed that map and read every one of its own symbols as absent. +// The Node.js and Go-TLS probes can both target the same executable, so this is +// reachable, not theoretical. +type symbolKey struct { + bin binaryKey + name string +} + +// Bounded so a node cycling through many image versions cannot grow this +// without limit. Entries are a handful of ints each. +const symbolCacheSize = 4096 + +var ( + symbolCacheMu sync.Mutex + symbolCache *lru.Cache[symbolKey, ProbeTarget] +) + +func init() { + symbolCache, _ = lru.New[symbolKey, ProbeTarget](symbolCacheSize) +} + +func binaryKeyFor(path string) (binaryKey, error) { + fi, err := os.Stat(path) + if err != nil { + return binaryKey{}, err + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return binaryKey{}, fmt.Errorf("stat unavailable for %s", path) + } + return binaryKey{ + dev: uint64(st.Dev), + ino: uint64(st.Ino), + size: uint64(fi.Size()), + mtimeNsec: fi.ModTime().UnixNano(), + }, nil +} + +// LookupSymbols resolves the named symbols in the binary at path. +// +// On a miss it reads the ELF symbol table and disassembles each function body +// to find its RET instructions; on a hit it does neither. That matters because +// the uncached path is expensive out of proportion to what it yields: node is a +// large statically linked binary, and readSymbols loads the whole .symtab and +// .dynsym — allocating a Go string per symbol — to locate six functions. A +// customer profile showed this at 17.5% of a core, repeated per pid, because +// instrumentNodejs is gated per Process and Node cluster mode runs many +// processes off one binary. +// +// Symbols that are absent are cached with Found=false, deliberately: a binary +// that does not export the probe points must not be re-parsed for every new +// pid. That negative case is the common one, since every non-Node executable +// that reaches here also fails to match. +func LookupSymbols(path string, names []string) (map[string]ProbeTarget, error) { + key, err := binaryKeyFor(path) + if err != nil { + return nil, err + } + + targets := make(map[string]ProbeTarget, len(names)) + var missing []string + symbolCacheMu.Lock() + for _, name := range names { + if t, ok := symbolCache.Get(symbolKey{bin: key, name: name}); ok { + targets[name] = t + } else { + missing = append(missing, name) + } + } + symbolCacheMu.Unlock() + + if len(missing) == 0 { + return targets, nil + } + + // One parse resolves every name still missing, so a partial hit costs no + // more than a full miss. + resolved, err := readProbeTargets(path, missing) + if err != nil { + // Could not read the binary at all — not cacheable, since a later + // attempt against a readable path may succeed. + return nil, err + } + + symbolCacheMu.Lock() + for name, t := range resolved { + symbolCache.Add(symbolKey{bin: key, name: name}, t) + targets[name] = t + } + symbolCacheMu.Unlock() + return targets, nil +} + +// readProbeTargets does the expensive work: one ELF open, one symbol table +// parse shared across all requested names, and one disassembly per found +// symbol. +func readProbeTargets(path string, names []string) (map[string]ProbeTarget, error) { + ef, err := OpenELFFile(path) + if err != nil { + return nil, err + } + defer ef.Close() + + targets := make(map[string]ProbeTarget, len(names)) + for _, name := range names { + s, err := ef.GetSymbol(name) + if err != nil { + targets[name] = ProbeTarget{} + continue + } + t := ProbeTarget{Address: s.Address(), Found: true} + // A symbol with no discoverable RET offsets still attaches an entry + // uprobe; only the uretprobes are skipped. + if offsets, err := s.ReturnOffsets(); err == nil { + t.ReturnOffsets = offsets + } + targets[name] = t + } + return targets, nil +} + +// attachUprobeAt and attachUretprobesAt mirror Symbol.AttachUprobe and +// Symbol.AttachUretprobes, but take a resolved address so attachment no longer +// requires holding an open ELFFile. +func attachUprobeAt(exe *link.Executable, prog *ebpf.Program, pid uint32, addr uint64) (link.Link, error) { + return exe.Uprobe("", prog, &link.UprobeOptions{Address: addr, PID: int(pid)}) +} + +func attachUretprobesAt(exe *link.Executable, prog *ebpf.Program, pid uint32, t ProbeTarget) ([]link.Link, error) { + if len(t.ReturnOffsets) == 0 { + return nil, fmt.Errorf("no return offsets") + } + var links []link.Link + for _, offset := range t.ReturnOffsets { + l, err := exe.Uprobe("", prog, &link.UprobeOptions{Address: t.Address + uint64(offset), PID: int(pid)}) + if err != nil { + return links, err + } + links = append(links, l) + } + return links, nil +} diff --git a/ebpftracer/symbol_cache_test.go b/ebpftracer/symbol_cache_test.go new file mode 100644 index 00000000..1feedbcc --- /dev/null +++ b/ebpftracer/symbol_cache_test.go @@ -0,0 +1,105 @@ +package ebpftracer + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// The cache must key on file identity, not on the path string. Probe paths are +// per-process (/proc//root/...) and every pid produces a different string +// for the same file, so a path-keyed cache would miss every time — which is the +// behaviour this cache exists to remove. +func TestLookupSymbolsCachesByFileIdentity(t *testing.T) { + bin, err := exec.LookPath("true") + if err != nil { + t.Skip("no /bin/true available") + } + + first, err := LookupSymbols(bin, []string{"main"}) + if err != nil { + t.Fatalf("first lookup: %v", err) + } + + // A second path referring to the same inode must hit the same entry. + link := filepath.Join(t.TempDir(), "true-hardlink") + if err := os.Link(bin, link); err != nil { + t.Skipf("cannot hardlink %s: %v", bin, err) + } + second, err := LookupSymbols(link, []string{"main"}) + if err != nil { + t.Fatalf("lookup via hardlink: %v", err) + } + + k1, err := binaryKeyFor(bin) + if err != nil { + t.Fatal(err) + } + k2, err := binaryKeyFor(link) + if err != nil { + t.Fatal(err) + } + if k1 != k2 { + t.Fatalf("same file yielded different cache keys:\n %+v\n %+v", k1, k2) + } + if len(first) != len(second) { + t.Errorf("cached result differs: %d vs %d entries", len(first), len(second)) + } +} + +// A binary that does not export the probe points must be recorded as +// Found=false rather than erroring, so it is never re-parsed. Non-Node +// executables reaching this path are the common case, and re-parsing them per +// pid is what made this hot. +func TestLookupSymbolsCachesNegativeResults(t *testing.T) { + bin, err := exec.LookPath("true") + if err != nil { + t.Skip("no /bin/true available") + } + targets, err := LookupSymbols(bin, []string{"definitely_not_a_real_symbol_xyzzy"}) + if err != nil { + t.Fatalf("absent symbol should not error: %v", err) + } + got, ok := targets["definitely_not_a_real_symbol_xyzzy"] + if !ok { + t.Fatal("absent symbol missing from result map; it must be cached as not-found") + } + if got.Found { + t.Error("absent symbol reported as found") + } +} + +func TestLookupSymbolsUnreadableBinaryIsNotCached(t *testing.T) { + missing := filepath.Join(t.TempDir(), "does-not-exist") + if _, err := LookupSymbols(missing, []string{"main"}); err == nil { + t.Fatal("expected error for unreadable binary") + } + if _, err := binaryKeyFor(missing); err == nil { + t.Error("expected stat error for missing file") + } +} + +// Distinct binaries must not share an entry. +func TestBinaryKeyDistinguishesFiles(t *testing.T) { + dir := t.TempDir() + a := filepath.Join(dir, "a") + b := filepath.Join(dir, "b") + if err := os.WriteFile(a, []byte("aaaa"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(b, []byte("bbbbbb"), 0o600); err != nil { + t.Fatal(err) + } + ka, err := binaryKeyFor(a) + if err != nil { + t.Fatal(err) + } + kb, err := binaryKeyFor(b) + if err != nil { + t.Fatal(err) + } + if ka == kb { + t.Fatal("different files produced the same cache key") + } +}