Change text fields to keyword for Metricbeat #10318

ruflin · 2019-01-24T14:03:25Z

Searching for type: text revealed a few fields which were text but I think should be keyword.

ruflin · 2019-01-24T14:04:34Z

metricbeat/module/kafka/consumergroup/_meta/fields.yml

@@ -40,7 +40,7 @@
      description: consumer offset into partition being read

    - name: meta
-      type: text
+      type: keyword


@urso objections?

If text indexing is needed, it should be added as a multi-field at .meta.text

ruflin · 2019-01-24T14:04:49Z

metricbeat/module/kibana/stats/_meta/fields.yml

@@ -9,7 +9,7 @@
      description: >
        Kibana instance UUID
    - name: name
-      type: text
+      type: keyword


@ycombinator Ok? Don't see why this should be text.

Yep, makes sense as keyword to me.

I screwed up. This should remain text IMHO. See my more-enlightened comment here: https://github.com/elastic/beats/pull/10318/files#r250673640

ruflin · 2019-01-24T14:05:18Z

metricbeat/module/php_fpm/process/_meta/fields.yml

@@ -34,7 +34,7 @@
    description: >
      The request method (GET, POST, etc) (of the current request)
  - name: request_uri
-    type: text
+    type: keyword


@webmat should we map this to url.original?

Yes, absolutely

webmat

LGTM if Ceph's children is made into a keyword as well.

Is this search limited to Metricbeat? Or did you search everywhere, and only Metricbeat had text field left?

webmat · 2019-01-24T14:57:51Z

metricbeat/module/kibana/stats/_meta/fields.yml

@@ -9,7 +9,7 @@
      description: >
        Kibana instance UUID
    - name: name
-      type: text
+      type: keyword


webmat · 2019-01-24T14:58:03Z

metricbeat/module/php_fpm/process/_meta/fields.yml

@@ -34,7 +34,7 @@
    description: >
      The request method (GET, POST, etc) (of the current request)
  - name: request_uri
-    type: text
+    type: keyword


Yes, absolutely

webmat · 2019-01-24T14:59:22Z

metricbeat/module/kafka/consumergroup/_meta/fields.yml

@@ -40,7 +40,7 @@
      description: consumer offset into partition being read

    - name: meta
-      type: text
+      type: keyword


If text indexing is needed, it should be added as a multi-field at .meta.text

metricbeat/module/ceph/osd_tree/_meta/fields.yml

webmat · 2019-01-24T15:05:39Z

Got an answer to my question via the tag. We should do the same for all beats. Adding a note to #8655, in the section "Fields changes"

ycombinator · 2019-01-24T16:13:50Z

metricbeat/docs/fields.asciidoc

@@ -10640,7 +10640,7 @@ Kibana instance UUID
 *`kibana.stats.name`*::
 +
 --
-type: text
+type: keyword


I think this needs to be text. This value comes from the server.name setting in kibana.yml. The setting is meant for display purposes, according to it's documentation:

# The Kibana server's name. This is used for display purposes. #server.name: "your-hostname"

So it could be a string like "Marketing's Fantastic Analytics UI". In that case I can see the benefit of letting ES analyze the string. Thoughts?

If searching for specific words (e.g. "marketing", to follow your example), then we simply need server.name.text for these searches.

I agree it's really useful in situations where these resources are not always well tagged & so on.

But the canonical field -- server.name -- should really be keyword. Defaulting to keyword all the time, then only adding text as a multi-field when needed ensures this progression over time (adding text) is never a breaking change.

That sounds good to me but I have a tangential question:

IIRC if the server.name field is mapped as keyword in ES, it wouldn't also get a server.name.text multi-field automatically. Wouldn't we need to explicitly specify that somehow in the fields.yml so ES knows to create the multi-field? Or do all keyword fields in fields.yml automatically get a multi-field mapping added to them in the template via dynamic templates or something?

Another point: having server.name as keyword will enable users to aggregate per kibana server. It's the reason in ECS, we've decided to push for keyword across the board, by default

Haha I missed your answer when posting my previous message.

The template will have to create the multi-field explicitly for the .text.

Going for keyword across the board is also a small performance gain, so the default is kw only. Then we add text strategically, where it makes sense.

I'm not sure the exact incantation to make the multi-field in Beats fields.yml.

Gotcha, I understand now. Thanks. I'm good with changing this to keyword for now and adding a text multi-field later if we need.

ruflin · 2019-01-25T12:27:33Z

@webmat For the text fields I focused on Metricbeat, but we should probably to a broader search.

ruflin · 2019-01-25T12:29:52Z

metricbeat/docs/fields.asciidoc

@@ -18573,7 +18573,7 @@ The request method (GET, POST, etc) (of the current request)
 *`php_fpm.process.request_uri`*::


@webmat Sounds like a field that should be mapped to ECS?

Yeah, agreed. url.original?

I noticed a few more things in this field defs:

user => user.name

script => file.path

content_length => http.request.body.size

request_method => http.request.method

