From c606c497fb850d9246494920739e8f3b7cd1966f Mon Sep 17 00:00:00 2001 From: Gabriel Pop Date: Fri, 11 Nov 2022 19:08:22 +0200 Subject: [PATCH 1/6] remove compute metadata cache --- .../module/gcp/metrics/compute/metadata.go | 74 ++++++++++++------- .../gcp/metrics/compute/metadata_test.go | 7 +- 2 files changed, 50 insertions(+), 31 deletions(-) diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go index 4fd84ece5ee..d50c7b4aaf5 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go @@ -9,32 +9,33 @@ import ( "fmt" "strconv" "strings" - "time" "google.golang.org/api/compute/v1" "google.golang.org/api/option" monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3" - "github.com/elastic/beats/v7/libbeat/common" "github.com/elastic/beats/v7/x-pack/metricbeat/module/gcp" "github.com/elastic/elastic-agent-libs/logp" ) +/* const ( cacheTTL = 30 * time.Second initialCacheSize = 13 ) +*/ // NewMetadataService returns the specific Metadata service for a GCP Compute resource func NewMetadataService(projectID, zone string, region string, regions []string, opt ...option.ClientOption) (gcp.MetadataService, error) { return &metadataCollector{ - projectID: projectID, - zone: zone, - region: region, - regions: regions, - opt: opt, - instanceCache: common.NewCache(cacheTTL, initialCacheSize), - logger: logp.NewLogger("metrics-compute"), + projectID: projectID, + zone: zone, + region: region, + regions: regions, + opt: opt, + // instanceCache: common.NewCache(cacheTTL, initialCacheSize), + computeInstances: make(map[string]*compute.Instance), + logger: logp.NewLogger("metrics-compute"), }, nil } @@ -55,13 +56,14 @@ type computeMetadata struct { } type metadataCollector struct { - projectID string - zone string - region string - regions []string - opt []option.ClientOption - instanceCache *common.Cache - logger *logp.Logger + projectID string + zone string + region string + regions []string + opt []option.ClientOption + // instanceCache *common.Cache + computeInstances map[string]*compute.Instance + logger *logp.Logger } // Metadata implements googlecloud.MetadataCollector to the known set of labels from a Compute TimeSeries single point of data. @@ -144,14 +146,26 @@ 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 string) (*compute.Instance, error) { - s.refreshInstanceCache(ctx) - instanceCachedData := s.instanceCache.Get(instanceID) - if instanceCachedData != nil { - if computeInstance, ok := instanceCachedData.(*compute.Instance); ok { - return computeInstance, nil + /* + s.refreshInstanceCache(ctx) + instanceCachedData := s.instanceCache.Get(instanceID) + if instanceCachedData != nil { + if computeInstance, ok := instanceCachedData.(*compute.Instance); ok { + return computeInstance, nil + } } + */ + + s.getComputeInstances(ctx) + + computeInstance, ok := s.computeInstances[instanceID] + if ok { + return computeInstance, nil } + // Remake the compute instances map to avoid having stale data. + s.computeInstances = make(map[string]*compute.Instance) + return nil, nil } @@ -171,12 +185,19 @@ func (s *metadataCollector) instanceZone(ts *monitoringpb.TimeSeries) string { return "" } -func (s *metadataCollector) refreshInstanceCache(ctx context.Context) { - // only refresh cache if it is empty - if s.instanceCache.Size() > 0 { +func (s *metadataCollector) getComputeInstances(ctx context.Context) { + /* + // only refresh cache if it is empty + if s.instanceCache.Size() > 0 { + return + } + s.logger.Debugf("refresh cache with Instances.AggregatedList API") + */ + + if len(s.computeInstances) > 0 { return } - s.logger.Debugf("refresh cache with Instances.AggregatedList API") + computeService, err := compute.NewService(ctx, s.opt...) if err != nil { s.logger.Errorf("error getting client from Compute service: %v", err) @@ -187,7 +208,8 @@ func (s *metadataCollector) refreshInstanceCache(ctx context.Context) { if err := req.Pages(ctx, func(page *compute.InstanceAggregatedList) error { for _, instancesScopedList := range page.Items { for _, instance := range instancesScopedList.Instances { - s.instanceCache.Put(strconv.Itoa(int(instance.Id)), instance) + // s.instanceCache.Put(strconv.Itoa(int(instance.Id)), instance) + s.computeInstances[strconv.Itoa(int(instance.Id))] = instance } } return nil diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go index 53f1878addd..26e7b437960 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go @@ -6,15 +6,12 @@ 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{ @@ -67,8 +64,8 @@ var fake = &monitoring.TimeSeries{ } var m = &metadataCollector{ - projectID: "projectID", - instanceCache: common.NewCache(30*time.Second, 13), + projectID: "projectID", + // instanceCache: common.NewCache(30*time.Second, 13), } func TestInstanceID(t *testing.T) { From bad28caf0159668e7b3a823be5656e49c2ab779e Mon Sep 17 00:00:00 2001 From: Gabriel Pop Date: Tue, 15 Nov 2022 11:58:21 +0200 Subject: [PATCH 2/6] add logs and change map key type --- .../module/gcp/metrics/compute/metadata.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go index d50c7b4aaf5..afe8ab091dc 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go @@ -34,7 +34,7 @@ func NewMetadataService(projectID, zone string, region string, regions []string, regions: regions, opt: opt, // instanceCache: common.NewCache(cacheTTL, initialCacheSize), - computeInstances: make(map[string]*compute.Instance), + computeInstances: make(map[uint64]*compute.Instance), logger: logp.NewLogger("metrics-compute"), }, nil } @@ -62,7 +62,7 @@ type metadataCollector struct { regions []string opt []option.ClientOption // instanceCache *common.Cache - computeInstances map[string]*compute.Instance + computeInstances map[uint64]*compute.Instance logger *logp.Logger } @@ -122,6 +122,7 @@ func (s *metadataCollector) instanceMetadata(ctx context.Context, instanceID, zo } if instance == nil { + s.logger.Debugf("couldn't find instance %s, call Instances.AggregatedList", instanceID) return computeMetadata, nil } @@ -158,13 +159,14 @@ func (s *metadataCollector) instance(ctx context.Context, instanceID string) (*c s.getComputeInstances(ctx) - computeInstance, ok := s.computeInstances[instanceID] + instanceIdInt, _ := strconv.Atoi(instanceID) + computeInstance, ok := s.computeInstances[uint64(instanceIdInt)] if ok { return computeInstance, nil } // Remake the compute instances map to avoid having stale data. - s.computeInstances = make(map[string]*compute.Instance) + s.computeInstances = make(map[uint64]*compute.Instance) return nil, nil } @@ -198,6 +200,8 @@ func (s *metadataCollector) getComputeInstances(ctx context.Context) { return } + s.logger.Debug("Compute API Instances.AggregatedList") + computeService, err := compute.NewService(ctx, s.opt...) if err != nil { s.logger.Errorf("error getting client from Compute service: %v", err) @@ -209,7 +213,7 @@ func (s *metadataCollector) getComputeInstances(ctx context.Context) { for _, instancesScopedList := range page.Items { for _, instance := range instancesScopedList.Instances { // s.instanceCache.Put(strconv.Itoa(int(instance.Id)), instance) - s.computeInstances[strconv.Itoa(int(instance.Id))] = instance + s.computeInstances[instance.Id] = instance } } return nil From b38858bac6a83107873d39b6baf78b5e37dbb5b3 Mon Sep 17 00:00:00 2001 From: Gabriel Pop Date: Wed, 16 Nov 2022 14:39:46 +0200 Subject: [PATCH 3/6] add import aliases --- x-pack/metricbeat/module/gcp/metrics/compute/metadata.go | 2 +- x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go index afe8ab091dc..db9a244bd04 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go @@ -10,7 +10,7 @@ import ( "strconv" "strings" - "google.golang.org/api/compute/v1" + compute "google.golang.org/api/compute/v1" "google.golang.org/api/option" monitoringpb "google.golang.org/genproto/googleapis/monitoring/v3" diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go index 26e7b437960..9e835b3f837 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go @@ -11,7 +11,7 @@ import ( "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" + monitoring "google.golang.org/genproto/googleapis/monitoring/v3" ) var fake = &monitoring.TimeSeries{ From 2d181312c753ce3d891300fcd39629eb8c195e56 Mon Sep 17 00:00:00 2001 From: Gabriel Pop Date: Wed, 23 Nov 2022 22:36:24 +0200 Subject: [PATCH 4/6] add changelog entry --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 5b40f808c12..b377bc55139 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -186,6 +186,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff] - Update README file on how to run Metricbeat on Kubernetes. {pull}33308[33308] - Add per-thread metrics to system_summary {pull}33614[33614] - Add GCP CloudSQL metadata {pull}33066[33066] +- Remove GCP Compute metadata cache {pull}33655[33655] *Packetbeat* From d9f41924bef9d796ed1c73d4f20f7e661922814a Mon Sep 17 00:00:00 2001 From: Gabriel Pop Date: Thu, 1 Dec 2022 13:45:28 +0200 Subject: [PATCH 5/6] cleanup comments --- .../module/gcp/metrics/compute/metadata.go | 40 +++---------------- 1 file changed, 5 insertions(+), 35 deletions(-) diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go index db9a244bd04..969a5e98c49 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go @@ -18,22 +18,14 @@ import ( "github.com/elastic/elastic-agent-libs/logp" ) -/* -const ( - cacheTTL = 30 * time.Second - initialCacheSize = 13 -) -*/ - // NewMetadataService returns the specific Metadata service for a GCP Compute resource func NewMetadataService(projectID, zone string, region string, regions []string, opt ...option.ClientOption) (gcp.MetadataService, error) { return &metadataCollector{ - projectID: projectID, - zone: zone, - region: region, - regions: regions, - opt: opt, - // instanceCache: common.NewCache(cacheTTL, initialCacheSize), + projectID: projectID, + zone: zone, + region: region, + regions: regions, + opt: opt, computeInstances: make(map[uint64]*compute.Instance), logger: logp.NewLogger("metrics-compute"), }, nil @@ -42,13 +34,10 @@ func NewMetadataService(projectID, zone string, region string, regions []string, // computeMetadata is an object to store data in between the extraction and the writing in the destination (to uncouple // reading and writing in the same method) type computeMetadata struct { - // projectID string zone string instanceID string machineType string - // ts *monitoringpb.TimeSeries - User map[string]string Metadata map[string]string Metrics interface{} @@ -147,16 +136,6 @@ 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 string) (*compute.Instance, error) { - /* - s.refreshInstanceCache(ctx) - instanceCachedData := s.instanceCache.Get(instanceID) - if instanceCachedData != nil { - if computeInstance, ok := instanceCachedData.(*compute.Instance); ok { - return computeInstance, nil - } - } - */ - s.getComputeInstances(ctx) instanceIdInt, _ := strconv.Atoi(instanceID) @@ -188,14 +167,6 @@ func (s *metadataCollector) instanceZone(ts *monitoringpb.TimeSeries) string { } func (s *metadataCollector) getComputeInstances(ctx context.Context) { - /* - // only refresh cache if it is empty - if s.instanceCache.Size() > 0 { - return - } - s.logger.Debugf("refresh cache with Instances.AggregatedList API") - */ - if len(s.computeInstances) > 0 { return } @@ -212,7 +183,6 @@ func (s *metadataCollector) getComputeInstances(ctx context.Context) { if err := req.Pages(ctx, func(page *compute.InstanceAggregatedList) error { for _, instancesScopedList := range page.Items { for _, instance := range instancesScopedList.Instances { - // s.instanceCache.Put(strconv.Itoa(int(instance.Id)), instance) s.computeInstances[instance.Id] = instance } } From 23b6c18b6470773b47b3ae1f282b1ddd064ae788 Mon Sep 17 00:00:00 2001 From: Gabriel Pop Date: Thu, 1 Dec 2022 21:42:51 +0200 Subject: [PATCH 6/6] remove comments --- .../metricbeat/module/gcp/metrics/compute/metadata.go | 11 +++++------ .../module/gcp/metrics/compute/metadata_test.go | 1 - 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go index 969a5e98c49..700a667164c 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go @@ -45,12 +45,11 @@ type computeMetadata struct { } type metadataCollector struct { - projectID string - zone string - region string - regions []string - opt []option.ClientOption - // instanceCache *common.Cache + projectID string + zone string + region string + regions []string + opt []option.ClientOption computeInstances map[uint64]*compute.Instance logger *logp.Logger } diff --git a/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go b/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go index 9e835b3f837..cee4484f615 100644 --- a/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go +++ b/x-pack/metricbeat/module/gcp/metrics/compute/metadata_test.go @@ -65,7 +65,6 @@ var fake = &monitoring.TimeSeries{ var m = &metadataCollector{ projectID: "projectID", - // instanceCache: common.NewCache(30*time.Second, 13), } func TestInstanceID(t *testing.T) {