From 60b7b6fcab736c2a1803c6460f9a1828a769669f Mon Sep 17 00:00:00 2001 From: Alex Kats Date: Thu, 18 Jan 2024 22:22:04 -0500 Subject: [PATCH 1/5] fix ec2 scraper attribute drop without describeTags --- .chloggen/update-ec2-scraper.yaml | 27 +++++++++++++++++++ .../internal/aws/ec2/ec2.go | 10 +++---- 2 files changed, 32 insertions(+), 5 deletions(-) create mode 100755 .chloggen/update-ec2-scraper.yaml diff --git a/.chloggen/update-ec2-scraper.yaml b/.chloggen/update-ec2-scraper.yaml new file mode 100755 index 000000000000..0c645045c78f --- /dev/null +++ b/.chloggen/update-ec2-scraper.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'bug_fix' + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: resourcedetectionprocessor + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Update to ec2 scraper so that core attributes are not dropped if describeTags returns an error (likely due to permissions) + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go index 8ac342466d8c..57994b6917b4 100644 --- a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go +++ b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go @@ -88,13 +88,13 @@ func (d *Detector) Detect(ctx context.Context) (resource pcommon.Resource, schem client := getHTTPClientSettings(ctx, d.logger) tags, err := connectAndFetchEc2Tags(meta.Region, meta.InstanceID, d.tagKeyRegexes, client) if err != nil { - return res, "", fmt.Errorf("failed fetching ec2 instance tags: %w", err) - } - for key, val := range tags { - res.Attributes().PutStr(tagPrefix+key, val) + d.logger.Warn("failed fetching ec2 instance tags", zap.Error(err)) + } else { + for key, val := range tags { + res.Attributes().PutStr(tagPrefix+key, val) + } } } - return res, conventions.SchemaURL, nil } From f4f76703b999456eaea167f668f703c3644af0ad Mon Sep 17 00:00:00 2001 From: Alex Kats Date: Mon, 22 Jan 2024 11:35:40 -0500 Subject: [PATCH 2/5] updated chlog --- .chloggen/update-ec2-scraper.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/update-ec2-scraper.yaml b/.chloggen/update-ec2-scraper.yaml index 0c645045c78f..d5a2208beb6d 100755 --- a/.chloggen/update-ec2-scraper.yaml +++ b/.chloggen/update-ec2-scraper.yaml @@ -10,7 +10,7 @@ component: resourcedetectionprocessor note: Update to ec2 scraper so that core attributes are not dropped if describeTags returns an error (likely due to permissions) # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. -issues: [] +issues: [30672] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. From 5047c33ae0314f64f2f503b2adf5109bb052dbed Mon Sep 17 00:00:00 2001 From: Alex Kats Date: Mon, 19 Feb 2024 23:16:41 -0500 Subject: [PATCH 3/5] Added test cases ec2 scraper + update to detector struct --- .../internal/aws/ec2/ec2.go | 13 +- .../internal/aws/ec2/ec2_test.go | 138 ++++++++++++++---- 2 files changed, 115 insertions(+), 36 deletions(-) diff --git a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go index 57994b6917b4..0d11cfb64d66 100644 --- a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go +++ b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go @@ -36,6 +36,7 @@ type Detector struct { tagKeyRegexes []*regexp.Regexp logger *zap.Logger rb *metadata.ResourceBuilder + ec2Client ec2iface.EC2API } func NewDetector(set processor.CreateSettings, dcfg internal.DetectorConfig) (internal.Detector, error) { @@ -86,7 +87,7 @@ func (d *Detector) Detect(ctx context.Context) (resource pcommon.Resource, schem if len(d.tagKeyRegexes) != 0 { client := getHTTPClientSettings(ctx, d.logger) - tags, err := connectAndFetchEc2Tags(meta.Region, meta.InstanceID, d.tagKeyRegexes, client) + tags, err := connectAndFetchEc2Tags(d.ec2Client, meta.Region, meta.InstanceID, d.tagKeyRegexes, client) if err != nil { d.logger.Warn("failed fetching ec2 instance tags", zap.Error(err)) } else { @@ -107,7 +108,10 @@ func getHTTPClientSettings(ctx context.Context, logger *zap.Logger) *http.Client return client } -func connectAndFetchEc2Tags(region string, instanceID string, tagKeyRegexes []*regexp.Regexp, client *http.Client) (map[string]string, error) { +func connectAndFetchEc2Tags(svc ec2iface.EC2API, region string, instanceID string, tagKeyRegexes []*regexp.Regexp, client *http.Client) (map[string]string, error) { + if svc != nil { + return fetchEC2Tags(svc, instanceID, tagKeyRegexes) + } sess, err := session.NewSession(&aws.Config{ Region: aws.String(region), HTTPClient: client}, @@ -115,9 +119,8 @@ func connectAndFetchEc2Tags(region string, instanceID string, tagKeyRegexes []*r if err != nil { return nil, err } - e := ec2.New(sess) - - return fetchEC2Tags(e, instanceID, tagKeyRegexes) + svc = ec2.New(sess) + return fetchEC2Tags(svc, instanceID, tagKeyRegexes) } func fetchEC2Tags(svc ec2iface.EC2API, instanceID string, tagKeyRegexes []*regexp.Regexp) (map[string]string, error) { diff --git a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go index b57ca85a2814..a761df27d729 100644 --- a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go +++ b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go @@ -97,6 +97,41 @@ func TestNewDetector(t *testing.T) { } } +// Define a mock client to mock connecting to an EC2 instance +type mockEC2ClientError struct { + ec2iface.EC2API +} + +// override the DescribeTags function to mock the output from an actual EC2 instance +func (m *mockEC2ClientError) DescribeTags(_ *ec2.DescribeTagsInput) (*ec2.DescribeTagsOutput, error) { + return nil, errors.New("Error fetching tags") +} + +type mockEC2Client struct { + ec2iface.EC2API +} + +// override the DescribeTags function to mock the output from an actual EC2 instance +func (m *mockEC2Client) DescribeTags(input *ec2.DescribeTagsInput) (*ec2.DescribeTagsOutput, error) { + if *input.Filters[0].Values[0] == "error" { + return nil, errors.New("error") + } + + tag1 := "tag1" + tag2 := "tag2" + resource1 := "resource1" + val1 := "val1" + val2 := "val2" + resourceType := "type" + + return &ec2.DescribeTagsOutput{ + Tags: []*ec2.TagDescription{ + {Key: &tag1, ResourceId: &resource1, ResourceType: &resourceType, Value: &val1}, + {Key: &tag2, ResourceId: &resource1, ResourceType: &resourceType, Value: &val2}, + }, + }, nil +} + func TestDetector_Detect(t *testing.T) { type fields struct { metadataProvider ec2provider.Provider @@ -105,11 +140,13 @@ func TestDetector_Detect(t *testing.T) { ctx context.Context } tests := []struct { - name string - fields fields - args args - want pcommon.Resource - wantErr bool + name string + fields fields + tagKeyRegexes []*regexp.Regexp + args args + want pcommon.Resource + wantErr bool + tagsProvider ec2iface.EC2API }{ { name: "success", @@ -139,6 +176,69 @@ func TestDetector_Detect(t *testing.T) { attr.PutStr("host.name", "example-hostname") return res }()}, + { + name: "success with tags", + fields: fields{metadataProvider: &mockMetadata{ + retIDDoc: ec2metadata.EC2InstanceIdentityDocument{ + Region: "us-west-2", + AccountID: "account1234", + AvailabilityZone: "us-west-2a", + InstanceID: "i-abcd1234", + ImageID: "abcdef", + InstanceType: "c4.xlarge", + }, + retHostname: "example-hostname", + isAvailable: true}}, + tagKeyRegexes: []*regexp.Regexp{regexp.MustCompile("^tag1$"), regexp.MustCompile("^tag2$")}, + args: args{ctx: context.Background()}, + want: func() pcommon.Resource { + res := pcommon.NewResource() + attr := res.Attributes() + attr.PutStr("cloud.account.id", "account1234") + attr.PutStr("cloud.provider", "aws") + attr.PutStr("cloud.platform", "aws_ec2") + attr.PutStr("cloud.region", "us-west-2") + attr.PutStr("cloud.availability_zone", "us-west-2a") + attr.PutStr("host.id", "i-abcd1234") + attr.PutStr("host.image.id", "abcdef") + attr.PutStr("host.type", "c4.xlarge") + attr.PutStr("host.name", "example-hostname") + attr.PutStr("ec2.tag.tag1", "val1") + attr.PutStr("ec2.tag.tag2", "val2") + return res + }(), + tagsProvider: &mockEC2Client{}, + }, + { + name: "success without tags returned from describeTags", + fields: fields{metadataProvider: &mockMetadata{ + retIDDoc: ec2metadata.EC2InstanceIdentityDocument{ + Region: "us-west-2", + AccountID: "account1234", + AvailabilityZone: "us-west-2a", + InstanceID: "i-abcd1234", + ImageID: "abcdef", + InstanceType: "c4.xlarge", + }, + retHostname: "example-hostname", + isAvailable: true}}, + args: args{ctx: context.Background()}, + want: func() pcommon.Resource { + res := pcommon.NewResource() + attr := res.Attributes() + attr.PutStr("cloud.account.id", "account1234") + attr.PutStr("cloud.provider", "aws") + attr.PutStr("cloud.platform", "aws_ec2") + attr.PutStr("cloud.region", "us-west-2") + attr.PutStr("cloud.availability_zone", "us-west-2a") + attr.PutStr("host.id", "i-abcd1234") + attr.PutStr("host.image.id", "abcdef") + attr.PutStr("host.type", "c4.xlarge") + attr.PutStr("host.name", "example-hostname") + return res + }(), + tagsProvider: &mockEC2ClientError{}, + }, { name: "endpoint not available", fields: fields{metadataProvider: &mockMetadata{ @@ -177,6 +277,8 @@ func TestDetector_Detect(t *testing.T) { metadataProvider: tt.fields.metadataProvider, logger: zap.NewNop(), rb: metadata.NewResourceBuilder(metadata.DefaultResourceAttributesConfig()), + tagKeyRegexes: tt.tagKeyRegexes, + ec2Client: tt.tagsProvider, } got, _, err := d.Detect(tt.args.ctx) @@ -191,32 +293,6 @@ func TestDetector_Detect(t *testing.T) { } } -// Define a mock client to mock connecting to an EC2 instance -type mockEC2Client struct { - ec2iface.EC2API -} - -// override the DescribeTags function to mock the output from an actual EC2 instance -func (m *mockEC2Client) DescribeTags(input *ec2.DescribeTagsInput) (*ec2.DescribeTagsOutput, error) { - if *input.Filters[0].Values[0] == "error" { - return nil, errors.New("error") - } - - tag1 := "tag1" - tag2 := "tag2" - resource1 := "resource1" - val1 := "val1" - val2 := "val2" - resourceType := "type" - - return &ec2.DescribeTagsOutput{ - Tags: []*ec2.TagDescription{ - {Key: &tag1, ResourceId: &resource1, ResourceType: &resourceType, Value: &val1}, - {Key: &tag2, ResourceId: &resource1, ResourceType: &resourceType, Value: &val2}, - }, - }, nil -} - func TestEC2Tags(t *testing.T) { tests := []struct { name string From a1e2affce215e76f14d5e1b1054ca01b6ee50001 Mon Sep 17 00:00:00 2001 From: Alex Kats Date: Fri, 23 Feb 2024 10:52:48 -0500 Subject: [PATCH 4/5] resolved conflict --- processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go index c41b469f5601..d640c6e94f44 100644 --- a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go +++ b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go @@ -87,7 +87,7 @@ func (d *Detector) Detect(ctx context.Context) (resource pcommon.Resource, schem if len(d.tagKeyRegexes) != 0 { client := getClientConfig(ctx, d.logger) - tags, err := connectAndFetchEc2Tags(meta.Region, meta.InstanceID, d.tagKeyRegexes, client) + tags, err := connectAndFetchEc2Tags(d.ec2Client, meta.Region, meta.InstanceID, d.tagKeyRegexes, client) if err != nil { d.logger.Warn("failed fetching ec2 instance tags", zap.Error(err)) } else { From 6728a411f121930d8a6f59bf4694d15440baa6dd Mon Sep 17 00:00:00 2001 From: Alex Kats Date: Sat, 2 Mar 2024 18:30:09 -0500 Subject: [PATCH 5/5] refactored interface --- .../internal/aws/ec2/ec2.go | 44 +++++++++++-------- .../internal/aws/ec2/ec2_test.go | 21 +++++++-- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go index d640c6e94f44..969ed76020ca 100644 --- a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go +++ b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go @@ -31,12 +31,29 @@ const ( var _ internal.Detector = (*Detector)(nil) +type ec2ifaceBuilder interface { + buildClient(region string, client *http.Client) (ec2iface.EC2API, error) +} + +type ec2ClientBuilder struct{} + +func (e *ec2ClientBuilder) buildClient(region string, client *http.Client) (ec2iface.EC2API, error) { + sess, err := session.NewSession(&aws.Config{ + Region: aws.String(region), + HTTPClient: client}, + ) + if err != nil { + return nil, err + } + return ec2.New(sess), nil +} + type Detector struct { metadataProvider ec2provider.Provider tagKeyRegexes []*regexp.Regexp logger *zap.Logger rb *metadata.ResourceBuilder - ec2Client ec2iface.EC2API + ec2ClientBuilder ec2ifaceBuilder } func NewDetector(set processor.CreateSettings, dcfg internal.DetectorConfig) (internal.Detector, error) { @@ -55,6 +72,7 @@ func NewDetector(set processor.CreateSettings, dcfg internal.DetectorConfig) (in tagKeyRegexes: tagKeyRegexes, logger: set.Logger, rb: metadata.NewResourceBuilder(cfg.ResourceAttributes), + ec2ClientBuilder: &ec2ClientBuilder{}, }, nil } @@ -86,8 +104,13 @@ func (d *Detector) Detect(ctx context.Context) (resource pcommon.Resource, schem res := d.rb.Emit() if len(d.tagKeyRegexes) != 0 { - client := getClientConfig(ctx, d.logger) - tags, err := connectAndFetchEc2Tags(d.ec2Client, meta.Region, meta.InstanceID, d.tagKeyRegexes, client) + httpClient := getClientConfig(ctx, d.logger) + ec2Client, err := d.ec2ClientBuilder.buildClient(meta.Region, httpClient) + if err != nil { + d.logger.Warn("failed to build ec2 client", zap.Error(err)) + return res, conventions.SchemaURL, nil + } + tags, err := fetchEC2Tags(ec2Client, meta.InstanceID, d.tagKeyRegexes) if err != nil { d.logger.Warn("failed fetching ec2 instance tags", zap.Error(err)) } else { @@ -108,21 +131,6 @@ func getClientConfig(ctx context.Context, logger *zap.Logger) *http.Client { return client } -func connectAndFetchEc2Tags(svc ec2iface.EC2API, region string, instanceID string, tagKeyRegexes []*regexp.Regexp, client *http.Client) (map[string]string, error) { - if svc != nil { - return fetchEC2Tags(svc, instanceID, tagKeyRegexes) - } - sess, err := session.NewSession(&aws.Config{ - Region: aws.String(region), - HTTPClient: client}, - ) - if err != nil { - return nil, err - } - svc = ec2.New(sess) - return fetchEC2Tags(svc, instanceID, tagKeyRegexes) -} - func fetchEC2Tags(svc ec2iface.EC2API, instanceID string, tagKeyRegexes []*regexp.Regexp) (map[string]string, error) { ec2Tags, err := svc.DescribeTags(&ec2.DescribeTagsInput{ Filters: []*ec2.Filter{{ diff --git a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go index a761df27d729..17916ef75356 100644 --- a/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go +++ b/processor/resourcedetectionprocessor/internal/aws/ec2/ec2_test.go @@ -6,6 +6,7 @@ package ec2 import ( "context" "errors" + "net/http" "regexp" "testing" @@ -36,6 +37,18 @@ type mockMetadata struct { var _ ec2provider.Provider = (*mockMetadata)(nil) +type mockClientBuilder struct{} + +func (e *mockClientBuilder) buildClient(_ string, _ *http.Client) (ec2iface.EC2API, error) { + return &mockEC2Client{}, nil +} + +type mockClientBuilderError struct{} + +func (e *mockClientBuilderError) buildClient(_ string, _ *http.Client) (ec2iface.EC2API, error) { + return &mockEC2ClientError{}, nil +} + func (mm mockMetadata) InstanceID(_ context.Context) (string, error) { if !mm.isAvailable { return "", errUnavailable @@ -146,7 +159,7 @@ func TestDetector_Detect(t *testing.T) { args args want pcommon.Resource wantErr bool - tagsProvider ec2iface.EC2API + tagsProvider ec2ifaceBuilder }{ { name: "success", @@ -207,7 +220,7 @@ func TestDetector_Detect(t *testing.T) { attr.PutStr("ec2.tag.tag2", "val2") return res }(), - tagsProvider: &mockEC2Client{}, + tagsProvider: &mockClientBuilder{}, }, { name: "success without tags returned from describeTags", @@ -237,7 +250,7 @@ func TestDetector_Detect(t *testing.T) { attr.PutStr("host.name", "example-hostname") return res }(), - tagsProvider: &mockEC2ClientError{}, + tagsProvider: &mockClientBuilderError{}, }, { name: "endpoint not available", @@ -278,7 +291,7 @@ func TestDetector_Detect(t *testing.T) { logger: zap.NewNop(), rb: metadata.NewResourceBuilder(metadata.DefaultResourceAttributesConfig()), tagKeyRegexes: tt.tagKeyRegexes, - ec2Client: tt.tagsProvider, + ec2ClientBuilder: tt.tagsProvider, } got, _, err := d.Detect(tt.args.ctx)