-
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
Update Cassandra protocol to use ECS fields #10093
Update Cassandra protocol to use ECS fields #10093
Conversation
2f9c1fe
to
57c2186
Compare
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.
Skimmed through it, LGTM.
Again a deeper look with packetbeat knowledge could be helpful.
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.
Looking good.
Only one question I'd like an answer to, before approving, and a nitpick.
"field": "@timestamp", | ||
"interval": "auto", | ||
"min_doc_count": 1, | ||
"time_zone": "America/New_York", |
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.
Can tz info here be problematic?
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.
I'm not at all sure why this got added or what effect it has. It looks like it is using a custom date histogram interval. I'll try switching that to automatic and see if this TZ goes away.
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.
I edited the object to remove the time_zone
and the dashboard still works OK. Pushed an updated.
type: alias | ||
path: cassandra.no_request | ||
migration: true | ||
|
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.
Nit: I personally like to move the aliases at the end of the file, so that the generated doc starts with the good stuff, and ends with the aliases.
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.
Also, I agree with moving this top level field to cassandra.no_request
👍
assert o["network.type"] == "ipv4" | ||
assert o["network.transport"] == "tcp" | ||
assert o["network.protocol"] == "cassandra" | ||
assert o["network.community_id"] == "1:bCORHZnGIk6GWYaE3Kn0DOpQCKE=" |
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.
It's not obvious to me here, but are you also populating source.[ip|port]
and destination.[ip|port]
? If so, could you add tests for that?
In ECS we recommend always filling source and destination, and also filling client/server when appropriate. But the source/destination pair should always be there.
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.
All of these updated "datasets" automatically populate the client/server with the source/destination if they have not been populated by the code.
Lines 221 to 231 in ce4f474
// client | |
if f.Client == nil && f.Source != nil { | |
client := ecs.Client(*f.Source) | |
f.Client = &client | |
} | |
// server | |
if f.Server == nil && f.Destination != nil { | |
server := ecs.Server(*f.Destination) | |
f.Server = &server | |
} |
I will be updating the python test at the end (after all of packetbeat is updated) to check for the existence of all of the common fields like source/destination/client/server/network.transport/event.dataset/event.start/etc.
That dashboard was updated too. Here's a summary of what fields changed. Part of elastic#7968 Changed - bytes_in -> source.bytes - bytes_out -> destination.bytes - notes -> error.message - responsetime -> event.duration (unit are now nanoseconds) - no_request -> cassandra.no_request (this was an undocumented field specific to this dataset) Added - source - destination - event.dataset = cassandra - event.end - event.start - network.community_id - network.transport = tcp - network.protocol = cassandra - network.bytes - network.type Unchanged Packetbeat Fields - status - type = cassandra (we might remove this since we have event.dataset)
9153c2a
to
367d31b
Compare
Rebased to resolve conflicts in ecs-migration.yml. |
That dashboard was updated too.
Here's a summary of what fields changed.
Part of #7968
Changed
Added
Unchanged Packetbeat Fields