feat(processtree): populate the process start time (SUB-7845)

[AlonLiwsky]

Summary

armotypes.Process.StartTime was declared but assigned nowhere in this repo: the periodic /proc scan read process stats and skipped the start-time field, so it was a hardcoded zero. This populates it, so later work can tell recycled process ids apart.

Nothing reads the new values yet. This is the groundwork for network-stream process attribution (SUB-7786) and pid-reuse hardening (SUB-7846), separated deliberately from the changes that give the value meaning.

Two representations, and the split matters:

• Boot-relative nanoseconds — /proc/<pid>/stat field 22 (clock ticks since boot) converted once via ticks × 10⁷ (USER_HZ=100). Lives in a new pid-keyed side map in the process-tree creator, exposed as GetProcessBootTimeNs. This is the sole identity source.
• Wall-clock Process.StartTime on tree nodes — derived from /proc/stat btime, which has whole-second resolution. Display only. Comparing it for identity is wrong by up to the btime skew.

The conversion happens exactly once, on the way in. Nothing downstream rescales: a division at emission would compile, parse, and even join correctly within one message while silently breaking cross-message identity by seven orders of magnitude.

The part that would otherwise silently do nothing

Three separate functions build a process node from an explicit field list and strip anything absent from it. All three omitted StartTime, so populating the source alone changes nothing downstream — a change that looks like it worked:

• buildBranchToShim — feeds every alert branch and every stream tree
• CopyProcess — the alert bulk manager's merged tree
• EnrichProcess — the bulk manager's merge of overlapping chains

Each now carries the field and each has its own named test, so a future field-strip regression fails by name rather than showing up as an empty value on the wire.

Coverage

The periodic scan runs every 30 seconds, which would leave every process shorter than one interval at zero — and short-lived processes are both the bulk of beacon-style connections and the drivers of pid churn. So a fork or exec that creates a node also reads field 22 on demand: same kernel source, same semantics, just fetched earlier. One read per node creation, never per event. A failed read (process already gone) leaves zero rather than guessing.

This is not the rejected "derive the start time from an exec event" option. That rejection was of event timestamps — an exec stamps the exec instant, not creation, so the value would change on re-exec. convertExecEvent/convertForkEvent/convertExitEvent still set StartTimeNs from the event wall-clock timestamp (epoch ns — a different clock domain), those assignments are untouched, and two tests pin that the value can never reach the side map.

Known gaps, both accepted: in Kubernetes mode the tree refuses to create nodes for host processes, so pre-existing host daemons stay at zero; and a process that dies before its creation event is processed stays at zero.

Recycled-pid guard

A fork's pid is newborn, so a surviving side-map entry can only belong to a process the kernel already recycled the pid away from. Exits linger for exitCleanup.cleanupDelay (5 minutes by default), so this is readily reachable: A exits on pid 4242, the kernel hands 4242 to B, B's fork event arrives, B reports A's creation time — and if B lives under one scan interval the scan never corrects it. That is worse than the zero it replaced, because a consumer joining on (pid, startTime) then concludes A and B are one process.

This guard is scoped to the side map this PR introduces and does not implement pid-reuse hardening. The shared tree node still carries the dead process's comm, cmdline and path — that is SUB-7846's job, and it remains to be done.

Cost of the on-demand read

A single /proc/<pid>/stat open-read-parse measures ~7.5 µs (Go benchmark, Linux container, warm). Every fork event performs one, because the recycled-pid guard drops any stale value before the read decision is made — so the "skip when already known" shortcut does not apply to the fork path.

Fork runs to thousands per second on a busy node. At 1,000/s that is ~7.5 ms of read time per second; at 3,000/s, ~22 ms/s. Under pt.mutex — the tree-wide write lock that every alert type contends on — that would all be hold time blocking readers.

So the fork path reads before acquiring the lock and stores the result under it: the same number of reads, none of them holding the lock, and zero added hold time from fork. This is a local reordering inside code this PR introduces, not a change to lock scope or discipline. Exec keeps its read inside ensureStartTime, where the skip-when-known check makes it conditional and it fires only on first sight of a pid.

TestHandleForkEvent_ReadsStartTimeWithoutHoldingTreeLock pins the property, using TryRLock so a regression fails rather than deadlocks.

The tradeoff: reading before the lock widens the window between event and read, so a pid recycled inside it yields the new incarnation's value. That is the same already-accepted race, slightly wider, and the reuse hardening is what detects it.

