Adds the ability to log the HTTP response status code to the log plugin.

pcornish · msavy · commit dd2e8336c15b · 2017-05-11T15:51:55.000+01:00
diff --git a/log-policy/src/main/apiman/plugin.json b/log-policy/src/main/apiman/plugin.json
@@ -1,6 +1,6 @@
 {
   "frameworkVersion" : 1.0,
   "name" : "Log Policy Plugin",
-  "description" : "This plugin has policies for logging to std out.",
+  "description" : "This plugin has policies for logging HTTP headers.",
   "version" : "${project.version}"
 }
diff --git a/log-policy/src/main/apiman/policyDefs/schemas/log-policyDef.schema b/log-policy/src/main/apiman/policyDefs/schemas/log-policyDef.schema
@@ -8,6 +8,18 @@
       "title": "Direction",
       "description": "You may choose to log either the Request headers, Response headers, or both.",
       "enum": [ "request", "response", "both" ]
+    },
+    "logHeaders": {
+      "title": "Log Headers",
+      "description": "Whether to log the HTTP headers.",
+      "type": "boolean",
+      "default": true
+    },
+    "logStatusCode": {
+      "title": "Log Status Code",
+      "description": "Whether to log the HTTP response status code.",
+      "type": "boolean",
+      "default": false
     }
   }
 }
diff --git a/log-policy/src/main/java/io/apiman/plugins/log_policy/LogHeadersPolicy.java b/log-policy/src/main/java/io/apiman/plugins/log_policy/LogHeadersPolicy.java
@@ -26,6 +26,7 @@
 import io.apiman.plugins.log_policy.beans.LogHeadersDirectionType;
 
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.TreeSet;
 
 /**
@@ -75,7 +76,7 @@ protected void doApply(ApiRequest request, IPolicyContext context, LogHeadersCon
         IApimanLogger logger = context.getLogger(getClass());
         String endpoint = request.getApi().getEndpoint();
         context.setAttribute(ENDPOINT_ATTRIBUTE, endpoint);
-        if (config.getDirection() != LogHeadersDirectionType.response) {
+        if (config.isLogHeaders() && config.getDirection() != LogHeadersDirectionType.response) {
             logHeaders(logger, request.getHeaders(), HttpDirection.REQUEST, endpoint);
         }
         chain.doApply(request);
@@ -90,26 +91,32 @@ protected void doApply(ApiResponse response, IPolicyContext context, LogHeadersC
         IApimanLogger logger = context.getLogger(getClass());
         if (config.getDirection() != LogHeadersDirectionType.request) {
             String endpoint = context.getAttribute(ENDPOINT_ATTRIBUTE, ""); //$NON-NLS-1$
-            logHeaders(logger, response.getHeaders(), HttpDirection.RESPONSE, endpoint);
+            if (config.isLogStatusCode()) {
+                logger.info(String.format("Status code %d for %s", response.getCode(), endpoint));
+            }
+            if (config.isLogHeaders()) {
+                logHeaders(logger, response.getHeaders(), HttpDirection.RESPONSE, endpoint);
+            }
         }
         chain.doApply(response);
     }
 
-	/**
+    /**
 	 * Logs the given headers to standard output.
 	 * @param logger 
 	 * @param headers
 	 * @param direction
 	 * @param endpoint
 	 */
 	private void logHeaders(IApimanLogger logger, final HeaderMap headers, final HttpDirection direction, final String endpoint) {
-        logger.info(String.format("Logging %d %s headers for %s", headers.size(), direction.getDescription(), endpoint)); //$NON-NLS-1$
-        TreeSet<String> sortedKeys = new TreeSet<>(headers.keySet());
+	    final StringBuilder sb = new StringBuilder();
+        sb.append(String.format("Logging %d %s headers for %s", headers.size(), direction.getDescription(), endpoint)); //$NON-NLS-1$
+        final Set<String> sortedKeys = new TreeSet<>(headers.keySet());
         for (String key : sortedKeys) {
             for (Entry<String, String> pair : headers.getAllEntries(key)) {
-                logger.info(String.format("Key : %s, Value : %s", pair.getKey(), pair.getValue())); //$NON-NLS-1$
+                sb.append(String.format("\r\nKey : %s, Value : %s", pair.getKey(), pair.getValue())); //$NON-NLS-1$
             }
         }
+        logger.info(sb.toString());
 	}
