From e545836afb147392866f8b92fe27875e43881638 Mon Sep 17 00:00:00 2001 From: Mariana Dima Date: Thu, 2 Apr 2020 13:42:28 +0200 Subject: [PATCH] Add test for documented fields check for metricsets without a http input (#17334) * add test for documented fields * changelog * work on test * add test for testdata * fmt update --- CHANGELOG.next.asciidoc | 1 + metricbeat/mb/testing/testdata.go | 32 +++++++-- metricbeat/mb/testing/testdata_test.go | 45 ++++++++++++ .../compute_vm/compute_vm_integration_test.go | 3 +- .../compute_vm_scaleset_integration_test.go | 2 +- .../container_instance_integration_test.go | 2 +- .../container_registry_integration_test.go | 2 +- .../container_service_integration_test.go | 2 +- .../database_account_integration_test.go | 2 +- .../azure/monitor/monitor_integration_test.go | 2 +- .../azure/storage/storage_integration_test.go | 2 +- .../module/azure/test/integration.go | 70 ------------------- 12 files changed, 81 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 2fe4a6d039a..34603af4707 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -289,6 +289,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] - Add final tests and move label to GA for the azure module in metricbeat. {pull}17319[17319] *Packetbeat* diff --git a/metricbeat/mb/testing/testdata.go b/metricbeat/mb/testing/testdata.go index a0cf85d4b01..3770a76d21c 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{}{} @@ -350,12 +365,19 @@ func documentedFieldCheck(foundKeys common.MapStr, knownKeys map[string]interfac 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/metricbeat/mb/testing/testdata_test.go b/metricbeat/mb/testing/testdata_test.go index 39cbf5cf8f3..442b97ba0f4 100644 --- a/metricbeat/mb/testing/testdata_test.go +++ b/metricbeat/mb/testing/testdata_test.go @@ -20,6 +20,8 @@ package testing import ( "testing" + "github.com/elastic/beats/v7/libbeat/common" + "github.com/stretchr/testify/assert" ) @@ -40,3 +42,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'") + +} 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 3e197f186b3..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,8 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(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 fc11506d890..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,7 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(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 700e606a039..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,7 +28,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(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 a820e252a36..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,7 +28,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(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 821d2049e87..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,7 +28,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(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 e8ab246834c..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,7 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(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 1feba658f85..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,7 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(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 532ef492775..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,7 +25,7 @@ func TestFetchMetricset(t *testing.T) { t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs) } assert.NotEmpty(t, events) - test.TestFieldsDocumentation(t, events) + mbtest.TestMetricsetFieldsDocumented(t, metricSet, events) } func TestData(t *testing.T) { 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 -}