Note for reviewers: a live wire value changes

armotypes.Process.StartTime is a time.Time tagged omitempty, and Go's encoder ignores omitempty on structs — so alert payloads already ship "startTime":"0001-01-01T00:00:00Z" today. This changes an existing value from the zero time to a real time rather than adding a new field. Nothing branches on it and there is no hash or equality over Process anywhere in the repo, so it is still additive in effect, but it is not invisible to consumers.

Out of scope and untouched: cmd/host, cmd/ecs, pkg/hostnetworksensor, and the Kubernetes-mode streaming gate.

Testing

Run in a Linux container — this repo cannot be built or tested on macOS, because inspektor-gadget/pkg/utils/host excludes darwin and every package under pkg/processtree imports it transitively:

docker run --rm -v "$PWD":/src -v "$(go env GOMODCACHE)":/go/pkg/mod -w /src \
  -e GOFLAGS=-mod=mod golang:1.25 go test ./pkg/processtree/... ./pkg/utils/...

Test-driven throughout: every test was watched failing for the right reason before the fix, at value level rather than only at compile level. Where a test was written after its implementation, the regression it claims to catch was verified by reverting the fix and watching it fail.

• Both tick→nanosecond conversions are pinned against an independently read field 22 times a literal 10⁷, so drift between the feeder's and the creator's constant fails deterministically. This replaced weaker checks: ns % 10⁷ == 0 only catches a 10× error when ticks % 10 != 0, and a wall-clock proximity check loses sensitivity on a freshly booted node.
• TestManager_GetContainerProcessTree_CarriesStartTime walks the real production path — procfs event → ReportEvent → creator → buildBranchToShim → GetContainerProcessTree — against a creator-populated tree rather than a hand-built fixture.

Full-suite baseline. go test ./pkg/... was recorded on a pristine checkout of origin/main before any change. The failure set is identical after: 19 failing tests before, 19 after, same names — all environmental (pkg/containerwatcher/v2/tracers needs a tracers.tar build artifact; pkg/validator needs privileges to mount /sys/fs/bpf). go vet clean; touched files gofmt-clean.

On -race: pkg/processtree/creator reports 7 data races. They are pre-existing and not from this PR — every frame sits in exit_manager.go around exitCleanupStopChan (startExitManager/stopExitManager/exitCleanupLoop). I ran -race on a detached checkout of origin/main and got the same 7 at the same code sites. That is SUB-7847, which has a fix in flight on a separate branch. No frame touches code added here.

AI Review

Reviewed by Claude Opus 5 in a fresh context with no prior knowledge of the change, against armosec-shared-rules:code-review-standards.

Verdict: APPROVE WITH COMMENTS.

Findings acted on:

• The on-demand reader had no test coverage — every other test on that path injects a fake reader, so the real /proc parse and its constant were unexercised. Confirmed by mutating the constant to 10⁶ and watching the whole suite stay green. Both conversions are now pinned deterministically, as described above.
• A recycled pid inherited the dead process's start time — mechanism reproduced, guard and regression test added above.
• Reader cleanups — /proc is now resolved once instead of re-stating the mount point per call, and the two btime/mount failure modes log instead of degrading silently.

Post-review changes (CodeRabbit)

CodeRabbit reviewed this PR and raised four points; all four are addressed.

• Real defect, mine. The recycled-pid guard cleared the boot-ns side-map entry but left the dead process's wall-clock Process.StartTime on the reused node, because ensureStartTime only assigns the display value when it is zero. For exactly the case the guard exists to handle, the identity value and the display value disagreed. My test asserted only the accessor, which is why it passed. Fixed, and the test now asserts both; the reset also moved to where reuse is actually detected, closing a narrower case where the side-map entry is gone but the node survives.
• /proc read under the tree write lock. Correct, and sharper than when the design accepted it — see Cost of the on-demand read above. The fork read is now taken outside the lock.
• (pid, startTimeNs) is not unique. Documented as a known limitation; the 10 ms tick quantization means a pid recycled inside one tick yields two indistinguishable incarnations. This propagates to consumers building a join key from the tuple, so it is recorded in the feature doc rather than left implicit.
• Markdownlint MD040. Fence tagged.

