feat: container_id in quark_process and go-quark

[biscout42]

No description provided.

[nicholasberlin]

PR Review: #373 — feat: container_id in quark_process and go-quark

Author: biscout42
Branch: container_id -> main
Commits: 1 | Files changed: 3 | +38 -15
URL: #373
Reviewed commit: 2c1008e
Reviewed by: gpt-5.6-sol

Summary

This PR adds a runtime-qualified container ID to the C process cache and Go binding, populating it during /proc scraping, process events, and fork inheritance. Two correctness issues need to be addressed: malformed systemd cgroups can be published as container IDs, and the kprobe backend cannot refresh an inherited ID when a launcher moves a child into its container cgroup before exec. The Buildkite check passes, but it does not cover these paths.

Findings

Significant

1. Reject malformed cgroup names before publishing an ID (quark.c:1627-1629). parse_container_cgroup() only checks the basename prefix and truncates at the last dot, so a full path such as /system.slice/docker-agent.service is accepted as docker://agent. This was mostly harmless while the result was only an internal Kubernetes lookup key, but the new field exposes the false identity directly to C and Go callers. Require an exact .scope suffix and a nonempty valid runtime ID, and add malformed full-path tests; the current negative cases omit / and fail earlier in safe_basename().

2. Refresh inherited IDs after kprobe cgroup migration (quark.c:544-546). Fork inheritance copies the parent's ID, but kprobe task samples never populate raw_task->cgroup, so a later exec cannot recompute it. In the common container-launch sequence where a runtime forks in its own cgroup, moves the child into a container cgroup, and then execs, the child retains the parent's ID or no ID. Capture or scrape the cgroup at kprobe exec time and test a child moved between recognized cgroups before exec.

Verdict

REQUEST_CHANGES — callers can receive a false or stale container identity in supported runtime paths.

[nicholasberlin]

Refresh inherited IDs after kprobe cgroup migration

TL;DR - I think we should disable container_id for the kprobe backend.

My understanding is that cgroup and container_id are initially scraped from /proc. But new tracefs (kprobe) events provide no cgroup information. On fork, the container_id is now inherited, but not cgroup. Also, if a process is migrated to a new cgroup, there's no way to tell.

It seems there's some things that can be done to add cgroup data to tracefs events, adding PERF_SAMPLE_CGROUP, and asking for cgroup creation events PERF_RECORD_CGROUP. Or at least that's my hazy understanding. I think that will also limit usage to 5.7 kernels and newer.

Basically, the cgroup data for the kprobe backend is bad to begin with. Making it better requires a newer kernel which defeats the purpose of the fallback. (falling back is a bad idea in general).

Perhaps, we should limit this change to only the ebpf backed:
• Keep /proc cgroup scraping for every backend—it provides useful startup state.
• Preserve existing kprobe Kubernetes enrichment based on that scraped cgroup.
• Do not inherit or publish container_id for kprobe; leave it NULL/empty.
• Populate container_id only when the active backend is eBPF.
• Check qq->stats.backend == QQ_EBPF, not qq->flags, because eBPF can fail and fall back to kprobe.

[nicholasberlin]

Reject malformed cgroup names before publishing an ID

This should be addressed, imo.