Skip to content

fix(profiler): keep partial results when a PID fails, and explain pprof failures - #139

Merged
blue4209211 merged 4 commits into
mainfrom
fix/profiler-per-pid-tolerance
Oct 5, 2026
Merged

blue4209211 merged 4 commits into
mainfrom
fix/profiler-per-pid-tolerance

Conversation

@mayankpande88

Copy link
Copy Markdown
Contributor

Summary

  • A PID the profiler can't attach to no longer fails the whole run. That can be a shell, a helper process, or a process that isn't the target runtime.

    • Every PID is profiled, and each one that succeeds publishes its result as before.
    • The run succeeds when at least one PID did, and emits a notice listing the skipped PIDs and why.
    • It fails only when every PID failed, with each reason: PID <n>: <reason>; PID <m>: <reason>.
  • A flamegraph that can't be rendered now fails that PID (bpf, perf, py-spy, austin). Before, the run ended as a success with no result.

  • Go pprof scrape failures name the cause:

    • the endpoint requires authentication (401/403);
    • no /debug/pprof handler (404);
    • nothing listening (connection refused).

    When the listening port can't be detected, the error says the default :8080 was tried.

Type of change

  • Bug fix
  • CI / build

Test plan

  • Unit tests:
    • the shared helper: mixed results, all failing, all succeeding, the notice, stagger and concurrency;
    • each profiler's all-PIDs-fail path;
    • the flamegraph-failure path for the four tools;
    • pprof error mapping and the port fallback.
  • New Docker e2e scripts, run in Code Verify:
    • test/e2e/python-austin-multi-pid.sh: a Python process next to a non-Python one, both start orders, with the PIDs found by the agent itself.
    • test/e2e/go-pprof.sh: Go servers with pprof open, behind basic auth, not registered, and with nothing listening.
Engineering detail
  • Shared helper. common.ProfilePIDs replaces eight copies of the worker-pool fan-out.
    • It keeps the stagger between PIDs, but no longer sleeps after the last one.
    • An empty PID list is now an error.
    • The alitto/pond dependency is dropped.
  • Result events are unchanged: one result event per published file. A partial success still emits the successful PID's result first, followed by the notice and progress: ended. Before, a failing PID ended the run with an error event, after any result already published, and PIDs not yet started were cancelled.
  • Go pprof:
    • Each PID gets its own copy of the job. Concurrent tasks were setting PID on a shared one.
    • The scrape runs through the commander, and failures are read from busybox wget's stderr.
  • The test fakes now lock in invoke, which -race needs once PIDs run on separate goroutines.

Local e2e: built the python and bpf profiler images from this branch (arm64) and ran test/e2e/python-austin-multi-pid.sh, test/e2e/go-pprof.sh and test/e2e/python-austin.sh; all passed.

  • Multi-PID: in both start orders, the Python PID's profile was published with samples (about 400 each), and the non-Python PID was named in a notice: Profiled 1 of 2 PIDs; skipped PID 8: could not launch profiler: austin (PID 8, /usr/bin/sleep): ….
  • Go pprof: the open pprof server produced a profile. The auth, no-handler and no-listener servers produced the pprof endpoint on :6060 requires authentication, no /debug/pprof handler on :9090 (is net/http/pprof registered?) and could not detect the listening port (…), so tried the default :8080: … nothing listening on :8080.
  • Negative control: against images built from main, both new scripts fail. The multi-process run ends with an error event and no notice, and the pprof errors are raw wget output.

Checks:

  • go build, go vet and go test -race ./internal/agent/profiler/... are clean on Linux.
  • golangci-lint shows no new findings, and fixes one existing QF1003.

Checklist

  • go build ./... and go test ./... pass locally
  • Docs (README, flags) updated if behavior changed: no flag or doc change
  • No breaking changes to the agent's stdout JSON envelope. Partial success now also emits a notice event, which is an existing event type.

Every profiler fanned out over the container's leaf PIDs and returned the
first error, which cancelled the PIDs not yet started and failed the whole
run. A container whose process tree has a shell, a helper process or
anything else the tool cannot attach to next to the real workload could
therefore never be profiled without --pid, even though the workload's own
PID would have produced a result.

Run every PID through one shared helper instead. Each successful PID
still publishes its own result event; the run succeeds when at least one
PID did and emits a notice naming the skipped PIDs and why. It fails only
when every PID failed, with each reason in the error:
"PID <n>: <reason>; PID <m>: <reason>". An empty PID list is now an error
rather than a success that publishes nothing.

The Go pprof profiler also set the PID on the shared job from concurrent
tasks, so a later PID could leak into an earlier PID's scrape; each PID
now gets its own copy of the job.

The helper uses a WaitGroup, so the worker-pool dependency is dropped.

The test fakes for the per-PID managers now lock in invoke, which the
helper calls from one goroutine per PID.
bpf, perf, py-spy and austin logged a flamegraph rendering error and
returned nil without publishing anything. With a single PID the run then
ended as a success with no result, leaving the caller waiting for a file
that would never come.

Return the error instead, so the PID counts as failed: the run still
succeeds when another PID published, and otherwise fails with the reason.
The pprof profiler reported every failed scrape as the raw nsenter+wget
command line and stderr, and when it could not find the target's
listening port it silently scraped :8080 instead, so a failure there
read as if the target's own pprof port were down.

Map the failures busybox wget reports to the cause:
- HTTP 401/403: the pprof endpoint on :<port> requires authentication
- HTTP 404: no /debug/pprof handler on :<port> (is net/http/pprof
  registered?)
- connection refused: nothing listening on :<port>

Anything else is passed through as before. :8080 is still tried when the
port cannot be detected, but a failure there now says the port could not
be detected and the default was tried.

The scrape now runs through the profiler's commander, like the other
profilers, so it can be exercised in tests.
python-austin-multi-pid.sh runs a Python program next to a non-Python
process, in both start orders, and lets the agent find the container's
leaf PIDs itself. The Python PID's profile must be published, the other
PID named in a notice, and the run must not fail.

go-pprof.sh builds a small Go server and scrapes it with the bpf image's
own wget: with pprof registered a profile is published; behind basic
auth, without a pprof handler, and with nothing listening, the agent's
error must give the matching reason.

Both run in Code Verify.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors concurrent profiling across all supported languages and tools by replacing the third-party pond library with a new custom common.ProfilePIDs helper. This helper runs profiling concurrently with a staggered delay and handles partial failures gracefully, ensuring that a single failing PID does not fail the entire run. Additionally, error reporting has been improved (especially for Go pprof scrape failures), fake managers have been updated with mutexes to serialize concurrent invocations in tests, and new E2E tests have been introduced. However, a critical compilation error was identified in the new common.ProfilePIDs implementation, where a non-existent Go method is called on a standard sync.WaitGroup.

Comment thread internal/agent/profiler/common/pids.go
@blue4209211
blue4209211 merged commit 04f4a18 into main Oct 5, 2026
13 checks passed
@blue4209211
blue4209211 deleted the fix/profiler-per-pid-tolerance branch October 5, 2026 17:22
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.

2 participants