request_duration => event.duration, with an adjustment to nanos

pid => process.pid

No longer sure if any of those were discussed in #10218

Looks like we should review php_fpm in detail again. Can you comment the above on #10218 so we can track it there?

ruflin · 2019-01-25T14:15:29Z

This should be ready for review.

webmat

The ceph children field still looks like it's text. Other than that, looking good :-)

Note that I trust your grepping skill. I haven't double checked whether you had missed any.

metricbeat/module/ceph/osd_tree/_meta/fields.yml

webmat · 2019-01-25T20:36:59Z

metricbeat/module/ceph/osd_tree/data.go

@@ -86,7 +86,7 @@ func eventsMapping(content []byte) ([]common.MapStr, error) {
 		nodeInfo := common.MapStr{}
 		if node.ID < 0 {
 			//bucket node
-			nodeInfo["children"] = childrenMap[node.Name]
+			nodeInfo["children"] = strings.Split(childrenMap[node.Name], ",")


CHANGELOG.next.asciidoc

webmat

LGTM

Searching for `type: text` revealed a few fields which were text but I think should be keyword.

ruflin added module review Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Jan 24, 2019

ruflin self-assigned this Jan 24, 2019

ruflin requested a review from webmat January 24, 2019 14:03

ruflin requested review from a team as code owners January 24, 2019 14:03

ruflin commented Jan 24, 2019

View reviewed changes

ruflin force-pushed the text-metricbeat branch from c10eac2 to 58936eb Compare January 24, 2019 14:05

ruflin mentioned this pull request Jan 24, 2019

Move Metricbeat module fields to ECS #10218

Closed

webmat reviewed Jan 24, 2019

View reviewed changes

ruflin mentioned this pull request Jan 24, 2019

Transition Beats to ECS #8655

Closed

webmat changed the title ~~Change text fields to keyword~~ Change text fields to keyword for Metricbeat Jan 24, 2019

ycombinator reviewed Jan 24, 2019

View reviewed changes

ruflin commented Jan 25, 2019

View reviewed changes

ruflin force-pushed the text-metricbeat branch from e0b2842 to a0cbe5e Compare January 25, 2019 12:38

webmat reviewed Jan 25, 2019

View reviewed changes

ruflin force-pushed the text-metricbeat branch from c6e6fca to a5cf580 Compare January 28, 2019 09:17

kaiyan-sheng reviewed Jan 28, 2019

View reviewed changes

CHANGELOG.next.asciidoc Show resolved Hide resolved

ruflin force-pushed the text-metricbeat branch from 8f4dd45 to 0f84273 Compare January 29, 2019 07:29

webmat approved these changes Jan 29, 2019

View reviewed changes

ruflin added 2 commits January 29, 2019 16:50

Change text fields to keyword

c5a65ef

Searching for `type: text` revealed a few fields which were text but I think should be keyword.

change children to array and update data.json files

e4edad5

ruflin added 3 commits January 29, 2019 16:50

change leftover field of children and update changelog

f855e19

add missing changelog entry

a19d4ce

update fields

d9a7c5d

ruflin force-pushed the text-metricbeat branch from 0f6356f to d9a7c5d Compare January 29, 2019 15:51

fix changelog

37d12a4

ruflin merged commit 0f3ea0d into elastic:master Jan 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change text fields to keyword for Metricbeat #10318

Change text fields to keyword for Metricbeat #10318

ruflin commented Jan 24, 2019

ruflin Jan 24, 2019

webmat Jan 24, 2019

ruflin Jan 24, 2019

ycombinator Jan 24, 2019

webmat Jan 24, 2019

ycombinator Jan 24, 2019 •

edited

Loading

ruflin Jan 24, 2019

webmat Jan 24, 2019

webmat left a comment

webmat Jan 24, 2019

webmat Jan 24, 2019

webmat Jan 24, 2019

webmat commented Jan 24, 2019

ycombinator Jan 24, 2019

webmat Jan 24, 2019

ycombinator Jan 24, 2019

webmat Jan 24, 2019

webmat Jan 24, 2019 •

edited

Loading

ycombinator Jan 24, 2019 •

edited

Loading

ruflin commented Jan 25, 2019

ruflin Jan 25, 2019

webmat Jan 25, 2019

ruflin Jan 28, 2019

ruflin commented Jan 25, 2019

webmat left a comment

webmat Jan 25, 2019

webmat left a comment

		@@ -18573,7 +18573,7 @@ The request method (GET, POST, etc) (of the current request)
		`php_fpm.process.request_uri`::

Change text fields to keyword for Metricbeat #10318

Change text fields to keyword for Metricbeat #10318

Conversation

ruflin commented Jan 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ycombinator Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webmat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webmat commented Jan 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webmat Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

ycombinator Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

ruflin commented Jan 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Jan 25, 2019

webmat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webmat left a comment

Choose a reason for hiding this comment

ycombinator Jan 24, 2019 •

edited

Loading

webmat Jan 24, 2019 •

edited

Loading

ycombinator Jan 24, 2019 •

edited

Loading