Skip to content

Serialize traces into a chunked buffer instead of one 5 MiB array - #13096

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 9 commits into
masterfrom
dougqh/chunked-buffer
Oct 30, 2026
Merged

gh-worker-dd-mergequeue-cf854d[bot] merged 9 commits into
masterfrom
dougqh/chunked-buffer

Conversation

@dougqh

@dougqh dougqh commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

What Does This Do

Replaces the trace writer's single 5 MiB serialization array with a chunked buffer, so the writer no longer holds a humongous array for the tracer's lifetime.

  • ChunkedBuffer (internal-api, toolbox): a write buffer made of chunks whose whole array object is at most 256 KiB, so no chunk is ever humongous on G1, ZGC or Shenandoah.
    • Allocates nothing until the first write.
    • On reset(), keeps the chunks the last payload used plus half of any earlier surplus, so a burst drains over a few flushes.
    • Capacity is exact, counted in data bytes, so payload sizes and counts are unchanged.
    • Typed puts never straddle a chunk boundary, so the fast path is a single bounds check.
    • By default, running out of room doesn't throw: the buffer discards further writes, and mark() rewinds the message and returns false. A subclass can throw instead by overriding onCapacityExceeded().
  • ChunkedStreamingBuffer (communication): the StreamingBuffer used by PayloadDispatcherImpl, which also serves CI Visibility and LLM Obs.
    • Throws BufferOverflowException at the first write that doesn't fit, like FlushingBuffer. Mappers rely on being interrupted there: CI Visibility and LLM Obs update event counts and processor decisions at the end of map(), so letting map() finish would count the rejected trace in the flushed payload.
    • StreamingBuffer.mark() returns a boolean to match ChunkedBuffer. It's always true for buffers that throw on overflow, which is every buffer used with MsgPackWriter.format().
  • Payload holds its body as a list of parts (one per chunk). writeTo no longer consumes it, so a payload can be written again.
  • Rewind on failure (first commit). On any exception from a mapper, format() now rewinds to the last mark, sends the completed traces as their own payload, and resets the mapper.
    • Before: only BufferOverflowException was handled. Anything else left a partial trace in the buffer, the next mark() absorbed it, and the agent rejected the whole payload, losing every trace sent with it.
    • Also fixes CiTestCovMapperV2 counting events before a trace was fully written, which could make the header count too high when a trace failed or overflowed partway.
  • Perf review rule J13: flag large retained byte[] buffers, and suggest ChunkedBuffer. The guide's LLM Obs example now separates per-call cost, which a long LLM call absorbs, from retained size, which it doesn't.

Motivation

The 5 MiB array (2 + 2 MiB for v0.5) is a G1 humongous object on heaps up to 16 GB. It's pinned in old gen, never compacted, and counted toward the concurrent-cycle trigger, a permanent addition to the app's live set.

Spring PetClinic at 128m (JDK 17, G1 with 1 MB regions, real Agent; class histogram after a full GC):

noagent before this PR Δ
Live objects 32.1 MB 50.6 MB 45.6 MB −5.0 MB
Heap used after full GC 31.5 MB 52.6 MB 46.8 MB −5.9 MB
Humongous regions 0 11 5 −6

The buffer was about 27% of the agent's live-set overhead on this app.

Serialization CPU (PayloadBufferBenchmark, v0.4, 5 MiB buffer, arm64): about +7% / +12% / +22% ns per formatted trace on JDK 8 / 17 / 21. This runs on the background writer thread, off the request path. It's an intentional trade for app-heap health.

Additional Notes

  • The other 5 humongous regions in the measurement are also the agent's, filed as follow-ups:
    • APMLP-2159: the DSM payload buffer, allocated even when Data Streams is off.
    • APMLP-2160: the client-stats buffer.
    • APMLP-2161: the agent jar's central directory, held by the JDK.
  • Not changed here:
    • The v0.5 dictionary (a second 2 MiB FlushingBuffer).
    • OkHttpUtils.ByteBufferRequestBody still consumes its buffers on writeTo, which matters if OkHttp retries a request after the body has started sending. Separate fix.
  • Heap sweep pending: the full 64–92m sweep (young GCs per request, where full GCs start) hasn't been run for this build yet.

Jira ticket: APMLP-2114

🤖 Generated with Claude Code

dougqh and others added 5 commits October 27, 2026 18:16
MsgPackWriter.format() only caught BufferOverflowException. Any other
throwable from a mapper (a concurrent modification, a bug) left the
partial message in the buffer, and the next mark() absorbed it, so the
whole payload failed to decode and every trace in it was lost.

Add StreamingBuffer.rewind(), which drops everything written since the
last mark. On a failure, format() now rewinds, sends the completed
messages as their own payload (mapper state such as v1's string table
may still refer to the rewound bytes), resets the mapper and rethrows.

CiTestCovMapperV2 counted events as it wrote them, so a trace that
failed or overflowed part-way left the payload's header count too
high. It now counts once the whole trace is written, as
LLMObsSpanMapper does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A toolbox buffer for serializing payloads of a few MiB without one large
array. Each chunk is a regular-sized object (at most 256 KiB, header
included) on G1, ZGC and Shenandoah, where a single multi-MiB array is
humongous or otherwise a large-object allocation.

- typed puts never straddle chunks, so the fast path is one bounds check
- mark()/rewind() keep only completed messages
- running out of capacity never throws: the buffer fails, discards
  writes, and the next mark() rewinds and returns false