-
 }
diff --git a/log-policy/src/main/java/io/apiman/plugins/log_policy/beans/LogHeadersConfigBean.java b/log-policy/src/main/java/io/apiman/plugins/log_policy/beans/LogHeadersConfigBean.java
@@ -27,6 +27,19 @@ public class LogHeadersConfigBean {
     @JsonProperty
     private LogHeadersDirectionType direction;
 
+    /**
+     * Whether to log the request/response headers.
+     * Defaults to {@code true} for backwards compatibility.
+     */
+    @JsonProperty
+    private boolean logHeaders = true;
+
+    /**
+     * Whether to log the response status code.
+     */
+    @JsonProperty
+    private boolean logStatusCode;
+
     /**
      * Constructor.
      */
@@ -47,4 +60,17 @@ public void setDirection(LogHeadersDirectionType direction) {
         this.direction = direction;
     }
 
+    /**
+     * @return whether to log the headers
+     */
+    public boolean isLogHeaders() {
+        return logHeaders;
+    }
+
+    /**
+     * @return whether to log the status code
+     */
+    public boolean isLogStatusCode() {
+        return logStatusCode;
+    }
 }
diff --git a/log-policy/src/test/java/io/apiman/plugins/log_policy/LogHeadersPolicyTest.java b/log-policy/src/test/java/io/apiman/plugins/log_policy/LogHeadersPolicyTest.java
@@ -23,7 +23,7 @@ public class LogHeadersPolicyTest extends ApimanPolicyTest {
 	 * A simple happy flow test to verify the policy does not blow up in our face.
 	 */
 	@Test
-	@Configuration("{ \"direction\" : \"both\" }")
+	@Configuration("{ \"direction\" : \"both\", \"logStatusCode\" : true }")
 	public void testLogHeadersWithoutAnyRequestHeaders() throws PolicyFailureError, Throwable {
 	    PrintStream out = System.out;
 	    ByteArrayOutputStream testOutput = new ByteArrayOutputStream();
@@ -35,11 +35,12 @@ public void testLogHeadersWithoutAnyRequestHeaders() throws PolicyFailureError,
     		output = redactDates(output);
     		output = normalize(output);
     		String expected = "INFO: Logging 0 HTTP Request headers for io.apiman.test.policies.EchoBackEndApi\n" +
-    		        "INFO: Logging 4 HTTP Response headers for io.apiman.test.policies.EchoBackEndApi\n" +
-    		        "INFO: Key : Content-Length, Value : 167\n" +
-    		        "INFO: Key : Content-Type, Value : application/json\n" +
-    		        "INFO: Key : Date, Value : XXX\n" +
-    		        "INFO: Key : Server, Value : apiman.policy-test\n" +
+                    "INFO: Status code 200 for io.apiman.test.policies.EchoBackEndApi\n" +
+                    "INFO: Logging 4 HTTP Response headers for io.apiman.test.policies.EchoBackEndApi\n" +
+    		        "Key : Content-Length, Value : 167\n" +
+    		        "Key : Content-Type, Value : application/json\n" +
+    		        "Key : Date, Value : XXX\n" +
+    		        "Key : Server, Value : apiman.policy-test\n" +
     		        "";
     		Assert.assertEquals(expected, output);
         } finally {
@@ -51,7 +52,7 @@ public void testLogHeadersWithoutAnyRequestHeaders() throws PolicyFailureError,
 	 * A simple happy flow test to verify the policy does not blow up in our face.
 	 */
 	@Test
-    @Configuration("{ \"direction\" : \"both\" }")
+    @Configuration("{ \"direction\" : \"both\", \"logStatusCode\" : true }")
 	public void testLogHeadersHappyFlow() throws PolicyFailureError, Throwable {
         PrintStream out = System.out;
         ByteArrayOutputStream testOutput = new ByteArrayOutputStream();
@@ -66,12 +67,13 @@ public void testLogHeadersHappyFlow() throws PolicyFailureError, Throwable {
             output = redactDates(output);
             output = normalize(output);
             String expected = "INFO: Logging 1 HTTP Request headers for io.apiman.test.policies.EchoBackEndApi\n" +
-                    "INFO: Key : X-Test-Name, Value : testGet\n" +
+                    "Key : X-Test-Name, Value : testGet\n" +
+                    "INFO: Status code 200 for io.apiman.test.policies.EchoBackEndApi\n" +
                     "INFO: Logging 4 HTTP Response headers for io.apiman.test.policies.EchoBackEndApi\n" +
-                    "INFO: Key : Content-Length, Value : 199\n" +
-                    "INFO: Key : Content-Type, Value : application/json\n" +
-                    "INFO: Key : Date, Value : XXX\n" +
-                    "INFO: Key : Server, Value : apiman.policy-test\n" +
+                    "Key : Content-Length, Value : 199\n" +
+                    "Key : Content-Type, Value : application/json\n" +
+                    "Key : Date, Value : XXX\n" +
+                    "Key : Server, Value : apiman.policy-test\n" +
                     "";
             Assert.assertEquals(expected, output);
         } finally {
@@ -98,7 +100,7 @@ public void testLogHeadersHappyFlowRequestOnly() throws PolicyFailureError, Thro
             output = redactDates(output);
             output = normalize(output);
             String expected = "INFO: Logging 1 HTTP Request headers for io.apiman.test.policies.EchoBackEndApi\n" +
-                    "INFO: Key : X-Test-Name, Value : testGet\n" +
+                    "Key : X-Test-Name, Value : testGet\n" +
                     "";
             Assert.assertEquals(expected, output);
         } finally {
@@ -110,7 +112,7 @@ public void testLogHeadersHappyFlowRequestOnly() throws PolicyFailureError, Thro
      * A simple happy flow test to verify the policy does not blow up in our face.
      */
     @Test
-    @Configuration("{ \"direction\" : \"response\" }")
+    @Configuration("{ \"direction\" : \"response\", \"logStatusCode\" : true }")
     public void testLogHeadersHappyFlowResponseOnly() throws PolicyFailureError, Throwable {
         PrintStream out = System.out;
         ByteArrayOutputStream testOutput = new ByteArrayOutputStream();
@@ -124,11 +126,12 @@ public void testLogHeadersHappyFlowResponseOnly() throws PolicyFailureError, Thr
             String output = testOutput.toString("UTF-8");
             output = redactDates(output);
             output = normalize(output);
-            String expected = "INFO: Logging 4 HTTP Response headers for io.apiman.test.policies.EchoBackEndApi\n" +
-                    "INFO: Key : Content-Length, Value : 199\n" +
-                    "INFO: Key : Content-Type, Value : application/json\n" +
-                    "INFO: Key : Date, Value : XXX\n" +
-                    "INFO: Key : Server, Value : apiman.policy-test\n" +
+            String expected = "INFO: Status code 200 for io.apiman.test.policies.EchoBackEndApi\n" +
+                    "INFO: Logging 4 HTTP Response headers for io.apiman.test.policies.EchoBackEndApi\n" +
+                    "Key : Content-Length, Value : 199\n" +
+                    "Key : Content-Type, Value : application/json\n" +
+                    "Key : Date, Value : XXX\n" +
+                    "Key : Server, Value : apiman.policy-test\n" +
                     "";
             Assert.assertEquals(expected, output);
         } finally {

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"frameworkVersion" : 1.0,`
`3`	`3`	`"name" : "Log Policy Plugin",`
`4`		`- "description" : "This plugin has policies for logging to std out.",`
	`4`	`+ "description" : "This plugin has policies for logging HTTP headers.",`
`5`	`5`	`"version" : "${project.version}"`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,18 @@`
`8`	`8`	`"title": "Direction",`
`9`	`9`	`"description": "You may choose to log either the Request headers, Response headers, or both.",`
`10`	`10`	`"enum": [ "request", "response", "both" ]`
	`11`	`+ },`
	`12`	`+ "logHeaders": {`
	`13`	`+ "title": "Log Headers",`
	`14`	`+ "description": "Whether to log the HTTP headers.",`
	`15`	`+ "type": "boolean",`
	`16`	`+ "default": true`
	`17`	`+ },`
	`18`	`+ "logStatusCode": {`
	`19`	`+ "title": "Log Status Code",`
	`20`	`+ "description": "Whether to log the HTTP response status code.",`
	`21`	`+ "type": "boolean",`
	`22`	`+ "default": false`
`11`	`23`	`}`
`12`	`24`	`}`
`13`	`25`	`}`