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") + } +}