From 02458b070d65cfaad3175977489a466f3761ff90 Mon Sep 17 00:00:00 2001 From: Mariana Date: Mon, 30 Mar 2020 18:26:17 +0200 Subject: [PATCH 1/5] add test for documented fields --- metricbeat/mb/testing/testdata.go | 27 ++++++++++++++++--- .../compute_vm/compute_vm_integration_test.go | 1 + .../compute_vm_scaleset_integration_test.go | 1 + .../container_instance_integration_test.go | 1 + .../container_registry_integration_test.go | 1 + .../container_service_integration_test.go | 1 + .../database_account_integration_test.go | 1 + .../azure/monitor/monitor_integration_test.go | 1 + .../azure/storage/storage_integration_test.go | 1 + 9 files changed, 32 insertions(+), 3 deletions(-) diff --git a/metricbeat/mb/testing/testdata.go b/metricbeat/mb/testing/testdata.go index a0cf85d4b01..e97ea4dfc26 100644 --- a/metricbeat/mb/testing/testdata.go +++ b/metricbeat/mb/testing/testdata.go @@ -171,6 +171,22 @@ func TestDataFilesWithConfig(t *testing.T, module, metricSet string, config Data } } +// TestMetricsetFieldsDocumented checks metricset fields are documented from metricsets that cannot run `TestDataFiles` test which contains this check +func TestMetricsetFieldsDocumented(t *testing.T, metricSet mb.MetricSet, events []mb.Event) { + var data []common.MapStr + for _, e := range events { + beatEvent := StandardizeEvent(metricSet, e, mb.AddMetricSetInfo) + data = append(data, beatEvent.Fields) + } + + if err := checkDocumented(data, nil); err != nil { + t.Errorf("%v: check if fields are documented in `metricbeat/%s/%s/_meta/fields.yml` "+ + "file or run 'make update' on Metricbeat folder to update fields in `metricbeat/fields.yml`", + err, metricSet.Module().Name(), metricSet.Name()) + } + +} + func runTest(t *testing.T, file string, module, metricSetName string, config DataConfig) { // starts a server serving the given file under the given url s := server(t, file, config.URL) @@ -215,7 +231,7 @@ func runTest(t *testing.T, file string, module, metricSetName string, config Dat return h1 < h2 }) - if err := checkDocumented(t, data, config.OmitDocumentedFieldsCheck); err != nil { + if err := checkDocumented(data, config.OmitDocumentedFieldsCheck); err != nil { t.Errorf("%v: check if fields are documented in `metricbeat/%s/%s/_meta/fields.yml` "+ "file or run 'make update' on Metricbeat folder to update fields in `metricbeat/fields.yml`", err, module, metricSetName) @@ -300,7 +316,7 @@ func writeDataJSON(t *testing.T, data common.MapStr, path string) { } // checkDocumented checks that all fields which show up in the events are documented -func checkDocumented(t *testing.T, data []common.MapStr, omitFields []string) error { +func checkDocumented(data []common.MapStr, omitFields []string) error { fieldsData, err := asset.GetFields("metricbeat") if err != nil { return err @@ -310,7 +326,6 @@ func checkDocumented(t *testing.T, data []common.MapStr, omitFields []string) er if err != nil { return err } - documentedFields := fields.GetKeys() keys := map[string]interface{}{} @@ -346,6 +361,12 @@ func documentedFieldCheck(foundKeys common.MapStr, knownKeys map[string]interfac found = true break } + // should cover scenarios as azure.compute_vm_scaleset.*.* + key = strings.Join(splits[0:pos], ".") + ".*.*" + if _, ok := knownKeys[key]; ok { + found = true + break + } } if found { continue diff --git a/x-pack/metricbeat/module/azure/compute_vm/compute_vm_integration_test.go b/x-pack/metricbeat/module/azure/compute_vm/compute_vm_integration_test.go index f5f988d41f3..2da62daaeec 100644 --- a/x-pack/metricbeat/module/azure/compute_vm/compute_vm_integration_test.go +++ b/x-pack/metricbeat/module/azure/compute_vm/compute_vm_integration_test.go @@ -25,6 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { diff --git a/x-pack/metricbeat/module/azure/compute_vm_scaleset/compute_vm_scaleset_integration_test.go b/x-pack/metricbeat/module/azure/compute_vm_scaleset/compute_vm_scaleset_integration_test.go index b48d7a65c5f..7403203ad12 100644 --- a/x-pack/metricbeat/module/azure/compute_vm_scaleset/compute_vm_scaleset_integration_test.go +++ b/x-pack/metricbeat/module/azure/compute_vm_scaleset/compute_vm_scaleset_integration_test.go @@ -25,6 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { diff --git a/x-pack/metricbeat/module/azure/container_instance/container_instance_integration_test.go b/x-pack/metricbeat/module/azure/container_instance/container_instance_integration_test.go index 4808c3fde5d..e12e879084e 100644 --- a/x-pack/metricbeat/module/azure/container_instance/container_instance_integration_test.go +++ b/x-pack/metricbeat/module/azure/container_instance/container_instance_integration_test.go @@ -28,6 +28,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { diff --git a/x-pack/metricbeat/module/azure/container_registry/container_registry_integration_test.go b/x-pack/metricbeat/module/azure/container_registry/container_registry_integration_test.go index 3f6fceb64d7..df38ea55a79 100644 --- a/x-pack/metricbeat/module/azure/container_registry/container_registry_integration_test.go +++ b/x-pack/metricbeat/module/azure/container_registry/container_registry_integration_test.go @@ -28,6 +28,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { diff --git a/x-pack/metricbeat/module/azure/container_service/container_service_integration_test.go b/x-pack/metricbeat/module/azure/container_service/container_service_integration_test.go index dfb1017c74f..d9dc1bc98b8 100644 --- a/x-pack/metricbeat/module/azure/container_service/container_service_integration_test.go +++ b/x-pack/metricbeat/module/azure/container_service/container_service_integration_test.go @@ -28,6 +28,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { diff --git a/x-pack/metricbeat/module/azure/database_account/database_account_integration_test.go b/x-pack/metricbeat/module/azure/database_account/database_account_integration_test.go index 0ec41fdd8a4..6fa35ee4698 100644 --- a/x-pack/metricbeat/module/azure/database_account/database_account_integration_test.go +++ b/x-pack/metricbeat/module/azure/database_account/database_account_integration_test.go @@ -25,6 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { diff --git a/x-pack/metricbeat/module/azure/monitor/monitor_integration_test.go b/x-pack/metricbeat/module/azure/monitor/monitor_integration_test.go index f37a87f18c8..b351592d0dd 100644 --- a/x-pack/metricbeat/module/azure/monitor/monitor_integration_test.go +++ b/x-pack/metricbeat/module/azure/monitor/monitor_integration_test.go @@ -25,6 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { diff --git a/x-pack/metricbeat/module/azure/storage/storage_integration_test.go b/x-pack/metricbeat/module/azure/storage/storage_integration_test.go index f3a3f906173..b611f82694b 100644 --- a/x-pack/metricbeat/module/azure/storage/storage_integration_test.go +++ b/x-pack/metricbeat/module/azure/storage/storage_integration_test.go @@ -25,6 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { From b936db0e94cfc383ad55e09a12f08607ec959142 Mon Sep 17 00:00:00 2001 From: Mariana Date: Mon, 30 Mar 2020 18:34:10 +0200 Subject: [PATCH 2/5] changelog --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 728432a10c6..4be1677eeba 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -279,6 +279,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Release vsphere module as GA. {issue}15798[15798] {pull}17119[17119] - Add Storage metricsets to GCP module {pull}15598[15598] - Added documentation for running Metricbeat in Cloud Foundry. {pull}17275[17275] +- Add test for documented fields check for metricsets without a http input. {issue}17315[17315] {pull}17334[17334] *Packetbeat* From bae1de40b56e17b7b071cb988bd7a3eaa9bed54b Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 31 Mar 2020 11:49:52 +0200 Subject: [PATCH 3/5] work on test --- metricbeat/mb/testing/testdata.go | 17 ++--- .../module/azure/test/integration.go | 70 ------------------- 2 files changed, 9 insertions(+), 78 deletions(-) diff --git a/metricbeat/mb/testing/testdata.go b/metricbeat/mb/testing/testdata.go index e97ea4dfc26..3770a76d21c 100644 --- a/metricbeat/mb/testing/testdata.go +++ b/metricbeat/mb/testing/testdata.go @@ -361,22 +361,23 @@ func documentedFieldCheck(foundKeys common.MapStr, knownKeys map[string]interfac found = true break } - // should cover scenarios as azure.compute_vm_scaleset.*.* - key = strings.Join(splits[0:pos], ".") + ".*.*" - if _, ok := knownKeys[key]; ok { - found = true - break - } } if found { continue } - - // last case `status_codes.*`: + // case `status_codes.*`: prefix := strings.Join(splits[0:len(splits)-1], ".") if _, ok := knownKeys[prefix+".*"]; ok { continue } + // should cover scenarios as status_codes.*.*` and `azure.compute_vm_scaleset.*.*` + if len(splits) > 2 { + prefix = strings.Join(splits[0:len(splits)-2], ".") + if _, ok := knownKeys[prefix+".*.*"]; ok { + continue + } + } + return errors.Errorf("field missing '%s'", foundKey) } } diff --git a/x-pack/metricbeat/module/azure/test/integration.go b/x-pack/metricbeat/module/azure/test/integration.go index 91a30175bca..7187e3b93f7 100644 --- a/x-pack/metricbeat/module/azure/test/integration.go +++ b/x-pack/metricbeat/module/azure/test/integration.go @@ -5,13 +5,8 @@ package test import ( - "errors" "os" "testing" - - "github.com/stretchr/testify/assert" - - "github.com/elastic/beats/v7/metricbeat/mb" ) // GetConfig function gets azure credentials for integration tests. @@ -45,68 +40,3 @@ func GetConfig(t *testing.T, metricSetName string) map[string]interface{} { "subscription_id": subId, } } - -// TestFieldsDocumentation func checks if all the documented fields have the expected type -func TestFieldsDocumentation(t *testing.T, events []mb.Event) { - for _, event := range events { - // RootField - checkIsDocumented("service.name", "string", event, t) - checkIsDocumented("cloud.provider", "string", event, t) - checkIsDocumented("cloud.region", "string", event, t) - checkIsDocumented("cloud.instance.name", "string", event, t) - checkIsDocumented("cloud.instance.id", "string", event, t) - - // MetricSetField - checkIsDocumented("azure.timegrain", "string", event, t) - checkIsDocumented("azure.subscription_id", "string", event, t) - checkIsDocumented("azure.namespace", "string", event, t) - checkIsDocumented("azure.resource.type", "string", event, t) - checkIsDocumented("azure.resource.group", "string", event, t) - } -} - -// checkIsDocumented function checks a given field type and compares it with the expected type for integration tests. -// this implementation is only temporary, will be replaced by issue https://github.com/elastic/beats/issues/17315 -func checkIsDocumented(metricName string, expectedType string, event mb.Event, t *testing.T) { - t.Helper() - - ok1, err1 := event.MetricSetFields.HasKey(metricName) - ok2, err2 := event.RootFields.HasKey(metricName) - if ok1 || ok2 { - if ok1 { - assert.NoError(t, err1) - metricValue, err := event.MetricSetFields.GetValue(metricName) - assert.NoError(t, err) - err = compareType(metricValue, expectedType, metricName) - assert.NoError(t, err) - t.Log("Succeed: Field " + metricName + " matches type " + expectedType) - } else if ok2 { - assert.NoError(t, err2) - rootValue, err := event.RootFields.GetValue(metricName) - assert.NoError(t, err) - err = compareType(rootValue, expectedType, metricName) - assert.NoError(t, err) - t.Log("Succeed: Field " + metricName + " matches type " + expectedType) - } - } else { - t.Log("Field " + metricName + " does not exist in metric set fields") - } -} - -func compareType(metricValue interface{}, expectedType string, metricName string) (err error) { - switch metricValue.(type) { - case float64: - if expectedType != "float" { - err = errors.New("Failed: Field " + metricName + " is not in type " + expectedType) - } - case string: - if expectedType != "string" { - err = errors.New("Failed: Field " + metricName + " is not in type " + expectedType) - } - case int64: - if expectedType != "int" { - err = errors.New("Failed: Field " + metricName + " is not in type " + expectedType) - } - } - return -} From 4b7f2785872b43ea45eab026513f3048067b1db7 Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 31 Mar 2020 15:21:13 +0200 Subject: [PATCH 4/5] add test for testdata --- metricbeat/mb/testing/testdata_test.go | 44 ++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/metricbeat/mb/testing/testdata_test.go b/metricbeat/mb/testing/testdata_test.go index 39cbf5cf8f3..7a3daf007ba 100644 --- a/metricbeat/mb/testing/testdata_test.go +++ b/metricbeat/mb/testing/testdata_test.go @@ -18,6 +18,7 @@ package testing import ( + "github.com/elastic/beats/v7/libbeat/common" "testing" "github.com/stretchr/testify/assert" @@ -40,3 +41,46 @@ func TestOmitDocumentedField(t *testing.T) { assert.Equal(t, tt.result, result) } } + +func TestDocumentedFieldCheck(t *testing.T) { + foundKeys := common.MapStr{ + "hello": "hello", + "elasticsearch.stats": "stats1", + } + omitfields := []string{ + "hello", + } + knownKeys := map[string]interface{}{ + "elasticsearch.stats": "key1", + } + err := documentedFieldCheck(foundKeys, knownKeys, omitfields) + //error should be nil, as `hello` field is ignored and `elasticsearch.stats` field is defined + assert.NoError(t, err) + + foundKeys = common.MapStr{ + "elasticsearch.stats.cpu": "stats2", + "elasticsearch.metrics.requests.count": "requests2", + } + + knownKeys = map[string]interface{}{ + "elasticsearch.stats.*": "key1", + "elasticsearch.metrics.*.*": "hello1", + } + // error should be nil as the foundKeys are covered by the `prefix` cases + err = documentedFieldCheck(foundKeys, knownKeys, omitfields) + assert.NoError(t, err) + + foundKeys = common.MapStr{ + "elasticsearch.stats.cpu": "stats2", + "elasticsearch.metrics.requests.count": "requests2", + } + + knownKeys = map[string]interface{}{ + "elasticsearch.*": "key1", + "elasticsearch.metrics.*": "hello1", + } + // error should not be nil as the foundKeys are not covered by the `prefix` cases + err = documentedFieldCheck(foundKeys, knownKeys, omitfields) + assert.Error(t, err, "field missing 'elasticsearch.stats.cpu'") + +} From e6a9a03da3406c4b3e74cc73e7d1691462ec5624 Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 31 Mar 2020 16:08:18 +0200 Subject: [PATCH 5/5] fmt update --- metricbeat/mb/testing/testdata_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/metricbeat/mb/testing/testdata_test.go b/metricbeat/mb/testing/testdata_test.go index 7a3daf007ba..442b97ba0f4 100644 --- a/metricbeat/mb/testing/testdata_test.go +++ b/metricbeat/mb/testing/testdata_test.go @@ -18,9 +18,10 @@ package testing import ( - "github.com/elastic/beats/v7/libbeat/common" "testing" + "github.com/elastic/beats/v7/libbeat/common" + "github.com/stretchr/testify/assert" )