From a0812403da1b7cbb3c473651bf4b0ce543a13b82 Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Thu, 5 Dec 2024 19:53:00 +0530 Subject: [PATCH 1/8] Retrieve instance tags at the same time to reduce number of entities in compass experience --- extension/entitystore/ec2Info.go | 69 ---------------- extension/entitystore/ec2Info_test.go | 79 ------------------- extension/entitystore/extension.go | 10 ++- extension/entitystore/extension_test.go | 47 +++++++---- extension/entitystore/serviceprovider.go | 33 +++++++- extension/entitystore/serviceprovider_test.go | 78 +++++++++++++++++- plugins/processors/awsentity/processor.go | 16 +++- .../processors/awsentity/processor_test.go | 48 +++++++---- 8 files changed, 185 insertions(+), 195 deletions(-) diff --git a/extension/entitystore/ec2Info.go b/extension/entitystore/ec2Info.go index cfb2eccba8..4201edcace 100644 --- a/extension/entitystore/ec2Info.go +++ b/extension/entitystore/ec2Info.go @@ -6,14 +6,12 @@ package entitystore import ( "context" "errors" - "strings" "sync" "time" "go.uber.org/zap" "github.com/aws/amazon-cloudwatch-agent/internal/ec2metadataprovider" - "github.com/aws/amazon-cloudwatch-agent/plugins/processors/ec2tagger" ) const ( @@ -29,7 +27,6 @@ const ( type EC2Info struct { InstanceID string AccountID string - AutoScalingGroup string // region is used while making call to describeTags Ec2 API for AutoScalingGroup Region string @@ -48,9 +45,6 @@ func (ei *EC2Info) initEc2Info() { if err := ei.setInstanceIDAccountID(); err != nil { return } - if err := ei.setAutoScalingGroup(); err != nil { - return - } ei.logger.Debug("Finished initializing EC2Info") } @@ -66,12 +60,6 @@ func (ei *EC2Info) GetAccountID() string { return ei.AccountID } -func (ei *EC2Info) GetAutoScalingGroup() string { - ei.mutex.RLock() - defer ei.mutex.RUnlock() - return ei.AutoScalingGroup -} - func (ei *EC2Info) setInstanceIDAccountID() error { for { metadataDoc, err := ei.metadataProvider.Get(context.Background()) @@ -99,63 +87,6 @@ func (ei *EC2Info) setInstanceIDAccountID() error { } } -func (ei *EC2Info) setAutoScalingGroup() error { - retry := 0 - for { - var waitDuration time.Duration - if retry < len(ec2tagger.BackoffSleepArray) { - waitDuration = ec2tagger.BackoffSleepArray[retry] - } else { - waitDuration = ec2tagger.BackoffSleepArray[len(ec2tagger.BackoffSleepArray)-1] - } - - wait := time.NewTimer(waitDuration) - select { - case <-ei.done: - wait.Stop() - return errors.New("shutdown signal received") - case <-wait.C: - } - - if retry > 0 { - ei.logger.Debug("Initial retrieval of tags and volumes", zap.Int("retry", retry)) - } - - if err := ei.retrieveAsgName(); err != nil { - ei.logger.Debug("Unable to fetch instance tags with imds", zap.Int("retry", retry), zap.Error(err)) - } else { - ei.logger.Debug("Retrieval of auto-scaling group tags succeeded") - return nil - } - - retry++ - } - -} - -func (ei *EC2Info) retrieveAsgName() error { - tags, err := ei.metadataProvider.InstanceTags(context.Background()) - if err != nil { - ei.logger.Debug("Failed to get tags through metadata provider", zap.Error(err)) - return err - } else if strings.Contains(tags, ec2tagger.Ec2InstanceTagKeyASG) { - asg, err := ei.metadataProvider.InstanceTagValue(context.Background(), ec2tagger.Ec2InstanceTagKeyASG) - if err != nil { - ei.logger.Error("Failed to get AutoScalingGroup through metadata provider", zap.Error(err)) - } else { - ei.logger.Debug("AutoScalingGroup retrieved through IMDS") - ei.mutex.Lock() - ei.AutoScalingGroup = asg - if asgLength := len(ei.AutoScalingGroup); asgLength > autoScalingGroupSizeMax { - ei.logger.Warn("AutoScalingGroup length exceeds characters limit and will be ignored", zap.Int("length", asgLength), zap.Int("character limit", autoScalingGroupSizeMax)) - ei.AutoScalingGroup = "" - } - ei.mutex.Unlock() - } - } - return nil -} - func newEC2Info(metadataProvider ec2metadataprovider.MetadataProvider, done chan struct{}, region string, logger *zap.Logger) *EC2Info { return &EC2Info{ metadataProvider: metadataProvider, diff --git a/extension/entitystore/ec2Info_test.go b/extension/entitystore/ec2Info_test.go index 6602752c5a..816e56a40d 100644 --- a/extension/entitystore/ec2Info_test.go +++ b/extension/entitystore/ec2Info_test.go @@ -6,7 +6,6 @@ package entitystore import ( "bytes" "log" - "strings" "testing" "time" @@ -86,83 +85,6 @@ func TestSetInstanceIDAccountID(t *testing.T) { } } -func TestRetrieveASGName(t *testing.T) { - type args struct { - metadataProvider ec2metadataprovider.MetadataProvider - } - tests := []struct { - name string - args args - wantErr bool - want EC2Info - }{ - { - name: "happy path", - args: args{ - metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"aws:autoscaling:groupName": tagVal3}}, - }, - wantErr: false, - want: EC2Info{ - AutoScalingGroup: tagVal3, - }, - }, - { - name: "happy path with multiple tags", - args: args{ - metadataProvider: &mockMetadataProvider{ - InstanceIdentityDocument: mockedInstanceIdentityDoc, - Tags: map[string]string{ - "aws:autoscaling:groupName": tagVal3, - "env": "test-env", - "name": "test-name", - }}, - }, - - wantErr: false, - want: EC2Info{ - AutoScalingGroup: tagVal3, - }, - }, - { - name: "AutoScalingGroup too large", - args: args{ - metadataProvider: &mockMetadataProvider{ - InstanceIdentityDocument: mockedInstanceIdentityDoc, - Tags: map[string]string{ - "aws:autoscaling:groupName": strings.Repeat("a", 256), - "env": "test-env", - "name": "test-name", - }}, - }, - - wantErr: false, - want: EC2Info{ - AutoScalingGroup: "", - }, - }, - { - name: "Success IMDS tags call but no ASG", - args: args{ - metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"name": tagVal3}}, - }, - wantErr: false, - want: EC2Info{ - AutoScalingGroup: "", - }, - }, - } - for _, tt := range tests { - logger, _ := zap.NewDevelopment() - t.Run(tt.name, func(t *testing.T) { - ei := &EC2Info{metadataProvider: tt.args.metadataProvider, logger: logger} - if err := ei.retrieveAsgName(); (err != nil) != tt.wantErr { - t.Errorf("retrieveAsgName() error = %v, wantErr %v", err, tt.wantErr) - } - assert.Equal(t, tt.want.AutoScalingGroup, ei.GetAutoScalingGroup()) - }) - } -} - func TestLogMessageDoesNotIncludeResourceInfo(t *testing.T) { type args struct { metadataProvider ec2metadataprovider.MetadataProvider @@ -201,7 +123,6 @@ func TestLogMessageDoesNotIncludeResourceInfo(t *testing.T) { logOutput := buf.String() log.Println(logOutput) assert.NotContains(t, logOutput, ei.GetInstanceID()) - assert.NotContains(t, logOutput, ei.GetAutoScalingGroup()) }) } } diff --git a/extension/entitystore/extension.go b/extension/entitystore/extension.go index a6af693cb3..f27fd38400 100644 --- a/extension/entitystore/extension.go +++ b/extension/entitystore/extension.go @@ -43,6 +43,7 @@ type serviceProviderInterface interface { addEntryForLogGroup(LogGroupName, ServiceAttribute) logFileServiceAttribute(LogFileGlob, LogGroupName) ServiceAttribute getServiceNameAndSource() (string, string) + getAutoScalingGroup() string } type EntityStore struct { @@ -173,6 +174,13 @@ func (e *EntityStore) GetServiceMetricAttributesMap() map[string]*string { return e.createAttributeMap() } +func (e *EntityStore) GetAutoScalingGroup() string { + if e.serviceprovider == nil { + return "" + } + return e.serviceprovider.getAutoScalingGroup() +} + // AddServiceAttrEntryForLogFile adds an entry to the entity store for the provided file glob -> (serviceName, environmentName) key-value pair func (e *EntityStore) AddServiceAttrEntryForLogFile(fileGlob LogFileGlob, serviceName string, environmentName string) { if e.serviceprovider != nil { @@ -221,7 +229,7 @@ func (e *EntityStore) createAttributeMap() map[string]*string { if e.mode == config.ModeEC2 { addNonEmptyToMap(attributeMap, InstanceIDKey, e.ec2Info.GetInstanceID()) - addNonEmptyToMap(attributeMap, ASGKey, e.ec2Info.GetAutoScalingGroup()) + addNonEmptyToMap(attributeMap, ASGKey, e.GetAutoScalingGroup()) } switch e.mode { case config.ModeEC2: diff --git a/extension/entitystore/extension_test.go b/extension/entitystore/extension_test.go index 5662cf1d88..8fe95d9b16 100644 --- a/extension/entitystore/extension_test.go +++ b/extension/entitystore/extension_test.go @@ -63,6 +63,11 @@ func (s *mockServiceProvider) getServiceNameAndSource() (string, string) { return "test-service-name", "UserConfiguration" } +func (s *mockServiceProvider) getAutoScalingGroup() string { + args := s.Called() + return args.Get(0).(string) +} + type mockMetadataProvider struct { InstanceIdentityDocument *ec2metadata.EC2InstanceIdentityDocument Tags map[string]string @@ -135,12 +140,10 @@ func TestEntityStore_EC2Info(t *testing.T) { { name: "happypath", ec2InfoInput: EC2Info{ - InstanceID: "i-1234567890", - AutoScalingGroup: "test-asg", + InstanceID: "i-1234567890", }, want: EC2Info{ - InstanceID: "i-1234567890", - AutoScalingGroup: "test-asg", + InstanceID: "i-1234567890", }, }, } @@ -200,8 +203,9 @@ func TestEntityStore_KubernetesMode(t *testing.T) { func TestEntityStore_createAttributeMaps(t *testing.T) { type fields struct { - ec2Info EC2Info - mode string + ec2Info EC2Info + mode string + emptyASG bool } tests := []struct { name string @@ -212,13 +216,12 @@ func TestEntityStore_createAttributeMaps(t *testing.T) { name: "HappyPath", fields: fields{ ec2Info: EC2Info{ - InstanceID: "i-123456789", - AutoScalingGroup: "test-asg", + InstanceID: "i-123456789", }, mode: config.ModeEC2, }, want: map[string]*string{ - ASGKey: aws.String("test-asg"), + ASGKey: aws.String("ASG-1"), InstanceIDKey: aws.String("i-123456789"), PlatformType: aws.String(EC2PlatForm), }, @@ -229,7 +232,8 @@ func TestEntityStore_createAttributeMaps(t *testing.T) { ec2Info: EC2Info{ InstanceID: "i-123456789", }, - mode: config.ModeEC2, + mode: config.ModeEC2, + emptyASG: true, }, want: map[string]*string{ InstanceIDKey: aws.String("i-123456789"), @@ -239,7 +243,8 @@ func TestEntityStore_createAttributeMaps(t *testing.T) { { name: "HappyPath_InstanceIdAndAsgMissing", fields: fields{ - mode: config.ModeEC2, + mode: config.ModeEC2, + emptyASG: true, }, want: map[string]*string{ PlatformType: aws.String(EC2PlatForm), @@ -249,8 +254,7 @@ func TestEntityStore_createAttributeMaps(t *testing.T) { name: "NonEC2", fields: fields{ ec2Info: EC2Info{ - InstanceID: "i-123456789", - AutoScalingGroup: "test-asg", + InstanceID: "i-123456789", }, mode: config.ModeOnPrem, }, @@ -263,6 +267,13 @@ func TestEntityStore_createAttributeMaps(t *testing.T) { ec2Info: tt.fields.ec2Info, mode: tt.fields.mode, } + sp := new(mockServiceProvider) + if tt.fields.emptyASG { + sp.On("getAutoScalingGroup").Return("") + } else { + sp.On("getAutoScalingGroup").Return("ASG-1") + } + e.serviceprovider = sp assert.Equalf(t, dereferenceMap(tt.want), dereferenceMap(e.createAttributeMap()), "createAttributeMap()") }) } @@ -320,6 +331,7 @@ func TestEntityStore_createLogFileRID(t *testing.T) { } sp := new(mockServiceProvider) sp.On("logFileServiceAttribute", glob, group).Return(serviceAttr) + sp.On("getAutoScalingGroup").Return("ASG-1") e := EntityStore{ mode: config.ModeEC2, ec2Info: EC2Info{InstanceID: instanceId, AccountID: accountId}, @@ -337,9 +349,10 @@ func TestEntityStore_createLogFileRID(t *testing.T) { entityattributes.AwsAccountId: aws.String(accountId), }, Attributes: map[string]*string{ - InstanceIDKey: aws.String(instanceId), - ServiceNameSourceKey: aws.String(ServiceNameSourceUserConfiguration), - PlatformType: aws.String(EC2PlatForm), + InstanceIDKey: aws.String(instanceId), + ServiceNameSourceKey: aws.String(ServiceNameSourceUserConfiguration), + PlatformType: aws.String(EC2PlatForm), + entityattributes.AutoscalingGroup: aws.String("ASG-1"), }, } assert.Equal(t, dereferenceMap(expectedEntity.KeyAttributes), dereferenceMap(entity.KeyAttributes)) @@ -619,7 +632,7 @@ func TestEntityStore_LogMessageDoesNotIncludeResourceInfo(t *testing.T) { logOutput := buf.String() log.Println(logOutput) assertIfNonEmpty(t, logOutput, es.ec2Info.GetInstanceID()) - assertIfNonEmpty(t, logOutput, es.ec2Info.GetAutoScalingGroup()) + assertIfNonEmpty(t, logOutput, es.GetAutoScalingGroup()) assertIfNonEmpty(t, logOutput, es.ec2Info.GetAccountID()) assert.True(t, es.ready.Load(), "EntityStore should be ready") }) diff --git a/extension/entitystore/serviceprovider.go b/extension/entitystore/serviceprovider.go index c65a0daf62..9c19ddd88f 100644 --- a/extension/entitystore/serviceprovider.go +++ b/extension/entitystore/serviceprovider.go @@ -59,6 +59,7 @@ type serviceprovider struct { metadataProvider ec2metadataprovider.MetadataProvider iamRole string imdsServiceName string + autoScalingGroup string region string done chan struct{} logger *zap.Logger @@ -81,7 +82,7 @@ func (s *serviceprovider) startServiceProvider() { unlimitedRetryer := NewRetryer(false, true, defaultJitterMin, defaultJitterMax, ec2tagger.BackoffSleepArray, infRetry, s.done, s.logger) limitedRetryer := NewRetryer(false, true, describeTagsJitterMin, describeTagsJitterMax, ec2tagger.ThrottleBackOffArray, maxRetry, s.done, s.logger) go unlimitedRetryer.refreshLoop(s.scrapeIAMRole) - go limitedRetryer.refreshLoop(s.scrapeImdsServiceName) + go limitedRetryer.refreshLoop(s.scrapeImdsServiceNameAndASG) } func (s *serviceprovider) GetIAMRole() string { @@ -96,6 +97,12 @@ func (s *serviceprovider) GetIMDSServiceName() string { return s.imdsServiceName } +func (s *serviceprovider) getAutoScalingGroup() string { + s.mutex.RLock() + defer s.mutex.RUnlock() + return s.autoScalingGroup +} + // addEntryForLogFile adds an association between a log file glob and a service attribute, as configured in the // CloudWatch Agent config. func (s *serviceprovider) addEntryForLogFile(logFileGlob LogFileGlob, serviceAttr ServiceAttribute) { @@ -206,12 +213,12 @@ func (s *serviceprovider) serviceAttributeFromIamRole() ServiceAttribute { } func (s *serviceprovider) serviceAttributeFromAsg() ServiceAttribute { - if s.ec2Info == nil || s.ec2Info.GetAutoScalingGroup() == "" { + if s.getAutoScalingGroup() == "" { return ServiceAttribute{} } return ServiceAttribute{ - Environment: "ec2:" + s.ec2Info.GetAutoScalingGroup(), + Environment: "ec2:" + s.autoScalingGroup, } } @@ -237,7 +244,7 @@ func (s *serviceprovider) scrapeIAMRole() error { s.mutex.Unlock() return nil } -func (s *serviceprovider) scrapeImdsServiceName() error { +func (s *serviceprovider) scrapeImdsServiceNameAndASG() error { tags, err := s.metadataProvider.InstanceTags(context.Background()) if err != nil { s.logger.Debug("Failed to get tags through metadata provider", zap.Error(err)) @@ -257,9 +264,27 @@ func (s *serviceprovider) scrapeImdsServiceName() error { break } } + if strings.Contains(tags, ec2tagger.Ec2InstanceTagKeyASG) { + asg, err := s.metadataProvider.InstanceTagValue(context.Background(), ec2tagger.Ec2InstanceTagKeyASG) + if err != nil { + s.logger.Error("Failed to get AutoScalingGroup through metadata provider", zap.Error(err)) + } else { + s.logger.Debug("AutoScalingGroup retrieved through IMDS") + s.mutex.Lock() + s.autoScalingGroup = asg + if asgLength := len(s.autoScalingGroup); asgLength > autoScalingGroupSizeMax { + s.logger.Warn("AutoScalingGroup length exceeds characters limit and will be ignored", zap.Int("length", asgLength), zap.Int("character limit", autoScalingGroupSizeMax)) + s.autoScalingGroup = "" + } + s.mutex.Unlock() + } + } if s.GetIMDSServiceName() == "" { s.logger.Debug("Service name not found through IMDS") } + if s.getAutoScalingGroup() == "" { + s.logger.Debug("AutoScalingGroup name not found through IMDS") + } return nil } diff --git a/extension/entitystore/serviceprovider_test.go b/extension/entitystore/serviceprovider_test.go index c049c03305..f8072ac191 100644 --- a/extension/entitystore/serviceprovider_test.go +++ b/extension/entitystore/serviceprovider_test.go @@ -4,6 +4,7 @@ package entitystore import ( + "strings" "testing" "time" @@ -202,10 +203,10 @@ func Test_serviceprovider_serviceAttributeFromAsg(t *testing.T) { s := &serviceprovider{} assert.Equal(t, ServiceAttribute{}, s.serviceAttributeFromAsg()) - s = &serviceprovider{ec2Info: &EC2Info{}} + s = &serviceprovider{autoScalingGroup: ""} assert.Equal(t, ServiceAttribute{}, s.serviceAttributeFromAsg()) - s = &serviceprovider{ec2Info: &EC2Info{AutoScalingGroup: "test-asg"}} + s = &serviceprovider{autoScalingGroup: "test-asg"} assert.Equal(t, ServiceAttribute{Environment: "ec2:test-asg"}, s.serviceAttributeFromAsg()) } @@ -230,7 +231,7 @@ func Test_serviceprovider_logFileServiceAttribute(t *testing.T) { assert.Equal(t, ServiceAttribute{ServiceName: ServiceNameUnknown, ServiceNameSource: ServiceNameSourceUnknown, Environment: "ec2:default"}, s.logFileServiceAttribute("glob", "group")) - s.ec2Info = &EC2Info{AutoScalingGroup: "test-asg"} + s.autoScalingGroup = "test-asg" assert.Equal(t, ServiceAttribute{ServiceName: ServiceNameUnknown, ServiceNameSource: ServiceNameSourceUnknown, Environment: "ec2:test-asg"}, s.logFileServiceAttribute("glob", "group")) s.iamRole = "test-role" @@ -336,8 +337,77 @@ func Test_serviceprovider_getImdsServiceName(t *testing.T) { logger: zap.NewExample(), metadataProvider: tt.metadataProvider, } - s.scrapeImdsServiceName() + s.scrapeImdsServiceNameAndASG() assert.Equal(t, tt.wantTagServiceName, s.GetIMDSServiceName()) }) } } + +func TestRetrieveASGName(t *testing.T) { + type args struct { + metadataProvider ec2metadataprovider.MetadataProvider + } + tests := []struct { + name string + args args + wantErr bool + want string + }{ + { + name: "happy path", + args: args{ + metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"aws:autoscaling:groupName": tagVal3}}, + }, + wantErr: false, + want: tagVal3, + }, + { + name: "happy path with multiple tags", + args: args{ + metadataProvider: &mockMetadataProvider{ + InstanceIdentityDocument: mockedInstanceIdentityDoc, + Tags: map[string]string{ + "aws:autoscaling:groupName": tagVal3, + "env": "test-env", + "name": "test-name", + }}, + }, + + wantErr: false, + want: tagVal3, + }, + { + name: "AutoScalingGroup too large", + args: args{ + metadataProvider: &mockMetadataProvider{ + InstanceIdentityDocument: mockedInstanceIdentityDoc, + Tags: map[string]string{ + "aws:autoscaling:groupName": strings.Repeat("a", 256), + "env": "test-env", + "name": "test-name", + }}, + }, + + wantErr: false, + want: "", + }, + { + name: "Success IMDS tags call but no ASG", + args: args{ + metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"name": tagVal3}}, + }, + wantErr: false, + want: "", + }, + } + for _, tt := range tests { + logger, _ := zap.NewDevelopment() + t.Run(tt.name, func(t *testing.T) { + sp := &serviceprovider{metadataProvider: tt.args.metadataProvider, logger: logger} + if err := sp.scrapeImdsServiceNameAndASG(); (err != nil) != tt.wantErr { + t.Errorf("retrieveAsgName() error = %v, wantErr %v", err, tt.wantErr) + } + assert.Equal(t, tt.want, sp.getAutoScalingGroup()) + }) + } +} diff --git a/plugins/processors/awsentity/processor.go b/plugins/processors/awsentity/processor.go index 7492050cf7..5a5d9e08c3 100644 --- a/plugins/processors/awsentity/processor.go +++ b/plugins/processors/awsentity/processor.go @@ -76,6 +76,15 @@ var getEC2InfoFromEntityStore = func() entitystore.EC2Info { return es.EC2Info() } +var getAutoScalingGroupFromEntityStore = func() string { + // Get the following metric attributes from the EntityStore: EC2.AutoScalingGroup + es := entitystore.GetEntityStore() + if es == nil { + return "" + } + return es.GetAutoScalingGroup() +} + var getServiceNameSource = func() (string, string) { es := entitystore.GetEntityStore() if es == nil { @@ -217,11 +226,10 @@ func (p *awsEntityProcessor) processMetrics(_ context.Context, md pmetric.Metric } entityPlatformType = entityattributes.AttributeEntityEC2Platform - ec2Info = getEC2InfoFromEntityStore() if entityEnvironmentName == EMPTY { - if ec2Info.GetAutoScalingGroup() != EMPTY { - entityEnvironmentName = entityattributes.DeploymentEnvironmentFallbackPrefix + ec2Info.GetAutoScalingGroup() + if getAutoScalingGroupFromEntityStore() != EMPTY { + entityEnvironmentName = entityattributes.DeploymentEnvironmentFallbackPrefix + getAutoScalingGroupFromEntityStore() } else { entityEnvironmentName = entityattributes.DeploymentEnvironmentDefault } @@ -234,7 +242,7 @@ func (p *awsEntityProcessor) processMetrics(_ context.Context, md pmetric.Metric ec2Attributes := EC2ServiceAttributes{ InstanceId: ec2Info.GetInstanceID(), - AutoScalingGroup: ec2Info.GetAutoScalingGroup(), + AutoScalingGroup: getAutoScalingGroupFromEntityStore(), ServiceNameSource: entityServiceNameSource, } if err := validate.Struct(ec2Attributes); err == nil { diff --git a/plugins/processors/awsentity/processor_test.go b/plugins/processors/awsentity/processor_test.go index bd263e0a4f..605db91b8f 100644 --- a/plugins/processors/awsentity/processor_test.go +++ b/plugins/processors/awsentity/processor_test.go @@ -73,16 +73,21 @@ func newMockGetServiceNameAndSource(service, source string) func() (string, stri } } -func newMockGetEC2InfoFromEntityStore(instance, accountId, asg string) func() entitystore.EC2Info { +func newMockGetEC2InfoFromEntityStore(instance, accountId string) func() entitystore.EC2Info { return func() entitystore.EC2Info { return entitystore.EC2Info{ - InstanceID: instance, - AccountID: accountId, - AutoScalingGroup: asg, + InstanceID: instance, + AccountID: accountId, } } } +func newMockGetAutoScalingGroupFromEntityStore(asg string) func() string { + return func() string { + return asg + } +} + // This helper function creates a test logger // so that it can send the log messages into a // temporary buffer for pattern matching @@ -289,6 +294,7 @@ func TestProcessMetricsResourceAttributeScraping(t *testing.T) { metrics pmetric.Metrics mockServiceNameSource func() (string, string) mockGetEC2InfoFromEntityStore func() entitystore.EC2Info + mockGetAutoScalingGroup func() string want map[string]any }{ { @@ -303,7 +309,8 @@ func TestProcessMetricsResourceAttributeScraping(t *testing.T) { platform: config.ModeEC2, metrics: generateMetrics(attributeServiceName, "test-service"), mockServiceNameSource: newMockGetServiceNameAndSource("test-service-name", "Instrumentation"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -320,7 +327,8 @@ func TestProcessMetricsResourceAttributeScraping(t *testing.T) { platform: config.ModeEC2, metrics: generateMetrics(attributeDeploymentEnvironment, "test-environment"), mockServiceNameSource: newMockGetServiceNameAndSource("unknown_service", "Unknown"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "unknown_service", @@ -338,7 +346,8 @@ func TestProcessMetricsResourceAttributeScraping(t *testing.T) { platform: config.ModeEC2, metrics: generateMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment"), mockServiceNameSource: newMockGetServiceNameAndSource("test-service-name", "Instrumentation"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", "test-auto-scaling"), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore("test-auto-scaling"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -377,7 +386,8 @@ func TestProcessMetricsResourceAttributeScraping(t *testing.T) { platform: config.ModeEC2, metrics: generateMetrics(), mockServiceNameSource: newMockGetServiceNameAndSource("unknown_service", "Unknown"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", "test-asg"), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore("test-asg"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "unknown_service", @@ -394,7 +404,8 @@ func TestProcessMetricsResourceAttributeScraping(t *testing.T) { platform: config.ModeEC2, metrics: generateMetrics(), mockServiceNameSource: newMockGetServiceNameAndSource("unknown_service", "Unknown"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "unknown_service", @@ -417,6 +428,9 @@ func TestProcessMetricsResourceAttributeScraping(t *testing.T) { if tt.mockGetEC2InfoFromEntityStore != nil { getEC2InfoFromEntityStore = tt.mockGetEC2InfoFromEntityStore } + if tt.mockGetAutoScalingGroup != nil { + getAutoScalingGroupFromEntityStore = tt.mockGetAutoScalingGroup + } p := newAwsEntityProcessor(&Config{EntityType: attributeService, ClusterName: tt.clusterName}, logger) p.config.Platform = tt.platform p.config.KubernetesMode = tt.kubernetesMode @@ -470,7 +484,7 @@ func TestProcessMetricsResourceEntityProcessing(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - getEC2InfoFromEntityStore = newMockGetEC2InfoFromEntityStore(tt.instance, tt.accountId, tt.asg) + getEC2InfoFromEntityStore = newMockGetEC2InfoFromEntityStore(tt.instance, tt.accountId) p := newAwsEntityProcessor(&Config{EntityType: entityattributes.Resource}, logger) p.config.Platform = config.ModeEC2 _, err := p.processMetrics(ctx, tt.metrics) @@ -537,7 +551,7 @@ func TestAWSEntityProcessorNoSensitiveInfoInLogs(t *testing.T) { resetGetEC2InfoFromEntityStore := getEC2InfoFromEntityStore asgName := "test-asg" - getEC2InfoFromEntityStore = newMockGetEC2InfoFromEntityStore("i-1234567890abcdef0", "123456789012", asgName) + getEC2InfoFromEntityStore = newMockGetEC2InfoFromEntityStore("i-1234567890abcdef0", "123456789012") defer func() { getEC2InfoFromEntityStore = resetGetEC2InfoFromEntityStore }() md := generateTestMetrics() @@ -624,7 +638,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { { name: "DatapointAttributeServiceNameOnly", metrics: generateDatapointMetrics(attributeServiceName, "test-service"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", "auto-scaling"), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -640,7 +654,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { name: "DatapointAttributeEnvironmentOnly", metrics: generateDatapointMetrics(attributeDeploymentEnvironment, "test-environment"), mockServiceNameAndSource: newMockGetServiceNameAndSource("test-service-name", "ClientIamRole"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service-name", @@ -654,7 +668,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { { name: "DatapointAttributeServiceNameAndEnvironment", metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment"), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -669,7 +683,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { name: "DatapointAttributeServiceAndEnvironmentNameUserConfiguration", checkDatapointAttributeRemoval: true, metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment", entityattributes.AttributeServiceNameSource, entityattributes.AttributeServiceNameSourceUserConfig, entityattributes.AttributeDeploymentEnvironmentSource, entityattributes.AttributeServiceNameSourceUserConfig), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -685,7 +699,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { name: "DatapointAttributeServiceNameUserConfigurationAndUserEnvironment", checkDatapointAttributeRemoval: true, metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment", entityattributes.AttributeServiceNameSource, entityattributes.AttributeServiceNameSourceUserConfig), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -703,7 +717,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { name: "DatapointAttributeEnvironmentNameUserConfigurationAndUserServiceName", checkDatapointAttributeRemoval: true, metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment", entityattributes.AttributeDeploymentEnvironmentSource, entityattributes.AttributeServiceNameSourceUserConfig), - mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012", ""), + mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", From c8244ffeafdf5bcae584b2ba0846a11824e8b322 Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Thu, 5 Dec 2024 20:53:30 +0530 Subject: [PATCH 2/8] updated unit tests --- plugins/processors/awsentity/processor.go | 1 + plugins/processors/awsentity/processor_test.go | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/plugins/processors/awsentity/processor.go b/plugins/processors/awsentity/processor.go index 5a5d9e08c3..bd1a63b079 100644 --- a/plugins/processors/awsentity/processor.go +++ b/plugins/processors/awsentity/processor.go @@ -226,6 +226,7 @@ func (p *awsEntityProcessor) processMetrics(_ context.Context, md pmetric.Metric } entityPlatformType = entityattributes.AttributeEntityEC2Platform + ec2Info = getEC2InfoFromEntityStore() if entityEnvironmentName == EMPTY { if getAutoScalingGroupFromEntityStore() != EMPTY { diff --git a/plugins/processors/awsentity/processor_test.go b/plugins/processors/awsentity/processor_test.go index 605db91b8f..0e1eb7584d 100644 --- a/plugins/processors/awsentity/processor_test.go +++ b/plugins/processors/awsentity/processor_test.go @@ -627,6 +627,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { metrics pmetric.Metrics mockServiceNameAndSource func() (string, string) mockGetEC2InfoFromEntityStore func() entitystore.EC2Info + mockGetAutoScalingGroup func() string want map[string]any wantDatapointAttributes map[string]any }{ @@ -639,6 +640,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { name: "DatapointAttributeServiceNameOnly", metrics: generateDatapointMetrics(attributeServiceName, "test-service"), mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore("auto-scaling"), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -655,6 +657,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { metrics: generateDatapointMetrics(attributeDeploymentEnvironment, "test-environment"), mockServiceNameAndSource: newMockGetServiceNameAndSource("test-service-name", "ClientIamRole"), mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service-name", @@ -669,6 +672,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { name: "DatapointAttributeServiceNameAndEnvironment", metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment"), mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -684,6 +688,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { checkDatapointAttributeRemoval: true, metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment", entityattributes.AttributeServiceNameSource, entityattributes.AttributeServiceNameSourceUserConfig, entityattributes.AttributeDeploymentEnvironmentSource, entityattributes.AttributeServiceNameSourceUserConfig), mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -700,6 +705,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { checkDatapointAttributeRemoval: true, metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment", entityattributes.AttributeServiceNameSource, entityattributes.AttributeServiceNameSourceUserConfig), mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -718,6 +724,7 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { checkDatapointAttributeRemoval: true, metrics: generateDatapointMetrics(attributeServiceName, "test-service", attributeDeploymentEnvironment, "test-environment", entityattributes.AttributeDeploymentEnvironmentSource, entityattributes.AttributeServiceNameSourceUserConfig), mockGetEC2InfoFromEntityStore: newMockGetEC2InfoFromEntityStore("i-123456789", "0123456789012"), + mockGetAutoScalingGroup: newMockGetAutoScalingGroupFromEntityStore(""), want: map[string]any{ entityattributes.AttributeEntityType: "Service", entityattributes.AttributeEntityServiceName: "test-service", @@ -743,6 +750,9 @@ func TestProcessMetricsDatapointAttributeScraping(t *testing.T) { if tt.mockGetEC2InfoFromEntityStore != nil { getEC2InfoFromEntityStore = tt.mockGetEC2InfoFromEntityStore } + if tt.mockGetAutoScalingGroup != nil { + getAutoScalingGroupFromEntityStore = tt.mockGetAutoScalingGroup + } p := newAwsEntityProcessor(&Config{ScrapeDatapointAttribute: true, EntityType: attributeService}, logger) p.config.Platform = config.ModeEC2 _, err := p.processMetrics(ctx, tt.metrics) From 54209144fdfd438fdda036bc777705d50763480d Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Mon, 9 Dec 2024 04:43:46 -0500 Subject: [PATCH 3/8] refactored unit tests --- extension/entitystore/ec2Info.go | 4 +- extension/entitystore/serviceprovider.go | 2 +- extension/entitystore/serviceprovider_test.go | 101 +++++++----------- 3 files changed, 39 insertions(+), 68 deletions(-) diff --git a/extension/entitystore/ec2Info.go b/extension/entitystore/ec2Info.go index 4201edcace..23d13eb0b1 100644 --- a/extension/entitystore/ec2Info.go +++ b/extension/entitystore/ec2Info.go @@ -25,8 +25,8 @@ const ( ) type EC2Info struct { - InstanceID string - AccountID string + InstanceID string + AccountID string // region is used while making call to describeTags Ec2 API for AutoScalingGroup Region string diff --git a/extension/entitystore/serviceprovider.go b/extension/entitystore/serviceprovider.go index 9c19ddd88f..1a1e85e0d2 100644 --- a/extension/entitystore/serviceprovider.go +++ b/extension/entitystore/serviceprovider.go @@ -59,7 +59,7 @@ type serviceprovider struct { metadataProvider ec2metadataprovider.MetadataProvider iamRole string imdsServiceName string - autoScalingGroup string + autoScalingGroup string region string done chan struct{} logger *zap.Logger diff --git a/extension/entitystore/serviceprovider_test.go b/extension/entitystore/serviceprovider_test.go index f8072ac191..486a73689a 100644 --- a/extension/entitystore/serviceprovider_test.go +++ b/extension/entitystore/serviceprovider_test.go @@ -293,12 +293,13 @@ func Test_serviceprovider_getIAMRole(t *testing.T) { } } -func Test_serviceprovider_getImdsServiceName(t *testing.T) { +func Test_serviceprovider_scrapeAndgetImdsServiceNameAndASG(t *testing.T) { tests := []struct { name string metadataProvider ec2metadataprovider.MetadataProvider wantTagServiceName string + wantASGName string }{ { name: "HappyPath_ServiceExists", @@ -330,84 +331,54 @@ func Test_serviceprovider_getImdsServiceName(t *testing.T) { metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"service": "test-service", "application": "test-application", "app": "test-app"}}, wantTagServiceName: "test-service", }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - s := &serviceprovider{ - logger: zap.NewExample(), - metadataProvider: tt.metadataProvider, - } - s.scrapeImdsServiceNameAndASG() - assert.Equal(t, tt.wantTagServiceName, s.GetIMDSServiceName()) - }) - } -} - -func TestRetrieveASGName(t *testing.T) { - type args struct { - metadataProvider ec2metadataprovider.MetadataProvider - } - tests := []struct { - name string - args args - wantErr bool - want string - }{ { - name: "happy path", - args: args{ - metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"aws:autoscaling:groupName": tagVal3}}, - }, - wantErr: false, - want: tagVal3, + name: "happy path with Only ASG tag", + metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"aws:autoscaling:groupName": tagVal3}}, + wantASGName: tagVal3, }, { name: "happy path with multiple tags", - args: args{ - metadataProvider: &mockMetadataProvider{ - InstanceIdentityDocument: mockedInstanceIdentityDoc, - Tags: map[string]string{ - "aws:autoscaling:groupName": tagVal3, - "env": "test-env", - "name": "test-name", - }}, - }, - - wantErr: false, - want: tagVal3, + metadataProvider: &mockMetadataProvider{ + InstanceIdentityDocument: mockedInstanceIdentityDoc, + Tags: map[string]string{ + "aws:autoscaling:groupName": tagVal3, + "env": "test-env", + "name": "test-name", + }}, + wantASGName: tagVal3, }, { name: "AutoScalingGroup too large", - args: args{ - metadataProvider: &mockMetadataProvider{ - InstanceIdentityDocument: mockedInstanceIdentityDoc, - Tags: map[string]string{ - "aws:autoscaling:groupName": strings.Repeat("a", 256), - "env": "test-env", - "name": "test-name", - }}, - }, - - wantErr: false, - want: "", + metadataProvider: &mockMetadataProvider{ + InstanceIdentityDocument: mockedInstanceIdentityDoc, + Tags: map[string]string{ + "aws:autoscaling:groupName": strings.Repeat("a", 256), + "env": "test-env", + "name": "test-name", + }}, + wantASGName: "", }, { - name: "Success IMDS tags call but no ASG", - args: args{ - metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"name": tagVal3}}, - }, - wantErr: false, - want: "", + name: "Success IMDS tags call with no ASG", + metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"name": tagVal3}}, + wantASGName: "", + }, + { + name: "Success IMDS tags call with both Service and ASG", + metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"aws:autoscaling:groupName": tagVal3, "service": "test-service", "application": "test-application", "app": "test-app"}}, + wantTagServiceName: "test-service", + wantASGName: tagVal3, }, } for _, tt := range tests { - logger, _ := zap.NewDevelopment() t.Run(tt.name, func(t *testing.T) { - sp := &serviceprovider{metadataProvider: tt.args.metadataProvider, logger: logger} - if err := sp.scrapeImdsServiceNameAndASG(); (err != nil) != tt.wantErr { - t.Errorf("retrieveAsgName() error = %v, wantErr %v", err, tt.wantErr) + s := &serviceprovider{ + logger: zap.NewExample(), + metadataProvider: tt.metadataProvider, } - assert.Equal(t, tt.want, sp.getAutoScalingGroup()) + s.scrapeImdsServiceNameAndASG() + assert.Equal(t, tt.wantTagServiceName, s.GetIMDSServiceName()) + assert.Equal(t, tt.wantASGName, s.getAutoScalingGroup()) }) } } From 0aba00c706eda5f84768ac9cc196b566dc7d66f6 Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Mon, 9 Dec 2024 10:10:26 -0500 Subject: [PATCH 4/8] removed check to retrieve ASG when not in EKS mode --- extension/entitystore/ec2Info.go | 5 +--- extension/entitystore/ec2Info_test.go | 33 --------------------------- extension/entitystore/extension.go | 2 +- 3 files changed, 2 insertions(+), 38 deletions(-) diff --git a/extension/entitystore/ec2Info.go b/extension/entitystore/ec2Info.go index 0fdb8064cc..fee5e2ca09 100644 --- a/extension/entitystore/ec2Info.go +++ b/extension/entitystore/ec2Info.go @@ -12,7 +12,6 @@ import ( "go.uber.org/zap" "github.com/aws/amazon-cloudwatch-agent/internal/ec2metadataprovider" - "github.com/aws/amazon-cloudwatch-agent/translator/config" ) const ( @@ -31,7 +30,6 @@ type EC2Info struct { // region is used while making call to describeTags Ec2 API for AutoScalingGroup Region string - kubernetesMode string metadataProvider ec2metadataprovider.MetadataProvider logger *zap.Logger @@ -89,10 +87,9 @@ func (ei *EC2Info) setInstanceIDAccountID() error { } } -func newEC2Info(metadataProvider ec2metadataprovider.MetadataProvider, kubernetesMode string, done chan struct{}, region string, logger *zap.Logger) *EC2Info { +func newEC2Info(metadataProvider ec2metadataprovider.MetadataProvider, done chan struct{}, region string, logger *zap.Logger) *EC2Info { return &EC2Info{ metadataProvider: metadataProvider, - kubernetesMode: kubernetesMode, done: done, Region: region, logger: logger, diff --git a/extension/entitystore/ec2Info_test.go b/extension/entitystore/ec2Info_test.go index 39b225431b..816e56a40d 100644 --- a/extension/entitystore/ec2Info_test.go +++ b/extension/entitystore/ec2Info_test.go @@ -14,7 +14,6 @@ import ( "go.uber.org/zap" "github.com/aws/amazon-cloudwatch-agent/internal/ec2metadataprovider" - "github.com/aws/amazon-cloudwatch-agent/translator/config" ) var mockedInstanceIdentityDoc = &ec2metadata.EC2InstanceIdentityDocument{ @@ -158,35 +157,3 @@ func TestNotInitIfMetadataProviderIsEmpty(t *testing.T) { }) } } - -func TestNoASGRetrievalInKubernetesMode(t *testing.T) { - type args struct { - metadataProvider ec2metadataprovider.MetadataProvider - kubernetesMode string - } - tests := []struct { - name string - args args - wantErr bool - want string - }{ - { - name: "EKSNoASGFromEC2Info", - args: args{ - metadataProvider: &mockMetadataProvider{InstanceIdentityDocument: mockedInstanceIdentityDoc, Tags: map[string]string{"aws:autoscaling:groupName": tagVal3}}, - kubernetesMode: config.ModeEKS, - }, - wantErr: false, - want: "", - }, - } - for _, tt := range tests { - logger, _ := zap.NewDevelopment() - t.Run(tt.name, func(t *testing.T) { - ei := &EC2Info{metadataProvider: tt.args.metadataProvider, kubernetesMode: tt.args.kubernetesMode, logger: logger} - go ei.initEc2Info() - time.Sleep(3 * time.Second) - assert.Equal(t, tt.want, ei.GetAutoScalingGroup()) - }) - } -} diff --git a/extension/entitystore/extension.go b/extension/entitystore/extension.go index f0da3f55e5..7f5c71389f 100644 --- a/extension/entitystore/extension.go +++ b/extension/entitystore/extension.go @@ -95,7 +95,7 @@ func (e *EntityStore) Start(ctx context.Context, host component.Host) error { e.serviceprovider = newServiceProvider(e.mode, e.config.Region, &e.ec2Info, e.metadataprovider, getEC2Provider, ec2CredentialConfig, e.done, e.logger) switch e.mode { case config.ModeEC2: - e.ec2Info = *newEC2Info(e.metadataprovider, e.kubernetesMode, e.done, e.config.Region, e.logger) + e.ec2Info = *newEC2Info(e.metadataprovider, e.done, e.config.Region, e.logger) go e.ec2Info.initEc2Info() // Instance metadata tags is not usable for EKS nodes // https://github.com/kubernetes/cloud-provider-aws/issues/762 From 157af1545d95055ed15782733a32c61d7ef10ef2 Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Tue, 17 Dec 2024 13:26:45 -0500 Subject: [PATCH 5/8] changed the limited retryer to unlimited until success retryer for tags retrieval --- extension/entitystore/serviceprovider.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/entitystore/serviceprovider.go b/extension/entitystore/serviceprovider.go index c6a9039586..bdd270765d 100644 --- a/extension/entitystore/serviceprovider.go +++ b/extension/entitystore/serviceprovider.go @@ -80,9 +80,9 @@ func (s *serviceprovider) startServiceProvider() { return } unlimitedRetryer := NewRetryer(false, true, defaultJitterMin, defaultJitterMax, ec2tagger.BackoffSleepArray, infRetry, s.done, s.logger) - limitedRetryer := NewRetryer(false, true, describeTagsJitterMin, describeTagsJitterMax, ec2tagger.ThrottleBackOffArray, maxRetry, s.done, s.logger) + unlimitedRetryerUntilSuccess := NewRetryer(true, true, describeTagsJitterMin, describeTagsJitterMax, ec2tagger.BackoffSleepArray, infRetry, s.done, s.logger) go unlimitedRetryer.refreshLoop(s.scrapeIAMRole) - go limitedRetryer.refreshLoop(s.scrapeImdsServiceNameAndASG) + go unlimitedRetryerUntilSuccess.refreshLoop(s.scrapeImdsServiceNameAndASG) } func (s *serviceprovider) GetIAMRole() string { From b2b4b3da9c610fd97beff9da53ba4556fe6aa1d3 Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Tue, 17 Dec 2024 15:49:21 -0500 Subject: [PATCH 6/8] lint fix --- extension/entitystore/ec2Info.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/entitystore/ec2Info.go b/extension/entitystore/ec2Info.go index fee5e2ca09..23d13eb0b1 100644 --- a/extension/entitystore/ec2Info.go +++ b/extension/entitystore/ec2Info.go @@ -29,7 +29,7 @@ type EC2Info struct { AccountID string // region is used while making call to describeTags Ec2 API for AutoScalingGroup - Region string + Region string metadataProvider ec2metadataprovider.MetadataProvider logger *zap.Logger From de21ba9d29bc6aabd6134e5a7254752b8c785192 Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Tue, 17 Dec 2024 15:59:32 -0500 Subject: [PATCH 7/8] lint fix --- plugins/processors/awsentity/processor_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/processors/awsentity/processor_test.go b/plugins/processors/awsentity/processor_test.go index 4f5d22cf96..f9051975bd 100644 --- a/plugins/processors/awsentity/processor_test.go +++ b/plugins/processors/awsentity/processor_test.go @@ -60,11 +60,11 @@ func newMockGetServiceNameAndSource(service, source string) func() (string, stri } } -func newMockGetEC2InfoFromEntityStore(instance, accountId string) func() entitystore.EC2Info { +func newMockGetEC2InfoFromEntityStore(instance, accountID string) func() entitystore.EC2Info { return func() entitystore.EC2Info { return entitystore.EC2Info{ InstanceID: instance, - AccountID: accountId, + AccountID: accountID, } } } From 63ff0bc05fe2c16415cbdc9267aff71dcc60853a Mon Sep 17 00:00:00 2001 From: Pooja Reddy Nathala Date: Thu, 19 Dec 2024 11:22:11 -0500 Subject: [PATCH 8/8] addressed review comments --- extension/entitystore/serviceprovider.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/extension/entitystore/serviceprovider.go b/extension/entitystore/serviceprovider.go index bdd270765d..d644f7b6cd 100644 --- a/extension/entitystore/serviceprovider.go +++ b/extension/entitystore/serviceprovider.go @@ -218,7 +218,7 @@ func (s *serviceprovider) serviceAttributeFromAsg() ServiceAttribute { } return ServiceAttribute{ - Environment: "ec2:" + s.autoScalingGroup, + Environment: "ec2:" + s.getAutoScalingGroup(), } } @@ -266,9 +266,7 @@ func (s *serviceprovider) scrapeImdsServiceNameAndASG() error { } if strings.Contains(tags, ec2tagger.Ec2InstanceTagKeyASG) { asg, err := s.metadataProvider.InstanceTagValue(context.Background(), ec2tagger.Ec2InstanceTagKeyASG) - if err != nil { - s.logger.Error("Failed to get AutoScalingGroup through metadata provider", zap.Error(err)) - } else { + if err == nil { s.logger.Debug("AutoScalingGroup retrieved through IMDS") s.mutex.Lock() s.autoScalingGroup = asg