Skip to content
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

Create a section to explain some conventions used in ECS. Explain 2 conventions. #89

Merged
merged 4 commits into from
Aug 20, 2018

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Aug 16, 2018

Explaining the multi-fields convention will help keep each field description more to the point. Right now there's an inconsistent quick explainer in many of the multi-fields, which is redundant. Having this section to explain it a bit better will let us clean these field descriptions up later.

I also like that we're already using keyword for most IDs (I haven't checked
them all out). But it's something I would have pushed for if that hadn't
been the case already.

…onventions.

Explaining the multi-fields convention will help keep each field description more to the point. Right now there's an inconsistent quick explainer in many of the multi-fields, which is redundant. Having this section to explain it a bit better will let us clean these descriptions up later.
README.md Outdated

ElasticSearch can index text multiple ways:

* `text` indexing allows for full text search, or searching arbitrary words that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to link to the ES docs here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I'll link to the docs in a few places, good point

README.md Outdated

* `text` indexing allows for full text search, or searching arbitrary words that
are part of the field.
* `keyword` indexing allows for exact match search (much faster) and allows for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And prefix which is pretty nice for version fields for example: https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-prefix-query.html

README.md Outdated
aggregations (what Kibana visualizations are built on).

In some cases only one type of indexing makes sense for a field. E.g. no need to
do full text search on an id, and nobody needs to do an exact match search on
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normally there is no need for an exact match search on 2 kb stack trace. Nobody is a bit strong :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually thinking of taking out that whole sentence.

README.md Outdated
Whenever both types of indexing are helpful, we use multi-fields indexing. The
convention used is the following:

* `foo`: `text` indexing. The top level of the field (its plain name) is used
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this convention change any of the existing fields we have? Mainly curious.

In general +1 on the convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well most of the non-multi-fields text fields are actually keyword. That's why I follow all this immediately with a mention to that effect.

But any time we do multi-field, we need to follow this convention, otherwise it will get confusing :-)

Also, this PR is a side-effect of the thinking/discussion that's happening around #87. And obviously we will need to tweak the wording here if we go with .keyword instead of .raw, but I'm fine with that :-)

@webmat webmat self-assigned this Aug 17, 2018
@webmat
Copy link
Contributor Author

webmat commented Aug 17, 2018

This is ready to be reviewed again

@ruflin ruflin merged commit e29b91c into elastic:master Aug 20, 2018
@ruflin
Copy link
Member

ruflin commented Aug 20, 2018

Thanks for adding the convention. Let's see what #87 will have as an affect on this.

@webmat webmat deleted the ecs-conventions branch August 20, 2018 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants