[Metricbeat] Avoid omitting of errors in Vsphere module and upgrade t…

…o use context.Context (#12816)
elastic · Jul 18, 2019 · 8f8d0b3 · 8f8d0b3
1 parent 7bba6ec
commit 8f8d0b3
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 31 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -150,6 +150,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Ramdisk is not filtered out when collecting disk performance counters in diskio metricset {issue}12814[12814] {pull}12829[12829]
 - Fix connections leak in redis module {pull}12914[12914]
 - Fix wrong uptime reporting by system/uptime metricset under Windows. {pull}12915[12915]
+- Print errors that were being omitted in vSphere metricsets {pull}12816[12816]
 
 *Packetbeat*
 

diff --git a/metricbeat/module/vsphere/datastore/datastore.go b/metricbeat/module/vsphere/datastore/datastore.go
@@ -76,17 +76,20 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 // Fetch methods implements the data gathering and data conversion to the right
 // format. It publishes the event which is then forwarded to the output. In case
 // of an error set the Error field of mb.Event or simply call report.Error().
-func (m *MetricSet) Fetch(reporter mb.ReporterV2) error {
-
-	ctx, cancel := context.WithCancel(context.Background())
+func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error {
+	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	client, err := govmomi.NewClient(ctx, m.HostURL, m.Insecure)
 	if err != nil {
 		return errors.Wrap(err, "error in NewClient")
 	}
 
-	defer client.Logout(ctx)
+	defer func() {
+		if err := client.Logout(ctx); err != nil {
+			m.Logger().Debug(errors.Wrap(err, "error trying to logout from vshphere"))
+		}
+	}()
 
 	c := client.Client
 
@@ -98,12 +101,15 @@ func (m *MetricSet) Fetch(reporter mb.ReporterV2) error {
 		return errors.Wrap(err, "error in CreateContainerView")
 	}
 
-	defer v.Destroy(ctx)
+	defer func() {
+		if err := v.Destroy(ctx); err != nil {
+			m.Logger().Debug(errors.Wrap(err, "error trying to destroy view from vshphere"))
+		}
+	}()
 
 	// Retrieve summary property for all datastores
 	var dst []mo.Datastore
-	err = v.Retrieve(ctx, []string{"Datastore"}, []string{"summary"}, &dst)
-	if err != nil {
+	if err = v.Retrieve(ctx, []string{"Datastore"}, []string{"summary"}, &dst); err != nil {
 		return errors.Wrap(err, "error in Retrieve")
 	}
 

diff --git a/metricbeat/module/vsphere/datastore/datastore_test.go b/metricbeat/module/vsphere/datastore/datastore_test.go
@@ -35,8 +35,8 @@ func TestFetchEventContents(t *testing.T) {
 	ts := model.Service.NewServer()
 	defer ts.Close()
 
-	f := mbtest.NewReportingMetricSetV2Error(t, getConfig(ts))
-	events, errs := mbtest.ReportingFetchV2Error(f)
+	f := mbtest.NewReportingMetricSetV2WithContext(t, getConfig(ts))
+	events, errs := mbtest.ReportingFetchV2WithContext(f)
 	if len(errs) > 0 {
 		t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs)
 	}
@@ -85,9 +85,9 @@ func TestData(t *testing.T) {
 	ts := model.Service.NewServer()
 	defer ts.Close()
 
-	f := mbtest.NewReportingMetricSetV2Error(t, getConfig(ts))
+	f := mbtest.NewReportingMetricSetV2WithContext(t, getConfig(ts))
 
-	if err := mbtest.WriteEventsReporterV2Error(f, t, ""); err != nil {
+	if err := mbtest.WriteEventsReporterV2WithContext(f, t, ""); err != nil {
 		t.Fatal("write", err)
 	}
 }

diff --git a/metricbeat/module/vsphere/host/host.go b/metricbeat/module/vsphere/host/host.go
@@ -81,17 +81,20 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 // Fetch methods implements the data gathering and data conversion to the right
 // format. It publishes the event which is then forwarded to the output. In case
 // of an error set the Error field of mb.Event or simply call report.Error().
-func (m *MetricSet) Fetch(reporter mb.ReporterV2) error {
-
-	ctx, cancel := context.WithCancel(context.Background())
+func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error {
+	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	client, err := govmomi.NewClient(ctx, m.HostURL, m.Insecure)
 	if err != nil {
 		return errors.Wrap(err, "error in NewClient")
 	}
 
-	defer client.Logout(ctx)
+	defer func() {
+		if err := client.Logout(ctx); err != nil {
+			m.Logger().Debug(errors.Wrap(err, "error trying to logout from vshphere"))
+		}
+	}()
 
 	c := client.Client
 
@@ -103,7 +106,11 @@ func (m *MetricSet) Fetch(reporter mb.ReporterV2) error {
 		return errors.Wrap(err, "error in CreateContainerView")
 	}
 
-	defer v.Destroy(ctx)
+	defer func() {
+		if err := v.Destroy(ctx); err != nil {
+			m.Logger().Debug(errors.Wrap(err, "error trying to destroy view from vshphere"))
+		}
+	}()
 
 	// Retrieve summary property for all hosts.
 	var hst []mo.HostSystem

diff --git a/metricbeat/module/vsphere/host/host_test.go b/metricbeat/module/vsphere/host/host_test.go
@@ -36,8 +36,8 @@ func TestFetchEventContents(t *testing.T) {
 	ts := model.Service.NewServer()
 	defer ts.Close()
 
-	f := mbtest.NewReportingMetricSetV2Error(t, getConfig(ts))
-	events, errs := mbtest.ReportingFetchV2Error(f)
+	f := mbtest.NewReportingMetricSetV2WithContext(t, getConfig(ts))
+	events, errs := mbtest.ReportingFetchV2WithContext(f)
 	if len(errs) > 0 {
 		t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs)
 	}
@@ -82,9 +82,9 @@ func TestData(t *testing.T) {
 	ts := model.Service.NewServer()
 	defer ts.Close()
 
-	f := mbtest.NewReportingMetricSetV2Error(t, getConfig(ts))
+	f := mbtest.NewReportingMetricSetV2WithContext(t, getConfig(ts))
 
-	if err := mbtest.WriteEventsReporterV2Error(f, t, ""); err != nil {
+	if err := mbtest.WriteEventsReporterV2WithContext(f, t, ""); err != nil {
 		t.Fatal("write", err)
 	}
 }

diff --git a/metricbeat/module/vsphere/virtualmachine/virtualmachine.go b/metricbeat/module/vsphere/virtualmachine/virtualmachine.go
@@ -86,16 +86,20 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 // Fetch methods implements the data gathering and data conversion to the right
 // format. It publishes the event which is then forwarded to the output. In case
 // of an error set the Error field of mb.Event or simply call report.Error().
-func (m *MetricSet) Fetch(reporter mb.ReporterV2) error {
-	ctx, cancel := context.WithCancel(context.Background())
+func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error {
+	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	client, err := govmomi.NewClient(ctx, m.HostURL, m.Insecure)
 	if err != nil {
 		return errors.Wrap(err, "error in NewClient")
 	}
 
-	defer client.Logout(ctx)
+	defer func() {
+		if err := client.Logout(ctx); err != nil {
+			m.Logger().Debug(errors.Wrap(err, "error trying to logout from vshphere"))
+		}
+	}()
 
 	c := client.Client
 
@@ -117,7 +121,11 @@ func (m *MetricSet) Fetch(reporter mb.ReporterV2) error {
 		return errors.Wrap(err, "error in CreateContainerView")
 	}
 
-	defer v.Destroy(ctx)
+	defer func() {
+		if err := v.Destroy(ctx); err != nil {
+			m.Logger().Debug(errors.Wrap(err, "error trying to destroy view from vshphere"))
+		}
+	}()
 
 	// Retrieve summary property for all machines
 	var vmt []mo.VirtualMachine
@@ -181,7 +189,7 @@ func (m *MetricSet) Fetch(reporter mb.ReporterV2) error {
 		}
 
 		if vm.Summary.Vm != nil {
-			networkNames, err := getNetworkNames(c, vm.Summary.Vm.Reference())
+			networkNames, err := getNetworkNames(ctx, c, vm.Summary.Vm.Reference())
 			if err != nil {
 				m.Logger().Debug(err.Error())
 			} else {
@@ -214,8 +222,8 @@ func getCustomFields(customFields []types.BaseCustomFieldValue, customFieldsMap
 	return outputFields
 }
 
-func getNetworkNames(c *vim25.Client, ref types.ManagedObjectReference) ([]string, error) {
-	ctx, cancel := context.WithCancel(context.Background())
+func getNetworkNames(ctx context.Context, c *vim25.Client, ref types.ManagedObjectReference) ([]string, error) {
+	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	var outputNetworkNames []string

diff --git a/metricbeat/module/vsphere/virtualmachine/virtualmachine_test.go b/metricbeat/module/vsphere/virtualmachine/virtualmachine_test.go
@@ -37,8 +37,8 @@ func TestFetchEventContents(t *testing.T) {
 	ts := model.Service.NewServer()
 	defer ts.Close()
 
-	f := mbtest.NewReportingMetricSetV2Error(t, getConfig(ts))
-	events, errs := mbtest.ReportingFetchV2Error(f)
+	f := mbtest.NewReportingMetricSetV2WithContext(t, getConfig(ts))
+	events, errs := mbtest.ReportingFetchV2WithContext(f)
 	if len(errs) > 0 {
 		t.Fatalf("Expected 0 error, had %d. %v\n", len(errs), errs)
 	}
@@ -82,9 +82,9 @@ func TestData(t *testing.T) {
 	ts := model.Service.NewServer()
 	defer ts.Close()
 
-	f := mbtest.NewReportingMetricSetV2Error(t, getConfig(ts))
+	f := mbtest.NewReportingMetricSetV2WithContext(t, getConfig(ts))
 
-	if err := mbtest.WriteEventsReporterV2Error(f, t, ""); err != nil {
+	if err := mbtest.WriteEventsReporterV2WithContext(f, t, ""); err != nil {
 		t.Fatal("write", err)
 	}
 }