Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ correlation is an upgrade, not a dependency.
| `profilesCacheRefreshRate` | duration | `1m` | Profile cache refresh rate |
| `procfsScanInterval` | duration | `30s` | Procfs scan interval |
| `procfsPidScanInterval` | duration | `5s` | Per-PID procfs scan interval |
| `syscallPollInterval` | duration | `5s` | How often the seccomp syscall tracer fetches its eBPF map for live event delivery (profile-building and rule-based alerting). Termination-time capture is handled separately and doesn't depend on this value — see [docs/features/syscall-poll-interval.md](features/syscall-poll-interval.md). |
| `maxDelaySeconds` | int | `30` | Max random delay for jitter |
| `maxJitterPercentage` | int | `5` | Max jitter percentage |

Expand Down
144 changes: 144 additions & 0 deletions docs/features/syscall-poll-interval.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# Syscall Tracer: On-Demand Fetching

Container profiles could end up with 0 or partial syscalls even when the container executed
tens of distinct syscalls (kubescape/node-agent#922). This documents the root cause and the fix,
including why the fix ended up restoring the pre-PR-#591 architecture's shape rather than
tuning the post-#591 one.

## Background: how this worked before PR #591

Before PR #591 ("Feature/refactor container watcher"), node-agent used a **non-destructive**
`Peek(mntns)` on the vendored native seccomp tracer (`pkg/gadgets/advise/seccomp/tracer`, old
`amirmalka/inspektor-gadget` fork): a plain `bpf_map_lookup_elem`, no delete. Two independent
consumers each registered this same callback and drove it on their own schedule:

- `ContainerProfileManager` called it on demand, exactly when saving a profile (periodic
checkpoint or termination) — never on a timer of its own.
- `RuleManager` ran its **own 5-second ticker per monitored container**
(`pkg/rulemanager/v1/rule_manager.go`, `syscallPeriod = 5 * time.Second`) purely for real-time,
syscall-based rule alerting, fully decoupled from profile-saving.

PR #591 moved the tracer to the OCI-gadget framework (`advise_seccomp` image), whose generic
map-iterator mechanism (`GADGET_MAPITER`, `pkg/operators/ebpf/maps.go` in the
`matthyx/inspektor-gadget` fork) reads batches via `BPF_MAP_LOOKUP_AND_DELETE_BATCH` — a
destructive drain. The refactor consolidated to a single shared tracer instance with one
internal periodic fetch (`operator.oci.ebpf.map-fetch-interval`, ended up at `30s`) whose decoded
events fan out to every consumer (`ContainerProfileManager`, `RuleManager`, metrics) alike. That
single shared schedule is what regressed rule-alerting latency from the original 5s to 30s, and
is also what caused #922: a container terminating between two 30s ticks lost whatever it
executed since the last one, because nothing else could recover it before the container's data
was removed.

Note: the destructiveness of the read was never actually why a single shared schedule was
required — nothing stops running two independent instances of the same gadget image, each
with its own attach and its own private map, at different cadences. The real reason a single
schedule existed is simpler: there was only one tracer instance and one internal ticker driving
it, and every consumer got whatever cadence that one ticker happened to use.

## The fix

Rather than picking a single fixed interval that compromises between "fast enough for alerting"
and "cheap enough to not matter," the tracer's own internal fetch schedule is removed entirely,
and *all* fetching — periodic or not — goes through one explicit call:

- **`ebpfoperator.TriggerManualMapFetch(names ...string)`** (added in
`matthyx/inspektor-gadget#12`) requests an immediate, out-of-band fetch from a running map
iterator, independent of any schedule. The OCI gadget is started with
`map-fetch-interval: "0"` and `map-fetch-count: "0"`, which — per `maps.go`'s iterator loop —
disables its own ticker entirely; the iterator then does nothing except wait to be triggered
(plus the existing flush-on-stop safety net when the whole gadget shuts down).
- **`SyscallTracer.Peek()`** (pkg/containerwatcher/v2/tracers/syscall.go) calls that trigger.
- **`SyscallTracer.runPeekLoop`**, started in `Start`, calls `Peek` on `syscallPollInterval`
(default `5s`, matching the original `RuleManager` cadence) — this is what now drives live
event delivery to every consumer (profile-building and rule alerting alike), restoring the
original alerting responsiveness. Unlike the pre-#591 design, this is **one global ticker**
regardless of how many containers are being traced (the old design ran one 5s ticker *per
container*), since one `TriggerManualMapFetch` call drains and dispatches every currently
tracked container's pending syscalls in a single batch.
- **`ContainerProfileManager.SetSyscallFlusher`** lets `TracerFactory` additionally wire
`Peek` to be called once more, out of that schedule, right before a container's final forced
profile save (`flushAndSettle` in pkg/containerprofilemanager/v1/helpers.go, called from both
termination paths in monitoring.go) — closing even the up-to-`syscallPollInterval` gap the
periodic schedule alone would still leave at exactly the moment a container disappears.
`flushAndSettle` waits a fixed `postSyscallFlushSettleDelay` (`500ms`) afterward for the
resulting data to land before the profile is snapshotted and cleared. `ContainerProfileManager`
no longer goes through the generic event queue/worker pool for syscalls at all (see
[Batching](#batching-bypassing-the-generic-pipeline-for-containerprofilemanager) below); the
wait covers `SyscallTracer.callback`'s own dispatch goroutine and any per-container
`SyncChannel` backpressure in `ContainerProfileManager.ReportSyscalls`, not queue latency. This
wiring only happens when the syscall tracer is actually enabled (`SyscallTracer.IsEnabled`), so
a disabled tracer costs nothing.

## Why this isn't a regression risk for alerting

`SyscallEventType` events fan out to `RuleManager` and a metrics counter through the generic
per-event pipeline (`pkg/containerwatcher/v2/event_handler_factory.go`). `RuleManager` evaluates
rules against each syscall event in real time and drives rule-based alerts through
`pkg/ruleadapters/adapters/syscall.go` (e.g. "unexpected syscall executed"), including
post-learning while `EnableRuntimeDetection` keeps the tracer running. Removing the periodic
schedule entirely (fetching only at `ContainerProfileManager`'s own save points, which happen
every `updateDataPeriod` — default 10 minutes — or at termination) would have delayed that
alerting path by the same amount, which is why `syscallPollInterval` still drives a real,
independent periodic `Peek()` call — it isn't only there for `ContainerProfileManager`
(see [Batching](#batching-bypassing-the-generic-pipeline-for-containerprofilemanager) below for
why `ContainerProfileManager` is no longer one of that fan-out's targets).

## Batching: bypassing the generic pipeline for ContainerProfileManager

`SyscallTracer.callback` decodes one eBPF fetch into a batch of syscall names per container. The
generic pipeline models one event per occurrence, so turning that batch into individual
`SyscallEventType` events (one `AddEventDirect`, dedup-key computation, and 2-handler dispatch
loop each) is pure per-item overhead for a data source that was never actually a discrete event
stream — it's a periodic snapshot of a persistent per-mntns bitmap. That overhead scales with how
many *distinct* syscalls a container has executed since its last fetch, which is worst right
after a container starts.

`RuleManager`'s rule matching and the generic dedup cache both key off a single
`event.syscall` value, so they still need one event per syscall — batching can't safely change
that without a breaking rule-schema change (a list field with `in`/`exists` semantics instead of
scalar `==`). `ContainerProfileManager` has no such constraint: it only ever folds each syscall
into a per-container `mapset.Set[string]`, so `SyscallTracer.callback` calls
`ContainerProfileManagerClient.ReportSyscalls(containerID, syscalls []string)` directly — a
single container-lock acquisition and size update for the whole batch — instead of emitting a
separate event per syscall for it to pick up off the queue. `containerProfileManager` is
correspondingly absent from `ehf.handlers[utils.SyscallEventType]`; `RuleManager` and `metrics`
are unaffected and still receive one event per syscall exactly as before.

Two things this direct call requires that the generic pipeline used to provide for free:

- **Filtering out unresolved container IDs.** The map covers every mount namespace on the node,
including ones never resolved to a container (host processes, etc.) —
`EventHandlerFactory.ProcessEvent`'s `if enrichedEvent.ContainerID == "" { return }` used to
drop those before they ever reached `ReportSyscall`. `callback` now does that check itself
before calling `ReportSyscalls`, to avoid spamming `ContainerProfileManager.withContainer`'s
"invalid empty containerID" error on every such row, every tick.
- **Not blocking the shared fetch-processing path.** Previously `ReportSyscall` reached
`ContainerProfileManager` through the generic queue and one of `WorkerPoolSize` pooled
goroutines, so a slow or blocked container only ever tied up one interchangeable worker.
`callback` is invoked synchronously, once per currently-traced container, by the single
goroutine that processes each fetch — and `ContainerProfileManager.ReportSyscalls` can itself
block (`withContainer` sends on a container's `SyncChannel`, which is bounded, while holding
that container's lock, when a profile crosses `MaxTsProfileSize`). `callback` therefore
dispatches `ReportSyscalls` on its own goroutine rather than calling it inline, so one
container's backpressure can never stall the processing of every other container's syscalls
for that tick, or the next `Peek`-triggered fetch.

## Concurrency

`ContainerProfileManager.syscallFlusher` (the callback `SetSyscallFlusher` registers) is stored
via `atomic.Pointer[func()]`, not a plain field. `SetSyscallFlusher` is called once during
startup wiring (`TracerFactory.CreateAllTracers`), but by that point `StartContainerCollection`
has already begun enumerating existing containers and starting their `monitorContainer`
goroutines — a container that starts and terminates in that window would call `flushAndSettle`
(reading `syscallFlusher`) concurrently with that write. A plain field would be an unsynchronized
data race there, silently able to reintroduce #922's exact bug at exactly the highest-churn
moment there is: node-agent startup or restart.

## Known limitations

`TriggerManualMapFetch` is fire-and-forget and `flushAndSettle`'s settle delay is a bounded wait
for the event pipeline, not an absolute guarantee tied to the fetch itself. Under severe
scheduler contention or a saturated event queue, the flushed events could in principle still
arrive after the delay elapses; termination would then fall back to whatever the tracer already
had from the last periodic `Peek` (at most `syscallPollInterval` old) — the same kind of race as
before, just far less likely and far smaller in scope than the original 30-second window.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ require (
zombiezen.com/go/sqlite v1.4.0 // indirect
)

replace github.com/inspektor-gadget/inspektor-gadget => github.com/matthyx/inspektor-gadget v0.0.0-20260824132042-5bba57473b53
replace github.com/inspektor-gadget/inspektor-gadget => github.com/matthyx/inspektor-gadget v0.0.0-20260825192450-2b683d258349

replace github.com/anchore/syft => github.com/kubescape/syft v1.32.0-ks.2

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -919,8 +919,8 @@ github.com/mailru/easyjson v0.0.0-20190312143242-1de009706dbe/go.mod h1:C1wdFJiN
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
github.com/mailru/easyjson v0.9.0 h1:PrnmzHw7262yW8sTBwxi1PdJA3Iw/EKBa8psRf7d9a4=
github.com/mailru/easyjson v0.9.0/go.mod h1:1+xMtQp2MRNVL/V1bOzuP3aP8VNwRW55fQUto+XFtTU=
github.com/matthyx/inspektor-gadget v0.0.0-20260824132042-5bba57473b53 h1:gFcmFBYF1u5p+T7MPY9WcvBafNIsvpnWNJb93XXhJkA=
github.com/matthyx/inspektor-gadget v0.0.0-20260824132042-5bba57473b53/go.mod h1:cwCFczq1LJ6Frpur0Vr5Ncic77a9ihki07Xpwzy+ItI=
github.com/matthyx/inspektor-gadget v0.0.0-20260825192450-2b683d258349 h1:9fxhaVRFGjJIOXR0NVU0+hQPI7ixrzYSa/bW1KWYATo=
github.com/matthyx/inspektor-gadget v0.0.0-20260825192450-2b683d258349/go.mod h1:cwCFczq1LJ6Frpur0Vr5Ncic77a9ihki07Xpwzy+ItI=
github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
Expand Down
8 changes: 8 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ const NodeNameEnvVar = "NODE_NAME"
const PodNameEnvVar = "POD_NAME"
const NamespaceEnvVar = "NAMESPACE_NAME"

// DefaultSyscallPollInterval is SyscallPollInterval's default, matching the pre-PR-#591
// RuleManager alerting cadence (see pkg/containerwatcher/v2/tracers/syscall.go). Exported so
// SyscallTracer's own fallback (used when constructed with a zero-value config.Config, e.g. in
// tests) can reference the same value instead of duplicating the literal.
const DefaultSyscallPollInterval = 5 * time.Second

// EventDedupConfig controls eBPF event deduplication before CEL rule evaluation.
// ProfileProjectionConfig controls rule-aware profile projection behaviour.
type ProfileProjectionConfig struct {
Expand Down Expand Up @@ -122,6 +128,7 @@ type Config struct {
ProfileProjection ProfileProjectionConfig `mapstructure:"profileProjection"`
ProfilesCacheRefreshRate time.Duration `mapstructure:"profilesCacheRefreshRate"`
StorageRPCBudget time.Duration `mapstructure:"storageRPCBudget"`
SyscallPollInterval time.Duration `mapstructure:"syscallPollInterval"`
RuleCoolDown rulecooldown.RuleCooldownConfig `mapstructure:"ruleCooldown"`
UpdateDataPeriod time.Duration `mapstructure:"updateDataPeriod"`
WorkerChannelSize int `mapstructure:"workerChannelSize"`
Expand Down Expand Up @@ -205,6 +212,7 @@ func LoadConfigOptional(path string, errNotFound bool) (Config, error) {
viper.SetDefault("exitCleanup::cleanupInterval", 30*time.Second)
viper.SetDefault("exitCleanup::cleanupDelay", 5*time.Minute)
viper.SetDefault("updateDataPeriod", 10*time.Minute)
viper.SetDefault("syscallPollInterval", DefaultSyscallPollInterval)
viper.SetDefault("workerChannelSize", 750000)
viper.SetDefault("blockEvents", false)
viper.SetDefault("celConfigCache::maxSize", 100000)
Expand Down
13 changes: 7 additions & 6 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,13 @@ func TestLoadConfig(t *testing.T) {
BulkRetryMaxDelayMs: 30000,
},
},
WorkerPoolSize: 3000,
EventBatchSize: 15000,
EventDedup: EventDedupConfig{Enabled: true, SlotsExponent: 18},
WorkerChannelSize: 750000,
BlockEvents: false,
ProcfsScanInterval: 30 * time.Second,
WorkerPoolSize: 3000,
EventBatchSize: 15000,
EventDedup: EventDedupConfig{Enabled: true, SlotsExponent: 18},
WorkerChannelSize: 750000,
BlockEvents: false,
ProcfsScanInterval: 30 * time.Second,
SyscallPollInterval: 5 * time.Second,
RuleCoolDown: rulecooldown.RuleCooldownConfig{
CooldownDuration: 1 * time.Hour,
CooldownAfterCount: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,19 @@ type ContainerProfileManagerClient interface {
ReportSymlinkEvent(containerID string, event utils.LinkEvent)
ReportHardlinkEvent(containerID string, event utils.LinkEvent)
ReportNetworkEvent(containerID string, event utils.NetworkEvent)
ReportSyscall(containerID string, syscall string)
// ReportSyscalls reports a batch of syscalls for a container in a single operation. The
// syscall tracer bypasses the generic per-event pipeline entirely for this method (see
// SyscallTracer.callback), so this is the only entry point for syscall data -
// ContainerProfileManagerClient intentionally has no single-syscall equivalent.
ReportSyscalls(containerID string, syscalls []string)
ReportDroppedEvent(containerID string)
RegisterForContainerEndOfLife(notificationChannel chan *containercollection.Container)
OnQueueError(profile *v1beta1.ContainerProfile, containerID string, err error)
// SetSyscallFlusher registers a callback invoked right before a container's final
// forced profile save (on termination or max-sniffing-time), requesting an immediate
// out-of-band fetch of any not-yet-polled syscalls for that container so they aren't
// silently discarded (see SyscallTracer.Peek, kubescape/node-agent#922).
SetSyscallFlusher(flush func())
}

type Enricher interface {
Expand Down
6 changes: 5 additions & 1 deletion pkg/containerprofilemanager/containerprofile_manager_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (a ContainerProfileManagerMock) ContainerCallback(_ containercollection.Pub
// noop
}

func (a ContainerProfileManagerMock) ReportSyscall(_ string, _ string) {
func (a ContainerProfileManagerMock) ReportSyscalls(_ string, _ []string) {
// noop
}

Expand Down Expand Up @@ -63,6 +63,10 @@ func (a ContainerProfileManagerMock) RegisterForContainerEndOfLife(_ chan *conta
// noop
}

func (a ContainerProfileManagerMock) SetSyscallFlusher(_ func()) {
// noop
}

func (a ContainerProfileManagerMock) ReportNetworkEvent(_ string, _ utils.NetworkEvent) {
// noop
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/containerprofilemanager/v1/containerprofile_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,26 @@ type ContainerProfileManager struct {
completionNotifier objectcache.CompletionNotifier

lifecycleTracker *otelsetup.ProfileLifecycleTracker

// syscallFlusher, if set, is called right before a container's final forced profile
// save to request an immediate out-of-band fetch of its not-yet-polled syscalls. See
// SetSyscallFlusher and kubescape/node-agent#922. Stored via atomic.Pointer: it's written
// once during startup wiring (TracerFactory.CreateAllTracers, after StartContainerCollection
// has already begun enumerating containers), and flushAndSettle reads it from whatever
// per-container monitorContainer goroutine happens to be running at that time - a plain
// field would be an unsynchronized data race between that write and those concurrent reads.
syscallFlusher atomic.Pointer[func()]
}

func (cpm *ContainerProfileManager) SetCompletionNotifier(n objectcache.CompletionNotifier) {
cpm.completionNotifier = n
}

// SetSyscallFlusher implements containerprofilemanager.ContainerProfileManagerClient.
func (cpm *ContainerProfileManager) SetSyscallFlusher(flush func()) {
cpm.syscallFlusher.Store(&flush)
}

func (cpm *ContainerProfileManager) notifyCompleted(containerID string) {
if cpm.completionNotifier != nil {
cpm.completionNotifier.NotifyContainerCompleted(containerID)
Expand Down
Loading
Loading