Adding test cases and incorporate review comments

CHANGELOG.asciidoc

@@ -177,6 +177,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
 - The `add_docker_metadata` and `add_kubernetes_metadata` processors are now GA, instead of Beta. {pull}6105[6105]
 - The node name can be discovered automatically by machine-id matching when beat deployed outside kubernetes cluster. {pull}6146[6146]
 - Panics will be written to the logger before exiting. {pull}6199[6199]
+- Add builder support for autodiscover and annotations builder {pull}6408[6408]
 
 *Auditbeat*
 

filebeat/autodiscover/builder/log_annotations/config.go → filebeat/autodiscover/builder/logs/config.go

@@ -1,4 +1,4 @@
-package log_annotations
+package logs
 
 import "github.com/elastic/beats/libbeat/common"
 

filebeat/autodiscover/builder/log_annotations/log_annotations.go → filebeat/autodiscover/builder/logs/logs.go

@@ -1,4 +1,4 @@
-package log_annotations
+package logs
 
 import (
 	"fmt"
@@ -12,15 +12,21 @@ import (
 )
 
 func init() {
-	autodiscover.Registry.AddBuilder("log.annotations", NewLogAnnotations)
+	autodiscover.Registry.AddBuilder("logs", NewLogAnnotations)
 }
 
+const (
+	multiline    = "multiline"
+	includeLines = "include_lines"
+	excludeLines = "exclude_lines"
+)
+
 type logAnnotations struct {
 	Key    string
 	Config []*common.Config
 }
 
-// Construct a log annotations builder
+// NewLogAnnotations builds a log annotations builder
 func NewLogAnnotations(cfg *common.Config) (autodiscover.Builder, error) {
 	config := defaultConfig()
 	err := cfg.Unpack(&config)
@@ -43,9 +49,7 @@ func (l *logAnnotations) CreateConfig(event bus.Event) []*common.Config {
 
 	var hints common.MapStr
 	hIface, ok := event["hints"]
-	if !ok {
-		return config
-	} else {
+	if ok {
 		hints, _ = hIface.(common.MapStr)
 	}
 
@@ -56,26 +60,23 @@ func (l *logAnnotations) CreateConfig(event bus.Event) []*common.Config {
 	//TODO: Add module support
 
 	tempCfg := common.MapStr{}
-	multiline := l.getMultiline(hints)
-
-	for k, v := range multiline {
-		tempCfg.Put(k, v)
+	mline := l.getMultiline(hints)
+	if len(mline) != 0 {
+		tempCfg.Put(multiline, mline)
 	}
-	if includeLines := l.getIncludeLines(hints); len(includeLines) != 0 {
-		tempCfg.Put("include_lines", includeLines)
+	if ilines := l.getIncludeLines(hints); len(ilines) != 0 {
+		tempCfg.Put(includeLines, ilines)
 	}
-	if excludeLines := l.getExcludeLines(hints); len(excludeLines) != 0 {
-		tempCfg.Put("exclude_lines", excludeLines)
+	if elines := l.getExcludeLines(hints); len(elines) != 0 {
+		tempCfg.Put(excludeLines, elines)
 	}
 
 	// Merge config template with the configs from the annotations
 	for _, c := range l.Config {
 		if err := c.Merge(tempCfg); err != nil {
-			logp.Debug("log.annotations", "config merge failed with error: %v", err)
+			logp.Debug("logs.builder", "config merge failed with error: %v", err)
 		} else {
-			cfg := common.MapStr{}
-			c.Unpack(cfg)
-			logp.Debug("log.annotations", "generated config %v", cfg.String())
+			logp.Debug("logs.builder", "generated config %v", *c)
 			config = append(config, c)
 		}
 	}
@@ -86,13 +87,13 @@ func (l *logAnnotations) CreateConfig(event bus.Event) []*common.Config {
 }
 
 func (l *logAnnotations) getMultiline(hints common.MapStr) common.MapStr {
-	return builder.GetHintMapStr(hints, l.Key, "multiline")
+	return builder.GetHintMapStr(hints, l.Key, multiline)
 }
 
 func (l *logAnnotations) getIncludeLines(hints common.MapStr) []string {
-	return builder.GetHintAsList(hints, l.Key, "include_lines")
+	return builder.GetHintAsList(hints, l.Key, includeLines)
 }
 
 func (l *logAnnotations) getExcludeLines(hints common.MapStr) []string {
-	return builder.GetHintAsList(hints, l.Key, "exclude_lines")
+	return builder.GetHintAsList(hints, l.Key, excludeLines)
 }

filebeat/autodiscover/builder/logs/logs_test.go

@@ -0,0 +1,145 @@
+package logs
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/elastic/beats/libbeat/common"
+	"github.com/elastic/beats/libbeat/common/bus"
+)
+
+func TestGenerateHints(t *testing.T) {
+	tests := []struct {
+		event  bus.Event
+		len    int
+		result common.MapStr
+	}{
+		// Hints without host should return nothing
+		{
+			event: bus.Event{
+				"hints": common.MapStr{
+					"metrics": common.MapStr{
+						"module": "prometheus",
+					},
+				},
+			},
+			len:    0,
+			result: common.MapStr{},
+		},
+		// Empty event hints should return default config
+		{
+			event: bus.Event{
+				"host": "1.2.3.4",
+				"kubernetes": common.MapStr{
+					"container": common.MapStr{
+						"name": "foobar",
+						"id":   "abc",
+					},
+				},
+				"docker": common.MapStr{
+					"container": common.MapStr{
+						"name": "foobar",
+						"id":   "abc",
+					},
+				},
+			},
+			len: 1,
+			result: common.MapStr{
+				"type": "docker",
+				"containers": map[string]interface{}{
+					"ids": []interface{}{"abc"},
+				},
+			},
+		},
+		// Hint with include|exclude_lines must be part of the input config
+		{
+			event: bus.Event{
+				"host": "1.2.3.4",
+				"kubernetes": common.MapStr{
+					"container": common.MapStr{
+						"name": "foobar",
+						"id":   "abc",
+					},
+				},
+				"docker": common.MapStr{
+					"container": common.MapStr{
+						"name": "foobar",
+						"id":   "abc",
+					},
+				},
+				"hints": common.MapStr{
+					"logs": common.MapStr{
+						"include_lines": "^test, ^test1",
+						"exclude_lines": "^test2, ^test3",
+					},
+				},
+			},
+			len: 1,
+			result: common.MapStr{
+				"type": "docker",
+				"containers": map[string]interface{}{
+					"ids": []interface{}{"abc"},
+				},
+				"include_lines": []interface{}{"^test", "^test1"},
+				"exclude_lines": []interface{}{"^test2", "^test3"},
+			},
+		},
+		// Hint with multiline config must have a multiline in the input config
+		{
+			event: bus.Event{
+				"host": "1.2.3.4",
+				"kubernetes": common.MapStr{
+					"container": common.MapStr{
+						"name": "foobar",
+						"id":   "abc",
+					},
+				},
+				"docker": common.MapStr{
+					"container": common.MapStr{
+						"name": "foobar",
+						"id":   "abc",
+					},
+				},
+				"hints": common.MapStr{
+					"logs": common.MapStr{
+						"multiline": common.MapStr{
+							"pattern": "^test",
+							"negate":  "true",
+						},
+					},
+				},
+			},
+			len: 1,
+			result: common.MapStr{
+				"type": "docker",
+				"containers": map[string]interface{}{
+					"ids": []interface{}{"abc"},
+				},
+				"multiline": map[string]interface{}{
+					"pattern": "^test",
+					"negate":  "true",
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		cfg := defaultConfig()
+		l := logAnnotations{
+			Key:    cfg.Key,
+			Config: cfg.Config,
+		}
+		cfgs := l.CreateConfig(test.event)
+		assert.Equal(t, len(cfgs), test.len)
+
+		if test.len != 0 {
+			config := common.MapStr{}
+			err := cfgs[0].Unpack(&config)
+			assert.Nil(t, err)
+
+			assert.Equal(t, config, test.result)
+		}
+
+	}
+}

filebeat/autodiscover/include.go

@@ -2,5 +2,5 @@ package autodiscover
 
 import (
 	// include all filebeat specific builders
-	_ "github.com/elastic/beats/filebeat/autodiscover/builder/log_annotations"
+	_ "github.com/elastic/beats/filebeat/autodiscover/builder/logs"
 )

libbeat/autodiscover/autodiscover_test.go

@@ -10,8 +10,6 @@ import (
 	"github.com/elastic/beats/libbeat/common/bus"
 
 	"github.com/stretchr/testify/assert"
-
-	"github.com/elastic/beats/libbeat/autodiscover/template"
 )
 
 type mockRunner struct {
@@ -105,7 +103,7 @@ func TestAutodiscover(t *testing.T) {
 	// Register mock autodiscover provider
 	busChan := make(chan bus.Bus, 1)
 	Registry = NewRegistry()
-	Registry.AddProvider("mock", func(b bus.Bus, mapper *template.Mapper, builders Builders, c *common.Config) (Provider, error) {
+	Registry.AddProvider("mock", func(b bus.Bus, c *common.Config) (Provider, error) {
 		// intercept bus to mock events
 		busChan <- b
 
@@ -208,7 +206,7 @@ func TestAutodiscoverHash(t *testing.T) {
 	busChan := make(chan bus.Bus, 1)
 
 	Registry = NewRegistry()
-	Registry.AddProvider("mock", func(b bus.Bus, mapper *template.Mapper, builders Builders, c *common.Config) (Provider, error) {
+	Registry.AddProvider("mock", func(b bus.Bus, c *common.Config) (Provider, error) {
 		// intercept bus to mock events
 		busChan <- b
 

libbeat/autodiscover/builder.go

@@ -53,8 +53,8 @@ func (r *registry) GetBuilder(name string) BuilderConstructor {
 	return r.builders[name]
 }
 
-// ConstructBuilder reads provider configuration and instatiate one
-func (r *registry) ConstructBuilder(c *common.Config) (Builder, error) {
+// BuildBuilder reads provider configuration and instatiate one
+func (r *registry) BuildBuilder(c *common.Config) (Builder, error) {
 	var config BuilderConfig
 	err := c.Unpack(&config)
 	if err != nil {

libbeat/autodiscover/builder/helper_test.go

@@ -0,0 +1,52 @@
+package builder
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/elastic/beats/libbeat/common"
+)
+
+func TestGenerateHints(t *testing.T) {
+	tests := []struct {
+		annotations map[string]string
+		result      common.MapStr
+	}{
+		// Empty annotations should return empty hints
+		{
+			annotations: map[string]string{},
+			result:      common.MapStr{},
+		},
+
+		// Scenarios being tested:
+		// logs/multiline.pattern must be a nested common.MapStr under hints.logs
+		// metrics/module must be found in hints.metrics
+		// not.to.include must not be part of hints
+		// period is annotated at both container and pod level. Container level value must be in hints
+		{
+			annotations: map[string]string{
+				"co.elastic.logs/multiline.pattern": "^test",
+				"co.elastic.metrics/module":         "prometheus",
+				"co.elastic.metrics/period":         "10s",
+				"co.elastic.metrics.foobar/period":  "15s",
+				"not.to.include":                    "true",
+			},
+			result: common.MapStr{
+				"logs": common.MapStr{
+					"multiline": common.MapStr{
+						"pattern": "^test",
+					},
+				},
+				"metrics": common.MapStr{
+					"module": "prometheus",
+					"period": "15s",
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		assert.Equal(t, GenerateHints(test.annotations, "foobar", "co.elastic."), test.result)
+	}
+}