Do not update event in parallel #1428
Conversation
| buf.Write(tmp) | ||
|
|
||
| buf.WriteString(`,"beat":`) | ||
| buf.Write(beat) |
There was a problem hiding this comment.
Shouldn't this also be passed through json.Marshal or otherwise escaped? We have to be careful with JSON injection if we're doing this by hand.
There was a problem hiding this comment.
json.Marshal is done line 62. Didn't want to marshal over and over again
tsg
added
needs_backport
| @@ -285,6 +295,49 @@ func (p *protocol) serializeDataFrame( | |||
| return nil | |||
| } | |||
|
|
|||
| func serializeEvent(event common.MapStr, beat []byte) ([]byte, error) { | |||
There was a problem hiding this comment.
Some unit tests for this function would be good.
There was a problem hiding this comment.
it's tested (block-box testing sendEvents), as we've got some logstash unit tests forwarding events via sendEvents with server part decoding json messages for validation.
|
This looks to me like a good solution for the bug we have now, I guess long term we'll have a proper Event type and we can add locking or immutability to it, right? |
|
LGTM. Can you add a changelog item? |
That's exactly what I have had in mind when doing the fix. |
urso
force-pushed
the
fix/1410-race-json-nil
branch
from
b266a37
to
35163ae
Compare
logstash output used to add metadata to an event. With potentially having
multiple outputs configured this is some bad practice. Instead the logstash
output plugin will add '@metadata' to the event when encoding and only if
'@metadata' is not already present in event
Resolves #1410