-
Notifications
You must be signed in to change notification settings - Fork 431
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
Add network.name to track network names with human language :-) #30
Conversation
CHANGELOG.md
Outdated
@@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file based on the | |||
|
|||
### Added | |||
* Add `event.action` field. #21 | |||
* Add `network.name`, to track network names right in the monitoring pipeline. #25 |
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 suggest striking right
from the sentence.
@@ -5,6 +5,12 @@ | |||
description: > | |||
Fields related to network data. | |||
fields: | |||
- name: name | |||
type: text |
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 would have chosen keyword here because I can see myself wanting to aggregate on this term. I personally wouldn't need to have this value analyzed/searchable, but maybe someone with lots of networks would.
So perhaps we should make this a keyword and reserve the option of making it a multi-field in the future if someone wants it to be searchable.
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.
Yes I totally agree we need to be able to aggregate on this field.
However this is absolutely a field I would want analyzed as well. E.g. Search for all "Guest" or "DMZ" networks. You don't even need such a big organization for this to make sense. As soon as you have more than one physical location you'll have multiple guest networks, as soon as you have more than one application (if you're doing things right), you'll have more than one DMZ, application, private networks. Having this field analyzed lets people filter on those in a simple manner.
I haven't looked actively, but is ECS trying to steer away from having fields indexed both ways? I'm personally not a huge fan of how things ended up, with the "field" and "field.keyword" convention. But I find I often want both types of indexing.
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 we ended up with for now is having indexed by default and have the .raw
convention instead. See for example: https://github.com/elastic/ecs/blob/master/schemas/url.yml#L26
More then happy to get some alternative suggestions on how to deal with it.
+1 on having this field as keyword and text.
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.
Ok thanks @ruflin, I see how multi-fields is done in the schemas yml files now.
Side note: I notice that the description of field href
seems to say the opposite of what the actual schema is.
Description says:
url.href
is keywordurl.href.analyzed
is text
Schema says:
url.href
is texturl.href.raw
is keyword
;-)
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.
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.
@webmat @praseodym Thanks, that was not intentional.
@karenzone Could you have a look at that?
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.
@webmat Do you plan to make it multi field in this PR or should merge as is and add multi field later?
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.
Well I wanted to wait for us to have this discussion next Monday. I want to discuss the naming convention for multi field and confusion I've seen it create in a past life.
One could say this was a long time coming, but this is ready for another review, and should be ready to go now ;-) |
Closes #25