[internal/aws/proxy] Migrate from AWS SDK v1 to v2#47244
[internal/aws/proxy] Migrate from AWS SDK v1 to v2#47244andrzej-stencel merged 26 commits intoopen-telemetry:mainfrom
Conversation
|
|
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
2d9fc25 to
88403eb
Compare
7669a9a to
d22bd92
Compare
|
Moving to draft while you work on this, please mark ready for review when CI passes. |
d22bd92 to
2dcc158
Compare
This change migrates the internal AWS proxy module from AWS SDK Go v1 to v2, removing dependency on the deprecated AWS SDK v1. Fixes open-telemetry#40461
Extract region detection from getAWSConfigSession into smaller, focused functions for better readability and maintainability.
- Remove unused `implicitGlobalRegion` field from partitionConfig struct - Remove unused `getServiceEndpointFromConfig` function - Fix errorlint: use %w for error wrapping in fmt.Errorf - Fix unused-parameter: rename unused ctx parameters to _ - Fix usetesting: replace context.Background() with t.Context()
…d consolidate proxy functions - Convert getRegionFromEC2Metadata to mockable function variable to prevent real HTTP calls to EC2 IMDS (169.254.169.254) during tests - Export GetProxyAddress and GetProxyURL from awsutil package - Remove duplicate getProxyAddress/getProxyURL from proxy package
- Remove hand-rolled partition table; use sts.NewDefaultEndpointResolverV2() for getServiceEndpoint covering all 8 AWS partitions - Remove duplicate cfg.RetryMaxAttempts and redundant cfg.Region assignment - Replace getPrimaryRegion with prefix-based matching for all partitions - Refactor setupMock to use t.Cleanup() with meaningful variable names - Fix data race in signing tests: use buffered channels + r.Clone() - Replace silent nil-guards with require.NotNil/require.NotEmpty - Expand TestGetServiceEndpoint with iso/isob/isoe/isof/eusc partitions - Remove TestLoadEnvConfigCreds (covered by awsutil package tests) Assisted-by: Claude Opus 4
- Fix misleading error message in getRegionFromMetadata: only mentions ECS and EC2 metadata (config/env vars are checked in resolveRegion) - Migrate from deprecated Director to Rewrite in ReverseProxy to address security concern (hop-by-hop headers handled automatically) - Unexport GetProxyAddress and GetProxyURL since they are not used outside the awsutil package
Also address round 2 review feedback from Aneurysm9: - Delete unused getProxyAddress/getProxyURL and their tests (dead code) - Simplify Rewrite comment (Director deprecation detail in commit history) - Read from r.In.Body instead of r.Out.Body for hash computation
0cb0ee1 to
c35f9bc
Compare
|
@mitali-salvi Hey, I was looking at issue #37728. Do you intend to migrate all AWS use cases from v1 to v2? If yes, then I would pick up another issue to work on. Thanks. |
|
@atoulme should be ready to review now |
|
@andrzej-stencel I would appreciate a review as well :) |
|
/workflow-approve |
|
/workflow-approve |
|
@codeboten can I get this PR merged please ? :) |
|
Thank you for your contribution @mitali-salvi! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. If you are getting started contributing, you can also join the CNCF Slack channel #opentelemetry-new-contributors to ask for guidance and get help. |
Migrates internal/aws/proxy from AWS SDK Go v1 to v2.
Link to tracking issue
Fixes #40461
Related: #37728
Testing
Documentation
No documentation changes needed — test-only fix.