Update ICMP protocol to use ECS fields #10062
Conversation
Only a few fields were changed. No dashboards were changed because there are no ICMP specific dashboards. Here's a summary of what fields changed. Part of elastic#7968 Changed - responsetime -> event.duration (unit are now nanoseconds) - bytes_in -> source.bytes - bytes_out -> destination.bytes Added - event.dataset = icmp - event.end - event.start - network.community_id - network.transport = icmp or ipv6-icmp - network.type = ipv4/ipv6 Unchanged Packetbeat Fields - status - type = icmp (we might remove this since we have event.dataset) - path = destination.ip (what is requested, not sure if this still makes sense)
There was a problem hiding this comment.
It think even though the fields cannot be migrated directly because of unit change it would still be worth to have them in ecs-migration.yml but with alias: false.We potentially can use this to generate a list of fields that users need to look at when upgrading.
| @@ -241,7 +244,7 @@ func (f *Fields) MarshalMapStr(m common.MapStr) error { | |||
| structField := typ.Field(i) | |||
| tag := structField.Tag.Get("ecs") | |||
| if tag == "" { | |||
| panic(errors.Errorf("missing tag on field %v", structField.Name)) | |||
| continue | |||
There was a problem hiding this comment.
Change in behaviour? Did not follow the code.
There was a problem hiding this comment.
This file is "new" code as part of the parent issue #7968. Yes, this is a behavior change. It needed to be less restrictive to allow for fields in the object that are not to be marshaled. These are helper fields used to derive and make decisions about other fields that are marshaled.
There was a problem hiding this comment.
Should this be mentioned in the changelog?
There was a problem hiding this comment.
No, this is new code that's never been released and it has no impact on the user.
| if len(trans.notes) == 1 { | ||
| evt.PutValue("error.message", trans.notes[0]) | ||
| } else if len(trans.notes) > 1 { | ||
| evt.PutValue("error.message", trans.notes) |
There was a problem hiding this comment.
From and ES perspective, I would say both are the same as message in Lucene is always an array. Having said that, do we really want to put an array into message or flatten it first?
There was a problem hiding this comment.
If there's more than one note/error then I'd rather sent them as discrete strings that are part of an array. As for whether to always send an array or not that doesn't really matter to me (or to ES). I'll probably refactor this when I get to the end of #7968, because I'm finding that each protocol had slightly different behavior w.r.t. notes. So if you have a preference for whether to always send an array or not let me know.
There was a problem hiding this comment.
No strong preference either.
That already exists in the file. beats/dev-tools/ecs-migration.yml Lines 808 to 831 in 8a04a80 |
| @@ -241,7 +244,7 @@ func (f *Fields) MarshalMapStr(m common.MapStr) error { | |||
| structField := typ.Field(i) | |||
| tag := structField.Tag.Get("ecs") | |||
| if tag == "" { | |||
| panic(errors.Errorf("missing tag on field %v", structField.Name)) | |||
| continue | |||
There was a problem hiding this comment.
Should this be mentioned in the changelog?
Only a few fields were changed. No dashboards were changed because there are no ICMP specific dashboards.
Here's a summary of what fields changed.
Part of #7968
Changed
Added
Unchanged Packetbeat Fields