Skip to content

docs: README#1

Open
necco-c wants to merge 1 commit into
masterfrom
readme/initial
Open

docs: README#1
necco-c wants to merge 1 commit into
masterfrom
readme/initial

Conversation

@necco-c
Copy link
Copy Markdown

@necco-c necco-c commented May 29, 2026

Reviewer notes for ciprof

⚠️ Reviewer action required

The author's GitHub account does not have admin on this repo, so the About description and topic tags below could not be applied automatically. Please apply them via the repo's web UI (top of the repo page → ⚙ next to "About") when merging:

About description (paste verbatim)

Zero-instrumentation eBPF build profiler for GitHub Actions: see where CI time goes, by step.

(108 chars)

Topic tags (15)

ebpf bpf linux github-actions ci ci-cd profiler profiling observability performance build-performance tracing yeet uprobes kernel

First cluster (ebpf, bpf, linux, github-actions, ci, ci-cd, profiler, profiling) is the obvious-search set. yeet, uprobes, and observability carry the category language across the corpus.

First cluster (ebpf, bpf, linux, github-actions, ci, ci-cd, profiler, profiling) is the obvious-search set. yeet, uprobes, and observability carry the category language across the corpus.

Flagged for review (grounding ≥ 2)

  • How it works / inet_csk_accept omission — earlier hand-produced README listed inet_csk_accept (fexit) as one of the BPF attach points. It is not in ciprof.bpf.c. The actual program list is sched_process_exec (tp_btf, default), do_execveat_common (fentry, alt via -DUSE_TP_BTF_EXEC=0), sched_process_exit (tp_btf), tcp_connect (fentry), tcp_close (fentry). This draft omits the accept hook. Confirm before publishing.
  • Requirements — kernel version floor — grounding 2. "Linux 5.10+ (5.15+ recommended)" is the conventional floor for fentry-on-BTF; nothing in the source states it. Existing README also says 5.10+; safe to ship if you want a number stated.
  • What you're looking at — DNS / hostnames — grounding 2. Source code (format.js) only formats IP:port or [ipv6]:port from kernel bytes. No DNS resolution. The earlier hand-produced README's caveat ("IP:port only") is consistent with the code; the earlier sample output showing registry.npmjs.org is not consistent with the code. One of them is wrong — almost certainly the sample output (mocked). Confirm.
  • What you're looking at — heuristic % assumption — grounding 2. state.js formatPct divides by 100 * 1024 * 1024 / 1000 which decodes to "100 MB/s assumed throughput." Stated in code; the framing as a "fixed 100 MB/s assumption" is the inference.
  • Honest caveats — overhead — grounding 3. The "small but not zero" framing is reasonable but the source quotes no microseconds-per-event number. Earlier draft said "~2µs per exec/exit event, ~1µs per TCP event" — that's a 3+ extrapolation, not in the code.
  • Community questions — overhead, strace/time comparison, multi-tenant safety — grounding 2-3. The ciprof-side facts (runner-PID filtering, no syscall trapping) are in the code. The overhead characterizations of strace/time and the "safe on shared infra" framing are general knowledge / inference, not source facts.
  • Building from source — gitignored artifacts — grounding 2. Makefile shows vmlinux.h and bin/ as generated; the repo currently has no .gitignore to confirm they're excluded. Conventional but flagged.
  • License — MIT contradiction — grounding 1 (the contradiction itself) + reviewer decision needed. BPF C file: SPDX-License-Identifier: GPL-2.0 and SEC("license") = "GPL". Repo: no LICENSE file. Existing README: states "MIT" with no file backing it. The draft above omits MIT and states only the GPL/BPF fact. Pick a license, add a LICENSE file.

Source-level contradictions worth resolving in the repo (not in the README)

  • LICENSE file missing. Either add LICENSE (MIT, Apache, or GPL — pick one) or remove license claims from the existing README.
  • inet_csk_accept hook was claimed in prior README drafts but never implemented. Either implement it or stop referencing it.

Template-level observations

None this run. The existing template caught all the categories of flag that surfaced (license, kernel version, DNS contradiction, overhead estimates, multi-tenant safety inference, strace/time comparison). No new rule worth promoting.

Replace the placeholder README with the full template-driven version
covering quick start, primer, use cases, output walkthrough, hook list,
caveats, FAQ, and build steps. Add GPL-2.0 LICENSE to match the BPF
program's SEC("license") = "GPL" declaration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant