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

Add default_field option to fields.yml #14341

Merged
merged 5 commits into from
Oct 31, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-developer.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,4 @@ The list below covers the major changes between 7.0.0-rc2 and master only.
- Makefile included in generator copies files from beats repository using `git archive` instead of cp. {pull}13193[13193]
- Strip debug symbols from binaries to reduce binary sizes. {issue}12768[12768]
- Compare event by event in `testadata` framework to avoid sorting problems {pull}13747[13747]
- Added a `default_field` option to fields in fields.yml to offer a way to exclude fields from the default_field list. {issue}14262[14262] {pull}14341[14341]
5 changes: 5 additions & 0 deletions auditbeat/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"

"github.com/elastic/beats/auditbeat/cmd"
"github.com/elastic/beats/libbeat/tests/system/template"
)

var systemTest *bool
Expand All @@ -41,3 +42,7 @@ func TestSystem(t *testing.T) {
main()
}
}

func TestTemplate(t *testing.T) {
ruflin marked this conversation as resolved.
Show resolved Hide resolved
template.TestTemplate(t, cmd.Name)
}
5 changes: 5 additions & 0 deletions filebeat/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"

"github.com/elastic/beats/filebeat/cmd"
"github.com/elastic/beats/libbeat/tests/system/template"
)

var systemTest *bool
Expand All @@ -40,3 +41,7 @@ func TestSystem(t *testing.T) {
main()
}
}

func TestTemplate(t *testing.T) {
template.TestTemplate(t, cmd.Name)
}
5 changes: 5 additions & 0 deletions heartbeat/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"

"github.com/elastic/beats/heartbeat/cmd"
"github.com/elastic/beats/libbeat/tests/system/template"
)

var systemTest *bool
Expand All @@ -40,3 +41,7 @@ func TestSystem(t *testing.T) {
main()
}
}

func TestTemplate(t *testing.T) {
template.TestTemplate(t, cmd.Name)
}
6 changes: 5 additions & 1 deletion journalbeat/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"

"github.com/elastic/beats/journalbeat/cmd"
"github.com/elastic/beats/libbeat/tests/system/template"
)

var systemTest *bool
Expand All @@ -37,8 +38,11 @@ func init() {

// Test started when the test binary is started. Only calls main.
func TestSystem(t *testing.T) {

if *systemTest {
main()
}
}

func TestTemplate(t *testing.T) {
template.TestTemplate(t, cmd.Name)
}
6 changes: 6 additions & 0 deletions libbeat/libbeat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ package main
import (
"flag"
"testing"

"github.com/elastic/beats/libbeat/tests/system/template"
)

var systemTest *bool
Expand All @@ -37,3 +39,7 @@ func TestSystem(t *testing.T) {
main()
}
}

func TestTemplate(t *testing.T) {
template.TestTemplate(t, "mockbeat")
}
5 changes: 3 additions & 2 deletions libbeat/mapping/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ type Field struct {
UrlTemplate []VersionizedString `config:"url_template"`
OpenLinkInCurrentTab *bool `config:"open_link_in_current_tab"`

Overwrite bool `config:"overwrite"`
Path string
Overwrite bool `config:"overwrite"`
DefaultField *bool `config:"default_field"`
Path string
}

