This is an automated email from the ASF dual-hosted git repository.
more pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new fcaf700 KNOX-2705 - Add correlattion id to logs as trace_id (#536)
fcaf700 is described below
commit fcaf700a205b9faa3b02b5faf1243d702c09efcc
Author: Sandeep Moré <[email protected]>
AuthorDate: Wed Feb 16 11:26:18 2022 -0500
KNOX-2705 - Add correlattion id to logs as trace_id (#536)
---
gateway-release/home/conf/gateway-log4j2.xml | 2 +-
.../org/apache/knox/gateway/GatewayFilter.java | 13 +-
.../knox/gateway/filter/CorrelationHandler.java | 14 ++-
gateway-spi/pom.xml | 5 +
.../gateway/dispatch/AbstractGatewayDispatch.java | 17 +++
.../gateway/dispatch/ConfigurableDispatchTest.java | 132 +++++++++++++++++++++
6 files changed, 172 insertions(+), 11 deletions(-)
diff --git a/gateway-release/home/conf/gateway-log4j2.xml
b/gateway-release/home/conf/gateway-log4j2.xml
index ef62df5..b2ba1d6 100644
--- a/gateway-release/home/conf/gateway-log4j2.xml
+++ b/gateway-release/home/conf/gateway-log4j2.xml
@@ -32,7 +32,7 @@
</Console>
<RollingFile name="drfa" fileName="${app.log.dir}/${app.log.file}"
filePattern="${app.log.dir}/${app.log.file}.%d{yyyy-MM-dd}">
<!-- Same as ISO8601 format but without the 'T' (log4j1
compatible) -->
- <PatternLayout pattern="%d{yyyy-MM-dd' 'HH:mm:ss,SSS} %-5p %c{2}
(%F:%M(%L)) - %m%n" />
+ <PatternLayout pattern="%d{yyyy-MM-dd' 'HH:mm:ss,SSS} %X{trace_id}
%-5p %c{2} (%F:%M(%L)) - %m%n" />
<TimeBasedTriggeringPolicy />
</RollingFile>
<!-- <RollingFile name="httpclient"
fileName="${app.log.dir}/${launcher.name}-http-client.log"
filePattern="${app.log.dir}/${launcher.name}-http-client.log.%d{yyyy-MM-dd}">-->
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
index 0667c5a..73ed23f 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
@@ -154,12 +154,13 @@ public class GatewayFilter implements Filter {
}
/* If request contains X-Request-Id header use it else use random uuid as
correlation id */
- final String reqID = ((HttpServletRequest)
servletRequest).getHeader(REQUEST_ID_HEADER_NAME);
- if (StringUtils.isBlank(reqID)) {
- assignCorrelationRequestId(UUID.randomUUID().toString());
- } else {
- assignCorrelationRequestId(reqID);
- }
+ final String reqID =
+ StringUtils.isBlank(((HttpServletRequest)
servletRequest).getHeader(REQUEST_ID_HEADER_NAME)) ?
+ UUID.randomUUID().toString() :
+ ((HttpServletRequest)
servletRequest).getHeader(REQUEST_ID_HEADER_NAME);
+
+ assignCorrelationRequestId(reqID);
+
// Populate Audit/correlation parameters
AuditContext auditContext = auditService.getContext();
if(auditContext == null) {
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
b/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
index ba5b1ff..69f84ee 100644
---
a/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
+++
b/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
@@ -27,22 +27,28 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.knox.gateway.audit.api.CorrelationService;
import org.apache.knox.gateway.audit.api.CorrelationServiceFactory;
import org.apache.knox.gateway.audit.log4j.correlation.Log4jCorrelationContext;
+import org.apache.logging.log4j.CloseableThreadContext;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.HandlerWrapper;
public class CorrelationHandler extends HandlerWrapper {
public static final String REQUEST_ID_HEADER_NAME = "X-Request-Id";
+ public static final String TRACE_ID = "trace_id";
@Override
public void handle( String target, Request baseRequest, HttpServletRequest
request, HttpServletResponse response )
throws IOException, ServletException {
CorrelationService correlationService =
CorrelationServiceFactory.getCorrelationService();
- /* If request id is specified use it */
- final String req_id = request.getHeader(REQUEST_ID_HEADER_NAME);
+ /* If request contains X-Request-Id header use it else use random uuid as
correlation id */
+ final String reqID =
+ StringUtils.isBlank(request.getHeader(REQUEST_ID_HEADER_NAME)) ?
+ UUID.randomUUID().toString() :
+ request.getHeader(REQUEST_ID_HEADER_NAME);
+
correlationService.attachContext(
- new Log4jCorrelationContext(StringUtils.isBlank(req_id) ?
UUID.randomUUID().toString() : req_id,
+ new Log4jCorrelationContext(reqID,
null, null));
- try {
+ try(CloseableThreadContext.Instance ctc =
CloseableThreadContext.put(TRACE_ID, reqID)) {
super.handle( target, baseRequest, request, response );
} finally {
correlationService.detachContext();
diff --git a/gateway-spi/pom.xml b/gateway-spi/pom.xml
index 50a9eca..63129b1 100644
--- a/gateway-spi/pom.xml
+++ b/gateway-spi/pom.xml
@@ -159,6 +159,11 @@
</dependency>
<dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-api</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>de.thetaphi</groupId>
<artifactId>forbiddenapis</artifactId>
</dependency>
diff --git
a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/AbstractGatewayDispatch.java
b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/AbstractGatewayDispatch.java
index 8e6a40f..3a4a2a0 100644
---
a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/AbstractGatewayDispatch.java
+++
b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/AbstractGatewayDispatch.java
@@ -18,9 +18,11 @@
package org.apache.knox.gateway.dispatch;
import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
import org.apache.knox.gateway.filter.GatewayResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.logging.log4j.ThreadContext;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -37,6 +39,8 @@ import java.util.Set;
public abstract class AbstractGatewayDispatch implements Dispatch {
private static final Set<String> REQUEST_EXCLUDE_HEADERS = new
HashSet<>(Arrays.asList(
"Host", "Authorization", "Content-Length", "Transfer-Encoding"));
+ protected static final String REQUEST_ID_HEADER_NAME = "X-Request-Id";
+ protected static final String TRACE_ID = "trace_id";
protected HttpClient client;
@@ -124,6 +128,19 @@ public abstract class AbstractGatewayDispatch implements
Dispatch {
public void copyRequestHeaderFields(HttpUriRequest outboundRequest,
HttpServletRequest inboundRequest) {
Enumeration<String> headerNames = inboundRequest.getHeaderNames();
+ /**
+ Add X-Request-Id headers to outgoing requests.
+ Only do this when
+ 1. incoming request does not have X-Request-Id header (if it has no need
to add)
+ 2. This header is not in exclude header list
+ 3. This header is present in the MDC context
+ **/
+ if(StringUtils.isBlank(inboundRequest.getHeader(REQUEST_ID_HEADER_NAME)) &&
+ !getOutboundRequestExcludeHeaders().contains( REQUEST_ID_HEADER_NAME )
&&
+ ThreadContext.containsKey(TRACE_ID)) {
+ outboundRequest.addHeader( REQUEST_ID_HEADER_NAME,
ThreadContext.get(TRACE_ID));
+ }
+
while( headerNames.hasMoreElements() ) {
String name = headerNames.nextElement();
if ( !outboundRequest.containsHeader( name )
diff --git
a/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java
b/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java
index f741347..8f38fd5 100644
---
a/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java
+++
b/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java
@@ -17,10 +17,12 @@
*/
package org.apache.knox.gateway.dispatch;
+import static
org.apache.knox.gateway.dispatch.AbstractGatewayDispatch.REQUEST_ID_HEADER_NAME;
import static org.apache.knox.gateway.dispatch.DefaultDispatch.SET_COOKIE;
import static
org.apache.knox.gateway.dispatch.DefaultDispatch.WWW_AUTHENTICATE;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;
import java.net.URI;
@@ -28,6 +30,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.UUID;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -40,11 +43,14 @@ import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.message.BasicHeader;
import org.apache.knox.test.TestUtils;
import org.apache.knox.test.mock.MockHttpServletResponse;
+import org.apache.logging.log4j.CloseableThreadContext;
import org.easymock.Capture;
import org.easymock.EasyMock;
import org.junit.Test;
public class ConfigurableDispatchTest {
+ public final String TRACE_ID = "trace_id";
+
@Test( timeout = TestUtils.SHORT_TIMEOUT )
public void testGetDispatchUrl() {
HttpServletRequest request;
@@ -592,4 +598,130 @@ public class ConfigurableDispatchTest {
assertThat(outboundResponse.getHeader(SET_COOKIE),
containsString("hadoop.auth="));
}
+ /**
+ * Test the case where the incoming request to Knox does not
+ * have X-Request-Id header.
+ * Expected outcome is that correlation id will be used
+ * as X-Request-Id value for outgoing requests.
+ */
+ @Test
+ public void testCorrelationIDXRequestIDHeader() {
+ ConfigurableDispatch dispatch = new ConfigurableDispatch();
+ final String reqID = UUID.randomUUID().toString();
+
+ Map<String, String> headers = new HashMap<>();
+ headers.put(HttpHeaders.ACCEPT, "abc");
+ headers.put("TEST", "test");
+
+ HttpServletRequest inboundRequest =
EasyMock.createNiceMock(HttpServletRequest.class);
+
EasyMock.expect(inboundRequest.getHeaderNames()).andReturn(Collections.enumeration(headers.keySet())).anyTimes();
+ Capture<String> capturedArgument = Capture.newInstance();
+
EasyMock.expect(inboundRequest.getHeader(EasyMock.capture(capturedArgument)))
+ .andAnswer(() -> headers.get(capturedArgument.getValue())).anyTimes();
+ EasyMock.replay(inboundRequest);
+
+ HttpUriRequest outboundRequest = new HttpGet();
+ try(CloseableThreadContext.Instance ctc =
CloseableThreadContext.put(TRACE_ID, reqID)) {
+ dispatch.copyRequestHeaderFields(outboundRequest, inboundRequest);
+ }
+
+ Header[] outboundRequestHeaders = outboundRequest.getAllHeaders();
+ assertThat(outboundRequestHeaders.length, is(3));
+
assertThat(outboundRequest.getHeaders(REQUEST_ID_HEADER_NAME)[0].getValue(),
is(reqID));
+ }
+
+ /**
+ * Test the case where the incoming request to Knox comtains
+ * X-Request-Id header.
+ * Expected outcome is that correlation id will NOT be used
+ * as X-Request-Id value instead the X-Request-Id value
+ * coming from the inbound request will be used.
+ */
+ @Test
+ public void testRequestHeaderXRequestID() {
+ ConfigurableDispatch dispatch = new ConfigurableDispatch();
+ final String reqID = UUID.randomUUID().toString();
+ final String headerReqID = "1234567890ABCD";
+
+ Map<String, String> headers = new HashMap<>();
+ headers.put(REQUEST_ID_HEADER_NAME, headerReqID);
+ headers.put(HttpHeaders.ACCEPT, "abc");
+ headers.put("TEST", "test");
+
+ HttpServletRequest inboundRequest =
EasyMock.createNiceMock(HttpServletRequest.class);
+
EasyMock.expect(inboundRequest.getHeaderNames()).andReturn(Collections.enumeration(headers.keySet())).anyTimes();
+ Capture<String> capturedArgument = Capture.newInstance();
+
EasyMock.expect(inboundRequest.getHeader(EasyMock.capture(capturedArgument)))
+ .andAnswer(() -> headers.get(capturedArgument.getValue())).anyTimes();
+ EasyMock.replay(inboundRequest);
+
+ HttpUriRequest outboundRequest = new HttpGet();
+ try(CloseableThreadContext.Instance ctc =
CloseableThreadContext.put(TRACE_ID, reqID)) {
+ dispatch.copyRequestHeaderFields(outboundRequest, inboundRequest);
+ }
+
+ Header[] outboundRequestHeaders = outboundRequest.getAllHeaders();
+ assertThat(outboundRequestHeaders.length, is(3));
+
assertThat(outboundRequest.getHeaders(REQUEST_ID_HEADER_NAME)[0].getValue(),
is(headerReqID));
+ }
+
+ /**
+ * Make sure X-Request-Id header is not added when it is configured
+ * in exclude header list.
+ * This test case tests case where X-Request-Id is passed from inbound
request.
+ */
+ @Test
+ public void testXRequestIDHeaderExcludeList() {
+ ConfigurableDispatch dispatch = new ConfigurableDispatch();
+ dispatch.setResponseExcludeHeaders(String.join(",", Arrays.asList("test",
REQUEST_ID_HEADER_NAME)));
+
+ Header[] headers = new Header[]{
+ new BasicHeader(REQUEST_ID_HEADER_NAME, UUID.randomUUID().toString()),
+ new BasicHeader(WWW_AUTHENTICATE, "negotiate"),
+ new BasicHeader("TEST", "testValue"),
+ };
+
+ HttpResponse inboundResponse = EasyMock.createNiceMock(HttpResponse.class);
+
EasyMock.expect(inboundResponse.getAllHeaders()).andReturn(headers).anyTimes();
+ EasyMock.replay(inboundResponse);
+
+ HttpServletResponse outboundResponse = new MockHttpServletResponse();
+ try(CloseableThreadContext.Instance ctc =
CloseableThreadContext.put(TRACE_ID, UUID.randomUUID().toString())) {
+ dispatch.copyResponseHeaderFields(outboundResponse, inboundResponse);
+ }
+
+ assertThat(outboundResponse.getHeaderNames().size(), is(1));
+ assertThat(outboundResponse.getHeader(WWW_AUTHENTICATE), is("negotiate"));
+ assertThat(outboundResponse.getHeader(REQUEST_ID_HEADER_NAME),
nullValue());
+ }
+
+ /**
+ * Make sure X-Request-Id header is not added when it is configured
+ * in exclude header list.
+ * This test case tests case where no-request id passed from inbound request.
+ */
+ @Test
+ public void testXRequestIDHeaderExcludeListNoReqHeader() {
+ ConfigurableDispatch dispatch = new ConfigurableDispatch();
+ dispatch.setResponseExcludeHeaders(String.join(",", Arrays.asList("test",
REQUEST_ID_HEADER_NAME)));
+
+ Header[] headers = new Header[]{
+ new BasicHeader(WWW_AUTHENTICATE, "negotiate"),
+ new BasicHeader("TEST", "testValue"),
+ };
+
+ HttpResponse inboundResponse = EasyMock.createNiceMock(HttpResponse.class);
+
EasyMock.expect(inboundResponse.getAllHeaders()).andReturn(headers).anyTimes();
+ EasyMock.replay(inboundResponse);
+
+ HttpServletResponse outboundResponse = new MockHttpServletResponse();
+ try(CloseableThreadContext.Instance ctc =
CloseableThreadContext.put(TRACE_ID, UUID.randomUUID().toString())) {
+ dispatch.copyResponseHeaderFields(outboundResponse, inboundResponse);
+ }
+
+ assertThat(outboundResponse.getHeaderNames().size(), is(1));
+ assertThat(outboundResponse.getHeader(WWW_AUTHENTICATE), is("negotiate"));
+ assertThat(outboundResponse.getHeader(REQUEST_ID_HEADER_NAME),
nullValue());
+ }
+
}
