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

Should have a limit to the amount of content to log #43

Closed
wonderfly opened this issue Jan 9, 2015 · 6 comments

Comments

@wonderfly
Copy link
Contributor

commented Jan 9, 2015

From yanivin...@gmail.com on October 18, 2011 07:20:47

Version of google-http-java-client (e.g. 1.5.0-beta)? 1.6.0-beta Java environment (e.g. Java 6, Android 2.3, App Engine)? All, but especially limited memory Describe the problem. When logging is enabled, we log the content of the HTTP request and HTTP response. There is currently no limit to the size of content that we will log. To implement this, the whole content is stored in memory as a String. For large content, this will result in an OutOfMemoryException.

See: https://code.google.com/p/google-http-java-client/source/browse/google-http-client/src/main/java/com/google/api/client/http/LogContent.java https://code.google.com/p/google-http-java-client/source/browse/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java How would you expect it to be fixed? We should either set a hard-coded limit -- like 100KB -- or make it configurable by having a logging limit variable with a reasonable default, possibly allowing to set it to -1 to indicate no limit.

Original issue: http://code.google.com/p/google-http-java-client/issues/detail?id=43

@wonderfly wonderfly self-assigned this Jan 9, 2015

@wonderfly

This comment has been minimized.

Copy link
Contributor Author

commented Jan 9, 2015

From yan...@google.com on October 18, 2011 07:21:53

Status: Accepted
Owner: rmis...@google.com
Cc: yan...@google.com
Labels: -Priority-Medium Priority-High Milestone-Version1.6.0 Component-HTTP

@wonderfly

This comment has been minimized.

Copy link
Contributor Author

commented Jan 9, 2015

From yan...@google.com on October 18, 2011 07:32:00

Actually we already have setDisableContentLogging methods on HttpRequest and HttpResponse. We might be able to instead convert that to a contentLoggingLimit, where 0 means don't log, and >0 means any size.

We might also want to avoid the confusing -1 idea, since I'm guessing even Integer.MAX_VALUE should be a more than reasonable upper bound.

@wonderfly

This comment has been minimized.

Copy link
Contributor Author

commented Jan 9, 2015

From roy.smit...@gmail.com on October 18, 2011 08:57:00

My 2c, is that either way, any code within "if (debugging) {}" should catch and sink exceptions, since they are incidental to the intention of the developer. Debugging should never change the behaviour of the app.

@wonderfly

This comment has been minimized.

Copy link
Contributor Author

commented Jan 9, 2015

From yan...@google.com on October 28, 2011 09:46:18

Labels: -Milestone-Version1.6.0 Milestone-Version1.7.0

@wonderfly

This comment has been minimized.

Copy link
Contributor Author

commented Jan 9, 2015

From yan...@google.com on November 16, 2011 08:01:33

http://codereview.appspot.com/5373102/

Status: Started

@wonderfly

This comment has been minimized.

Copy link
Contributor Author

commented Jan 9, 2015

From rmis...@google.com on November 17, 2011 07:36:22

Status: Fixed

@wonderfly wonderfly closed this Jan 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.