Reviewer confirmed all seven contract points hold, verifying several by mutation: removing StartTime from each of the three copy sites, removing the side-map delete in exitByPid, and injecting a clock-domain violation into handleForkEvent each fail a dedicated test.

Noted and deliberately not addressed here, as out of scope: utils.CreateProcessTree (pkg/utils/process.go:129, reached from the malware manager) is a fourth node builder that still strips StartTime, so malware alerts will carry a zero start time while other alert types carry a real one. Tracked separately.

Ticket

SUB-7845 — Populate the process start time (node-agent).

Parent: SUB-7784. Unblocks SUB-7786 (streaming attribution) and SUB-7846 (pid-reuse hardening).

Summary by CodeRabbit

• New Features

  • Added process creation-time tracking with both precise identity timestamps and display-friendly wall-clock timestamps.
  • Preserved process start times across process trees, container views, event conversion, and process duplication.
  • Added process start-time lookup for known and active processes.

• Bug Fixes

  • Prevented stale or recycled process IDs from inheriting previous start-time data.
  • Improved cleanup when processes exit or are removed.

• Documentation

  • Documented timestamp semantics, limitations, fallback behavior, and testing requirements.

[coderabbitai]

Warning

Review limit reached

@AlonLiwsky, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 176f5bd7-0ef7-4394-8c84-b61ccce60db0

📥 Commits

Reviewing files that changed from the base of the PR and between a163e77 and e575a26.

📒 Files selected for processing (1)

• pkg/processtree/creator/processtree_creator.go

📝 Walkthrough

Walkthrough

Changes

Process creation time now uses boot-relative nanoseconds for identity and an optional wall-clock value for display. Procfs feeders and readers perform conversion, process trees preserve both values, PID cleanup prevents recycled-PID inheritance, and public APIs expose boot-time lookup.

Process start-time tracking

