[Filebeat] add support for ndjson results to httpjson input #23428
Conversation
botelastic
bot
added
the
needs_team
leehinman
added
enhancement
Filebeat
botelastic
bot
removed
the
needs_team
|
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
leehinman
force-pushed
the
httpjson_ndjson
branch
from
1c261f1
to
d545c36
Compare
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
- if response Content-Type is "application/x-ndjson", collect the json objects in an array and assign array to key "ndjson-results" in a new json object - if config response.force_ndjson is set, set response Content-Type header to "application/x-ndjson"
leehinman
force-pushed
the
httpjson_ndjson
branch
from
d545c36
to
fe138a3
Compare
| [float] | ||
| ==== `response.force_ndjson` | ||
|
|
||
| If the Content-Type header is set to "application/x-ndjson" then the ndjson results will be available in a new array called "ndjson-results". |
There was a problem hiding this comment.
I would add a comment about supporting ndjson at the top of the docs instead for visibility.
| @@ -80,6 +83,10 @@ func (rp *responseProcessor) startProcessing(stdCtx context.Context, trCtx *tran | |||
| go func() { | |||
| defer close(ch) | |||
|
|
|||
| if rp.forceNdjson { | |||
| resp.Header.Set("Content-Type", "application/x-ndjson") | |||
There was a problem hiding this comment.
Is this just for testing purposes? If not, maybe there are other use cases for modifying response headers, and enabling writing to them in transform_xxx.go is a more general approach without adding specific config fields. WDYT?
There was a problem hiding this comment.
Unfortunately not just for testing, some endpoints send back ndjson with content types of text/plain or application/json. So we need some way to force ndjson code path. That being said I like the idea of the general approach, let me give that a try.
There was a problem hiding this comment.
After reading "if config response.force_ndjson is set, set response Content-Type
header to "application/x-ndjson" in the description, my first thought was that this sounded fairly specific. If we have several builtin handlers for different content types then maybe a generic overwrite header option would work and make future extension easier.
replace ndjson-results with root level array Co-authored-by: Marc Guasch <marc-gr@users.noreply.github.com>
- use json.Decoder instead of json.UnMarshall - if only 1 json object is present it becomes the root - if more than on json object is present a root level array is returned
leehinman
force-pushed
the
httpjson_ndjson
branch
from
499955c
to
f203efb
Compare
|
I opened #23478, if it looks good to you, maybe we can merge it and then you just need to create a new ndjson decoder and use the |
|
yeah, I like the encode_as & decode_as stuff from #23478 better. Cunnighams Law at work. Closing this one. |
What does this PR do?
adds support for ndjson results to httpjson input.
objects in an array and assign array to key "ndjson-results" in a new
json object
header to "application/x-ndjson"
Why is it important?
Some HTTP API endpoints return results as ndjson not json
Checklist
- [ ] I have commented my code, particularly in hard-to-understand areas- [ ] I have made corresponding change to the default configuration filesCHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.