Repository navigation
[SLES-3039] fix(traces): send oversized trace payloads alone instead of blocking the queue - #1418
clifordshelton wants to merge 6 commits into
Conversation
A trace payload larger than the 12 MB batch cap was put back at the front of the trace queue by `get_batch` and never sent, so every later payload in that sandbox queued behind it until the Lambda ran out of memory. Large traces (tens of thousands of spans) exceed the cap once the extension adds its tags to every span. - Split oversized payloads before queuing them: chunks move into separate payloads, and a chunk that is still too large has its spans split into several chunks of the same trace, as tracer partial flushing does. Trace-level `_dd.p.*` tags are copied onto the first span of each piece. A single span larger than the cap is dropped with a warning. - `get_batch` now sends a payload that is over the cap on its own as a batch by itself instead of putting it back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 1 Pipeline job failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 833e9b8 | Docs | View more details | Give us feedback! |
There was a problem hiding this comment.
🟡 Changes recommended
The sizing reserve can incorrectly drop a near-limit span that would fit in a payload.
1 open finding
What changed in this PR
Splits oversized enriched trace payloads to prevent queue blocking and Lambda OOMs.
Changes:
- Splits payloads by chunks or spans before aggregation.
- Sends every split payload through trace and OTLP pipelines.
- Adds an aggregator fallback for indivisible oversized payloads.
| File | Description |
|---|---|
bottlecap/src/traces/payload_split.rs |
Implements payload splitting and tests. |
bottlecap/src/traces/trace_processor.rs |
Integrates splitting and returns multiple builders. |
bottlecap/src/traces/trace_aggregator.rs |
Prevents oversized payloads from blocking the queue. |
bottlecap/src/traces/mod.rs |
Exposes the splitting module. |
bottlecap/src/otlp/agent.rs |
Queues all generated payload pieces. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The room for copied `_dd.p.*` tags was reserved from every span's budget, although only the first span of each piece gains them and spans that already carry them gain nothing. A span within a few bytes of the limit, such as a large root span, could be dropped even though it fit. Charge each span only for the tags it lacks, and only when it starts a new piece. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@DataDog review |
![datadog-prod-us1-5[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Protobuf framing and copied trace tags are included in the split budget, and both ingestion paths enqueue every resulting piece. The queue fallback prevents an oversized entry from blocking later traces.
🤖 Bits Code Review · Commit a64cb43
lucaspimentel
left a comment
lucaspimentel
left a comment
There was a problem hiding this comment.
Found one metadata-preservation issue in the splitting logic. The unchanged splitter's existing tests pass, but focused regression probes for whole-chunk and span splitting both reproduce the loss described below.
Review by an AI agent
Tracers set `_dd.tags.process` only on the first span of a payload's first chunk, so after splitting, only the payload holding that span carried the process tags. Promote them to the payload-level tags before splitting, as the Agent does, so every output inherits them and their size is counted. An existing payload-level value takes priority. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Match the Go trace-agent, which never splits a trace payload: an oversized payload is sent on its own. Large traces should be split by the tracer with partial flushing (`DD_TRACE_PARTIAL_FLUSH_ENABLED`), which keeps them complete and well under the batch cap. This removes the payload splitting, the trace-level and process tag propagation, and the `process_traces` return type change. What remains is the `get_batch` change: a payload larger than the cap on its own is sent as a batch by itself instead of being put back at the front of the queue, where it blocked every later payload until the Lambda ran out of memory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lym953
left a comment
lym953
left a comment
There was a problem hiding this comment.
Could the splitting utils be reused for other Serverless agents? If so, we may considering moving them to serverless-components.
| let mut pieces = Vec::new(); | ||
| let mut current = Vec::new(); | ||
| let mut current_len = 0; | ||
| let mut dropped = 0; |
There was a problem hiding this comment.
nit: rename as dropped_cnt for clarity
clifordshelton
changed the title
|
@DataDog review |
There was a problem hiding this comment.
More details
Oversized payloads are emitted as standalone batches, allowing subsequent payloads to drain while preserving ordinary batching behavior. The completed static review identified no reportable regression.
🤖 Bits Code Review · Commit cb1806a
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
mergequeue build completed successfully, but the github api returned an error while merging the pr.
DetailsError: PUT https://api.github.com/repos/DataDog/datadog-lambda-extension/pulls/1418/merge: 405 6 of 6 required status checks are expected. [] (Request ID: BDDE:E6328:1A07042:55C7D46:6ACFAA1A) FullStacktrace: |
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
devflow unqueued this merge request: It did not become mergeable within the expected time |
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: waiting
and removed
mergequeue-status: error
labels
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: removed
and removed
mergequeue-status: waiting
labels
…USTSEC-2026-0285 waiver (#1425) ## Problem The "Bottlecap (Rust) / Audit" check fails on every branch, for example #1418. `main` fails the same way. On 2026-10-14, RustSec published three advisories for `rustls` 0.23.43, the version in `Cargo.lock`. `cargo audit` reports them as vulnerabilities and exits with an error. | Advisory | Problem | Fixed in | |---|---|---| | RUSTSEC-2026-0346 | The `Debug` output of client session values shows TLS secrets | 0.23.46 | | RUSTSEC-2026-0348 | Brotli certificate decompression can allocate 1 GiB before authentication | 0.23.46 | | RUSTSEC-2026-0349 | A forged ServerHello can make later connections use only classical key exchange | 0.23.46 | ## Why this PR ignores the advisories We cannot use `rustls` 0.23.46 yet. The block is the same as for RUSTSEC-2026-0285: - On crates.io, `rustls` 0.23.46 requires `aws-lc-rs` 1.18 or later, the same as 0.23.45. - `aws-lc-rs` 1.18 uses AWS-LC-FIPS 4.x, which has no FIPS 140-3 certification yet. Our FIPS builds must use a certified crypto module. For this reason, #1389 reverted the bump to 0.23.45, and `Cargo.toml` keeps `rustls` below 0.23.45. - The non-FIPS builds do not use `aws-lc-rs`, but both builds use the same `Cargo.lock`, so they get the same `rustls` version. ## Impact - **RUSTSEC-2026-0346** applies only to a custom client session store that formats session values with `Debug`. The bottlecap source has no custom session store. - **RUSTSEC-2026-0348** applies only when the `brotli` feature of `rustls` is on. `Cargo.lock` has no `brotli` crate, so the feature is off. - **RUSTSEC-2026-0349** can cause a loss of post-quantum protection for some connections. Keys are not disclosed, and authentication is not bypassed. ## Fix Add the three IDs to the two ignore lists for `cargo audit`: - `bottlecap/.cargo/audit.toml`, which local runs read. - The `ignore` input of the audit step in `rs_ci.yml`. CI reads only this list. ## Testing ### Steps 1. In `bottlecap/` on `main`, run `cargo audit --ignore RUSTSEC-2026-0285`. This is the command that CI runs. 2. On this branch, run `cargo audit` with no flags. This reads `.cargo/audit.toml`. 3. On this branch, run `cargo audit --json` with the four `--ignore` flags from `rs_ci.yml`. ### Result | Check | Before | After | |---|---|---| | `cargo audit`, CI ignore list | 3 vulnerabilities, exit with error | 0 vulnerabilities | | `cargo audit`, `.cargo/audit.toml` | Not run | 0 vulnerabilities | `cargo-audit` 0.22.1 ran on macOS. The CI Audit job on this PR is the check for Linux. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Overview
A trace payload larger than the trace aggregator's 12 MB batch cap (
MAX_CONTENT_SIZE_BYTES) could never be sent.TraceAggregator::get_batchput it back at the front of the queue and returned an empty batch, andget_batchesstops at the first empty batch. Every later trace in that execution environment then queued behind it, and the extension's memory grew until the Lambda ran out of memory (Runtime.OutOfMemory).This happens with very large single traces. A trace with tens of thousands of spans can be well over 12 MB once the extension adds its tags to every span, and since #902 the extension accepts trace requests up to 50 MiB.
This PR changes
get_batchso a payload that is larger than the cap on its own is sent as a batch by itself instead of being put back. This matches the Go trace-agent (appendChunksinpkg/trace/writer/trace.go), which flushes its buffer and always sends an oversized payload on its own, so one payload can never block the queue. If the intake rejects it, it is dropped after the existing retries, as in the Go agent.Why no splitting: an earlier revision of this PR also split oversized payloads in the extension. Following review, that was reverted so bottlecap keeps matching the Go agent. Very large traces should be split by the tracer with partial flushing (
DD_TRACE_PARTIAL_FLUSH_ENABLED=true), which is how APM handles them.Testing
DD_TRACE_PARTIAL_FLUSH_ENABLED=true- click heretest_get_batch_oversized_payload_does_not_block_queue: an oversized payload between two small ones is sent as its own batch and the payloads behind it still go out.cargo test --lib(684 passed),cargo clippy --all-targets -- -D warningsandcargo fmt --checkare clean.provided.al2023arm64 function behind SQS (datadog-lambda-go,periodically,1000flush, 1792 MB), with ~1,300-span traces and occasional ~35k-span traces, at about 2 invocations per second per execution environment:DD_TRACE_PARTIAL_FLUSH_ENABLED=true🤖 Generated with Claude Code