Og/continue protobuf nesting depth limit by EricaJ6 · Pull Request #26099 · vectordotdev/vector

EricaJ6 · 2026-08-12T23:47:27Z

Note: this continues #25417, which itself continued #25053. That branch had gone stale against
master and had two outstanding review items. Since we hit this bug as a production outage we have
picked the work up rather than let it sit. All original authorship is preserved in the commit
history and in the changelog fragment; thanks to @connoryy and @ganelo for the original design,
which we have not altered beyond the review feedback below.

Summary

Log events nested beyond prost's decode recursion limit can be encoded but never decoded. Vector
writes such a record into a disk buffer successfully, and from that point the sink cannot read it
back and the process refuses to start at all. Recovery requires deleting the buffer directory and
losing everything queued in it.

This PR carries #25417 forward with two changes:

Merged master. The branch was conflicted. Resolving it surfaced three compile breaks, all
from master moving underneath the branch: Bufferable gained a GroupedFinalizable supertrait
that the branch's test fixture did not satisfy; master's TryWriteOutcome<T> refactor replaced
the Result<()> and Option<T> returns the branch was written against; and the blanket
impl<T> Bufferable for T conflicted with the explicit impls this branch introduces.
Made overflow behaviour state-independent, per review feedback. Previously an over-nested
item was pruned while the disk had room but forwarded whole once the disk reported full, so with
a 100 MB disk overflowing to memory the same event took a different path at 99 MB than at 100 MB.

The fix moves the routing decision out of the disk backend and into BufferSender, where the
WhenFull policy already lives:

Bufferable::is_fully_encodable(&self) -> bool is a new non-consuming counterpart to
filter_unencodable, defaulting to true. It lets policy be decided before any filtering.
EventArray implements it as check_event_array_nesting_cost(self).is_ok(), reusing the existing
encode-time gate so the routing decision and the eventual encode cannot disagree.
BufferSender::send under WhenFull::Overflow hands an item failing that check to the overflow
stage intact, before the base stage is consulted at all. The overflow stage may have no
wire-format constraint (an in-memory stage does not), so pruning sub-items on its behalf would
discard events it could accept.
SenderAdapter::try_send loses its is_buffer_full() short-circuit and the KNOWN LIMITATION
comment documenting the deferral. Filtering there is now unconditional, so an item is treated
identically at 99% full and 100% full.
BufferWriter::is_buffer_full returns to private visibility, since nothing outside the writer
needs it now.

One behaviour change beyond the review request, called out because it shifts metrics: removing the
short-circuit also affects WhenFull::DropNewest at capacity. Previously the item was returned
Full unfiltered and counted as an intentional drop. Now it is filtered first, so over-budget
events are counted as unintentional drops via track_dropped and only the remainder is reported
Full. We believe this is more correct, and it is required for state-independence, but it is a
visible change.

References

Closes: Protobuf default recursion depth limit of 32 - fails disk buffering, codec, and Vector source/sink #19315
Related: fix(buffers): add nesting depth limit to prevent protobuf decode corruption #25417 (this PR continues it), fix(buffers): add nesting depth limit to prevent protobuf decode corruption #25053 (the original)
Related: Hitting "failed to decode record immediately after encoding it" frequently #20212 and failed to seek to position where writer left off #25642 appear to be symptoms of the same root cause. Hitting "failed to decode record immediately after encoding it" frequently #20212 reports
writer entered inconsistent state: failed to decode record immediately after encoding it, and
its debug output shows a data-file rollover immediately beforehand, which is the boundary where
the writer must decode a record back in order to return it to the caller. We believe that report
is nesting depth rather than the oversized record suspected in the thread.
Related: fix(disk buffer): recover from decode errors during initialization seek #25691 makes the startup case recoverable but does not stop the poison being written.

Vector configuration

Minimal configuration that reproduces the bug on 0.57.0, with no Kubernetes and no interruption of
any kind:

data_dir: /var/lib/vector
sources:
  stdin:
    type: stdin
transforms:
  parse:
    type: remap
    inputs: [stdin]
    source: |
      . = parse_json!(string!(.message), 128)
sinks:
  poison:
    type: http
    inputs: [parse]
    uri: http://127.0.0.1:9/        # discard port, never drains
    encoding:
      codec: json
    healthcheck:
      enabled: false
    buffer:
      type: disk
      max_size: 268435500
      when_full: drop_newest

Feed a few hundred copies of one 34-object-level line on stdin, holding stdin open for about 15
seconds so the buffer flushes and the reader reads the record back:

