reporter: give memory traces a dedicated API instead of abusing ReportTraceEvent - #3192
Open
gnurizen wants to merge 1 commit into
Open
reporter: give memory traces a dedicated API instead of abusing ReportTraceEvent#3192gnurizen wants to merge 1 commit into
gnurizen wants to merge 1 commit into
Conversation
gnurizen
marked this pull request as ready for review
June 22, 2026 11:36
gnurizen
changed the title
gnurizen
changed the title
…moryTraces path
Memory profiles used to ride on the otel TraceReporter contract: the
oomprof adapter built a fake libpf.Trace per sample, stuffed alloc/free
counters onto it via a synthetic MemoryPayloadFrame, and called
ReportTraceEvent. That worked but pushed memory-specific shape into the
otel-facing trace interface and burned one writer-lock acquisition per
sample.
Memory profiling upstream is evolving in a different direction and we
want to reduce fork drift so this greases the skids for the future.
* `ParcaReporter` is now an interface (reporter/iface.go) embedding
the otel `reporter.Reporter` + `ExecutableReporter` and adding
`ReportMetrics` and the new `ReportMemoryTraces(samples, meta)`.
* The concrete struct is renamed `arrowReporter`; `New(...)` returns
the interface.
* `arrowReporter.ReportMemoryTraces` writes memory rows v2-only,
computes labels once per batch, takes the v2 writer lock once,
and emits 2-4 rows (inuse_objects / inuse_space and, when
enabled, alloc_objects / alloc_space) per sample.
* The oomprof callback is wired through a new `oomprofAdapter`
(reporter/oomprof_adapter.go) that implements `oomprof.Reporter`,
chunks incoming batches into groups of 100, and forwards each
chunk to `ReportMemoryTraces`.
That lets us delete:
* `extractMemoryPayload` and its callers in `ReportTraceEvent`
(the synthetic-payload-frame side channel).
* The `oomprofMemoryFrame = 0xFF` shim plus its `case` in the
v1 stacktrace builder and the v2 location builder.
* The v1 and v2 `case TraceOriginMemory:` branches in
`reportTraceEvent` / `reportTraceEventV2` (memory no longer
flows through there).
* `SampleEvents` on the concrete reporter, now replaced by the
adapter.
Bonus: the v2 memory path now actually increments `r.memorySamples`
(the previous code only incremented it on the dead v1 branch).
No `main.go` source edits are required — the interface return from
`reporter.New` covers `Start`, `Stop`, `ReportTraceEvent`,
`ReportExecutable`, `ReportMetrics`, and the new
`ReportMemoryTraces`, satisfying every consumer (tracer, metrics,
parcagpu).
gnurizen
force-pushed
the
oomprof-payload-frames
branch
from
August 21, 2026 15:56
40e8601 to
6e972e1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Memory profiling is evolving in a different direction upstream and custom probes are deleting the whole trace origin machinery. Have oomprof use a new "parca" API instead of (ab)using ReportTraceEvent. We wanted to do this for efficiency reasons anyways. See commit message for details.