picatz / picatz/flowstate

observability: continuous profiling and runtime debugging, and how they join the trace story

Open
#524 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

design kind/design-record observability
Dominant language
Go
Stars
9
Forks
0
Avg merge
3h 3m
Merged PRs (30d)
509

Description

Problem

#522 asks for one system with four views: trace, logs, metrics, and a profile of
the worker that ran a step. Profiles are the piece not started. #422 names it
directly ("the profile story") and defers it on a stated tripwire (profiles
stable in opentelemetry-go, or the collector's profiles pipeline reaching GA).
This issue is the research pass that tripwire asked for, and a recommendation
for the smallest first slice that does not wait on either.

Measured against the tree today: no net/http/pprof import anywhere, no
/debug/pprof route, no CPU or heap profiling endpoint on flow server or
flow server dev. The only place a pprof profile is used at all is
pkg/flowstate/v1/engine/goroutineleak_test.go, added by #501, which builds
with GOEXPERIMENT=goroutineleakprofile and reads the goroutineleak profile
once per CI run to assert the async coroutine drain does not leak. That is a
test-time consumer, not an operator-facing surface. Nothing today lets a person
running flow server in production look at a live profile of the worker.

Where profiling would actually pay off here, ranked

Being honest about the shape of this system matters before recommending
anything: a durable workflow engine's worker spends most of its wall time
blocked on Temporal, on outbound HTTP the http task makes, and on timers. A
CPU profile of a mostly-idle worker is not the thing an operator needs, and
"add continuous profiling" as a blanket goal would mostly produce flame graphs
of runtime.gopark.

Ranked by where CPU or memory profiling data actually explains something a
trace or a metric cannot:

  1. Goroutine leak in the async drain. #501 already found the shape of this
    bug class exists (the scope-unwind join in asyncStep.wait) and built a
    CI-time detector for it. The natural next step is not a new profiling
    pipeline, it's making that same signal reachable live, not just in a
    scheduled test run; see the flight recorder discussion below.
  2. CEL evaluation pegging a worker. DefaultCostLimit bounds CEL by cost,
    not time, per this repo's own bounding doctrine, but a legitimate expression
    that is expensive-but-within-budget across many concurrent evaluations is
    exactly a CPU-profile-shaped problem: it shows up as "the worker is slow"
    with no single request to blame, and a trace only tells you which spans took
    long, not which function inside the evaluator burned the time.
  3. A parser under a pathological document. Named directly in this repo's
    "bound anything that consumes untrusted input" doctrine (the YAML alias
    billion-laughs case). A profile is the tool for confirming a bound holds
    under the adversarial input it was designed for, more than it is an
    always-on production surface.
  4. Memory growth across Continue-As-New. Plausible and worth watching, but
    nobody has reported this yet, and Continue-As-New exists specifically to
    bound history size, and the mechanism this repo already has is closer to a
    direct fix (checking Continue-As-New actually fires on schedule) than a
    profiling problem. Ranked lowest of the four because it is a hypothesis, not
    an observed pain point.

Everything below is scoped against #1-#3. #4 is worth a metric (a gauge on
history size or continuation count) more than a profile.

The flight recorder (runtime/trace, Go 1.25+)

This is the most interesting piece for this system specifically, and worth
taking seriously rather than filing as "someday."

What it is: trace.NewFlightRecorder keeps a bounded in-memory ring of
execution trace data (goroutine scheduling, blocking, stack traces, flow
events between goroutines) and you call WriteTo to snapshot whatever is
currently in the ring. Overhead is low enough that Go's own docs describe
continuous tracing as viable in production since 1.21, and the recorder adds
no cost when not snapshotting beyond the ring's own upkeep. Bounds are
explicit and set by the caller: MinAge, how far back the ring reliably
retains data (the docs suggest roughly 2x your problem's time window, so 10s
for a 5s deadline), and MaxBytes (expect roughly 2-10 MB/s of trace data
depending on load, so a 1 MiB buffer is typical). Nothing about it interacts
with Temporal's own goroutine management: it observes the runtime, it does
not participate in scheduling, so it says nothing special about Temporal SDK
goroutines beyond what it says about any other goroutine on the process.

What would trigger a snapshot here, concretely:

  • A workflow failing in a way both drivers must agree cares about: the two
    drivers disagreeing on outcome for the same input is exactly the kind of
    rare, hard-to-reproduce failure a flight recorder exists for.
  • A deadline exceeded on a step, since CLAUDE.md already names "CEL evaluation
    is bounded by cost, not by time" as a known gap, and a snapshot at the moment a
    bound is hit is the direct evidence for whether time, not cost, is actually
    the resource under pressure.
  • The goroutineleak detector firing, connecting #501's CI-time check to a
    live trigger: if the same detection ran in production (or an operator asked
    for it), a snapshot at the moment of detection captures the goroutine's
    actual blocking history instead of just its final stack.

Cost: a go tool trace snapshot file is not small once written, and it
carries stack traces and goroutine labels, which, per #522's invariant 2,
means it needs the same containment discipline that traces, logs, and metrics
already have if any of that data could carry a workflow name, step id, or
value from evaluated CEL. Snapshot triggers must fire deliberately (an
explicit failure path, not "always dump on any error") both to bound the data
volume and to keep the containment surface small. This is a local capture
mechanism, not something OTLP or a collector understands: it produces a Go
trace file, consumed by go tool trace, which is a real gap for the "one
system with four views" goal in #522: this data does not join the trace
story through OTLP, it sits beside it as an artifact an operator has to know
to go get.

Span-to-profile correlation (grafana/otel-profiling-go)

What it does: wraps an existing TracerProvider and attaches trace_id /
span_id as pprof labels on CPU profile samples, using pprof's own label
propagation so descendant spans inherit the trace id. It does not run a
profiler itself; you still run runtime/pprof or a Pyroscope client
separately, and it only tags the samples so a backend (Grafana's Traces-to-
Profiles) can join a slow span to the CPU profile taken during it.

What it costs: minimal in dependencies, and the labeling itself is cheap
(pprof labels are a documented, lightweight mechanism). The real cost is
everything upstream of it: you need continuous CPU profiling running
somewhere for the labels to attach to anything.

Stability: this is a small library (roughly a hundred stars, under fifty
commits at last check) with an explicit limitation that only CPU profiling is
fully supported today. It is not part of the OTel Go SDK; it is Grafana's own
glue for Pyroscope specifically, which conflicts with #522's invariant 4
("portability first... nothing may assume [ClickHouse/Grafana]"). Adopting it
would mean adopting a Grafana-specific correlation mechanism ahead of the
OTel profiling signal actually reaching a stability level where a
vendor-neutral equivalent exists.

Plainly: this is premature. Not because the idea is bad, but because
running continuous CPU profiling on a worker whose CPU time is rarely the
bottleneck (see the ranking above) to feed a correlation library tied to one
vendor's backend is solving a problem this repo does not have yet, ahead of
solving the one it does (an operator has no way to pull a profile at all,
continuous or otherwise).

The eBPF profiler (open-telemetry/opentelemetry-ebpf-profiler)

Whole-system, cross-language, host-level or DaemonSet, no code
instrumentation. It requires root (or CAP_BPF/CAP_PERF_ADMIN/
CAP_SYS_RESOURCE) and Linux 4.19+ (amd64) or 5.5+ (arm64). It is pre-v1,
untagged in the sense pkg.go.dev flags ("not the latest module version"), and
implements an experimental proto extension, not a finished OTel signal.

This does not fit inside flowstate's own deployment shape at all. flowstate
does not control the host, does not want to require root for the process
that runs it, and this tool profiles the whole machine, not the flowstate
process specifically. It is squarely an operator's infrastructure choice: a
Kubernetes platform team can run this DaemonSet against every pod on a node
including flowstate's, entirely outside anything this repo ships or
configures. Flowstate should not obstruct that (nothing in the process
should assume it is the only thing being profiled, and resource attributes
should stay consistent enough that an operator's eBPF profiler and
flowstate's own traces can be correlated by service.name if the operator
wants to), but flowstate should not build or document an integration with
it. It is not this repo's tool to adopt; it is the infrastructure layer
below it.

OpenTelemetry profiling signal status