{"d":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"leaf":"x"}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}

Before this change, pass one logs:

ERROR sink{component_id=poison component_type=http}: vector_buffers::internal_events:
Error encountered during buffer read. error=failed to decoded record:
InvalidProtobufPayload error_code="decode_failed" error_type="reader_failed" stage="processing"

and restarting against the same buffer directory refuses to come up:

ERROR vector::topology::builder: Configuration error. error=Sink "poison": error occurred
when building buffer: failed to build individual stage 0: failed to seek to position where
writer left off: failed to validate the last written record: failed to decoded record:
InvalidProtobufPayload

At 33 object levels both passes are clean.

How did you test this PR?

Unit tests. Two new regression tests in
lib/vector-buffers/src/variants/disk_v2/tests/filter_metrics.rs cover the near-full and
already-full cases from the review, asserting the unencodable item reaches the overflow stage with
its original event count rather than a pruned one. The pre-existing filter-accounting test in the
same module still passes, which matters because both share the FilterableBatch fixture.

test unencodable_item_overflows_intact_when_base_is_full ... ok
test unencodable_item_overflows_intact_when_base_has_room ... ok
test filter_drops_are_reported_as_unintentional_buffer_drops ... ok

Empirically validating the frame budget. We bisected the failure depth on 0.57.0 in a clean
single process and costed frames from event.proto at 3 per object level and 2 per array level:

payload	decodes	fails
all objects	33 levels, 99 frames	34 levels, 102 frames
1 object plus N arrays, `{"d": [[[...]]]}`	N=48, `3 + 2*48 = 99`	N=49, `3 + 2*49 = 101`

Two structurally different shapes bracket prost's limit of 100 to a single frame, and 99 is the
last value that decodes. That was measured independently of this PR and matches
MAX_VALUE_NESTING_FRAMES = 99 exactly, which we take as good corroboration of the constant chosen
here.

Production context. We hit this on a multi-tenant aggregator when an application logged a stack
trace with several nested causedBy clauses. The buffer sat on ephemeral storage shared across
container restarts, so the poisoned record survived every restart and the process crash looped
indefinitely until the volume was recreated. Two knock-on effects worth noting: a poisoned sink
cannot drain, so it hangs shutdown until the grace period expires and is then killed (out of roughly
fifty components, only the poisoned sinks failed to drain); and once a sink reaches its size
boundary and has to hand a record back to the caller, it fails with the writer entered inconsistent state error from #20212.

Open question for reviewers. The already-full test uses an in-memory base stage rather than a
disk stage, because reliably driving disk-v2's is_buffer_full() to true under the minimum-size
config needs careful record and buffer size tuning, as the prior code comment in this module noted.
We think the substitution is sound for this property, since the unencodable-item decision is taken
in BufferSender before any backend is consulted, so the base stage's type and occupancy are both
immaterial, and that is exactly the invariant being asserted. Happy to invest in a disk-based
variant if you would prefer it.

Is this a breaking change?

Yes
No

Does this PR include user facing changes?

Yes. Please add a changelog fragment based on our guidelines.
No. A maintainer will apply the no-changelog label to this PR.

The existing changelog.d/protobuf_nesting_depth_limit.fix.md fragment has been extended to cover
the overflow behaviour.

…uption Prost enforces a recursion limit of 100 during protobuf decoding. In Vector's proto schema, each level of map nesting costs ~3 prost recursion levels, causing decode failures at Value tree depth 34. Events that encode successfully but fail to decode corrupt the disk buffer irreversibly — subsequent reads and even startup validation fail with InvalidProtobufPayload, requiring manual buffer deletion. Add a MAX_NESTING_DEPTH (33) check at three protobuf encoding boundaries: - EventArray::encode (disk buffer writes) - NativeSerializer::encode (native codec over TCP/file) - VectorSink stream filter (gRPC vector-to-vector path) Events exceeding the limit are rejected before encoding. The vector sink emits ComponentEventsDropped (intentional) to the standard metrics pipeline so operators can monitor rejected events via component_discarded_events_total.

The protobuf encoding path encodes both the event value and the event metadata value into the same message. A deeply nested metadata value would bypass the nesting check and cause the same corruption. Check both values in event_exceeds_max_nesting_depth and check_event_array_nesting_depth.

