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

[Winlogbeat] Fix duplicated type entry #10373

Merged
merged 3 commits into from
Jan 29, 2019
Merged

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Jan 28, 2019

The type entry was duplicated and had text and keyword. Now changed to keyword only.

The type entry was duplicated and had text and keyword. No changed to keyword only.
@ruflin ruflin requested a review from a team as a code owner January 28, 2019 13:54
@ruflin ruflin self-assigned this Jan 28, 2019
@@ -184,7 +184,6 @@

- name: xml
type: keyword
type: text
Copy link
Member Author

Choose a reason for hiding this comment

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

@andrewkroh Not sure which one was the original intention.

Copy link
Member

Choose a reason for hiding this comment

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

It was text because the value is very long and similar to message. If you change it to keyword then ignore_above needs to be increased from our default.

Copy link
Contributor

Choose a reason for hiding this comment

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

If full text indexing is deemed important here, we should add it as a multi-field at eventlog.xml.text.

Increasing ignore_above wouldn't help much, I would think.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm also good with keeping this as text (I think at the moment it is text because yaml takes the last definition). I don't really see how this field would be used for aggregations or exact match queries.

@ruflin ruflin mentioned this pull request Jan 28, 2019
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -184,7 +184,6 @@

- name: xml
type: keyword
type: text
Copy link
Contributor

Choose a reason for hiding this comment

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

If full text indexing is deemed important here, we should add it as a multi-field at eventlog.xml.text.

Increasing ignore_above wouldn't help much, I would think.

@ruflin
Copy link
Member Author

ruflin commented Jan 29, 2019

I changed this back to text as I think this is really a field that will not be used as keyword and should be text only. So overall this PR is now mainly a cleanup of the fields.yml removing the duplicated entry.

@ruflin ruflin merged commit 2bf21e5 into elastic:master Jan 29, 2019
@ruflin
Copy link
Member Author

ruflin commented Jan 29, 2019

Merged for now as this is now only a code cleanup. We can continue the discussion here or on the other Winlogbeat PR if needed.

@ruflin ruflin deleted the winlobeat-text branch January 29, 2019 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants