-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix panic due to no type #4331
Fix panic due to no type #4331
Conversation
Some Beats no longer add a "type", but the LS output was still requiring it. This hardcodes the type in `@metadata` to `doc`, to keep BWC with older Logstashes/configs. It also removes `"type"` from a few kafka integration tests, to make sure we don't depend on it.
jenkins, test it again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WFG
typ := event["type"].(string) | ||
buf.WriteString(`"@metadata":{"type":`) | ||
encodeString(buf, typ) | ||
buf.WriteString(`"@metadata":{"type":"doc","beat":`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if there is a type
like we have in packetbeat? I think it is ok to just have it outside the meta info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i think it's safer to hardcode the metadata one to doc
, as otherwise existing Logstash configs will put it in _type
.
Can we have a BC-breaking changelog entry? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WFG
Some Beats no longer add a "type", but the LS output was still requiring it.
This hardcodes the type in
@metadata
todoc
, to keep BWC with older Logstashes/configs.It also removes
"type"
from a few kafka integration tests, to make sure we don't dependon it.