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

Enable SDKv2 s3 debug logs #207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kaovilai
Copy link
Contributor

@kaovilai kaovilai commented Jun 28, 2024

For aws sdkv2, there are no logging by default. This PR enables debug logging from the SDK.

Helps with vmware-tanzu/velero#7543

Sets ClientLogMode https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/logging/#clientlogmode and add plugin logger to config/options

This should assists in debugging requests/responses, especially with the recent rise in issues

Signed-off-by: Tiger Kaovilai tkaovila@redhat.com

image: ghcr.io/kaovilai/velero-plugin-for-aws:sdk-v2-logging

Dev Notes

image build command

docker build --platform=linux/amd64 -t ghcr.io/kaovilai/$(basename $PWD):$(git branch --show-current) . && docker push ghcr.io/kaovilai/$(basename $PWD):$(git branch --show-current)

@kaovilai kaovilai changed the title Enable sdkv2 s3 debug logs Enable SDKv2 s3 debug logs Jun 28, 2024
@kaovilai kaovilai force-pushed the sdk-v2-logging branch 2 times, most recently from aec0092 to 55e92a6 Compare June 28, 2024 22:18
}),
),
config.WithLogger(awsLogger),
config.WithClientLogMode(aws.LogRequest|aws.LogResponse|aws.LogRetries|aws.LogSigning),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please feedback on which logs to enable and/or if we need to make this configurable.

@kaovilai kaovilai force-pushed the sdk-v2-logging branch 2 times, most recently from 9e16cf8 to d736b23 Compare June 28, 2024 22:24
@kaovilai kaovilai marked this pull request as ready for review June 28, 2024 22:24
Comment on lines 143 to 152
cfg, err := newConfigBuilder(o.log).WithTLSSettings(insecureSkipTLSVerify, caCert).Build()
cfg, err := newConfigBuilder(o.log).WithTLSSettings(insecureSkipTLSVerify, caCert).WithClientLogMode().Build()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this is not needed, unless we want logs for region discovery too.

@kaovilai kaovilai force-pushed the sdk-v2-logging branch 8 times, most recently from d87a2e9 to f6f468e Compare July 2, 2024 02:00
@kaovilai kaovilai force-pushed the sdk-v2-logging branch 4 times, most recently from b12f7f3 to 41b19b5 Compare July 12, 2024 20:00
sseago
sseago previously approved these changes Jul 12, 2024
@kaovilai
Copy link
Contributor Author

This might also help determine vmware-tanzu/velero#8152 as to which value was passed in header x-amz-content-sha256

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

remove commented code.

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants