[Metricbeat] Fix getting compute instance metadata with partial zone/…

…region config (#18757) * use zone for getting instance metadata
elastic · May 28, 2020 · f4c65f3 · f4c65f3
1 parent f6bd8c6
commit f4c65f3
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 84 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -206,6 +206,7 @@ field. You can revert this change by configuring tags for the module and omittin
 - Remove specific win32 api errors from events in perfmon. {issue}18292[18292] {pull}18361[18361]
 - Fix application_pool metricset after pdh changes. {pull}18477[18477]
 - Fix tags_filter for cloudwatch metricset in aws. {pull}18524[18524]
+- Fix getting gcp compute instance metadata with partial zone/region in config. {pull}18757[18757]
 - Add missing network.sent_packets_count metric into compute metricset in googlecloud module. {pull}18802[18802]
 
 *Packetbeat*

diff --git a/x-pack/metricbeat/module/googlecloud/stackdriver/compute/metadata.go b/x-pack/metricbeat/module/googlecloud/stackdriver/compute/metadata.go
@@ -15,7 +15,7 @@ import (
 	monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3"
 
 	"github.com/elastic/beats/v7/libbeat/common"
-
+	"github.com/elastic/beats/v7/libbeat/logp"
 	"github.com/elastic/beats/v7/x-pack/metricbeat/module/googlecloud"
 )
 
@@ -27,6 +27,7 @@ func NewMetadataService(projectID, zone string, region string, opt ...option.Cli
 		region:        region,
 		opt:           opt,
 		instanceCache: common.NewCache(30*time.Second, 13),
+		logger:        logp.NewLogger("stackdriver-compute"),
 	}, nil
 }
 
@@ -55,12 +56,13 @@ type metadataCollector struct {
 	computeMetadata *computeMetadata
 
 	instanceCache *common.Cache
+	logger        *logp.Logger
 }
 
 // Metadata implements googlecloud.MetadataCollector to the known set of labels from a Compute TimeSeries single point of data.
 func (s *metadataCollector) Metadata(ctx context.Context, resp *monitoringpb.TimeSeries) (googlecloud.MetadataCollectorData, error) {
 	if s.computeMetadata == nil {
-		_, err := s.instanceMetadata(ctx, s.instanceID(resp), s.zone, s.region)
+		_, err := s.instanceMetadata(ctx, s.instanceID(resp), s.instanceZone(resp))
 		if err != nil {
 			return googlecloud.MetadataCollectorData{}, err
 		}
@@ -104,8 +106,8 @@ func (s *metadataCollector) Metadata(ctx context.Context, resp *monitoringpb.Tim
 }
 
 // instanceMetadata returns the labels of an instance
-func (s *metadataCollector) instanceMetadata(ctx context.Context, instanceID, zone string, region string) (*computeMetadata, error) {
-	i, err := s.instance(ctx, instanceID, zone, region)
+func (s *metadataCollector) instanceMetadata(ctx context.Context, instanceID, zone string) (*computeMetadata, error) {
+	i, err := s.instance(ctx, instanceID, zone)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error trying to get data from instance '%s' in zone '%s'", instanceID, zone)
 	}
@@ -139,7 +141,7 @@ func (s *metadataCollector) instanceMetadata(ctx context.Context, instanceID, zo
 }
 
 // instance returns data from an instance ID using the cache or making a request
-func (s *metadataCollector) instance(ctx context.Context, instanceID, zone string, region string) (*compute.Instance, error) {
+func (s *metadataCollector) instance(ctx context.Context, instanceID, zone string) (*compute.Instance, error) {
 	service, err := compute.NewService(ctx, s.opt...)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error getting client from Compute service")
@@ -152,29 +154,11 @@ func (s *metadataCollector) instance(ctx context.Context, instanceID, zone strin
 		}
 	}
 
-	if region != "" {
-		regionData, err := service.Regions.Get(s.projectID, region).Do()
-		if err != nil {
-			return nil, errors.Wrapf(err, "error getting region information for '%s'", region)
-		}
-
-		zones := regionData.Zones
-		for _, zone := range zones {
-			zString := strings.Split(zone, "/")
-			zName := zString[len(zString)-1]
-			instanceData, err := service.Instances.Get(s.projectID, zName, instanceID).Do()
-			if err != nil {
-				continue
-			}
-			s.instanceCache.Put(instanceID, instanceData)
-			return instanceData, nil
-		}
-	}
-
 	if zone != "" {
 		instanceData, err := service.Instances.Get(s.projectID, zone, instanceID).Do()
 		if err != nil {
-			return nil, errors.Wrapf(err, "error getting instance information for instance with ID '%s'", instanceID)
+			s.logger.Warnf("failed to get instance information for instance '%s' in zone '%s', skipping metadata for instance", instanceID, zone)
+			return nil, nil
 		}
 		s.instanceCache.Put(instanceID, instanceData)
 		return instanceData, nil
@@ -189,3 +173,11 @@ func (s *metadataCollector) instanceID(ts *monitoringpb.TimeSeries) string {
 
 	return ""
 }
+
+func (s *metadataCollector) instanceZone(ts *monitoringpb.TimeSeries) string {
+	if ts.Resource != nil && ts.Resource.Labels != nil {
+		return ts.Resource.Labels[googlecloud.TimeSeriesResponsePathForECSAvailabilityZone]
+	}
+
+	return ""
+}
diff --git a/x-pack/metricbeat/module/googlecloud/stackdriver/compute/metadata_test.go b/x-pack/metricbeat/module/googlecloud/stackdriver/compute/metadata_test.go
@@ -0,0 +1,82 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License;
+// you may not use this file except in compliance with the Elastic License.
+
+package compute
+
+import (
+	"testing"
+	"time"
+
+	"github.com/golang/protobuf/ptypes/timestamp"
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/genproto/googleapis/api/metric"
+	"google.golang.org/genproto/googleapis/api/monitoredres"
+	"google.golang.org/genproto/googleapis/monitoring/v3"
+
+	"github.com/elastic/beats/v7/libbeat/common"
+)
+
+var fake = &monitoring.TimeSeries{
+	Resource: &monitoredres.MonitoredResource{
+		Type: "gce_instance",
+		Labels: map[string]string{
+			"instance_id": "4624337448093162893",
+			"project_id":  "elastic-metricbeat",
+			"zone":        "us-central1-a",
+		},
+	},
+	Metadata: &monitoredres.MonitoredResourceMetadata{
+		UserLabels: map[string]string{
+			"user": "label",
+		},
+	},
+	Metric: &metric.Metric{
+		Labels: map[string]string{
+			"instance_name": "instance-1",
+		},
+		Type: "compute.googleapis.com/instance/cpu/usage_time",
+	},
+	MetricKind: metric.MetricDescriptor_GAUGE,
+	ValueType:  metric.MetricDescriptor_DOUBLE,
+	Points: []*monitoring.Point{{
+		Value: &monitoring.TypedValue{
+			Value: &monitoring.TypedValue_DoubleValue{DoubleValue: 0.0041224284852319215},
+		},
+		Interval: &monitoring.TimeInterval{
+			StartTime: &timestamp.Timestamp{
+				Seconds: 1569932700,
+			},
+			EndTime: &timestamp.Timestamp{
+				Seconds: 1569932700,
+			},
+		},
+	}, {
+		Value: &monitoring.TypedValue{
+			Value: &monitoring.TypedValue_DoubleValue{DoubleValue: 0.004205757571772513},
+		},
+		Interval: &monitoring.TimeInterval{
+			StartTime: &timestamp.Timestamp{
+				Seconds: 1569932640,
+			},
+			EndTime: &timestamp.Timestamp{
+				Seconds: 1569932640,
+			},
+		},
+	}},
+}
+
+var m = &metadataCollector{
+	projectID:     "projectID",
+	instanceCache: common.NewCache(30*time.Second, 13),
+}
+
+func TestInstanceID(t *testing.T) {
+	instanceID := m.instanceID(fake)
+	assert.Equal(t, "4624337448093162893", instanceID)
+}
+
+func TestInstanceZone(t *testing.T) {
+	zone := m.instanceZone(fake)
+	assert.Equal(t, "us-central1-a", zone)
+}
diff --git a/x-pack/metricbeat/module/googlecloud/stackdriver/response_parser_test.go b/x-pack/metricbeat/module/googlecloud/stackdriver/response_parser_test.go
@@ -7,68 +7,9 @@ package stackdriver
 import (
 	"testing"
 
-	"github.com/golang/protobuf/ptypes/timestamp"
 	"github.com/stretchr/testify/assert"
-	"google.golang.org/genproto/googleapis/api/metric"
-	"google.golang.org/genproto/googleapis/api/monitoredres"
-	"google.golang.org/genproto/googleapis/monitoring/v3"
 )
 
-var fake *monitoring.TimeSeries = &monitoring.TimeSeries{
-	Resource: &monitoredres.MonitoredResource{
-		Type: "gce_instance",
-		Labels: map[string]string{
-			"instance_id": "4624337448093162893",
-			"project_id":  "elastic-metricbeat",
-			"zone":        "us-central1-a",
-		},
-	},
-	Metadata: &monitoredres.MonitoredResourceMetadata{
-		UserLabels: map[string]string{
-			"user": "label",
-		},
-	},
-	Metric: &metric.Metric{
-		Labels: map[string]string{
-			"instance_name": "instance-1",
-		},
-		Type: "compute.googleapis.com/instance/cpu/usage_time",
-	},
-	MetricKind: metric.MetricDescriptor_GAUGE,
-	ValueType:  metric.MetricDescriptor_DOUBLE,
-	Points: []*monitoring.Point{{
-		Value: &monitoring.TypedValue{
-			Value: &monitoring.TypedValue_DoubleValue{DoubleValue: 0.0041224284852319215},
-		},
-		Interval: &monitoring.TimeInterval{
-			StartTime: &timestamp.Timestamp{
-				Seconds: 1569932700,
-			},
-			EndTime: &timestamp.Timestamp{
-				Seconds: 1569932700,
-			},
-		},
-	}, {
-		Value: &monitoring.TypedValue{
-			Value: &monitoring.TypedValue_DoubleValue{DoubleValue: 0.004205757571772513},
-		},
-		Interval: &monitoring.TimeInterval{
-			StartTime: &timestamp.Timestamp{
-				Seconds: 1569932640,
-			},
-			EndTime: &timestamp.Timestamp{
-				Seconds: 1569932640,
-			},
-		},
-	}},
-}
-
-var metrics = []string{
-	"compute.googleapis.com/instance/cpu/utilization",
-	"compute.googleapis.com/instance/disk/read_bytes_count",
-	"compute.googleapis.com/http/server/response_latencies",
-}
-
 func TestCleanMetricNameString(t *testing.T) {
 	cases := []struct {
 		title              string