Alpha, per OTel's own documentation, with the standard caveat that Alpha
means further breaking change without notice. This lags traces, metrics, and
logs, which are all Stable. #422 already recorded the right tripwire for
this: revisit when profiles reach Stable in opentelemetry-go or the
collector's profiles pipeline reaches GA, whichever comes first, and not on
a calendar date. Nothing in this research changes that call: the signal has
not moved to Stable since #422 was filed, and the one thing that has
changed since (this issue's tripwire check) is worth writing down here so
"revisit" has a place to leave a note the next time someone checks.

Security posture of a debug endpoint

This repo already has the right model to extend: cmd/flow/routing.go
mounts /healthz deliberately unauthenticated and deliberately
empty-handed, and "an unauthenticated endpoint that describes the deployment is
reconnaissance served on request" is the comment already sitting there. A
pprof endpoint is a much larger version of the same problem: /debug/pprof/*
exposes command-line arguments, environment-derived build info, full stack
traces of every goroutine (which can carry argument values), and heap dumps
that can carry live data. Per this repo's fail-closed doctrine, that surface
cannot be default-mounted the way net/http/pprof's package-level init()
tempts you to wire it (registering onto http.DefaultServeMux as a side
effect of import, which this repo's explicit-mux style already avoids by
construction).

If a debug surface is added, it needs: its own listener bound to loopback or
a separate address by default, not merged onto the RPC mux /healthz sits
on; authentication when it is not loopback-only; and a decision on whether it
ships behind a flag that defaults off, mirroring how --insecure-no-auth
already requires an explicit, logged opt-in elsewhere in this file.

Recommended first slice

Small, and deliberately does not touch OTel's profiling signal, Grafana's
correlation library, or eBPF:

  1. A flow server --debug-addr flag (default unset) that, when set,
    binds a second http.Server on that address running the standard
    net/http/pprof handlers, built by hand onto its own mux rather than the
    package-level default mux. Off by default, loopback-friendly, separate
    port from the RPC surface, no auth wiring needed for the common case
    (operator SSHes to the box or port-forwards), matching the fail-closed
    posture --insecure-no-auth already sets a precedent for.
  2. A documented go tool pprof / go tool trace runbook, not new code
    beyond (1): how an operator pulls a CPU profile or a goroutine dump from a
    worker they suspect is CEL-bound or leaking, using the endpoint from (1).
  3. A flow debug snapshot (or similar) command wrapping the flight
    recorder
    , triggered manually at first (an operator runs it when they
    already know something is wrong), before any automatic trigger. This gets
    the ring buffer running with sane bounds and proves the mechanism works
    against this codebase before wiring it to fire automatically on a
    deadline-exceeded or leak-detected condition. The automatic trigger from
    the flight recorder section above is the natural next step once the manual
    path is proven, not part of this first slice.
  4. Extend #501's goroutine-leak detector from CI-only to a debug-endpoint
    check
    , reusing endpoint (1): the same goroutineleak profile the deep
    tier already reads once a week becomes something an operator can ask for
    live, at cost of nothing new to build once the endpoint exists.

Recommend deferring

  • grafana/otel-profiling-go: premature per the analysis above. Revisit
    only if continuous CPU profiling is adopted for an independent reason
    first (it is not needed to ship the first slice).
  • OTel's own profiling signal / SDK support: Alpha. #422's tripwire
    stands: Stable in opentelemetry-go, or the collector's profiles pipeline
    GA, whichever comes first.
  • opentelemetry-ebpf-profiler: not this repo's tool to adopt. Keep
    resource attributes consistent enough that an operator running it
    independently can still correlate by service.name, and do not build
    against it.
  • Automatic flight-recorder snapshot triggers (deadline exceeded,
    driver-disagreement, leak detected firing a snapshot without an operator
    asking): wait until the manual path in the first slice above is in use and
    has produced at least one snapshot worth looking at, so the trigger
    conditions are chosen from a real snapshot rather than guessed.

What would have to be true to adopt the newer pieces

  • OTel profiling signal reaches Stable in go.opentelemetry.io/otel (or the
    collector's profiles pipeline reaches GA), #422's tripwire, restated here
    because it now gates this workstream too.
  • A real report of CPU or memory pressure from CEL evaluation or a worker
    under load, not a hypothesis. The ranking above put this second, and
    second means "the next slice after the debug endpoint proves useful," not
    "unconditionally worth building."
  • If Grafana/Pyroscope-style correlation is ever pursued, it has to be
    balanced against #522 invariant 4 (portability first), either paired with
    an equivalent vendor-neutral mechanism, or accepted explicitly as an
    optional, non-default integration the way ClickHouse already is.

Relationship to #522 and #422

Filed as one of the workstreams #522 asks each area to file separately.
Answers #422's open item on the profile story with a recommendation to wait
on adoption of the OTel signal itself, while giving #522's goal (one
operator moving between four views without re-deriving how they relate) a
concrete, small, honest first step that does not require waiting: a debug
endpoint an operator can reach, a runbook for using it, and a flight-recorder
snapshot command that connects directly to the leak class #501 already
proved exists and is watching for weekly.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with cmd/flow/routing.go and the existing flow server flag and listener setup, then read the referenced issues #422, #501, and #522. Review how a separate debug listener could fit the current fail-closed posture and how the documented go tool pprof and go tool trace workflow would be described. Done means a bounded first-slice recommendation and runbook with the OpenTelemetry and vendor-specific options explicitly deferred.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, documentation, observability
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.