Skip to content

refactor: simplify node-agent, fix bugs, cut hot-path work - #65

Open
entlein wants to merge 14 commits into
mirrormainfrom
refactor/simplify
Open

refactor: simplify node-agent, fix bugs, cut hot-path work#65
entlein wants to merge 14 commits into
mirrormainfrom
refactor/simplify

Conversation

@entlein

@entlein entlein commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

Simplification and bug-fix pass over node-agent: remove dead code and vestigial machinery, fix real correctness/latency bugs, cut per-event work on the detection hot path, and pin the shared type modules to k8sstormcenter builds. 44 files, +257 / −2,387.

What changed

Dead code / vestigial removal

  • Unwired Prometheus metrics stack (764 lines; OTEL is the only backend), IMDS cloud-metadata path, dead CEL type/helper/converter registration, unregistered procfs adapter, the per-container monitor goroutine + its never-read PID maps.
  • The always-nil TaskBasedEnricher/thirdPartyEnricher extension point across the watcher, both constructors, the tracer factory and all tracers — also drops a per-event []uint64 allocation.
  • MetricsMock, the fimmanager pass-through wrapper (folded into main), and the dead runtime-detection Fatal guard.

Bug fixes

  • SBOM diff-id slice cloned before reverse (was mispairing every layer with another layer's size); HTTPExporter hostname now set; rate limiter suppresses all over-limit alerts; FIM events attributed to the triggering process instead of node-agent itself.
  • getContainerInfo uses IG's O(1) GetContainer and drops an unbounded, never-invalidated cache of stale pointers.
  • dnsmanager reverse lookup bounded with a 2s context (was unbounded on the hot path).
  • networkstream: reverse-DNS/pod resolution moved out of the storage mutex (double-checked), the per-container enrich goroutine no longer retries forever on context.Background(), and the interval send retries instead of dropping on a transient error.
  • host-sensor kernel-var walk closes each fd per file instead of deferring every open fd until the whole /proc/sys/kernel walk returns.
  • NodeProfileManager loop honors shutdown; HealthManager serves a private mux so port 7888 no longer exposes pprof.

Hot path

  • The highest-volume open rule (wasPathOpened) uses an O(1) exact map lookup on the literal Opens.Values; dynamic/wildcard entries are still matched via CompareDynamic on Opens.Patterns. ~205× on a 500-entry profile. Per-rule/per-event pprof.Do label allocation removed.

Cleanup / dependencies

  • One shared exporter bus for the rule and malware managers (was two identical); reuse storage's MergeStrings; pin armoapi-go/utils-k8s-go to k8sstormcenter builds via replace directives.

Performance — CI benchmark (this build vs latest release, real Kind cluster)

  • Memory: 314.5 MB → 264.5 MB (−15.9%), reproducible across runs.
  • CPU: flat (~0.21 cores, within run-to-run noise) — no regression.

Verification

Built and validated through the fork-ci build → component-tests → benchmark chain, all green:

The PR-triggered component-tests check fails at build-and-push-image: Username and password required — a known infra limitation (PR workflows have no registry push credentials; unrelated PRs fail identically). Functional verification is the fork-ci dispatch runs linked above.

Paired with kubescape/storage refactor/simplify.

entlein and others added 13 commits August 10, 2026 23:01
@entlein @claude
refactor: delete dead code in node-agent
0fcecdb 
Remove the unwired Prometheus metrics implementation and its two-impl seam
(OTEL is the only wired metrics backend), the unused IMDS cloud-metadata
path, dead CEL type/helper/converter registration, the unregistered procfs
rule adapter, and the vestigial per-container monitor goroutine with its
never-read shim/pid maps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
refactor: remove unused third-party enricher extension point
b63e3ab 
The TaskBasedEnricher/thirdPartyEnricher hook had no implementation and was
threaded as nil from main through the container-watcher constructors, the
tracer factory, and every tracer. Remove the interface, the parameter and
struct fields at all sites, and the enrichEvent helper (tracers now call the
event callback directly). This also drops the per-event []uint64 syscall-list
allocation the helper required in the open/exec/hardlink/symlink tracers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
fix: correct SBOM layer sizing, HTTP alert hostname, and alert rate l…
353bb35 
…imiting

- syftutil.NewSource: clone the diff-id slice before reversing it, so the
  in-place reverse no longer mutates the original that toRootFS and toLayers
  read, which had paired every layer with another layer's size.
- HTTPExporter: set the host field from os.Hostname() so HTTP alerts and FIM
  reports carry a hostname instead of an empty string.
- HTTPExporter rate limiter: suppress every over-limit alert for the rest of
  the window instead of only the one that crosses the threshold; the
  limit-reached notification is still emitted once.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
perf: exact-match Opens.Values in the open-path rule instead of scanning
b080cc3 
wasPathOpened and wasPathOpenedWithFlags ranged the entire Opens.Values set
calling CompareDynamic on each entry, although the projection routes every
dynamic entry to Patterns and keeps only literal paths in Values (see
projection_apply). Query Values by exact map lookup, matching the exec-path
rule. On a 500-entry profile this drops the Values check from ~50us to ~260ns
per call. Add a benchmark covering the exact-match path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
refactor: pin armoapi-go and utils-k8s-go to k8sstormcenter builds
8f4bf4c 
Redirect the shared armoapi-go and utils-k8s-go type modules to the
k8sstormcenter builds via replace directives, at versions matching the
current pins (armoapi-go v0.0.742, utils-k8s-go v0.0.35). Content is
unchanged at these versions; storage moves from v0.0.696/v0.0.30 to the
common pin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
refactor: drop MetricsMock in favor of MetricsNoop
27a481a 
MetricsMock had a single consumer (the reconciler test's countingMetrics,
which embeds a MetricsManager only to satisfy the interface and overrides the
two methods it asserts on). Embed MetricsNoop instead and delete the mock.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
refactor: merge the fimmanager wrapper into main wiring
f812282 
fimmanager was a thin pass-through around hostfimsensor whose EnableFIM guard
duplicated main's, with dead IsRunning/GetStatus/cloudMetadata surface.
Construct the host FIM sensor directly in main and delete the package.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
fix: attribute FIM events to the triggering process, not node-agent
4d14d6c 
getProcessInfo read /proc/self and os.Getpid(), labelling every fanotify FIM
event with node-agent's own pid, comm, and cmdline. Use the fanotify event's
Pid (the process that caused the event) and read that process's /proc entry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
perf: drop per-event pprof.Do labelling from the detection hot path
1857cd1 
pprof.Do wrapped the CEL evaluation per rule per event and the handler
dispatch per event, allocating a pprof label set on every call solely to
attribute CPU profiles. Call the work directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
refactor: reuse dynamicpathdetector.MergeStrings for cloud-service merge
9a81bb3 
node-agent's MergeCloudServices duplicated storage's MergeStrings (string
set-union). Call MergeStrings, which node-agent already imports, and delete
the copy. MergeStrings also returns deterministic order (existing then new
uniques) rather than map-iteration order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
fix: honor shutdown in node-profile loop, isolate the health mux, dro…
fd881d2 
…p dead guard

- NodeProfileManager.Start's goroutine selected on nothing but time.Sleep, so
  it ran past context cancellation; select on ctx.Done() during the initial
  delay and the interval ticker so it exits on shutdown.
- HealthManager served http.DefaultServeMux, which the blank net/http/pprof
  import also populates, so the health port exposed pprof. Serve a private
  ServeMux with only the liveness/readiness probes.
- Remove the unreachable runtime-detection Fatal in the network-tracing else
  branch (both flags are false there; the true branch already wires DNS when
  runtime detection is on).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
refactor: build one shared exporter bus for rule and malware managers
f5a039e 
Runtime detection and malware detection each called InitExporters with the
same cfg.Exporters, standing up two identical ExporterBus instances
(duplicate HTTP clients, alertmanager clients, and bulk-manager goroutines)
when both were enabled. Build the bus once and share it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein @claude
fix: latency, leak, and fd bugs in the event/network/host-sensor paths
d6324a0 
- getContainerInfo: use ContainerCollection.GetContainer for an O(1) lookup and
  drop the unbounded, never-invalidated containerCache that held stale pointers.
- dnsmanager: bound the hot-path reverse lookup with a 2s context so a slow
  resolver can no longer stall DNS-event processing.
- networkstream: run the reverse-DNS and pod-by-IP resolution outside the
  storage mutex (double-checked) so I/O no longer blocks ingestion; stop the
  per-container enrich goroutine from retrying forever on context.Background();
  and retry the interval send instead of dropping it on a transient error.
- host-sensor: scope the kernel-var file Close per file (readKernelVar) instead
  of deferring every open fd until the whole /proc/sys/kernel walk returns.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein entlein changed the title Refactor/simplify refactor: simplify node-agent, fix bugs, cut hot-path work Aug 11, 2026
@entlein @claude
perf: use the no-op metrics manager unless metrics are actually exported
8cfd25e 
The OTEL metrics manager was constructed unconditionally, so every
Report*/Set*/Inc* call did work even when nothing consumes it: the default
deployment sets prometheusExporterEnabled=false and no OTEL endpoint, which
leaves the global providers no-op, yet each call still built its attribute
cache key and entered the instrument. Measured on the default config with 20
rules per event, that is ~7.2us and 61 allocations per event.

Select the OTEL implementation only when prometheusExporterEnabled is set or
an OTEL endpoint/exporter env var is present — the same condition already used
to gate Go runtime metrics — and fall back to the existing MetricsNoop
otherwise. The MetricsManager interface is write-only, so no behaviour depends
on the recorded values. Also skips the startup pod GET that only served the
cgroup gauges.

The chart maps nodeAgent.config.prometheusExporter=enable to
prometheusExporterEnabled, which the benchmark sets, so metric-scraping and
profiling runs are unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A8UV2B7b6dpDJQci31aC6c
@entlein
entlein force-pushed the refactor/simplify branch from c8290dd to 8cfd25e Compare August 13, 2026 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@entlein