Merge pull request #3 from perfx/tomcat-fix

Fix late getRequest and getResponse calls in Tomcat
SAP · Jul 11, 2018 · 50ece39 · 50ece39
2 parents 588889a + 711de83
commit 50ece39
Show file tree

Hide file tree

Showing 24 changed files with 534 additions and 419 deletions.
diff --git a/README.md b/README.md
@@ -27,7 +27,7 @@ All in all, you should do the following:
 * adjust your logging configuration accordingly.
 
 
-Say, you want to make use of the *servlet filter* feature, then you need to add the following dependency to your POM with property `cf-logging-version` referring to the latest nexus version (currently `2.2.0`):
+Say, you want to make use of the *servlet filter* feature, then you need to add the following dependency to your POM with property `cf-logging-version` referring to the latest nexus version (currently `2.2.1`):
 
 ```xml
 <properties>

diff --git a/cf-java-logging-support-core/pom.xml b/cf-java-logging-support-core/pom.xml
@@ -32,7 +32,7 @@
     <parent>
         <groupId>com.sap.hcp.cf.logging</groupId>
         <artifactId>cf-java-logging-support-parent</artifactId>
-        <version>2.2.0</version>
+        <version>2.2.1-SNAPSHOT</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <build>

diff --git a/...ogging-support-core/src/main/java/com/sap/hcp/cf/logging/common/RequestRecordBuilder.java b/...ogging-support-core/src/main/java/com/sap/hcp/cf/logging/common/RequestRecordBuilder.java
@@ -0,0 +1,59 @@
+package com.sap.hcp.cf.logging.common;
+
+import com.sap.hcp.cf.logging.common.RequestRecord.Direction;
+
+public class RequestRecordBuilder {
+
+	private final RequestRecord requestRecord;
+
+	private RequestRecordBuilder(String layerKey) {
+		this.requestRecord = new RequestRecord(layerKey);
+	}
+
+	private RequestRecordBuilder(String layerKey, Direction direction) {
+		this.requestRecord = new RequestRecord(layerKey, direction);
+	}
+
+	public static RequestRecordBuilder requestRecord(String layerKey) {
+		return new RequestRecordBuilder(layerKey);
+	}
+
+	public static RequestRecordBuilder requestRecord(String layerKey, Direction direction) {
+		return new RequestRecordBuilder(layerKey, direction);
+	}
+
+	public RequestRecord build() {
+		return requestRecord;
+	}
+
+	public RequestRecordBuilder addTag(String fieldKey, String tag) {
+		requestRecord.addTag(fieldKey, tag);
+		return this;
+	}
+
+	public RequestRecordBuilder addOptionalTag(boolean optionalFieldCanBeLogged, String fieldKey, String tag) {
+
+		if (!optionalFieldCanBeLogged && tag != null) {
+			requestRecord.addTag(fieldKey, Defaults.REDACTED);
+		}
+
+		if (!optionalFieldCanBeLogged && tag.equals(Defaults.UNKNOWN)) {
+			requestRecord.addTag(fieldKey, tag);
+		}
+
+		if (optionalFieldCanBeLogged) {
+			requestRecord.addTag(fieldKey, tag);
+		}
+		return this;
+	}
+
+	public RequestRecordBuilder addContextTag(String fieldKey, String tag) {
+		requestRecord.addContextTag(fieldKey, tag);
+		return this;
+	}
+
+	public RequestRecordBuilder addValue(String fieldKey, Value value) {
+		requestRecord.addValue(fieldKey, value);
+		return this;
+	}
+}
diff --git a/...g-support-core/src/main/java/com/sap/hcp/cf/logging/common/RequestRecordConfigurator.java b/...g-support-core/src/main/java/com/sap/hcp/cf/logging/common/RequestRecordConfigurator.java
diff --git a/...common/RequestRecordConfiguratorTest.java → ...ging/common/RequestRecordBuilderTest.java b/...common/RequestRecordConfiguratorTest.java → ...ging/common/RequestRecordBuilderTest.java
@@ -1,6 +1,6 @@
 package com.sap.hcp.cf.logging.common;
 
-import static com.sap.hcp.cf.logging.common.RequestRecordConfigurator.to;
+import static com.sap.hcp.cf.logging.common.RequestRecordBuilder.requestRecord;
 import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
@@ -10,52 +10,48 @@
 import com.fasterxml.jackson.jr.ob.JSON;
 import com.fasterxml.jackson.jr.ob.JSONObjectException;
 
-public class RequestRecordConfiguratorTest {
+public class RequestRecordBuilderTest {
 
     @Test
     public void testAddingSingleActivatedOptionalTagToRequestRecord() throws JSONObjectException, IOException {
-        RequestRecord requestRecord = new RequestRecord("TEST");
         boolean canBeLogged = true;
         String key = "TestKey";
         String tag = "TestTag";
 
-        to(requestRecord).addOptionalTag(canBeLogged, key, tag);
+		RequestRecord requestRecord = requestRecord("TEST").addOptionalTag(canBeLogged, key, tag).build();
 
         assertEquals(tag, getFieldFromRequestRecord(requestRecord, key));
     }
 
     @Test
     public void testAddingSingleForbiddenOptionalTagToRequestRecord() throws JSONObjectException, IOException {
-        RequestRecord requestRecord = new RequestRecord("TEST");
         boolean canBeLogged = false;
         String key = "TestKey";
         String tag = "TestTag";
 
-        to(requestRecord).addOptionalTag(canBeLogged, key, tag);
+		RequestRecord requestRecord = requestRecord("TEST").addOptionalTag(canBeLogged, key, tag).build();
 
         assertEquals(Defaults.REDACTED, getFieldFromRequestRecord(requestRecord, key));
     }
 
     @Test
     public void testAddingSingleForbiddenOptionalNullTagToRequestRecord() throws JSONObjectException, IOException {
-        RequestRecord requestRecord = new RequestRecord("TEST");
         boolean canBeLogged = false;
         String key = "TestKey";
         String tag = Defaults.UNKNOWN;
 
-        to(requestRecord).addOptionalTag(canBeLogged, key, tag);
+		RequestRecord requestRecord = requestRecord("TEST").addOptionalTag(canBeLogged, key, tag).build();
 
         assertEquals(Defaults.UNKNOWN, getFieldFromRequestRecord(requestRecord, key));
     }
 
     @Test
     public void testAddingSingleActivatedOptionalNullTagToRequestRecord() throws JSONObjectException, IOException {
-        RequestRecord requestRecord = new RequestRecord("TEST");
         boolean canBeLogged = true;
         String key = "TestKey";
         String tag = Defaults.UNKNOWN;
 
-        to(requestRecord).addOptionalTag(canBeLogged, key, tag);
+		RequestRecord requestRecord = requestRecord("TEST").addOptionalTag(canBeLogged, key, tag).build();
 
         assertEquals(Defaults.UNKNOWN, getFieldFromRequestRecord(requestRecord, key));
     }

diff --git a/cf-java-logging-support-jersey/pom.xml b/cf-java-logging-support-jersey/pom.xml
@@ -9,7 +9,7 @@
         <relativePath>../pom.xml</relativePath>
         <groupId>com.sap.hcp.cf.logging</groupId>
         <artifactId>cf-java-logging-support-parent</artifactId>
-        <version>2.2.0</version>
+        <version>2.2.1-SNAPSHOT</version>
     </parent>
 
     <name>cf-java-logging-support-jersey</name>

diff --git a/...ing-support-jersey/src/main/java/com/sap/hcp/cf/logging/jersey/filter/RequestHandler.java b/...ing-support-jersey/src/main/java/com/sap/hcp/cf/logging/jersey/filter/RequestHandler.java
@@ -1,7 +1,5 @@
 package com.sap.hcp.cf.logging.jersey.filter;
 
-import static com.sap.hcp.cf.logging.common.RequestRecordConfigurator.to;
-
 import java.net.URI;
 
 import com.sap.hcp.cf.logging.common.Defaults;
@@ -11,6 +9,7 @@
 import com.sap.hcp.cf.logging.common.LogOptionalFieldsSettings;
 import com.sap.hcp.cf.logging.common.LongValue;
 import com.sap.hcp.cf.logging.common.RequestRecord;
+import com.sap.hcp.cf.logging.common.RequestRecordBuilder;
 
 public class RequestHandler {
     final LogOptionalFieldsSettings logOptionalFieldsSettings;
@@ -38,38 +37,28 @@ public RequestRecord handle(RequestContextAdapter adapter) {
             }
         }
 
-        RequestRecord lrec = new RequestRecord(adapter.getName(), adapter.getDirection());
-        lrec.start();
-
-        addHeaders(adapter, lrec);
-
+		boolean isSensitiveConnectionData = logOptionalFieldsSettings.isLogSensitiveConnectionData();
+		boolean isLogRemoteUserField = logOptionalFieldsSettings.isLogRemoteUserField();
+		boolean isLogRefererField = logOptionalFieldsSettings.isLogRefererField();
+		RequestRecord lrec = RequestRecordBuilder.requestRecord(adapter.getName(), adapter.getDirection())
+				.addTag(Fields.REQUEST, getValue(getRequestUri(adapter)))
+				.addTag(Fields.METHOD, getValue(adapter.getMethod()))
+				.addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_IP, getValue(adapter.getUri().getAuthority()))
+				.addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_HOST, getValue(adapter.getUri().getHost()))
+				.addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_PORT,
+						Integer.toString(adapter.getUri().getPort()))
+				.addOptionalTag(isSensitiveConnectionData, Fields.X_FORWARDED_FOR,
+						getHeader(adapter, HttpHeaders.X_FORWARDED_FOR))
+				.addOptionalTag(isLogRemoteUserField, Fields.REMOTE_USER, getValue(adapter.getUser()))
+				.addOptionalTag(isLogRefererField, Fields.REFERER, getHeader(adapter, HttpHeaders.REFERER))
+				.addContextTag(Fields.REQUEST_ID, getHeader(adapter, HttpHeaders.X_VCAP_REQUEST_ID))
+				.addValue(Fields.REQUEST_SIZE_B, new LongValue(adapter.getRequestSize())).build();
+
+		lrec.start();
         return lrec;
 
     }
 
-    private void addHeaders(RequestContextAdapter adapter, RequestRecord lrec) {
-
-        lrec.addTag(Fields.REQUEST, getValue(getRequestUri(adapter)));
-        lrec.addTag(Fields.METHOD, getValue(adapter.getMethod()));
-
-        boolean isSensitiveConnectionData = logOptionalFieldsSettings.isLogSensitiveConnectionData();
-
-        to(lrec).addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_IP, getValue(adapter.getUri().getAuthority()))
-                .addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_HOST, getValue(adapter.getUri().getHost()))
-                .addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_PORT, Integer.toString(adapter.getUri()
-                                                                                                       .getPort()))
-                .addOptionalTag(isSensitiveConnectionData, Fields.X_FORWARDED_FOR, getHeader(adapter,
-                                                                                             HttpHeaders.X_FORWARDED_FOR))
-                .addOptionalTag(logOptionalFieldsSettings.isLogRemoteUserField(), Fields.REMOTE_USER, getValue(adapter
-                                                                                                                      .getUser()))
-                .addOptionalTag(logOptionalFieldsSettings.isLogRefererField(), Fields.REFERER, getHeader(adapter,
-                                                                                                         HttpHeaders.REFERER));
-        lrec.addContextTag(Fields.REQUEST_ID, getHeader(adapter, HttpHeaders.X_VCAP_REQUEST_ID));
-
-        lrec.addValue(Fields.REQUEST_SIZE_B, new LongValue(adapter.getRequestSize()));
-
-    }
-
     private String getValue(String value) {
         return value != null ? value : Defaults.UNKNOWN;
     }

diff --git a/cf-java-logging-support-log4j2/pom.xml b/cf-java-logging-support-log4j2/pom.xml
@@ -11,7 +11,7 @@
         <relativePath>../pom.xml</relativePath>
         <groupId>com.sap.hcp.cf.logging</groupId>
         <artifactId>cf-java-logging-support-parent</artifactId>
-        <version>2.2.0</version>
+        <version>2.2.1-SNAPSHOT</version>
     </parent>
 
     <dependencies>

diff --git a/cf-java-logging-support-logback/pom.xml b/cf-java-logging-support-logback/pom.xml
@@ -10,7 +10,7 @@
         <relativePath>../pom.xml</relativePath>
         <groupId>com.sap.hcp.cf.logging</groupId>
         <artifactId>cf-java-logging-support-parent</artifactId>
-        <version>2.2.0</version>
+        <version>2.2.1-SNAPSHOT</version>
     </parent>
 
     <dependencies>

diff --git a/cf-java-logging-support-servlet/pom.xml b/cf-java-logging-support-servlet/pom.xml
@@ -9,7 +9,7 @@
     <parent>
         <groupId>com.sap.hcp.cf.logging</groupId>
         <artifactId>cf-java-logging-support-parent</artifactId>
-        <version>2.2.0</version>
+        <version>2.2.1-SNAPSHOT</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
 

diff --git a/...-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/LoggingAsyncContextImpl.java b/...-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/LoggingAsyncContextImpl.java
@@ -11,56 +11,34 @@
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
 
 import org.slf4j.MDC;
 
 public class LoggingAsyncContextImpl implements AsyncContext {
 
 	private AsyncContext asyncContext;
 
-	public LoggingAsyncContextImpl(AsyncContext asyncContext, final RequestLoggingVisitor loggingVisitor) {
+	public LoggingAsyncContextImpl(AsyncContext asyncContext, final RequestLogger requestLogger) {
 		this.asyncContext = asyncContext;
 		asyncContext.addListener(new AsyncListener() {
 
 			@Override
 			public void onTimeout(AsyncEvent event) throws IOException {
-				generateLog(loggingVisitor);
+				requestLogger.logRequest();
 			}
 
-			private void generateLog(final RequestLoggingVisitor loggingVisitor) {
-				ServletRequest request = getRequest();
-				ServletResponse response = getResponse();
-				if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) {
-					HttpServletRequest httpRequest = (HttpServletRequest) request;
-					HttpServletResponse httpResponse = (HttpServletResponse) response;
-					Map<String, String> contextMap = getContextMap();
-					Map<String, String> currentContextMap = MDC.getCopyOfContextMap();
-					try {
-						MDC.setContextMap(contextMap);
-						loggingVisitor.logRequest(httpRequest, httpResponse);
-					} finally {
-						if (currentContextMap != null) {
-							MDC.setContextMap(currentContextMap);
-						}
-					}
-				}
-			}
-
-
 			@Override
 			public void onStartAsync(AsyncEvent event) throws IOException {
 			}
 
 			@Override
 			public void onError(AsyncEvent event) throws IOException {
-				generateLog(loggingVisitor);
+				requestLogger.logRequest();
 			}
 
 			@Override
 			public void onComplete(AsyncEvent event) throws IOException {
-				generateLog(loggingVisitor);
+				requestLogger.logRequest();
 			}
 		});
 	}

diff --git a/...let/src/main/java/com/sap/hcp/cf/logging/servlet/filter/LoggingContextRequestWrapper.java b/...let/src/main/java/com/sap/hcp/cf/logging/servlet/filter/LoggingContextRequestWrapper.java
@@ -8,9 +8,9 @@
 
 public class LoggingContextRequestWrapper extends HttpServletRequestWrapper {
 
-	private RequestLoggingVisitor loggingVisitor;
+	private RequestLogger loggingVisitor;
 
-	public LoggingContextRequestWrapper(HttpServletRequest request, RequestLoggingVisitor loggingVisitor) {
+	public LoggingContextRequestWrapper(HttpServletRequest request, RequestLogger loggingVisitor) {
 		super(request);
 		this.loggingVisitor = loggingVisitor;
 	}