Skip to content

[Filebeat] List of text fields to be converted#10372

Closed
ruflin wants to merge 5 commits intoelastic:masterfrom
ruflin:filebeat-text-types
Closed

[Filebeat] List of text fields to be converted#10372
ruflin wants to merge 5 commits intoelastic:masterfrom
ruflin:filebeat-text-types

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented Jan 28, 2019

This PR is for discussing the current text fields in Filebeat and if the type should be changed.

@ruflin
[Filebeat] List of text fields to be converted
74fa8e3 
This PR is for discussing the current text fields in Filebeat and if the type should be changed.
@ruflin ruflin added discuss Issue needs further discussion. Filebeat Filebeat ecs Team:Integrations Label for the Integrations team labels Jan 28, 2019
@ruflin ruflin self-assigned this Jan 28, 2019
@ruflin ruflin requested review from webmat and ycombinator January 28, 2019 13:50
@ruflin ruflin requested review from a team as code owners January 28, 2019 13:50
@ruflin ruflin mentioned this pull request Jan 28, 2019
Closed
webmat
webmat reviewed Jan 28, 2019
View reviewed changes
Copy link
Copy Markdown
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.

You've turned up a lot of things, good stuff!

I think when in doubt, we should always go for keyword indexing at the canonical field (or go for non-indexed if really it doesn't make sense to index); and leave users or future us add .text later on, when needed. This new convention reduces chances of breaking changes a lot, for all fields where we adopt it.

Tried to respond to everything below :-)

@ycombinator
Copy link
Copy Markdown
Contributor

ycombinator commented Jan 29, 2019

Probably a question for @ruflin:

In many ECS conversion PRs we say we want to treat a field as keyword for now and later, if necessary, add a text ES multi-field to it. I like this idea a lot. But I'm curious if this is possible today in Filebeat. Concretely, is it possible to specify in fields.yml today a field as a keyword and also specify that it should have a text multi-field, and will that result in the right ES template mapping?

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Jan 29, 2019

@ycombinator Yes:

  fields:
    - name: multifield_field
      type: keyword
      multi_fields:
        - name: foo
          type: text

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Jan 29, 2019

@ycombinator Could you tackle the Stack modules changes?

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Jan 29, 2019

I opened PR for all changes except the Elastic Stack. @ycombinator Can you ping here as soon as you have 1 (or multiple PR's) open for the fields so I can close this PR here?

@ycombinator
Copy link
Copy Markdown
Contributor

ycombinator commented Jan 29, 2019

@ruflin will do, it's item 5 on my TODO list for today so will probably get to it in a couple hours unless other stuff pops up (which never happens, of course).

This was referenced Jan 29, 2019
Merged
Merged
@ycombinator
Copy link
Copy Markdown
Contributor

ycombinator commented Jan 29, 2019 •
edited
Loading

@ruflin @webmat I've opened 2 PRs for the stack module changes:

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Jan 30, 2019

Thanks @ycombinator

Closing this PR as all changes have been addressed in follow up PR's.

@ruflin ruflin closed this Jan 30, 2019
@ruflin ruflin deleted the filebeat-text-types branch January 30, 2019 07:32
ycombinator added a commit that referenced this pull request Jan 30, 2019
@ycombinator
[Filebeat] Changes to text fields in logstash module (#10417)
ed37eea 
This PR is an offshoot of conversations and decisions made in #10372 w.r.t `text` fields, but scoped to the `logstash` module.
ycombinator added a commit that referenced this pull request Jan 31, 2019
@ycombinator
[Filebeat] Changes to text fields in elasticsearch module (#10414)
33b789c 
This PR is an offshoot of conversations and decisions made in #10372 w.r.t `text` fields, but scoped to the `elasticsearch` module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ycombinator ycombinator ycombinator left review comments

+2 more reviewers

@webmat webmat webmat left review comments

@pgomulka pgomulka pgomulka left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ruflin ruflin

Labels

discuss Issue needs further discussion. ecs Filebeat Filebeat Team:Integrations Label for the Integrations team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ruflin @ycombinator @webmat @pgomulka