Repository: square/retrofit
Issue: Add a less verbose logging level

Currently from what I can see there's no logging and debug logging. A level that would log the request without the response would be extremely useful.


Issue comments:
A way to leave out the POST body would be great too. I'd like to see request logs, but not the personal data being sent.


Pull request review comments:

Patch that resolved the issue:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7974b9f27e..b68962b302 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,14 @@
 Change Log
 ==========
 
+Version 1.2.0 *(In Development)*
+--------------------------------
+
+ * Change `setDebug` to `setLogLevel` on `RestAdapter` and `RestAdapter.Builder` and provide
+   two levels of logging via `LogLevel`.
+ * Query parameters can now be added in a request interceptor.
+
+
 Version 1.1.1 *(2013-06-25)*
 ----------------------------
 
diff --git a/retrofit/src/main/java/retrofit/RestAdapter.java b/retrofit/src/main/java/retrofit/RestAdapter.java
index a447a0fc22..c318a3d8c0 100644
--- a/retrofit/src/main/java/retrofit/RestAdapter.java
+++ b/retrofit/src/main/java/retrofit/RestAdapter.java
@@ -113,6 +113,16 @@ public interface Log {
     void log(String message);
   }
 
+  /** Controls the level of logging. */
+  public enum LogLevel {
+    /** No logging. */
+    NONE,
+    /** Log only the request method and URL and the response status code and execution time. */
+    BASIC,
+    /** Log the headers, body, and metadata for both requests and responses. */
+    FULL
+  }
+
   private final Server server;
   private final Client.Provider clientProvider;
   private final Executor httpExecutor;
@@ -122,11 +132,11 @@ public interface Log {
   private final Profiler profiler;
   private final ErrorHandler errorHandler;
   private final Log log;
-  private volatile boolean debug;
+  private volatile LogLevel logLevel;
 
   private RestAdapter(Server server, Client.Provider clientProvider, Executor httpExecutor,
       Executor callbackExecutor, RequestInterceptor requestInterceptor, Converter converter,
-      Profiler profiler, ErrorHandler errorHandler, Log log, boolean debug) {
+      Profiler profiler, ErrorHandler errorHandler, Log log, LogLevel logLevel) {
     this.server = server;
     this.clientProvider = clientProvider;
     this.httpExecutor = httpExecutor;
@@ -136,12 +146,15 @@ private RestAdapter(Server server, Client.Provider clientProvider, Executor http
     this.profiler = profiler;
     this.errorHandler = errorHandler;
     this.log = log;
-    this.debug = debug;
+    this.logLevel = logLevel;
   }
 
-  /** Toggle debug logging on or off. */
-  public void setDebug(boolean debug) {
-    this.debug = debug;
+  /** Change the level of logging. */
+  public void setLogLevel(LogLevel loglevel) {
+    if (logLevel == null) {
+      throw new NullPointerException("Log level may not be null.");
+    }
+    this.logLevel = loglevel;
   }
 
   /** Create an implementation of the API defined by the specified {@code service} interface. */
@@ -228,8 +241,10 @@ private Object invokeRequest(RestMethodInfo methodDetails, Object[] args) {
           Thread.currentThread().setName(THREAD_PREFIX + url.substring(serverUrl.length()));
         }
 
-        if (debug) {
+        if (logLevel == LogLevel.FULL) {
           request = logAndReplaceRequest(request);
+        } else if (logLevel == LogLevel.BASIC) {
+          logRequestLine(request);
         }
 
         Object profilerObject = null;
@@ -248,8 +263,10 @@ private Object invokeRequest(RestMethodInfo methodDetails, Object[] args) {
           profiler.afterCall(requestInfo, elapsedTime, statusCode, profilerObject);
         }
 
-        if (debug) {
+        if (logLevel == LogLevel.FULL) {
           response = logAndReplaceResponse(url, response, elapsedTime);
+        } else if (logLevel == LogLevel.BASIC) {
+          logResponseLine(url, response, elapsedTime);
         }
 
         Type type = methodDetails.responseObjectType;
@@ -300,9 +317,17 @@ private Object invokeRequest(RestMethodInfo methodDetails, Object[] args) {
     }
   }
 
+  private void logRequestLine(Request request) {
+    log.log(String.format("---> HTTP %s %s", request.getMethod(), request.getUrl()));
+  }
+
+  private void logResponseLine(String url, Response response, long elapsedTime) {
+    log.log(String.format("<--- HTTP %s %s (%sms)", response.getStatus(), url, elapsedTime));
+  }
+
   /** Log request headers and body. Consumes request body and returns identical replacement. */
   private Request logAndReplaceRequest(Request request) throws IOException {
-    log.log(String.format("---> HTTP %s %s", request.getMethod(), request.getUrl()));
+    logRequestLine(request);
 
     for (Header header : request.getHeaders()) {
       log.log(header.getName() + ": " + header.getValue());
@@ -338,7 +363,7 @@ private Request logAndReplaceRequest(Request request) throws IOException {
   /** Log response headers and body. Consumes response body and returns identical replacement. */
   private Response logAndReplaceResponse(String url, Response response, long elapsedTime)
       throws IOException {
-    log.log(String.format("<--- HTTP %s %s (%sms)", response.getStatus(), url, elapsedTime));
+    logResponseLine(url, response, elapsedTime);
 
     for (Header header : response.getHeaders()) {
       log.log(header.getName() + ": " + header.getValue());
@@ -414,7 +439,7 @@ public static class Builder {
     private Profiler profiler;
     private ErrorHandler errorHandler;
     private Log log;
-    private boolean debug;
+    private LogLevel logLevel = LogLevel.NONE;
 
     /** API server base URL. */
     public Builder setServer(String endpoint) {
@@ -522,9 +547,12 @@ public Builder setLog(Log log) {
       return this;
     }
 
-    /** Enable debug logging. */
-    public Builder setDebug(boolean debug) {
-      this.debug = debug;
+    /** Change the level of logging. */
+    public Builder setLogLevel(LogLevel logLevel) {
+      if (logLevel == null) {
+        throw new NullPointerException("Log level may not be null.");
+      }
+      this.logLevel = logLevel;
       return this;
     }
 
@@ -535,7 +563,7 @@ public RestAdapter build() {
       }
       ensureSaneDefaults();
       return new RestAdapter(server, clientProvider, httpExecutor, callbackExecutor,
-          requestInterceptor, converter, profiler, errorHandler, log, debug);
+          requestInterceptor, converter, profiler, errorHandler, log, logLevel);
     }
 
     private void ensureSaneDefaults() {


Test patch:
diff --git a/retrofit/src/test/java/retrofit/RestAdapterTest.java b/retrofit/src/test/java/retrofit/RestAdapterTest.java
index bf6d603f0b..1b5b13cdcf 100644
--- a/retrofit/src/test/java/retrofit/RestAdapterTest.java
+++ b/retrofit/src/test/java/retrofit/RestAdapterTest.java
@@ -10,12 +10,12 @@
 import java.util.concurrent.Executor;
 import org.junit.Before;
 import org.junit.Test;
-import retrofit.converter.ConversionException;
-import retrofit.http.GET;
 import retrofit.client.Client;
 import retrofit.client.Header;
 import retrofit.client.Request;
 import retrofit.client.Response;
+import retrofit.converter.ConversionException;
+import retrofit.http.GET;
 import retrofit.mime.TypedInput;
 import retrofit.mime.TypedString;
 
@@ -32,10 +32,27 @@
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 import static retrofit.Profiler.RequestInformation;
+import static retrofit.RestAdapter.LogLevel.BASIC;
+import static retrofit.RestAdapter.LogLevel.FULL;
 import static retrofit.Utils.SynchronousExecutor;
 
 public class RestAdapterTest {
-  private static List<Header> NO_HEADERS = Collections.emptyList();
+  private static final List<Header> NO_HEADERS = Collections.emptyList();
+
+  /** Not all servers play nice and add content-type headers to responses. */
+  private static final TypedInput NO_MIME_BODY = new TypedInput() {
+    @Override public String mimeType() {
+      return null;
+    }
+
+    @Override public long length() {
+      return 2;
+    }
+
+    @Override public InputStream in() throws IOException {
+      return new ByteArrayInputStream("{}".getBytes("UTF-8"));
+    }
+  };
 
   private interface Example {
     @GET("/") Object something();
@@ -85,7 +102,34 @@ private interface Example {
     verify(mockProfiler).afterCall(any(RequestInformation.class), anyInt(), eq(200), same(data));
   }
 
-  @Test public void logSuccessfulRequestResponseOnDebugWhenResponseBodyPresent() throws Exception {
+  @Test public void logRequestResponseBasic() throws Exception {
+    final List<String> logMessages = new ArrayList<String>();
+    RestAdapter.Log log = new RestAdapter.Log() {
+      public void log(String message) {
+        logMessages.add(message);
+      }
+    };
+
+    Example example = new RestAdapter.Builder() //
+        .setClient(mockClient)
+        .setExecutors(mockRequestExecutor, mockCallbackExecutor)
+        .setServer("http://example.com")
+        .setProfiler(mockProfiler)
+        .setLog(log)
+        .setLogLevel(BASIC)
+        .build()
+        .create(Example.class);
+
+    when(mockClient.execute(any(Request.class))) //
+        .thenReturn(new Response(200, "OK", NO_HEADERS, new TypedString("{}")));
+
+    example.something();
+    assertThat(logMessages).hasSize(2);
+    assertThat(logMessages.get(0)).isEqualTo("---> HTTP GET http://example.com/");
+    assertThat(logMessages.get(1)).matches("<--- HTTP 200 http://example.com/ \\([0-9]+ms\\)");
+  }
+
+  @Test public void logSuccessfulRequestResponseFullWhenResponseBodyPresent() throws Exception {
     final List<String> logMessages = new ArrayList<String>();
     RestAdapter.Log log = new RestAdapter.Log() {
       public void log(String message) {
@@ -99,7 +143,7 @@ public void log(String message) {
         .setServer("http://example.com")
         .setProfiler(mockProfiler)
         .setLog(log)
-        .setDebug(true)
+        .setLogLevel(FULL)
         .build()
         .create(Example.class);
 
@@ -115,7 +159,7 @@ public void log(String message) {
     assertThat(logMessages.get(4)).isEqualTo("<--- END HTTP (2-byte body)");
   }
 
-  @Test public void logSuccessfulRequestResponseOnDebugWhenResponseBodyAbsent() throws Exception {
+  @Test public void logSuccessfulRequestResponseFullWhenResponseBodyAbsent() throws Exception {
     final List<String> logMessages = new ArrayList<String>();
     RestAdapter.Log log = new RestAdapter.Log() {
       public void log(String message) {
@@ -129,7 +173,7 @@ public void log(String message) {
         .setServer("http://example.com")
         .setProfiler(mockProfiler)
         .setLog(log)
-        .setDebug(true)
+        .setLogLevel(FULL)
         .build()
         .create(Example.class);
 
@@ -144,30 +188,14 @@ public void log(String message) {
     assertThat(logMessages.get(3)).isEqualTo("<--- END HTTP (0-byte body)");
   }
 
-  /** Not all servers play nice and add content-type headers to responses. */
-  TypedInput inputMissingMimeType = new TypedInput() {
-
-    @Override public String mimeType() {
-      return null;
-    }
-
-    @Override public long length() {
-      return 2;
-    }
-
-    @Override public InputStream in() throws IOException {
-      return new ByteArrayInputStream("{}".getBytes());
-    }
-  };
-
   @Test public void successfulRequestResponseWhenMimeTypeMissing() throws Exception {
     when(mockClient.execute(any(Request.class))) //
-        .thenReturn(new Response(200, "OK", NO_HEADERS, inputMissingMimeType));
+        .thenReturn(new Response(200, "OK", NO_HEADERS, NO_MIME_BODY));
 
     example.something();
   }
 
-  @Test public void logSuccessfulRequestResponseOnDebugWhenMimeTypeMissing() throws Exception {
+  @Test public void logSuccessfulRequestResponseFullWhenMimeTypeMissing() throws Exception {
     final List<String> logMessages = new ArrayList<String>();
     RestAdapter.Log log = new RestAdapter.Log() {
       public void log(String message) {
@@ -181,12 +209,12 @@ public void log(String message) {
         .setServer("http://example.com")
         .setProfiler(mockProfiler)
         .setLog(log)
-        .setDebug(true)
+        .setLogLevel(FULL)
         .build()
         .create(Example.class);
 
     when(mockClient.execute(any(Request.class))) //
-        .thenReturn(new Response(200, "OK", NO_HEADERS, inputMissingMimeType));
+        .thenReturn(new Response(200, "OK", NO_HEADERS, NO_MIME_BODY));
 
     example.something();
     assertThat(logMessages).hasSize(5);
@@ -245,7 +273,7 @@ public void log(String message) {
     }
   }
 
-  @Test public void logErrorRequestResponseOnDebugWhenMimeTypeMissing() throws Exception {
+  @Test public void logErrorRequestResponseFullWhenMimeTypeMissing() throws Exception {
     final List<String> logMessages = new ArrayList<String>();
     RestAdapter.Log log = new RestAdapter.Log() {
       public void log(String message) {
@@ -259,12 +287,12 @@ public void log(String message) {
         .setServer("http://example.com")
         .setProfiler(mockProfiler)
         .setLog(log)
-        .setDebug(true)
+        .setLogLevel(FULL)
         .build()
         .create(Example.class);
 
     Response responseMissingMimeType = //
-        new Response(403, "Forbidden", NO_HEADERS, inputMissingMimeType);
+        new Response(403, "Forbidden", NO_HEADERS, NO_MIME_BODY);
 
     when(mockClient.execute(any(Request.class))).thenReturn(responseMissingMimeType);
 
@@ -283,7 +311,7 @@ public void log(String message) {
     assertThat(logMessages.get(4)).isEqualTo("<--- END HTTP (2-byte body)");
   }
 
-  @Test public void logErrorRequestResponseOnDebugWhenResponseBodyAbsent() throws Exception {
+  @Test public void logErrorRequestResponseFullWhenResponseBodyAbsent() throws Exception {
     final List<String> logMessages = new ArrayList<String>();
     RestAdapter.Log log = new RestAdapter.Log() {
       public void log(String message) {
@@ -297,7 +325,7 @@ public void log(String message) {
         .setServer("http://example.com")
         .setProfiler(mockProfiler)
         .setLog(log)
-        .setDebug(true)
+        .setLogLevel(FULL)
         .build()
         .create(Example.class);