// ObjectTypeCfg defines type and configuration of object attributes
Expand Down
45 changes: 30 additions & 15 deletions libbeat/template/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,27 @@ var (

const scalingFactorKey = "scalingFactor"

type fieldState struct {
DefaultField bool
Path string
}

// Process recursively processes the given fields and writes the template in the given output
func (p *Processor) Process(fields mapping.Fields, path string, output common.MapStr) error {
for _, field := range fields {
func (p *Processor) Process(fields mapping.Fields, state *fieldState, output common.MapStr) error {
if state == nil {
// Set the defaults.
state = &fieldState{DefaultField: true}
}

for _, field := range fields {
if field.Name == "" {
continue
}

field.Path = path
field.Path = state.Path
if field.DefaultField == nil {
field.DefaultField = &state.DefaultField
}
var indexMapping common.MapStr

switch field.Type {
Expand All @@ -69,12 +81,6 @@ func (p *Processor) Process(fields mapping.Fields, path string, output common.Ma
case "alias":
indexMapping = p.alias(&field)
case "group":
var newPath string
if path == "" {
newPath = field.Name
} else {
newPath = path + "." + field.Name
}
indexMapping = common.MapStr{}
if field.Dynamic.Value != nil {
indexMapping["dynamic"] = field.Dynamic.Value
Expand All @@ -93,17 +99,26 @@ func (p *Processor) Process(fields mapping.Fields, path string, output common.Ma
}
}

if err := p.Process(field.Fields, newPath, properties); err != nil {
groupState := &fieldState{Path: field.Name, DefaultField: true}
if state.Path != "" {
groupState.Path = state.Path + "." + field.Name
}
if field.DefaultField != nil {
groupState.DefaultField = *field.DefaultField
}
if err := p.Process(field.Fields, groupState, properties); err != nil {
return err
}
indexMapping["properties"] = properties
default:
indexMapping = p.other(&field)
}

switch field.Type {
case "", "keyword", "text":
addToDefaultFields(&field)
if field.DefaultField == nil || *field.DefaultField {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like that the nil check above means a DefaultField==nil behaves as an implicit default to true. It's my understanding that the check in line 59 prevents this from ever being nil, but still looks dangerous to me. I would turn the nil check into an error.

This really doesn't matter much unless we expect that some day the default for 'default_field' will be false, so feel free to ignore.

switch field.Type {
case "", "keyword", "text":
addToDefaultFields(&field)
}
}

if len(indexMapping) > 0 {
Expand Down Expand Up @@ -207,7 +222,7 @@ func (p *Processor) keyword(f *mapping.Field) common.MapStr {

if len(f.MultiFields) > 0 {
fields := common.MapStr{}
p.Process(f.MultiFields, "", fields)
p.Process(f.MultiFields, nil, fields)
property["fields"] = fields
}

Expand Down Expand Up @@ -243,7 +258,7 @@ func (p *Processor) text(f *mapping.Field) common.MapStr {

if len(f.MultiFields) > 0 {
fields := common.MapStr{}
p.Process(f.MultiFields, "", fields)
p.Process(f.MultiFields, nil, fields)
properties["fields"] = fields
}

Expand Down
82 changes: 80 additions & 2 deletions libbeat/template/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ func TestPropertiesCombine(t *testing.T) {
}

p := Processor{EsVersion: *version}
err = p.Process(fields, "", output)
err = p.Process(fields, nil, output)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -570,7 +570,7 @@ func TestProcessNoName(t *testing.T) {
}

p := Processor{EsVersion: *version}
err = p.Process(fields, "", output)
err = p.Process(fields, nil, output)
if err != nil {
t.Fatal(err)
}
Expand All @@ -589,3 +589,81 @@ func TestProcessNoName(t *testing.T) {

assert.Equal(t, expectedOutput, output)
}

func TestProcessDefaultField(t *testing.T) {
// NOTE: This package stores global state. It must be cleared before this test.
defaultFields = nil

var (
enableDefaultField = true
disableDefaultField = false
)

fields := mapping.Fields{
// By default foo will be included in default_field.
mapping.Field{
Name: "foo",
Type: "keyword",
},
// bar is explicitly set to be included in default_field.
mapping.Field{
Name: "bar",
Type: "keyword",
DefaultField: &enableDefaultField,
},
// baz is explicitly set to be excluded from default_field.
mapping.Field{
Name: "baz",
Type: "keyword",
DefaultField: &disableDefaultField,
},
mapping.Field{
Name: "nested",
Type: "group",
DefaultField: &enableDefaultField,
Fields: mapping.Fields{
mapping.Field{
Name: "bar",
Type: "keyword",
},
},
},
// The nested group is disabled default_field but one of the children
// has explicitly requested to be included.
mapping.Field{
Name: "nested",
Type: "group",
DefaultField: &disableDefaultField,
Fields: mapping.Fields{
mapping.Field{
Name: "foo",
Type: "keyword",
DefaultField: &enableDefaultField,
},
mapping.Field{
Name: "baz",
Type: "keyword",
},
},
},
}

version, err := common.NewVersion("7.0.0")
if err != nil {
t.Fatal(err)
}

p := Processor{EsVersion: *version}
output := common.MapStr{}
if err = p.Process(fields, nil, output); err != nil {
t.Fatal(err)
}

assert.Len(t, defaultFields, 4)
assert.Contains(t, defaultFields,
"foo",
"bar",
"nested.foo",
"nested.bar",
)
}
2 changes: 1 addition & 1 deletion libbeat/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (t *Template) load(fields mapping.Fields) (common.MapStr, error) {
// Start processing at the root
properties := common.MapStr{}
processor := Processor{EsVersion: t.esVersion, Migration: t.migration}
if err := processor.Process(fields, "", properties); err != nil {
if err := processor.Process(fields, nil, properties); err != nil {
return nil, err
}
output := t.Generate(properties, dynamicTemplates)
Expand Down
86 changes: 86 additions & 0 deletions libbeat/tests/system/template/template.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

package template

import (
"strings"
"testing"

"github.com/elastic/beats/libbeat/asset"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/template"
"github.com/elastic/beats/libbeat/version"
)

// MaxDefaultFieldLength is the limit on the number of default_field values
// allowed by the test. This is less that the 1024 limit of Elasticsearch to
// give a little room for custom fields and the expansion of `fields.*`.
const MaxDefaultFieldLength = 1000
andrewkroh marked this conversation as resolved.
Show resolved Hide resolved
andrewkroh marked this conversation as resolved.
Show resolved Hide resolved
andrewkroh marked this conversation as resolved.
Show resolved Hide resolved

// TestTemplate executes tests on the Beat's index template.
func TestTemplate(t *testing.T, beatName string) {
t.Run("default_field length", testTemplateDefaultFieldLength(beatName))
}

// testTemplateDefaultFieldLength constructs a template based on the embedded
// fields.yml data verifies that the length is less than 1000.
func testTemplateDefaultFieldLength(beatName string) func(*testing.T) {
return func(t *testing.T) {
// 7.0 is when default_field was introduced.
esVersion, err := common.NewVersion("7.0.0")
if err != nil {
t.Fatal(err)
}

// Generate a template based on the embedded fields.yml data.
tmpl, err := template.New(version.GetDefaultVersion(), beatName, *esVersion, template.TemplateConfig{}, false)
if err != nil {
t.Fatal(err)
}

fieldsBytes, err := asset.GetFields(beatName)
if err != nil {
t.Fatal("Failed to get embedded fields.yml asset data:", err)
}

fields, err := tmpl.LoadBytes(fieldsBytes)
if err != nil {
t.Fatal("Failed to load template bytes:", err)
}

templateMap := tmpl.Generate(fields, nil)

v, _ := templateMap.GetValue("settings.index.query.default_field")
defaultValue, ok := v.([]string)
if !ok {
t.Fatalf("settings.index.query.default_field value has an unexpected type: %T", v)
}

Copy link
Member

Choose a reason for hiding this comment

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

What about also executing a elasticsearch query here in addition to catch potential other errors. Say for example ES decides to lower it to 512?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would need to be added somewhere else IMO. I put the test here so that it could be executed quickly as a unit test and not have an external dependency on ES.

This could be put into system test. Each beat should probably call setup to install its template and ILM policy, index a document, then perform a query.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, this would fit well here: https://github.com/elastic/beats/blob/master/filebeat/tests/system/test_base.py#L30 Perhaps someone in the team from @urso could follow up with this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewkroh @ruflin I've created #14352 to make a change in libbeat to be more proactive to detect that error.

if len(defaultValue) > MaxDefaultFieldLength {
t.Fatalf("Too many fields (%d>%d) in %v index template"+
"settings.index.query.default_field for comfort. By default "+
"Elasticsearch has a limit of 1024 fields in a query so we need "+
"to keep the number of fields below 1024. Adding 'default_field: "+
"false' to fields or groups in a fields.yml can be used to "+
"reduce the number of text/keyword fields that end up in "+
"default_field.",
len(defaultValue), MaxDefaultFieldLength, strings.Title(beatName))
}
t.Logf("%v template has %d fields in default_field.", strings.Title(beatName), len(defaultValue))
}
}
Loading