Traced through prost 0.12's decode path and Vector's generated proto code to derive the exact formula: 4 outer recursion levels (EventArray → LogArray → Log → fields map) + 3 per Value map nesting level (Value message → ValueMap message → map entry). 4 + 3*32 = 100 = RECURSION_LIMIT exactly, confirmed by empirical test probing depths 28-38.

Empirical testing revealed that different proto encoding paths have different prost recursion overhead: - Log Object root (Log.fields path): 4 outer levels → fails at depth 33 - Log non-Object root (Log.value path): 5 outer levels → fails at depth 32 - Metadata value: fails at depth 33 - Trace events: fails at depth 33 With MAX_NESTING_DEPTH=33, the non-Object root path and metadata path had gaps where our check passed but prost decode failed. Lowering to 32 closes all gaps — verified by probing all four paths across depths 28-36 and confirming zero cases where our check passes but prost fails.

Verify that the EventWrapper decode path (used by the vector sink's gRPC receiver) is safe at MAX_NESTING_DEPTH. EventWrapper has fewer outer proto wrappers than EventArray, so our limit is conservative for this path.

Remove the approximate formula (5 + 3*N <= 100) and hedged outer overhead range (3-5) that were not fully verified. The comment now states only what we know for certain: the value was determined empirically and unit tests verify the boundary.

…oryy/vector into connor/protobuf-nesting-depth-limit

The metadata_full encoding path (EventArray → *Array → Event → Metadata → Value) is the tightest proto path, using exactly 100 of prost's 100 recursion budget at MAX_NESTING_DEPTH=32. This was the only path without a roundtrip success test — only gate rejection and decode failure tests existed for metadata. Add explicit prost encode/decode roundtrip tests for both log and metric metadata at depth 32 to prove the tightest path works.

Consolidate the scattered nesting depth tests into a data-driven framework that exhaustively covers every proto encoding path where a Value can appear: - Log value (via Log.fields), Log metadata (via metadata_full) - Trace value (via Trace.fields), Trace metadata (via metadata_full) - Metric metadata (via metadata_full) - Both EventArray and EventWrapper top-level wrappers The key test (`max_nesting_depth_is_correct_for_all_proto_paths`) verifies MAX_NESTING_DEPTH is exactly right: 1. ALL paths must roundtrip at depth 32 (limit isn't too high) 2. At least one path must fail decode at depth 33 (limit isn't too low) Adding a new proto wrapper message or Value-carrying field requires adding it to `all_proto_paths()`, and the test immediately surfaces if the limit needs adjustment.

…ing paths Instead of maintaining a list of individual proto paths, create events with ALL Value-carrying fields at max depth simultaneously. The proto conversion code populates every field (including deprecated ones like Log.metadata), so a single roundtrip per event type covers every proto path automatically. If a new Value-carrying field is added to event.proto, the conversion code must populate it, and these tests cover it with zero maintenance. No path enumeration to keep in sync.

…oryy/vector into spawn-src-94650

Add two isolated tests: - log_fields_has_headroom_beyond_max_depth: proves Log.fields can handle depth 33 (the loosest path has 3 entries of headroom) - metadata_full_fails_beyond_max_depth: proves metadata_full fails at depth 33 (the tightest path, exactly 100/100 at depth 32) Together these demonstrate that MAX_NESTING_DEPTH=32 is constrained specifically by the metadata_full encoding path.

Replace the two one-sided tests with a single per_path_boundaries test that verifies both sides of both paths: - Log.fields (loosest): succeeds at depth 33, fails at depth 34 - metadata_full (tightest): succeeds at depth 32, fails at depth 33 This proves the exact prost budget for each path and that the uniform MAX_NESTING_DEPTH=32 is set by metadata_full specifically.

The metadata_full encoding path has one more proto wrapper message than the event data path (Log.fields/Trace.fields), so it hits prost's recursion limit at a lower nesting depth. Instead of penalizing event data with the metadata path's stricter limit, use separate constants: - MAX_NESTING_DEPTH (33): for event data values (Log.fields, Trace.fields) - MAX_METADATA_NESTING_DEPTH (32): for metadata values (via metadata_full) Both limits are verified by per_path_boundaries: each constant succeeds at its value and fails at +1 via raw prost encode/decode.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20c5cbd4b8

ℹ️ 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".

pront · 2026-08-13T15:15:59Z

@pront thanks for the quick review on #25417. We hit this as a production outage last week so we have picked the branch up: master is merged and the conflicts resolved, and we have made the overflow behavior state-independent as you asked, moving the routing decision out of the disk backend into BufferSender where the WhenFull policy already lives, so an unencodable item goes to the overflow stage intact at any occupancy. Regression tests added for both the near-full and already-full cases. One thing flagged in the description for your call: the already-full test uses an in-memory base stage rather than disk, since driving disk-v2's is_buffer_full() to true reliably needs careful size tuning, and happy to build the disk-based variant if you would prefer it.

@EricaJ6 Thanks for picking this up!

The in-memory substitution actually exposed an issue: the base stage’s type is material here, BufferSender applies is_fully_encodable() to every overflow stage.

base: memory, currently empty
overflow: disk
item: too deeply nested for protobuf

Memory could store the item safely, so it should remain there. Instead, the current code skips memory immediately and sends it to disk, where it is filtered or dropped.

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

pront · 2026-08-13T16:48:38Z

@EricaJ6 some CI checks are failing, please see list of checks to run here: https://github.com/vectordotdev/vector/blob/master/CONTRIBUTING.md?plain=1#L120

EricaJ6 · 2026-08-13T17:41:28Z

@pront thanks for the quick review on #25417. We hit this as a production outage last week so we have picked the branch up: master is merged and the conflicts resolved, and we have made the overflow behavior state-independent as you asked, moving the routing decision out of the disk backend into BufferSender where the WhenFull policy already lives, so an unencodable item goes to the overflow stage intact at any occupancy. Regression tests added for both the near-full and already-full cases. One thing flagged in the description for your call: the already-full test uses an in-memory base stage rather than disk, since driving disk-v2's is_buffer_full() to true reliably needs careful size tuning, and happy to build the disk-based variant if you would prefer it.

@EricaJ6 Thanks for picking this up!

The in-memory substitution actually exposed an issue: the base stage’s type is material here, BufferSender applies is_fully_encodable() to every overflow stage.
base: memory, currently empty
overflow: disk
item: too deeply nested for protobuf
Memory could store the item safely, so it should remain there. Instead, the current code skips memory immediately and sends it to disk, where it is filtered or dropped.

@pront good catch, you are right. The check needs to be asked of the base stage rather than applied to every topology. Working on the fix now, gating the diversion on whether the base stage actually has a wire-format constraint so a memory base keeps the item instead of pushing it to disk. Adding a regression test for the memory -> disk case you described, and sorting out the CI failures at the same time.

…nstraint

EricaJ6 · 2026-08-13T18:50:10Z

@pront fixed, thanks for catching that. The diversion is now gated on SenderAdapter::requires_encodable_items(), following the provides_instrumentation() precedent, so the check is asked of the specific stage rather than assumed of every topology. A memory base now keeps the item instead of pushing it to a disk overflow that has to drop it. Added a regression test for exactly the memory -> disk case you described, since none of my existing tests used a memory base, which is why the gap got through. Ran locally on the pinned 1.95 toolchain, all passing: make check-clippy (full workspace, --all-targets --all-features), make test (3409 tests, 13 skipped), make check-changelog-fragments, and make fmt.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e165b150e

ℹ️ 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".

EricaJ6 · 2026-08-13T20:58:13Z

All checks have passed!

pront · 2026-08-17T16:49:32Z

All checks have passed! ..

Hello, please see #26099 (comment).

Also worth taking a look at this: https://github.com/vectordotdev/vector/blob/master/AI_POLICY.md#ai-review-comments.

EricaJ6 · 2026-08-17T17:48:15Z

All checks have passed! ..

Hello, please see #26099 (comment).

Also worth taking a look at this: https://github.com/vectordotdev/vector/blob/master/AI_POLICY.md#ai-review-comments.

Hi @pront, all should be resolved, refer to this.

This PR is a major blocker, so we would appreciate an approval. Once merged, will it make into the August release or is there a different timeline you can share?

pront · 2026-08-17T18:45:55Z

@codex review

Once merged, will it make into the August release or is there a different timeline you can share?

@EricaJ6 you can refer to https://calendar.vector.dev/. If a PR is merged before the next release date then it will be included in the release. If this PR isn't merged by then, then I would recommend forking and creating a custom release to unblock.

chatgpt-codex-connector · 2026-08-17T18:48:41Z

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: d40de33c33

ℹ️ 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".

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

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

EricaJ6 · 2026-08-17T18:58:13Z

@codex review

Once merged, will it make into the August release or is there a different timeline you can share?

@EricaJ6 you can refer to https://calendar.vector.dev/. If a PR is merged before the next release date then it will be included in the release. If this PR isn't merged by then, then I would recommend forking and creating a custom release to unblock.

Thank you @pront for sharing the release calendar. Looks like this PR will be merged before next week's August 25 Vector release.

chatgpt-codex-connector · 2026-08-17T18:58:23Z

To use Codex here, create a Codex account and connect to github.

connoryy added 30 commits March 26, 2026 10:06

test: add EventWrapper path boundary test for vector sink gRPC

59de3d4

Verify that the EventWrapper decode path (used by the vector sink's gRPC receiver) is safe at MAX_NESTING_DEPTH. EventWrapper has fewer outer proto wrappers than EventArray, so our limit is conservative for this path.

docs: remove proto field names from MAX_NESTING_DEPTH comment

fbd398c

docs: simplify test comments to remove proto-specific terminology

2191d9b

make error slightly more informative

5046ec5

move imports

390c30b

add test

3eb9fb4

fix conditional to new function signature

39a9924

style: auto-fix lint/format errors

d835bc8

Merge branch 'master' into connor/protobuf-nesting-depth-limit

220be47

fix metrics case and test failure

573dafb

Merge branch 'connor/protobuf-nesting-depth-limit' of github.com:conn…

4d53caf

…oryy/vector into connor/protobuf-nesting-depth-limit

Merge branch 'master' into connor/protobuf-nesting-depth-limit

7936f12

style: auto-fix lint/format errors

6800e85

Fix clippy doc_markdown lints in nesting depth tests

9ea18cd

Merge branch 'connor/protobuf-nesting-depth-limit' of github.com:conn…

30e6a86

…oryy/vector into spawn-src-94650

style: auto-fix lint/format errors

bf5119c

style: auto-fix lint/format errors

1021e1e

Add Trace to flat events test for uniformity

fbd2ad2

style: auto-fix lint/format errors

f0fc05a

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread lib/vector-buffers/src/topology/channel/sender.rs Outdated

Comment thread lib/vector-core/src/event/ser.rs

Merge branch 'master' into og/continue-protobuf-nesting-depth-limit

4426682

pront reviewed Aug 13, 2026

View reviewed changes

Comment thread changelog.d/protobuf_nesting_depth_limit.fix.md Outdated

Update changelog.d/protobuf_nesting_depth_limit.fix.md

23da462

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

EricaJ6 added 2 commits August 13, 2026 11:47

fix(buffers): gate overflow diversion on the base stage's encoding co…

e47d598

…nstraint

Merge branch 'master' into og/continue-protobuf-nesting-depth-limit

4e997b0

EricaJ6 added 2 commits August 13, 2026 12:21

Merge branch 'master' into og/continue-protobuf-nesting-depth-limit

a426b6b

Merge branch 'master' into og/continue-protobuf-nesting-depth-limit

9e165b1

chatgpt-codex-connector Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread lib/vector-core/src/event/ser.rs Outdated

Comment thread lib/vector-buffers/src/topology/channel/sender.rs

pront and others added 2 commits August 17, 2026 10:33

fix(buffers): use a common protobuf nesting budget

f7ca90a

Merge branch 'master' into og/continue-protobuf-nesting-depth-limit

d40de33

pront reviewed Aug 17, 2026

View reviewed changes

Comment thread changelog.d/protobuf_nesting_depth_limit.fix.md Outdated

Update changelog.d/protobuf_nesting_depth_limit.fix.md

39b74b4

pront enabled auto-merge August 17, 2026 18:50

pront approved these changes Aug 17, 2026

View reviewed changes

pront added this pull request to the merge queue Aug 17, 2026

Merged via the queue into vectordotdev:master with commit 2273959 Aug 17, 2026
59 checks passed

github-actions Bot locked and limited conversation to collaborators Aug 17, 2026

Conversation

EricaJ6 commented Aug 12, 2026

Summary

References

Vector configuration

How did you test this PR?

Is this a breaking change?

Does this PR include user facing changes?

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

pront commented Aug 13, 2026

Uh oh!

Uh oh!

pront commented Aug 13, 2026

Uh oh!

EricaJ6 commented Aug 13, 2026

Uh oh!

EricaJ6 commented Aug 13, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

EricaJ6 commented Aug 13, 2026

Uh oh!

pront commented Aug 17, 2026

Uh oh!

EricaJ6 commented Aug 17, 2026

Uh oh!

pront commented Aug 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 17, 2026

Uh oh!

Uh oh!

EricaJ6 commented Aug 17, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants