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

aws/client: Fix HTTP debug log EventStream payloads #2000

Merged
merged 2 commits into from Jun 20, 2018

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Jun 20, 2018

Fixes the SDK's HTTP client debug logging to not log the HTTP response
body for EventStreams. This prevents the SDK from buffering a very large
amount of data to be logged at once. The aws.LogDebugWithEventStreamBody
should be used to log the event stream events.

Also fixes a bug in the SDK's response logger which will buffer the
response body's content if LogDebug is enabled but LogDebugWithHTTPBody
is not.

@jasdel jasdel self-assigned this Jun 20, 2018
@jasdel jasdel requested a review from xibz June 20, 2018 00:08
if err != nil {
lw.Logger.Log(fmt.Sprintf(logRespErrMsg, req.ClientInfo.ServiceName, req.Operation.Name, err))
return
}
lw.Logger.Log(fmt.Sprintf(logRespMsg, req.ClientInfo.ServiceName, req.Operation.Name, string(body)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lw wouldn't be initialized here when it should of been if LogDebugWithHTTPBody isn't set.

Fixes the SDK's HTTP client debug logging to not log the HTTP response
body for EventStreams. This prevents the SDK from buffering a very large
amount of data to be logged at once. The aws.LogDebugWithEventStreamBody
should be used to log the event stream events.

Also fixes a bug in the SDK's response logger which will buffer the
response body's content if LogDebug is enabled but LogDebugWithHTTPBody
is not.
@jasdel jasdel merged commit 26e2f20 into aws:master Jun 20, 2018
@jasdel jasdel deleted the bug/EventStreamBodyLog branch June 20, 2018 17:17
jasdel added a commit that referenced this pull request Jun 20, 2018
@awstools awstools mentioned this pull request Jun 20, 2018
@diehlaws diehlaws added needs-review This issue or pull request needs review from a core team member. and removed review-needed labels Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants