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

[metricbeat] [gcp] remove compute metadata cache #33655

Merged
merged 11 commits into from Dec 5, 2022
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Expand Up @@ -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*

Expand Down
80 changes: 53 additions & 27 deletions x-pack/metricbeat/module/gcp/metrics/compute/metadata.go
Expand Up @@ -9,32 +9,33 @@ import (
"fmt"
"strconv"
"strings"
"time"

"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"

"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 (
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove these const since we are not using it anymore? :)

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[uint64]*compute.Instance),
logger: logp.NewLogger("metrics-compute"),
}, nil
}

Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Can we get rid of this too?

computeInstances map[uint64]*compute.Instance
logger *logp.Logger
}

// Metadata implements googlecloud.MetadataCollector to the known set of labels from a Compute TimeSeries single point of data.
Expand Down Expand Up @@ -120,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
}

Expand All @@ -144,14 +147,27 @@ 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)

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[uint64]*compute.Instance)

return nil, nil
}

Expand All @@ -171,12 +187,21 @@ 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")

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)
Expand All @@ -187,7 +212,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[instance.Id] = instance
}
}
return nil
Expand Down
Expand Up @@ -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"
monitoring "google.golang.org/genproto/googleapis/monitoring/v3"
)

var fake = &monitoring.TimeSeries{
Expand Down Expand Up @@ -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),
Copy link
Member

Choose a reason for hiding this comment

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

Can we get rid of this too?

}

func TestInstanceID(t *testing.T) {
Expand Down