Layer / File(s)	Summary
Timestamp contracts and event conversion pkg/ebpf/events/procfs.go, pkg/processtree/conversion/*, docs/features/process-start-time.md	Procfs and process events define separate identity and display timestamp semantics. Conversion tests verify both values.
Procfs start-time acquisition pkg/processtree/feeder/*, pkg/processtree/creator/starttime_reader.go	Procfs boot time and process ticks convert to boot-relative nanoseconds and optional wall-clock timestamps. Tests cover conversion and unavailable procfs data.
Process-tree start-time lifecycle pkg/processtree/creator/*	The creator records start times for procfs, fork, and exec paths, avoids rereads on exec, cleans exited PIDs, and replaces stale entries for recycled PIDs.
Process-tree API and propagation pkg/processtree/process_tree_manager*, pkg/processtree/container/*, pkg/utils/processtree_merge*, pkg/containerwatcher/v2/tracers/procfs.go	Manager lookup, container branches, copied nodes, enriched nodes, and procfs tracer events preserve process start times. Tests validate end-to-end propagation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProcfsFeeder
  participant Procfs
  participant ProcessTreeCreator
  participant ProcessTreeManager
  ProcfsFeeder->>Procfs: Read boot time and process stat ticks
  Procfs-->>ProcfsFeeder: Return boot time and start ticks
  ProcfsFeeder->>ProcessTreeCreator: Send StartTimeNs and StartTimeWall
  ProcessTreeCreator->>Procfs: Read start time for new fork or exec PID
  Procfs-->>ProcessTreeCreator: Return process start-time values
  ProcessTreeCreator->>ProcessTreeManager: Expose GetProcessBootTimeNs

Loading

Suggested reviewers: matthyx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 52.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: populating process start times in the process tree.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

pkg/processtree/feeder/procfs_feeder.go (1)

77-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the project logger instead of os.Stderr.

The rest of this package and the sibling newProcfsStartTimeReader in pkg/processtree/creator/starttime_reader.go report failures with logger.L().Warning. A raw fmt.Fprintf to stderr bypasses log level, structure, and collection.

♻️ Proposed change

 	if stat, err := fs.Stat(); err == nil {
 		pf.bootTime = time.Unix(int64(stat.BootTime), 0)
 	} else {
 		// Wall-clock start times will stay zero; boot-relative identity is unaffected.
-		fmt.Fprintf(os.Stderr, "procfs feeder: failed to read btime, StartTimeWall disabled: %v\n", err)
+		logger.L().Warning("procfs feeder: failed to read btime, StartTimeWall disabled", helpers.Error(err))
 	}

Add the github.com/kubescape/go-logger and github.com/kubescape/go-logger/helpers imports, and drop the os import if it becomes unused.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/processtree/feeder/procfs_feeder.go` around lines 77 - 83, Replace the
raw fmt.Fprintf(os.Stderr, ...) call in the procfs feeder boot-time error branch
with the project logger, using logger.L().Warning and helpers as established by
newProcfsStartTimeReader. Add the required go-logger imports and remove the os
import if no longer used, while preserving the existing error message and
behavior.

pkg/processtree/creator/starttime_reader.go (1)

12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

USER_HZ is hardcoded twice in two packages, with only a comment to keep them in agreement. The same tick-to-nanosecond conversion is defined independently in the creator and the feeder. If one changes, the same process receives two different boot-relative identities, each internally consistent, and nothing fails to compile. Export the constant once and have both call sites use it. A shared constant also gives a single place to switch to sysconf(_SC_CLK_TCK) if the 100 Hz assumption ever needs to hold on a non-100 Hz kernel.

• pkg/processtree/creator/starttime_reader.go#L12-L16: remove the local nsPerTick and reference the shared constant.
• pkg/processtree/feeder/procfs_feeder.go#L18-L25: move ticksPerSecond and nsPerTick into a small shared package (for example pkg/processtree/conversion) and export them.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/processtree/creator/starttime_reader.go` around lines 12 - 16, Centralize
the tick-to-nanosecond conversion constants by moving ticksPerSecond and
nsPerTick from pkg/processtree/feeder/procfs_feeder.go lines 18-25 into a shared
package such as pkg/processtree/conversion, exporting them. Remove the local
nsPerTick from pkg/processtree/creator/starttime_reader.go lines 12-16 and
update its conversion to use the shared exported constant; update the feeder to
use the same shared symbols.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/features/process-start-time.md`:
- Around line 30-32: Update the formula code fence near the process start-time
documentation to include the text language identifier, changing the unlabeled
fence around the boot-relative nanoseconds formula to a text-labeled fence.
- Around line 16-23: Update the process identity guidance around
GetProcessBootTimeNs and StartTimeNs to state that StartTimeNs is quantized to
USER_HZ ticks and is not always unique. Document that processes created within
the same tick may share the same (pid, StartTimeNs) tuple, and remove claims
that this value is exact or uniquely discriminating.

In `@pkg/processtree/creator/processtree_creator.go`:
- Around line 184-196: Move the pt.readStartTime I/O out of the locked
handleForkEvent and handleExecEvent paths: read the start-time values before
acquiring pt.mutex, then pass ns and wall into ensureStartTime so it only
updates pidStartTimeNs and proc.StartTime while locked. Preserve the fork-path
stale-entry deletion so the newly read value wins, and optionally retain a
pre-check using GetProcessBootTimeNs to avoid repeated reads for known PIDs.
- Around line 203-211: Update the recycled-PID handling near ensureStartTime to
also reset the reused process node’s wall-clock StartTime to its zero value when
deleting pidStartTimeNs[event.PID]. Keep the side-map cleanup and
ensureStartTime flow intact so the new incarnation’s creation time is assigned
consistently.

---

Nitpick comments:
In `@pkg/processtree/creator/starttime_reader.go`:
- Around line 12-16: Centralize the tick-to-nanosecond conversion constants by
moving ticksPerSecond and nsPerTick from pkg/processtree/feeder/procfs_feeder.go
lines 18-25 into a shared package such as pkg/processtree/conversion, exporting
them. Remove the local nsPerTick from
pkg/processtree/creator/starttime_reader.go lines 12-16 and update its
conversion to use the shared exported constant; update the feeder to use the
same shared symbols.

In `@pkg/processtree/feeder/procfs_feeder.go`:
- Around line 77-83: Replace the raw fmt.Fprintf(os.Stderr, ...) call in the
procfs feeder boot-time error branch with the project logger, using
logger.L().Warning and helpers as established by newProcfsStartTimeReader. Add
the required go-logger imports and remove the os import if no longer used, while
preserving the existing error message and behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ac56916-13ea-4e53-804f-7687ee3a8d60

📥 Commits

Reviewing files that changed from the base of the PR and between ce05eec and 86ee216.

📒 Files selected for processing (21)

• docs/features/process-start-time.md
• pkg/containerwatcher/v2/tracers/procfs.go
• pkg/ebpf/events/procfs.go
• pkg/processtree/container/container_processtree.go
• pkg/processtree/container/container_processtree_test.go
• pkg/processtree/conversion/convert.go
• pkg/processtree/conversion/convert_test.go
• pkg/processtree/conversion/types.go
• pkg/processtree/creator/exit_manager.go
• pkg/processtree/creator/processtree_creator.go
• pkg/processtree/creator/processtree_creator_interface.go
• pkg/processtree/creator/starttime_reader.go
• pkg/processtree/creator/starttime_test.go
• pkg/processtree/feeder/procfs_feeder.go
• pkg/processtree/feeder/procfs_feeder_test.go
• pkg/processtree/process_tree_manager.go
• pkg/processtree/process_tree_manager_interface.go
• pkg/processtree/process_tree_manager_mock.go
• pkg/processtree/process_tree_manager_test.go
• pkg/utils/processtree_merge.go
• pkg/utils/processtree_merge_test.go

[matthyx]

Two notes on the recycled-pid guard, left inline. Everything else checks out: suite green, and the -race failures really are pre-existing (every frame lands on startExitManager/stopExitManager, none on the new code).

[matthyx]

This drops the stale identity but leaves pendingExits[pid] holding the dead process's entry. cleanupDelay later (5 min default) exitByPid then deletes the live recycled process's node along with this freshly-read entry — so the guard's benefit expires within the delay and the process goes back to reading as unknown.

Verified on this head: after performExitCleanup() the node is gone and GetProcessBootTimeNs(4242) is 0 again.

Deleting the pending exit alone isn't right either — A's children would never be reparented and B would inherit them. Retiring the predecessor here does work, and every test in this PR still passes with it:

if _, pending := pt.pendingExits[event.PID]; pending {
    pt.exitByPid(event.PID) // reparents A's children, removes A's node
    ok = false              // fall through to getOrCreateProcess for B
}

Fine to defer to SUB-7846 — but then the feature doc's "the entry is deleted at the same point the tree node is deleted" should say that node may by then belong to a different process.

[AlonLiwsky]

Confirmed — I reproduced it on this head before deciding. After performExitCleanup() the node is gone and GetProcessBootTimeNs(4242) reads 0 again, so the guard's benefit does expire within cleanupDelay. And you are right that dropping the pending entry alone would be worse: A's children would never be reparented and B would inherit them.

Taking your second option and deferring the fix, for a scope reason rather than a disagreement. Retiring the predecessor means calling exitByPid from the fork path — reparenting children and deleting nodes — which is shared process-tree lifecycle that every alert type depends on, in a PR that otherwise only populates a field nothing reads yet. It is also already scoped to SUB-7846, which describes flushing a pending exit when a fork reuses a pid as a layer that needs no start time and is independent of this change. Your snippet is essentially that layer, and I would rather it land there with its own tests for the reparenting behaviour than ride in on an additive change.

Doc updated as you asked, and I went a bit further than the one line since the whole paragraph was overclaiming. It now states that the dead process's pendingExits entry survives the fork; that the delayed cleanup runs exitByPid on what is by then the live successor's node, taking the freshly-read start time with it; that the pid reverts to reading as unknown; and — on the line you quoted — that the node deleted may belong to a different process than the one whose exit scheduled the deletion. It also records that the fix is to retire the predecessor, reparenting rather than merely dropping the pending entry, so whoever picks up SUB-7846 does not have to rediscover why the simpler version is wrong.

Separately, your other comment led to a real fix in the same commit — the wipe is now gated on pendingExits.

[matthyx]

Minor: the wipe runs before the read result is consulted, and applyStartTime no-ops on ns == 0. So a fork event on an existing node whose on-demand read fails leaves both values zero even when the periodic scan had already recorded a correct one.

Reachability is low (the ordered queue is a min-heap on timestamp, so a fork normally lands before the procfs event) and "unknown" beats "wrong", so the trade-off is defensible. But the comment above asserts more than the code can know — an existing node also just means some other path created it first. Gating the wipe on pendingExits presence, per the comment above, makes it precise rather than heuristic.

[AlonLiwsky]

Fixed in a163e77 — gated the wipe on pendingExits presence, as you suggested.

You were right that the comment asserted more than the code could know. Wiping on "an existing node" meant a fork whose on-demand read then failed discarded a value the periodic scan had already recorded correctly, leaving both halves zero. I reproduced that before fixing it. Gating on a pending exit makes the signal precise — the previous holder actually exited — rather than heuristic, and it makes the comment true.

Your reachability assessment was right, and so was "unknown beats wrong" — but that only holds when the alternative is a wrong value, not when we are also throwing away a correct one. With the gate, that case no longer arises.

New test TestHandleForkEvent_DoesNotWipeKnownStartTimeWithoutAPendingExit covers it. The recycled-pid test is unchanged and still passes, since a genuine reuse always has the pending exit present.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/processtree/feeder/procfs_feeder_test.go`:
- Around line 127-148: The wall-clock assertion in the process start-time test
uses an elapsed-time-dependent reference. Build the expected wall-clock value
from the captured btime and stat.Starttime using the specified nanosecond
conversion, then compare event.StartTimeWall against that deterministic value
with an appropriate precision.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a0c18ee-b5da-46bb-bb0e-42354071652b

📥 Commits

Reviewing files that changed from the base of the PR and between 86ee216 and a163e77.

📒 Files selected for processing (21)

• docs/features/process-start-time.md
• pkg/containerwatcher/v2/tracers/procfs.go
• pkg/ebpf/events/procfs.go
• pkg/processtree/container/container_processtree.go
• pkg/processtree/container/container_processtree_test.go
• pkg/processtree/conversion/convert.go
• pkg/processtree/conversion/convert_test.go
• pkg/processtree/conversion/types.go
• pkg/processtree/creator/exit_manager.go
• pkg/processtree/creator/processtree_creator.go
• pkg/processtree/creator/processtree_creator_interface.go
• pkg/processtree/creator/starttime_reader.go
• pkg/processtree/creator/starttime_test.go
• pkg/processtree/feeder/procfs_feeder.go
• pkg/processtree/feeder/procfs_feeder_test.go
• pkg/processtree/process_tree_manager.go
• pkg/processtree/process_tree_manager_interface.go
• pkg/processtree/process_tree_manager_mock.go
• pkg/processtree/process_tree_manager_test.go
• pkg/utils/processtree_merge.go
• pkg/utils/processtree_merge_test.go

🚧 Files skipped from review as they are similar to previous changes (17)

• pkg/processtree/process_tree_manager.go
• pkg/processtree/process_tree_manager_interface.go
• pkg/processtree/conversion/convert_test.go
• pkg/utils/processtree_merge_test.go
• pkg/processtree/creator/starttime_reader.go
• pkg/processtree/creator/exit_manager.go
• pkg/processtree/process_tree_manager_mock.go
• pkg/processtree/container/container_processtree.go
• pkg/processtree/creator/processtree_creator_interface.go
• pkg/processtree/container/container_processtree_test.go
• pkg/utils/processtree_merge.go
• pkg/ebpf/events/procfs.go
• pkg/processtree/feeder/procfs_feeder.go
• pkg/processtree/conversion/convert.go
• pkg/processtree/creator/processtree_creator.go
• pkg/containerwatcher/v2/tracers/procfs.go
• pkg/processtree/conversion/types.go

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
fd -a 'procfs_feeder_test\.go$' . || true

echo "== relevant file lines =="
file="$(fd 'procfs_feeder_test\.go$' . | head -n1)"
if [ -n "${file:-}" ]; then
  sed -n '1,220p' "$file" | cat -n
fi

echo "== search feeder/bootTime/stat usage =="
rg -n "StartTimeWall|StartTimeNs|bootTime|btime|NewProc|stat\\.Starttime|USER_HZ|10_000_000" .

Repository: kubescape/node-agent

Length of output: 32768

---

Make the wall-clock assertion deterministic.

The 5*time.Minute window against time.Now() can fail if this test runs after the process has been alive for that long. Use the captured btime from the same /proc/stat read and stat.Starttime * 1_000_000 * time.Nanosecond to build the expected wall-clock value, then compare event.StartTimeWall to that value.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/processtree/feeder/procfs_feeder_test.go` around lines 127 - 148, The
wall-clock assertion in the process start-time test uses an
elapsed-time-dependent reference. Build the expected wall-clock value from the
captured btime and stat.Starttime using the specified nanosecond conversion,
then compare event.StartTimeWall against that deterministic value with an
appropriate precision.

Source: MCP tools

[matthyx]

thanks!