Sounds good.  If you are using logging in production, I agree,  we do need to be able to strip out the unnecessary details. I just took the first cut at re-implementing this logging:  i added a few private methods to deal with logging and they should help us to achieve better consistency in logging all API details across different methods (helps also if you would need to do any parsing for  automated reports) and this also moves the bulk of the logging stuff into these methods, so the main execution path should look much cleaner.  The code also removes the bulk of the base64 stuff, both from from POST requests and GET responses. I also changed this so that the appKey details are not logged with the requests. We can build on this and improve it further in later versions (possibly adding LoggerHelper to manage all of the API logging). I'll submit this with all the other review changes.
 
On Feb 19, 2014, at 2:30 AM, thobach <notifications@github.com> wrote:

> In src/com/mambu/apisdk/util/RequestExecutorImpl.java:
> 
> > @@ -104,12 +105,17 @@ public String executeRequest(String urlString, ParamsMap params, Method method,
> >  			params.addParam(APPLICATION_KEY, applicationKey);
> >  
> >  			// Log the App key (the first 3 and last 3 chars only)
> > -			final int keyLength = applicationKey.length();
> > -			final int printLength = 3;
> > -			// Mambu App Keys are very long but just to prevent any errors need to ensure there is enough to print
> > -			if (keyLength >= printLength)
> > -				LOGGER.info("Added Application key=" + applicationKey.substring(0, printLength) + "..."
> > -						+ applicationKey.substring(keyLength - printLength, keyLength));
> > +			// Wrap LOGGER calls with a check for the Logger Level:
> > +			// See http://www.sw-engineering-candies.com/blog-1/thewronguseofloggingcanbeaperformancekillerinjava
> > +			if (LOGGER.isLoggable(Level.INFO)) {
> Ok, I understand the problem with the logs for getImage() and getDocuments() and I feel that problem should be solved differently. Can't we strip out the base64 encoded image and document from the result in the logs? We use the Mambu Java API Wrapper in another project as well where we upload PDFs to Mambu and it makes reading the logs nearly impossible with these base64 encoded documents. We still need the logs so the solution "if (LOGGER.isLoggable(Level.INFO)) {...}" doesn't really help there.
> I'd be glad if you can find a solution to strip out the big chunks of the API responses.
> 
> —
> Reply to this email directly or view it on GitHub.
>