Migrate EC2 metadata to SDKv2 - #1992
Conversation
| var getMetaDataProvider = func(ctx context.Context) ec2metadataprovider.MetadataProvider { | ||
| mdCredentialConfig := &configaws.CredentialsConfig{} | ||
| cfg, err := mdCredentialConfig.LoadConfig(ctx) | ||
| if err != nil { |
There was a problem hiding this comment.
It's already logged in config/aws/v2/credentials.go.
| type metadataClient struct { | ||
| metadataFallbackDisabled *ec2metadata.EC2Metadata | ||
| metadataFallbackEnabled *ec2metadata.EC2Metadata | ||
| v2Client *imds.Client |
There was a problem hiding this comment.
a comment or todo would be nice for why to keep both and what to do later on.
There was a problem hiding this comment.
I don't think there is a TODO unless we want to mention that we're going to remove the v1Client once IMDSv1 is removed.
| Standard: retry.NewStandard(func(options *retry.StandardOptions) { | ||
| options.MaxAttempts = retries + 1 // MaxAttempts include the first attempt | ||
| }), |
There was a problem hiding this comment.
Is the reason for this custom retryer so that we can update the max attempts? Otherwise, I don't think we would need one. NewFromConfig for IMDS has the "standard" retryer already: https://github.com/aws/aws-sdk-go-v2/blob/dcbed91b6c6235022f15eda6ea526dbb91e1cb81/feature/ec2/imds/api_client.go#L117-L119.
If that is also the case, we can use the pattern described in https://docs.aws.amazon.com/sdk-for-go/v2/developer-guide/configure-retries-timeouts.html#customizing-behavior under Limiting the max number of attempts. This is more of a nit; I'm not opposed to what you have already.
There was a problem hiding this comment.
The custom IMDS retryer is to allow retries for not just the default retryable codes/errors, but basically any AWS error from IMDS. In SDKv2, IMDS returns https://github.com/aws/aws-sdk-go-v2/blob/dcbed91b6c6235022f15eda6ea526dbb91e1cb81/feature/ec2/imds/request_middleware.go#L185-L191, which may or may not fall under the standard retryables.
I'll try changing the pattern in the next PR.
There was a problem hiding this comment.
Tried it out and remembered why I couldn't use it. retry.AddWithMaxAttempts returns an aws.Retryer. The new IMDS retryer and retry.Standard both implement the aws.RetryerV2 interface, so it wouldn't satisfy the new functions it needs to cover.
Squashed application of the aws-sdk-v2 migration (feature/aws-sdk-v2) onto latest main. Replaces the SDKv1 credential chain, service clients, retryers, and request handlers with SDKv2 equivalents, and removes the hand-vendored SDKv1 cloudwatch/cloudwatchlogs services (~44k lines). Source PRs (Jeffrey Chien): - #1981 Support credential chain for AWS SDK Go v2 - #1985 Migrate CloudWatch exporter to SDKv2 - #1989 Migrate CloudWatch logs output plugin to SDKv2 - #1992 Migrate EC2 metadata to SDKv2 - #1994 Migrate ECS service discovery to SDKv2 - #1995 Migrate wizard, downloader, and translator to SDKv2 - #2001 Remove SDKv1 direct dependencies - Use a shared HTTP client to reduce file descriptor usage aws-sdk-go (v1) is no longer a direct dependency. cfg/aws now exposes the v2 credential chain (CredentialsConfig.LoadConfig), STS regional->partitional fallback, smithy custom-header middleware, and a shared BuildableClient.
Squashed application of the aws-sdk-v2 migration (feature/aws-sdk-v2) onto latest main. Replaces the SDKv1 credential chain, service clients, retryers, and request handlers with SDKv2 equivalents, and removes the hand-vendored SDKv1 cloudwatch/cloudwatchlogs services (~44k lines). Source PRs (Jeffrey Chien): - #1981 Support credential chain for AWS SDK Go v2 - #1985 Migrate CloudWatch exporter to SDKv2 - #1989 Migrate CloudWatch logs output plugin to SDKv2 - #1992 Migrate EC2 metadata to SDKv2 - #1994 Migrate ECS service discovery to SDKv2 - #1995 Migrate wizard, downloader, and translator to SDKv2 - #2001 Remove SDKv1 direct dependencies - Use a shared HTTP client to reduce file descriptor usage aws-sdk-go (v1) is no longer a direct dependency. cfg/aws now exposes the v2 credential chain (CredentialsConfig.LoadConfig), STS regional->partitional fallback, smithy custom-header middleware, and a shared BuildableClient.
Squashed application of the aws-sdk-v2 migration (feature/aws-sdk-v2) onto latest main. Replaces the SDKv1 credential chain, service clients, retryers, and request handlers with SDKv2 equivalents, and removes the hand-vendored SDKv1 cloudwatch/cloudwatchlogs services (~44k lines). Source PRs (Jeffrey Chien): - #1981 Support credential chain for AWS SDK Go v2 - #1985 Migrate CloudWatch exporter to SDKv2 - #1989 Migrate CloudWatch logs output plugin to SDKv2 - #1992 Migrate EC2 metadata to SDKv2 - #1994 Migrate ECS service discovery to SDKv2 - #1995 Migrate wizard, downloader, and translator to SDKv2 - #2001 Remove SDKv1 direct dependencies - Use a shared HTTP client to reduce file descriptor usage aws-sdk-go (v1) is no longer a direct dependency. cfg/aws now exposes the v2 credential chain (CredentialsConfig.LoadConfig), STS regional->partitional fallback, smithy custom-header middleware, and a shared BuildableClient.
3 participants
Description of the issue
The next one in the list after the exporters is the EC2 tagger.
Description of changes
internal/ec2metadataprovideraws-sdk-go/aws/ec2metadatatoaws-sdk-go-v2/feature/ec2/imdsEnableFallbackternarygetMetadata()helperinternal/retryer/v2/imdsretryer.goimplementing AWS SDK v2 retry interfaceplugins/processors/ec2tagger/ec2ProviderTypeto support v2 credential loadingextension/entitystore/cfg/aws/v2credential configurationec2ProviderTypeandnativeCredentialfieldsLicense
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
Kept unit tests as intact as possible while adding some additional coverage.
PR Build: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/21230132305
Ran integration tests against branch: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/21231010826
Ran the agent with metric collection enabled along with
append_dimensionsfor volume, instance ID, and instance type. Can see it successfully send and can see metrics in CloudWatch.Can see the entity store attempt to get the tags from IMDS
Can see it successfully get the instance identity document
Requirements
Before commiting your code, please do the following steps.
make fmtandmake fmt-shmake lintIntegration Tests
To run integration tests against this PR, add the
ready for testinglabel.