- chunks are allocated on first use, and reset() keeps only the chunks
  the completed messages needed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The trace writer kept a single 5 MiB array (2 + 2 MiB for v0.5) for its
lifetime: a humongous object on heaps up to 16 GB, pinned in old gen
and counted toward G1's concurrent-cycle trigger. PayloadDispatcherImpl
now writes into a ChunkedStreamingBuffer: 256 KiB chunks, allocated on
first use, trimmed after each send to what the last payload needed.

- StreamingBuffer.mark() returns false when a message didn't fit and
  was rewound, so the chunked buffer never throws for overflow;
  format() handles that the same way as BufferOverflowException.
  Existing buffers return true and keep throwing.
- ChunkedBuffer counts capacity in data bytes, so payload sizes and
  counts are unchanged.
- Payload holds its body as parts, and writeTo no longer consumes it,
  so a payload can be written again. This also fixes LLM Obs payloads
  repeating their headers on a partial channel write.

PetClinic at 128m (JDK 17, G1, 1 MB regions): live set -5 MB, heap
used after full GC -5.9 MB, humongous regions 11 -> 5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Buffer

Add J13: an array object over 256 KiB is a large-object allocation on
every region-based collector, and humongous on G1 with 1 MB regions
past 512 KiB. A retained one adds to the app's live set for the
tracer's lifetime (flag with confidence, SEV-2); a per-flush one takes
the humongous slow path (flag as measure, SEV-3). Points to
ChunkedBuffer and records the power-of-two header trap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- ChunkedBuffer.reset() keeps the chunks the last payload used plus half
  the surplus the previous reset kept, so payloads that vary in size don't
  free and reallocate chunks every cycle; a burst drains over a few
  resets.
- Payload.writeFully replaces the hand-written write loops in the payload
  classes; CI Visibility payloads write duplicates of their header so a
  payload can be written again.
- Benchmarks share DDSpanHelper.singleRootTrace; TraceMapperBenchmark uses
  the shared BlackholeBuffer instead of a nested copy.
- J13: humongous threshold at 512 KiB for G1; 256-512 KiB and per-flush
  arrays are flag-as-measure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dougqh dougqh added type: feature Enhancements and improvements comp: core Tracer core tag: performance Performance related changes tag: ai generated Largely based on code generated by an AI or LLM labels Oct 28, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dougqh
dougqh marked this pull request as ready for review October 28, 2026 01:21
@dougqh
dougqh requested review from a team as code owners October 28, 2026 01:21
@dougqh
dougqh removed the request for review from a team October 28, 2026 01:21
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T01:26:17.846164Z 05993fd Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@dougqh
dougqh requested a review from mtoffl01 October 28, 2026 01:21

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05993fdb9b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@datadog-datadog-prod-us1-2 datadog-datadog-prod-us1-2 Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bits Code Review: FAIL

Silent chunked-buffer overflow advances mapper state for rejected traces, producing malformed CI Visibility and LLM Observability payload counts. It also clears cached LLM processor decisions, causing processors to run again during retry.

Open Bits AI session

🤖 Bits Code Review · Commit 05993fd

@datadog-datadog-prod-us1-2

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 14.76 s 14.62 s [+0.2%; +1.7%] (maybe worse)
startup:insecure-bank:tracing:Agent 13.57 s 13.68 s [-1.4%; -0.1%] (maybe better)
startup:petclinic:appsec:Agent 17.24 s 17.13 s [-0.2%; +1.4%] (no difference)
startup:petclinic:iast:Agent 17.03 s 17.06 s [-0.9%; +0.6%] (no difference)
startup:petclinic:profiling:Agent 16.63 s 17.03 s [-3.5%; -1.3%] (significantly better)
startup:petclinic:sca:Agent 17.22 s 16.89 s [+0.9%; +2.9%] (maybe worse)
startup:petclinic:tracing:Agent 16.24 s 16.19 s [-0.5%; +1.2%] (no difference)

Commit: d71bb54c · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

With sticky failure, a message that overflowed let Mapper.map() run to
the end before mark() rejected it. Mappers rely on overflow interrupting
them: CiTestCycleMapperV1, CiTestCovMapperV2 and LLMObsSpanMapper count
events at the end of map(), so the rejected trace was counted in the
payload flushed before the retry. Its header then claimed more events
than its body held, and the intake would reject it. LLMObsSpanMapper
also cleared its processor decisions, so the retry ran the user's
processor a second time.

ChunkedBuffer gets an onCapacityExceeded() hook, a no-op by default (the
toolbox buffer stays sticky). ChunkedStreamingBuffer overrides it to
throw BufferOverflowException, matching FlushingBuffer, so mappers are
interrupted exactly as before.

Tests run the CI test cycle, CI coverage and LLM Obs overflow paths
through the chunked buffer; they fail without the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…LLMObs example

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The J13 retained-size check stays; the LLMObs example goes back to how it
reads on master.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dougqh

dougqh commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-10-30 00:05:19 UTC ℹ️ Start processing command /merge


2026-10-30 00:05:25 UTC ℹ️ MergeQueue: Pull request is not mergeable yet

It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.

  • Run /code blockers to see what is blocking it.
  • Run /remove to cancel it.

2026-10-30 00:53:08 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in master is approximately 2h (p90).


2026-10-30 02:21:51 UTC ℹ️ MergeQueue: This merge request was merged

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit e71aee5 into master Oct 30, 2026
623 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the dougqh/chunked-buffer branch October 30, 2026 02:21
@github-actions github-actions Bot added this to the 1.69.0 milestone Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM tag: performance Performance related changes type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants