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
…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.
This commit gives memory traces their own seam:
* `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
changed the title
gnurizen
changed the title
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.
What
Splits memory profiles off the otel
TraceReportercontract onto a dedicatedParcaReporter.ReportMemoryTracespath.Previously memory profiles rode on
ReportTraceEvent: the oomprof adapter built a fakelibpf.Traceper sample, stuffed alloc/free counters onto it via a synthetic memory payload frame (oomprofMemoryFrame = 0xFF) or in another incarnation stuffed the values in TraceEventMeta and taggedmeta.Origin = TraceOriginMemory. That pushed memory-specific shape into the otel-facing trace interface and took one writer-lock acquisition per sample. Stuffing memory profiles throughReportTraceEventwasn't really buying us anything and if we're gonna have the agent do general memory profiling we want to fix the fine grained locking issue. So this removes some fork deltas and improves performance.Pairs with a fork-side change in opentelemetry-ebpf-profiler that removes the now-unused
TRACE_MEMORYtrace origin. This PR should merge before that fork change is pinned into parca-agent, sincemainhere still referencesTraceOriginMemoryin the old memory path.