Limit growth of OTLP trace buffer#12052
Conversation
mcculls
added
type: feature
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR improves OTLP trace export robustness in dd-trace-core by tracking buffered payload size to proactively flush before scheduled intervals, enforcing a hard OTLP protobuf buffer cap, and preventing flush failures from terminating the trace-processing thread.
Changes:
- Add
sizeInBytes()toOtlpTraceCollectorand implement it for JSON and protobuf OTLP collectors (leveraging newJsonWriter.sizeInBytes()andOtlpProtoBuffer.sizeInBytes()). - Proactively flush OTLP traces in
OtlpPayloadDispatcherwhen buffered size crosses ~5 MiB. - Add a 64 MiB hard cap to
OtlpProtoBuffergrowth and guardTraceProcessingWorker.flushIfNecessary()against flush exceptions.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dd-trace-core/src/test/java/datadog/trace/common/writer/OtlpPayloadDispatcherTest.java | Updates test collector to support size tracking for new proactive-flush behavior. |
| dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceProtoCollector.java | Exposes protobuf-buffered size via sizeInBytes() for threshold-based flushing. |
| dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceJsonCollector.java | Exposes JSON-buffered size via JsonWriter.sizeInBytes() for threshold-based flushing. |
| dd-trace-core/src/main/java/datadog/trace/core/otlp/trace/OtlpTraceCollector.java | Adds the sizeInBytes() API to enable dispatcher-side payload growth control. |
| dd-trace-core/src/main/java/datadog/trace/core/otlp/common/OtlpProtoBuffer.java | Adds 64 MiB cap and size reporting to prevent unbounded OTLP protobuf buffering. |
| dd-trace-core/src/main/java/datadog/trace/common/writer/TraceProcessingWorker.java | Prevents flush exceptions from killing the processing loop. |
| dd-trace-core/src/main/java/datadog/trace/common/writer/OtlpPayloadDispatcher.java | Adds proactive flush when collector-reported buffer size exceeds threshold. |
| components/json/src/main/java/datadog/json/JsonWriter.java | Adds sizeInBytes() to measure buffered JSON output (after flushing). |
Comments suppressed due to low confidence (2)
dd-trace-core/src/main/java/datadog/trace/core/otlp/common/OtlpProtoBuffer.java:22
- The class Javadoc says this buffer "doesn't have a bounded length", but the new
MAX_CAPACITY_BYTESmakes it explicitly bounded. Updating the Javadoc will prevent future confusion about expected behavior.
public final class OtlpProtoBuffer {
// hard limit to avoid unbounded buffering; matches OTLP spec's recommended default
private static final int MAX_CAPACITY_BYTES = 64 << 20; // 64 MiB
dd-trace-core/src/main/java/datadog/trace/core/otlp/common/OtlpProtoBuffer.java:146
- The new 64 MiB hard cap in
checkCapacity()is a significant behavior change, but there is no test coverage ensuring an oversized payload reliably fails fast with the expected exception/message (and without corrupting buffer state). Consider adding a unit test that grows the buffer beyond the cap and asserts the failure mode.
// (uses long arithmetic so overflow can be detected before allocating)
long newSize = ((long) oldSize + required + initialCapacity - 1) & -initialCapacity;
if (newSize > MAX_CAPACITY_BYTES) {
throw new IllegalStateException(
"OTLP payload exceeds maximum buffer size of "
+ MAX_CAPACITY_BYTES
+ " bytes: "
+ oldSize
+ " bytes buffered, "
+ required
+ " more requested");
}
ByteBuffer newBuffer = ByteBuffer.allocate((int) newSize);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4cc8f8423
ℹ️ 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".
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
b4cc8f8 to
c7effa5
Compare
|
🎯 Code Coverage (details) 🔗 Commit SHA: 7ea79e4 | Docs | Datadog PR Page | Give us feedback! |
c7effa5 to
56590b8
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56590b87ef
ℹ️ 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".
56590b8 to
98c4491
Compare
dd-octo-sts
Bot
added
the
tag: ai generated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98c4491b2c
ℹ️ 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".
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Left minor comments of JSON component
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new flush path bounds OTLP buffering and the executable JSON accounting check matched emitted bytes for escaped Unicode span values. One resilience hazard remains: flush now catches every Throwable, including fatal JVM errors and sender failures that previously reached worker failure accounting; no additional tests recommended because the existing tests already cover the changed branches and the remaining concern is error-policy behavior.
📊 Validated against 2 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 98c4491 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
98c4491 to
b098984
Compare
* Proactively flush OTLP traces between scheduled flushes to try to keep payload size below 5 MiB * Add hard limit of 64 MiB to OTLP payloads, as recommended in https://opentelemetry.io/docs/specs/otlp/ Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
b098984 to
7ea79e4
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
dd-trace-core/src/test/java/datadog/trace/common/writer/OtlpPayloadDispatcherTest.java:120
- Because
collectoris a shared field, this test leaves state behind (spansToExportandfakeSizeInBytes) when it intentionally does not flush. That makes the suite order-dependent/flaky (later tests can see leftover spans and/or trigger unexpected proactive flushes). Clear/reset the collector state at the end of the test (or instantiate a new collector per test).
void belowFlushThresholdDoesNotTriggerProactiveFlush() {
OtlpPayloadDispatcher dispatcher = new OtlpPayloadDispatcher(sender, collector);
collector.fakeSizeInBytes = (5 << 20) - 1; // one byte under FLUSH_THRESHOLD_BYTES
dispatcher.addTrace(singletonList(sampledSpan()));
verifyNoInteractions(sender);
}
dd-trace-core/src/test/java/datadog/trace/common/writer/OtlpPayloadDispatcherTest.java:130
- This test sets
collector.fakeSizeInBytesbut doesn’t reset it afterwards. Since the same collector instance is reused across tests, this can make later tests order-dependent by triggering unexpected proactive flushes.
@Test
void atFlushThresholdTriggersProactiveFlush() {
OtlpPayloadDispatcher dispatcher = new OtlpPayloadDispatcher(sender, collector);
collector.fakeSizeInBytes = 5 << 20; // exactly FLUSH_THRESHOLD_BYTES
dispatcher.addTrace(singletonList(sampledSpan()));
verify(sender).send(any(OtlpPayload.class));
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ea79e4a7e
ℹ️ 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".
What Does This Do
Motivation
Avoids the following exception when large amounts of trace data is collected:
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]