From a09d9fc673577d0c4769363bb56e6acde0f6c194 Mon Sep 17 00:00:00 2001 From: sayden Date: Thu, 4 Apr 2019 23:35:00 +0200 Subject: [PATCH 1/4] Add moduleFlag and ExtraConfig --- metricbeat/mb/testing/data/data_test.go | 46 +++++++++++++++++-------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/metricbeat/mb/testing/data/data_test.go b/metricbeat/mb/testing/data/data_test.go index 54d9eb15f58..bc318db2782 100644 --- a/metricbeat/mb/testing/data/data_test.go +++ b/metricbeat/mb/testing/data/data_test.go @@ -51,12 +51,14 @@ const ( var ( // Use `go test -generate` to update files. generateFlag = flag.Bool("generate", false, "Write golden files") + moduleFlag = flag.String("module", "", "Write golden files") ) type Config struct { - Type string - URL string - Suffix string + Type string + URL string + Suffix string + ModuleConfig map[string]interface{} `yaml:"module_config"` } func TestAll(t *testing.T) { @@ -83,13 +85,13 @@ func TestAll(t *testing.T) { config.Suffix = "json" } - getTestdataFiles(t, config.URL, moduleName, metricSetName, config.Suffix) + getTestdataFiles(t, moduleName, metricSetName, config) } } -func getTestdataFiles(t *testing.T, url, module, metricSet, suffix string) { +func getTestdataFiles(t *testing.T, module, metricSet string, config Config) { - ff, err := filepath.Glob(getMetricsetPath(module, metricSet) + "/_meta/testdata/*." + suffix) + ff, err := filepath.Glob(getMetricsetPath(module, metricSet) + "/_meta/testdata/*." + config.Suffix) if err != nil { t.Fatal(err) } @@ -105,28 +107,35 @@ func getTestdataFiles(t *testing.T, url, module, metricSet, suffix string) { for _, f := range files { t.Run(f, func(t *testing.T) { - runTest(t, f, module, metricSet, url, suffix) + if *moduleFlag != "" { + if *moduleFlag == module { + runTest(t, f, module, metricSet, config) + } + } else { + runTest(t, f, module, metricSet, config) + } }) } } -func runTest(t *testing.T, file string, module, metricSetName, url, suffix string) { +func runTest(t *testing.T, file string, module, metricSetName string, config Config) { // starts a server serving the given file under the given url - s := server(t, file, url) + s := server(t, file, config.URL) defer s.Close() - metricSet := mbtesting.NewMetricSet(t, getConfig(module, metricSetName, s.URL)) + moduleConfig := getConfig(module, metricSetName, s.URL, config) + metricSet := mbtesting.NewMetricSet(t, moduleConfig) var events []mb.Event var errs []error switch v := metricSet.(type) { case mb.ReportingMetricSetV2: - metricSet := mbtesting.NewReportingMetricSetV2(t, getConfig(module, metricSetName, s.URL)) + metricSet := mbtesting.NewReportingMetricSetV2(t, moduleConfig) events, errs = mbtesting.ReportingFetchV2(metricSet) case mb.ReportingMetricSetV2Error: - metricSet := mbtesting.NewReportingMetricSetV2Error(t, getConfig(module, metricSetName, s.URL)) + metricSet := mbtesting.NewReportingMetricSetV2Error(t, moduleConfig) events, errs = mbtesting.ReportingFetchV2Error(metricSet) default: t.Fatalf("unknown type: %T", v) @@ -176,7 +185,7 @@ func runTest(t *testing.T, file string, module, metricSetName, url, suffix strin assert.Equal(t, string(expected), string(output)) - if strings.HasSuffix(file, "docs."+suffix) { + if strings.HasSuffix(file, "docs."+config.Suffix) { writeDataJSON(t, data[0], module, metricSetName) } } @@ -201,6 +210,7 @@ func checkDocumented(t *testing.T, data []common.MapStr) { if err != nil { t.Fatal(err) } + documentedFields := fields.GetKeys() keys := map[string]interface{}{} @@ -226,12 +236,18 @@ func checkDocumented(t *testing.T, data []common.MapStr) { } // GetConfig returns config for elasticsearch module -func getConfig(module, metricSet, url string) map[string]interface{} { - return map[string]interface{}{ +func getConfig(module, metricSet, url string, config Config) map[string]interface{} { + moduleConfig := map[string]interface{}{ "module": module, "metricsets": []string{metricSet}, "hosts": []string{url}, } + + for k, v := range config.ModuleConfig { + moduleConfig[k] = v + } + + return moduleConfig } // server starts a server with a mock output From 47e36c099c18bc9a8ce3eed8a32f58b1f14126ba Mon Sep 17 00:00:00 2001 From: sayden Date: Fri, 5 Apr 2019 16:55:01 +0200 Subject: [PATCH 2/4] Address changes --- metricbeat/mb/testing/data/data_test.go | 53 ++++++++++++++++++------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/metricbeat/mb/testing/data/data_test.go b/metricbeat/mb/testing/data/data_test.go index bc318db2782..52142a06b45 100644 --- a/metricbeat/mb/testing/data/data_test.go +++ b/metricbeat/mb/testing/data/data_test.go @@ -51,14 +51,15 @@ const ( var ( // Use `go test -generate` to update files. generateFlag = flag.Bool("generate", false, "Write golden files") - moduleFlag = flag.String("module", "", "Write golden files") + moduleFlag = flag.String("module", "", "Choose a module to test") ) type Config struct { - Type string - URL string - Suffix string - ModuleConfig map[string]interface{} `yaml:"module_config"` + Type string + URL string + Suffix string + Module map[string]interface{} `yaml:"module"` + OmitDocumentedFieldsCheck []string `yaml:"omit_documented_fields_check"` } func TestAll(t *testing.T) { @@ -90,6 +91,11 @@ func TestAll(t *testing.T) { } func getTestdataFiles(t *testing.T, module, metricSet string, config Config) { + if *moduleFlag != "" { + if *moduleFlag != module { + return + } + } ff, err := filepath.Glob(getMetricsetPath(module, metricSet) + "/_meta/testdata/*." + config.Suffix) if err != nil { @@ -107,13 +113,7 @@ func getTestdataFiles(t *testing.T, module, metricSet string, config Config) { for _, f := range files { t.Run(f, func(t *testing.T) { - if *moduleFlag != "" { - if *moduleFlag == module { - runTest(t, f, module, metricSet, config) - } - } else { - runTest(t, f, module, metricSet, config) - } + runTest(t, f, module, metricSet, config) }) } } @@ -163,7 +163,7 @@ func runTest(t *testing.T, file string, module, metricSetName string, config Con return h1 < h2 }) - checkDocumented(t, data) + checkDocumented(t, data, config.OmitDocumentedFieldsCheck) output, err := json.MarshalIndent(&data, "", " ") if err != nil { @@ -200,7 +200,7 @@ func writeDataJSON(t *testing.T, data common.MapStr, module, metricSet string) { } // checkDocumented checks that all fields which show up in the events are documented -func checkDocumented(t *testing.T, data []common.MapStr) { +func checkDocumented(t *testing.T, data []common.MapStr, omitFields []string) { fieldsData, err := asset.GetFields("metricbeat") if err != nil { t.Fatal(err) @@ -220,8 +220,14 @@ func checkDocumented(t *testing.T, data []common.MapStr) { for _, d := range data { flat := d.Flatten() + keys: for k := range flat { if _, ok := keys[k]; !ok { + for _, omitField := range omitFields { + if omitDocumentedField(k, omitField) { + continue keys + } + } // If a field is defined as object it can also be defined as `status_codes.*` // So this checks if such a key with the * exists by removing the last part. splits := strings.Split(k, ".") @@ -235,6 +241,23 @@ func checkDocumented(t *testing.T, data []common.MapStr) { } } +func omitDocumentedField(field, omitField string) bool { + if strings.Contains(omitField, "*") { + //Omit every key prefixed with chars before "*" + prefixedField := strings.Trim(omitField, ".*") + if strings.Contains(field, prefixedField) { + return true + } + } else { + //Omit only if key matches exactly + if field == omitField { + return true + } + } + + return false +} + // GetConfig returns config for elasticsearch module func getConfig(module, metricSet, url string, config Config) map[string]interface{} { moduleConfig := map[string]interface{}{ @@ -243,7 +266,7 @@ func getConfig(module, metricSet, url string, config Config) map[string]interfac "hosts": []string{url}, } - for k, v := range config.ModuleConfig { + for k, v := range config.Module { moduleConfig[k] = v } From 7945ca844cd2505d40fe230675c34fa84dde847a Mon Sep 17 00:00:00 2001 From: sayden Date: Fri, 5 Apr 2019 17:43:06 +0200 Subject: [PATCH 3/4] Add some comments for maintainers --- metricbeat/mb/testing/data/data_test.go | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/metricbeat/mb/testing/data/data_test.go b/metricbeat/mb/testing/data/data_test.go index 52142a06b45..4647624488b 100644 --- a/metricbeat/mb/testing/data/data_test.go +++ b/metricbeat/mb/testing/data/data_test.go @@ -55,10 +55,36 @@ var ( ) type Config struct { + // The type of the test to run, usually `http`. Type string + + // URL of the endpoint that must be tested depending on each module URL string + + // Suffix is the extension of the source file with the input contents. Defaults to `json`, `plain` is also a common use. Suffix string + + // Module is a map of specific configs that will be appended to a module configuration prior initializing it. + // For example, the following config in yaml: + // module: + // namespace: test + // foo: bar + // + // Will produce the following module config: + // - module: http + // metricsets: + // - json + // period: 10s + // hosts: ["localhost:80"] + // path: "/" + // namespace: "test" + // foo: bar + // + // (notice last two lines) Module map[string]interface{} `yaml:"module"` + + // OmitDocumentedFieldsCheck is a list of fields that must be omitted from the function that checks if the field + // is contained in {metricset}/_meta/fields.yml OmitDocumentedFieldsCheck []string `yaml:"omit_documented_fields_check"` } @@ -241,6 +267,13 @@ func checkDocumented(t *testing.T, data []common.MapStr, omitFields []string) { } } +// omitDocumentedField returns true if 'field' is exactly like 'omitField' or if 'field' equals the prefix of 'omitField' +// if the latter contains a dot.wildcard ".*". For example: +// field: hello, omitField: world false +// field: hello, omitField: hello true +// field: elasticsearch.stats omitField: elasticsearch.stats true +// field: elasticsearch.stats.hello.world omitField: elasticsearch.* true +// field: elasticsearch.stats.hello.world omitField: * true func omitDocumentedField(field, omitField string) bool { if strings.Contains(omitField, "*") { //Omit every key prefixed with chars before "*" From 090004134ed498c4c838a527efbd8c97e717f34d Mon Sep 17 00:00:00 2001 From: sayden Date: Mon, 8 Apr 2019 11:57:10 +0200 Subject: [PATCH 4/4] Address some changes --- metricbeat/mb/testing/data/data_test.go | 83 ++++++++++++++++--------- 1 file changed, 55 insertions(+), 28 deletions(-) diff --git a/metricbeat/mb/testing/data/data_test.go b/metricbeat/mb/testing/data/data_test.go index 4647624488b..4ab0aba8abe 100644 --- a/metricbeat/mb/testing/data/data_test.go +++ b/metricbeat/mb/testing/data/data_test.go @@ -30,6 +30,8 @@ import ( "strings" "testing" + "github.com/pkg/errors" + "github.com/mitchellh/hashstructure" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" @@ -56,13 +58,13 @@ var ( type Config struct { // The type of the test to run, usually `http`. - Type string + Type string // URL of the endpoint that must be tested depending on each module - URL string + URL string // Suffix is the extension of the source file with the input contents. Defaults to `json`, `plain` is also a common use. - Suffix string + Suffix string // Module is a map of specific configs that will be appended to a module configuration prior initializing it. // For example, the following config in yaml: @@ -81,11 +83,11 @@ type Config struct { // foo: bar // // (notice last two lines) - Module map[string]interface{} `yaml:"module"` + Module map[string]interface{} `yaml:"module"` // OmitDocumentedFieldsCheck is a list of fields that must be omitted from the function that checks if the field // is contained in {metricset}/_meta/fields.yml - OmitDocumentedFieldsCheck []string `yaml:"omit_documented_fields_check"` + OmitDocumentedFieldsCheck []string `yaml:"omit_documented_fields_check"` } func TestAll(t *testing.T) { @@ -98,6 +100,12 @@ func TestAll(t *testing.T) { moduleName := s[4] metricSetName := s[5] + if *moduleFlag != "" { + if *moduleFlag != moduleName { + continue + } + } + configFile, err := ioutil.ReadFile(f) if err != nil { log.Printf("yamlFile.Get err #%v ", err) @@ -117,12 +125,6 @@ func TestAll(t *testing.T) { } func getTestdataFiles(t *testing.T, module, metricSet string, config Config) { - if *moduleFlag != "" { - if *moduleFlag != module { - return - } - } - ff, err := filepath.Glob(getMetricsetPath(module, metricSet) + "/_meta/testdata/*." + config.Suffix) if err != nil { t.Fatal(err) @@ -246,25 +248,32 @@ func checkDocumented(t *testing.T, data []common.MapStr, omitFields []string) { for _, d := range data { flat := d.Flatten() - keys: - for k := range flat { - if _, ok := keys[k]; !ok { - for _, omitField := range omitFields { - if omitDocumentedField(k, omitField) { - continue keys - } - } - // If a field is defined as object it can also be defined as `status_codes.*` - // So this checks if such a key with the * exists by removing the last part. - splits := strings.Split(k, ".") - prefix := strings.Join(splits[0:len(splits)-1], ".") - if _, ok := keys[prefix+".*"]; ok { - continue + if err := documentedFieldCheck(flat, keys, omitFields); err != nil { + t.Fatal(err) + } + } +} + +func documentedFieldCheck(foundKeys common.MapStr, knownKeys map[string]interface{}, omitFields []string) error { + for foundKey := range foundKeys { + if _, ok := knownKeys[foundKey]; !ok { + for _, omitField := range omitFields { + if omitDocumentedField(foundKey, omitField) { + return nil } - t.Fatalf("check if fields are documented error: key missing '%s'", k) } + // If a field is defined as object it can also be defined as `status_codes.*` + // So this checks if such a key with the * exists by removing the last part. + splits := strings.Split(foundKey, ".") + prefix := strings.Join(splits[0:len(splits)-1], ".") + if _, ok := knownKeys[prefix+".*"]; ok { + continue + } + return errors.Errorf("check if fields are documented error: key missing '%s'", foundKey) } } + + return nil } // omitDocumentedField returns true if 'field' is exactly like 'omitField' or if 'field' equals the prefix of 'omitField' @@ -276,13 +285,13 @@ func checkDocumented(t *testing.T, data []common.MapStr, omitFields []string) { // field: elasticsearch.stats.hello.world omitField: * true func omitDocumentedField(field, omitField string) bool { if strings.Contains(omitField, "*") { - //Omit every key prefixed with chars before "*" + // Omit every key prefixed with chars before "*" prefixedField := strings.Trim(omitField, ".*") if strings.Contains(field, prefixedField) { return true } } else { - //Omit only if key matches exactly + // Omit only if key matches exactly if field == omitField { return true } @@ -291,6 +300,24 @@ func omitDocumentedField(field, omitField string) bool { return false } +func TestOmitDocumentedField(t *testing.T) { + tts := []struct { + a, b string + result bool + }{ + {a: "hello", b: "world", result: false}, + {a: "hello", b: "hello", result: true}, + {a: "elasticsearch.stats", b: "elasticsearch.stats", result: true}, + {a: "elasticsearch.stats.hello.world", b: "elasticsearch.*", result: true}, + {a: "elasticsearch.stats.hello.world", b: "*", result: true}, + } + + for _, tt := range tts { + result := omitDocumentedField(tt.a, tt.b) + assert.Equal(t, tt.result, result) + } +} + // GetConfig returns config for elasticsearch module func getConfig(module, metricSet, url string, config Config) map[string]interface{} { moduleConfig := map[string]interface{}{