Fix double newline in NDJSON bulk body when using RawEncoding#49557
Fix double newline in NDJSON bulk body when using RawEncoding#49557trilamsr wants to merge 3 commits intoelastic:mainfrom
Conversation
|
This pull request doesn't have a |
🤖 GitHub commentsJust comment with:
|
|
💚 CLA has been signed |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
When events are pre-encoded in the queue (via event_encoder.go), the
encoded bytes include a trailing newline from the Marshal/AddRaw call.
When the bulk body assembler later writes these bytes via
AddRaw(RawEncoding{...}), it unconditionally appends another newline,
producing an empty line (\n\n) in the NDJSON bulk body.
While Elasticsearch tolerates empty lines in bulk requests,
ES-compatible endpoints like Axiom and OpenSearch reject them with:
400 Bad Request: invalid event at index 1: ReadObject: expect { or ,
or } or n, but found \u0000
The fix checks whether RawEncoding bytes already end with a newline
and skips the additional one if so. This preserves backward
compatibility: RawEncoding bytes without a trailing newline (e.g. from
json.Marshal) still get the newline appended as before.
Applied to both jsonEncoder and gzipEncoder paths.
138d525 to
1c08869
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change fixes a bug in NDJSON bulk request body encoding where pre-encoded events using 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
668eadc to
457c007
Compare
When events are pre-encoded in the queue (via event_encoder.go), the
encoded bytes include a trailing newline from the Marshal/AddRaw call.
When the bulk body assembler later writes these bytes via
AddRaw(RawEncoding{...}), it unconditionally appends another newline,
producing an empty line (\n\n) in the NDJSON bulk body.
While Elasticsearch tolerates empty lines in bulk requests,
ES-compatible endpoints like Axiom and OpenSearch reject them with:
400 Bad Request: invalid event at index 1: ReadObject: expect { or ,
or } or n, but found \u0000
The fix checks whether RawEncoding bytes already end with a newline
and skips the additional one if so. This preserves backward
compatibility: RawEncoding bytes without a trailing newline (e.g. from
json.Marshal) still get the newline appended as before.
Applied to both jsonEncoder and gzipEncoder paths.
457c007 to
abeb91b
Compare
Proposed commit message
Fix double newline in NDJSON bulk body when using RawEncoding
When events are pre-encoded in the queue (via
event_encoder.go), the encoded bytes include a trailing newline from theMarshal/AddRawcall. When the bulk body assembler later writes these bytes viaAddRaw(RawEncoding{...}), it unconditionally appends another newline, producing an empty line (\n\n) in the NDJSON bulk body.While Elasticsearch tolerates empty lines in bulk requests, ES-compatible endpoints like Axiom and OpenSearch reject them with:
Closes #49558
What does this PR do?
Checks whether
RawEncodingbytes already end with a newline and skips the additionalWriteByte('\n')/Write(nl)if so. Applied to bothjsonEncoderandgzipEncoderpaths.Backward compatible:
RawEncodingbytes without a trailing newline still get the newline appended.How to test this locally
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration files🤖 Generated with Claude Code