Repository navigation
Conversation
`get_key` coerced the `key_field` value to bytes whatever its type, and `Value::Null` coerces to the string "<null>". An event with an explicit `key: null` was therefore produced with the literal key "<null>", while an event without the field was produced with no key, as users expect. Treat a null value like a missing one for log and trace events. Closes vectordotdev#24728
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
More details
Null log and trace keys now use Kafka’s existing absent-key handling, avoiding the literal <null> key while preserving non-null key conversion.
🤖 Bits Code Review · Commit 3b27e8a
This branch has not been deployed
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
With
key_fieldset, an event whose key field is explicitlynullwas produced with the literal string<null>as its Kafka key, becauseget_keycoerced the value to bytes whatever its type. An event without the field was already produced with no key.get_keynow treats a null value like a missing one for log and trace events. Metric tags need no change:MetricTags::getalready skips null values.References
Issue: #24728
Vector configuration
Input
{"test": "test", "key": null}now produces a message with no key, like{"test": "test"}.How did you test this PR?
Added
sinks::kafka::request_builder::tests::kafka_get_key_null_is_no_key, which checks that a null key gives no key for log and trace events and that a string key is unchanged. It fails without the change (Some(b"<null>")). The Kafka integration tests were not run.Does this PR include user facing changes?
For a new deprecation announcement, follow the deprecation policy and add a
deprecation.d/notice instead.no-changeloglabel to this PR.Contributor Guidelines
@vectordotdev/vectorto reach out to us regarding this PR.git merge origin masterandgit push.This PR was written with Claude Opus 5.5 as a coding agent and reviewed with Codex.
Closes: #24728