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 test for documented fields check for metricsets without a http input #17334

Merged
merged 6 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all 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.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,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*
Expand Down
32 changes: 27 additions & 5 deletions metricbeat/mb/testing/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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{}{}

Expand Down Expand Up @@ -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
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about adding unit tests for this function? I see it has started to grow and I foresee we will keep updating it for a while

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well spotted, added a test in testdata_test.go

return errors.Errorf("field missing '%s'", foundKey)
}
}
Expand Down
45 changes: 45 additions & 0 deletions metricbeat/mb/testing/testdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ package testing
import (
"testing"

"github.com/elastic/beats/v7/libbeat/common"

"github.com/stretchr/testify/assert"
)

Expand All @@ -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'")

}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
70 changes: 0 additions & 70 deletions x-pack/metricbeat/module/azure/test/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}