Skip to content

Commit a7decb9

Browse files
KarstenSchnitterKarstenSchnitter
authored
Merge pull request #81 from SAP/request-id-support
Refactor Request-Id Handling
2 parents 872788c + 0d4f2eb commit a7decb9

File tree

7 files changed

+85
-56
lines changed

7 files changed

+85
-56
lines changed

‎cf-java-logging-support-core/src/main/java/com/sap/hcp/cf/logging/common/LogContext.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ public class LogContext {
2121
put(Fields.CORRELATION_ID, Defaults.UNKNOWN);
2222
put(Fields.TENANT_ID, Defaults.UNKNOWN);
2323
put(Fields.TENANT_SUBDOMAIN, Defaults.UNKNOWN);
24-
put(Fields.REQUEST_ID, null);
2524
put(Fields.COMPONENT_ID, Defaults.UNKNOWN);
2625
put(Fields.COMPONENT_NAME, Defaults.UNKNOWN);
2726
put(Fields.COMPONENT_TYPE, Defaults.COMPONENT_TYPE);

‎cf-java-logging-support-core/src/main/java/com/sap/hcp/cf/logging/common/request/HttpHeaders.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
public enum HttpHeaders implements HttpHeader {
1515

1616
CONTENT_LENGTH("content-length"), CONTENT_TYPE("content-type"), REFERER("referer"), X_FORWARDED_FOR(
17-
"x-forwarded-for"), X_VCAP_REQUEST_ID("x-vcap-request-id"), CORRELATION_ID("X-CorrelationID",
17+
"x-forwarded-for"), X_VCAP_REQUEST_ID("x-vcap-request-id", Fields.REQUEST_ID, true), CORRELATION_ID("X-CorrelationID",
1818
Fields.CORRELATION_ID, true, X_VCAP_REQUEST_ID), TENANT_ID("tenantid", Fields.TENANT_ID, true);
1919

2020
private HttpHeaders(String name) {

‎cf-java-logging-support-core/src/test/java/com/sap/hcp/cf/logging/common/request/HttpHeadersTest.java

Lines changed: 62 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,84 @@
11
package com.sap.hcp.cf.logging.common.request;
22

3+
import static org.hamcrest.MatcherAssert.assertThat;
34
import static org.hamcrest.Matchers.containsInAnyOrder;
45
import static org.hamcrest.Matchers.empty;
56
import static org.hamcrest.Matchers.equalTo;
67
import static org.hamcrest.Matchers.is;
78
import static org.hamcrest.Matchers.nullValue;
8-
import static org.junit.Assert.assertThat;
99

10+
import org.junit.Before;
1011
import org.junit.Test;
1112

1213
import com.sap.hcp.cf.logging.common.Defaults;
1314
import com.sap.hcp.cf.logging.common.Fields;
15+
import com.sap.hcp.cf.logging.common.LogContext;
1416

1517
public class HttpHeadersTest {
1618

17-
@Test
18-
public void hasCorrectNumberOfTypes() throws Exception {
19-
assertThat(HttpHeaders.values().length, is(equalTo(7)));
20-
}
19+
@Before
20+
public void resetLogContext() {
21+
LogContext.resetContextFields();
22+
HttpHeaders.propagated().stream().map(HttpHeader::getField).forEach(LogContext::remove);
23+
}
2124

22-
@Test
23-
public void hasCorrectNames() throws Exception {
24-
assertThat(HttpHeaders.CONTENT_LENGTH.getName(), is("content-length"));
25-
assertThat(HttpHeaders.CONTENT_TYPE.getName(), is("content-type"));
26-
assertThat(HttpHeaders.CORRELATION_ID.getName(), is("X-CorrelationID"));
27-
assertThat(HttpHeaders.REFERER.getName(), is("referer"));
28-
assertThat(HttpHeaders.TENANT_ID.getName(), is("tenantid"));
29-
assertThat(HttpHeaders.X_FORWARDED_FOR.getName(), is("x-forwarded-for"));
30-
assertThat(HttpHeaders.X_VCAP_REQUEST_ID.getName(), is("x-vcap-request-id"));
31-
}
25+
@Test
26+
public void hasCorrectNumberOfTypes() throws Exception {
27+
assertThat(HttpHeaders.values().length, is(equalTo(7)));
28+
}
3229

33-
@Test
34-
public void hasCorrectFields() throws Exception {
35-
assertThat(HttpHeaders.CONTENT_LENGTH.getField(), is(nullValue()));
36-
assertThat(HttpHeaders.CONTENT_TYPE.getField(), is(nullValue()));
37-
assertThat(HttpHeaders.CORRELATION_ID.getField(), is(Fields.CORRELATION_ID));
38-
assertThat(HttpHeaders.REFERER.getField(), is(nullValue()));
39-
assertThat(HttpHeaders.TENANT_ID.getField(), is(Fields.TENANT_ID));
40-
assertThat(HttpHeaders.X_FORWARDED_FOR.getField(), is(nullValue()));
41-
assertThat(HttpHeaders.X_VCAP_REQUEST_ID.getField(), is(nullValue()));
42-
}
30+
@Test
31+
public void hasCorrectNames() throws Exception {
32+
assertThat(HttpHeaders.CONTENT_LENGTH.getName(), is("content-length"));
33+
assertThat(HttpHeaders.CONTENT_TYPE.getName(), is("content-type"));
34+
assertThat(HttpHeaders.CORRELATION_ID.getName(), is("X-CorrelationID"));
35+
assertThat(HttpHeaders.REFERER.getName(), is("referer"));
36+
assertThat(HttpHeaders.TENANT_ID.getName(), is("tenantid"));
37+
assertThat(HttpHeaders.X_FORWARDED_FOR.getName(), is("x-forwarded-for"));
38+
assertThat(HttpHeaders.X_VCAP_REQUEST_ID.getName(), is("x-vcap-request-id"));
39+
}
4340

44-
@Test
45-
public void defaultFieldValueIsUnknownWithoutConfiguredField() throws Exception {
46-
assertThat(HttpHeaders.CONTENT_LENGTH.getFieldValue(), is(Defaults.UNKNOWN));
47-
assertThat(HttpHeaders.CONTENT_TYPE.getFieldValue(), is(Defaults.UNKNOWN));
48-
assertThat(HttpHeaders.REFERER.getFieldValue(), is(Defaults.UNKNOWN));
49-
assertThat(HttpHeaders.X_FORWARDED_FOR.getFieldValue(), is(Defaults.UNKNOWN));
50-
assertThat(HttpHeaders.X_VCAP_REQUEST_ID.getFieldValue(), is(Defaults.UNKNOWN));
51-
}
41+
@Test
42+
public void hasCorrectFields() throws Exception {
43+
assertThat(HttpHeaders.CONTENT_LENGTH.getField(), is(nullValue()));
44+
assertThat(HttpHeaders.CONTENT_TYPE.getField(), is(nullValue()));
45+
assertThat(HttpHeaders.CORRELATION_ID.getField(), is(Fields.CORRELATION_ID));
46+
assertThat(HttpHeaders.REFERER.getField(), is(nullValue()));
47+
assertThat(HttpHeaders.TENANT_ID.getField(), is(Fields.TENANT_ID));
48+
assertThat(HttpHeaders.X_FORWARDED_FOR.getField(), is(nullValue()));
49+
assertThat(HttpHeaders.X_VCAP_REQUEST_ID.getField(), is(Fields.REQUEST_ID));
50+
}
5251

53-
@Test
54-
public void hasCorrectAliases() throws Exception {
55-
assertThat(HttpHeaders.CONTENT_LENGTH.getAliases(), is(empty()));
56-
assertThat(HttpHeaders.CONTENT_TYPE.getAliases(), is(empty()));
57-
assertThat(HttpHeaders.CORRELATION_ID.getAliases(), containsInAnyOrder(HttpHeaders.X_VCAP_REQUEST_ID));
58-
assertThat(HttpHeaders.REFERER.getAliases(), is(empty()));
59-
assertThat(HttpHeaders.TENANT_ID.getAliases(), is(empty()));
60-
assertThat(HttpHeaders.X_FORWARDED_FOR.getAliases(), is(empty()));
61-
assertThat(HttpHeaders.X_VCAP_REQUEST_ID.getAliases(), is(empty()));
62-
}
52+
@Test
53+
public void defaultFieldValueIsUnknownWithoutConfiguredField() throws Exception {
54+
assertThat(HttpHeaders.CONTENT_LENGTH.getFieldValue(), is(Defaults.UNKNOWN));
55+
assertThat(HttpHeaders.CONTENT_TYPE.getFieldValue(), is(Defaults.UNKNOWN));
56+
assertThat(HttpHeaders.REFERER.getFieldValue(), is(Defaults.UNKNOWN));
57+
assertThat(HttpHeaders.X_FORWARDED_FOR.getFieldValue(), is(Defaults.UNKNOWN));
58+
}
6359

64-
@Test
65-
public void propagatesCorrectHeaders() throws Exception {
66-
assertThat(HttpHeaders.propagated(), containsInAnyOrder(HttpHeaders.CORRELATION_ID, HttpHeaders.TENANT_ID));
67-
}
60+
@Test
61+
public void defaultFieldValueIsNullForProgatedHeaders() throws Exception {
62+
for (HttpHeader header: HttpHeaders.propagated()) {
63+
assertThat("Default of field <" + header.getField() + "> from header <" + header.getName() +
64+
"> should be null", header.getFieldValue(), is(nullValue()));
65+
}
66+
}
67+
68+
@Test
69+
public void hasCorrectAliases() throws Exception {
70+
assertThat(HttpHeaders.CONTENT_LENGTH.getAliases(), is(empty()));
71+
assertThat(HttpHeaders.CONTENT_TYPE.getAliases(), is(empty()));
72+
assertThat(HttpHeaders.CORRELATION_ID.getAliases(), containsInAnyOrder(HttpHeaders.X_VCAP_REQUEST_ID));
73+
assertThat(HttpHeaders.REFERER.getAliases(), is(empty()));
74+
assertThat(HttpHeaders.TENANT_ID.getAliases(), is(empty()));
75+
assertThat(HttpHeaders.X_FORWARDED_FOR.getAliases(), is(empty()));
76+
assertThat(HttpHeaders.X_VCAP_REQUEST_ID.getAliases(), is(empty()));
77+
}
78+
79+
@Test
80+
public void propagatesCorrectHeaders() throws Exception {
81+
assertThat(HttpHeaders.propagated(), containsInAnyOrder(HttpHeaders.CORRELATION_ID, HttpHeaders.TENANT_ID, HttpHeaders.X_VCAP_REQUEST_ID));
82+
}
6883

6984
}

‎cf-java-logging-support-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/RequestLogger.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public void logRequest() {
4242
requestRecord.stop();
4343
addRequestHandlingParameters();
4444
generateLog();
45+
requestRecord.resetContext();
4546
}
4647

4748
private void addRequestHandlingParameters() {

‎cf-java-logging-support-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/RequestLoggingFilter.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import com.sap.hcp.cf.logging.common.LogContext;
1717
import com.sap.hcp.cf.logging.common.LogOptionalFieldsSettings;
18+
import com.sap.hcp.cf.logging.common.request.HttpHeader;
1819
import com.sap.hcp.cf.logging.common.request.HttpHeaders;
1920
import com.sap.hcp.cf.logging.common.request.RequestRecord;
2021
import com.sap.hcp.cf.logging.servlet.dynlog.DynLogEnvironment;
@@ -144,10 +145,19 @@ private void doFilterRequest(HttpServletRequest httpRequest, HttpServletResponse
144145
if (dynamicLogLevelProcessor != null) {
145146
dynamicLogLevelProcessor.removeDynamicLogLevelFromMDC();
146147
}
147-
LogContext.resetContextFields();
148+
resetLogContext();
148149
}
149150
}
150151

152+
153+
private void resetLogContext() {
154+
for (HttpHeader header : HttpHeaders.propagated()) {
155+
LogContext.remove(header.getField());
156+
}
157+
LogContext.resetContextFields();
158+
}
159+
160+
151161
private void doFilter(FilterChain chain, HttpServletRequest httpRequest, HttpServletResponse httpResponse)
152162
throws IOException, ServletException {
153163
if (chain != null) {

‎cf-java-logging-support-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/RequestRecordFactory.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public RequestRecord create(HttpServletRequest request) {
2828
RequestRecordBuilder rrb = requestRecord("[SERVLET]").addTag(Fields.REQUEST, getFullRequestUri(request))
2929
.addTag(Fields.METHOD, request.getMethod())
3030
.addTag(Fields.PROTOCOL, getValue(request.getProtocol()))
31-
.addContextTag(Fields.REQUEST_ID, getHeader(request, HttpHeaders.X_VCAP_REQUEST_ID))
3231
.addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_IP, getValue(request.getRemoteAddr()))
3332
.addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_HOST, getValue(request.getRemoteHost()))
3433
.addOptionalTag(isSensitiveConnectionData, Fields.REMOTE_PORT,
@@ -38,7 +37,7 @@ public RequestRecord create(HttpServletRequest request) {
3837
.addOptionalTag(isLogRemoteUserField, Fields.REMOTE_USER, getValue(request.getRemoteUser()))
3938
.addOptionalTag(isLogRefererField, Fields.REFERER, getHeader(request, HttpHeaders.REFERER));
4039
for (HttpHeader header : HttpHeaders.propagated()) {
41-
rrb.addContextTag(header.getField(), getHeader(request, header));
40+
rrb.addContextTag(header.getField(), getHeaderValue(request, header, null));
4241
}
4342
return rrb.build();
4443
}

‎cf-java-logging-support-servlet/src/test/java/com/sap/hcp/cf/logging/servlet/filter/RequestLoggingFilterTest.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.sap.hcp.cf.logging.servlet.filter;
22

3+
import static org.hamcrest.Matchers.nullValue;
34
import static org.hamcrest.core.Is.is;
45
import static org.hamcrest.core.IsNot.not;
56
import static org.hamcrest.text.IsEmptyString.isEmptyOrNullString;
@@ -24,6 +25,7 @@
2425
import javax.servlet.http.HttpServletRequest;
2526
import javax.servlet.http.HttpServletResponse;
2627

28+
import org.hamcrest.Matchers;
2729
import org.junit.Before;
2830
import org.junit.Rule;
2931
import org.junit.Test;
@@ -84,10 +86,11 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
8486
@Test
8587
public void testSimple() throws IOException, ServletException {
8688
FilterChain mockFilterChain = mock(FilterChain.class);
89+
8790
new RequestLoggingFilter().doFilter(mockReq, mockResp, mockFilterChain);
8891
assertThat(getField(Fields.REQUEST), is(Defaults.UNKNOWN));
8992
assertThat(getField(Fields.CORRELATION_ID), not(isEmptyOrNullString()));
90-
assertThat(getField(Fields.REQUEST_ID), is(Defaults.UNKNOWN));
93+
assertThat(getField(Fields.REQUEST_ID), is(nullValue()));
9194
assertThat(getField(Fields.REMOTE_HOST), is(Defaults.UNKNOWN));
9295
assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN));
9396
assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN));
@@ -110,7 +113,7 @@ public void doFilter(ServletRequest request, ServletResponse response)
110113
new RequestLoggingFilter().doFilter(mockReq, mockResp, mockFilterChain);
111114
assertThat(getField(Fields.REQUEST), is(Defaults.UNKNOWN));
112115
assertThat(getField(Fields.CORRELATION_ID), not(isEmptyOrNullString()));
113-
assertThat(getField(Fields.REQUEST_ID), is(Defaults.UNKNOWN));
116+
assertThat(getField(Fields.REQUEST_ID), is(nullValue()));
114117
assertThat(getField(Fields.REMOTE_HOST), is(Defaults.UNKNOWN));
115118
assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN));
116119
assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN));
@@ -132,7 +135,7 @@ public void doFilter(ServletRequest request, ServletResponse response)
132135
new RequestLoggingFilter().doFilter(mockReq, mockResp, mockFilterChain);
133136
assertThat(getField(Fields.REQUEST), is(Defaults.UNKNOWN));
134137
assertThat(getField(Fields.CORRELATION_ID), not(isEmptyOrNullString()));
135-
assertThat(getField(Fields.REQUEST_ID), is(Defaults.UNKNOWN));
138+
assertThat(getField(Fields.REQUEST_ID), is(nullValue()));
136139
assertThat(getField(Fields.REMOTE_HOST), is(Defaults.UNKNOWN));
137140
assertThat(getField(Fields.COMPONENT_ID), is(Defaults.UNKNOWN));
138141
assertThat(getField(Fields.CONTAINER_ID), is(Defaults.UNKNOWN));
@@ -204,6 +207,7 @@ public void testExplicitCorrelationId() throws IOException, ServletException {
204207
new RequestLoggingFilter().doFilter(mockReq, mockResp, mockFilterChain);
205208
assertThat(getField(Fields.CORRELATION_ID), is(CORRELATION_ID));
206209
assertThat(getField(Fields.CORRELATION_ID), not(REQUEST_ID));
210+
assertThat(getField(Fields.REQUEST_ID), is(REQUEST_ID));
207211
assertThat(getField(Fields.TENANT_ID), is(Defaults.UNKNOWN));
208212
}
209213

@@ -217,7 +221,8 @@ public void testExplicitTenantId() throws IOException, ServletException {
217221
}
218222

219223
protected String getField(String fieldName) throws JSONObjectException, IOException {
220-
return JSON.std.mapFrom(getLastLine()).get(fieldName).toString();
224+
Object fieldValue = JSON.std.mapFrom(getLastLine()).get(fieldName);
225+
return fieldValue == null ? null : fieldValue.toString();
221226
}
222227

223228
private String getLastLine() {

0 commit comments

Comments
 (0)