chore(codecs): centralize events_dropped emission for batch encoding errors#25199
Merged
chore(codecs): centralize events_dropped emission for batch encoding errors#25199
events_dropped emission for batch encoding errors#25199Conversation
…rors Move events_dropped emission from individual internal events inside serializers to a single wrapper in (Transformer, BatchEncoder)::encode_input. This ensures all batch encoding error paths (Arrow IPC and Parquet) consistently emit events_dropped without requiring each new error path to remember to add it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
domain: sinks
pront
added
the
no-changelog
Covers the build_record_batch ArrowJsonDecode error path where a schema expects int64 but the event contains a string value. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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: 2caa7428ec
ℹ️ 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".
…nting Replace EncoderWriteError with a direct ComponentEventsDropped emission in the batch encode wrapper. EncoderWriteError was incrementing component_errors_total and logging "Failed writing bytes." which double-counted errors (codec-specific events already increment component_errors_total) and was misleading (the failure was encoding, not writing). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0def5bf25
ℹ️ 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".
Move ComponentEventsDropped and UNINTENTIONAL imports inside the cfg(feature = "codecs-arrow") impl block to avoid unused import errors when the feature is disabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pront
changed the title
pront
changed the title
events_dropped emission for batch encoding errors
thomasqueirozb
approved these changes
Apr 16, 2026
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e66904204b
ℹ️ 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".
The new EncoderRecordBatchError fires from build_record_batch's RecordBatchCreation and ArrowJsonDecode paths, so type-mismatch and decoder-build failures emit a granular component_errors_total counter at stage="sending" with a specific error_code, instead of relying solely on the downstream SinkRequestBuildError at stage="processing". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…atch Drives (Transformer, BatchEncoder)::encode_input through ArrowStreamSerializer with an Int64 schema field and a string-valued event to trigger the ArrowJsonDecode path in build_record_batch. Asserts both EncoderRecordBatchError and ComponentEventsDropped are recorded so a regression in either emission fails the test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Motivation
After #25156 (Parquet encoding),
events_droppedemission across batch encoding error paths was inconsistent. Parquet covered most paths but missedbuild_record_batchfailures (e.g. type mismatch); Arrow IPC covered almost none. Every new error path required the author to remember to add emission manually, which is easy to miss.What this changes
events_droppedemission to a single wrapper at(Transformer, BatchEncoder)::encode_inputso all current and future codec error paths drop accounting "for free".EncoderRecordBatchErrorcodec event sobuild_record_batchfailures (RecordBatchCreation,ArrowJsonDecode) reportcomponent_errors_totalwith a granularerror_codeatstage="sending", instead of relying solely on the downstreamSinkRequestBuildErroratstage="processing".Notes
non_log_countpartial drop in Parquet (non-log events filtered before encoding) can double-count if the batch also fails to encode. Rare intersection; acceptable.ParquetSerializer::encodestill has no dedicated codec event. Tracking as a follow-up so this PR stays scoped to drop-emission centralization plus thebuild_record_batchgranularity gap.How did you test this PR?
make check-clippycleancargo nextest run -p codecs --features arrow,parquet278 passedcargo nextest run --package vector --lib sinks::util::encoding11 passed (includes newtest_encode_batch_arrow_emits_record_batch_error_on_type_mismatch)healthchecks.enabled=false, confirmingcomponent_discarded_events_totalandcomponent_errors_totalincrement as expected for each.Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References