Skip to content

Commit dc564b3

Browse files
ChristophEichhornChristophEichhorn
committed
Make sure request records are closed
Change-Id: I5ee991d101fe2c6283cf1f6aa9a80a00052a7ce5
1 parent a4c9cc8 commit dc564b3

File tree

5 files changed

+64
-49
lines changed

5 files changed

+64
-49
lines changed

‎cf-java-logging-support-core/src/test/java/com/sap/hcp/cf/logging/common/converter/TestJsonMessageConverter.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public void testLogRecordMsgNotFlattened() {
5858
RequestRecord lrec = new RequestRecord(LOG_PROVIDER);
5959
String lmsg = lrec.toString();
6060
assertThat(formatMsg(jmc, lmsg), is(lmsg));
61+
lrec.close();
6162
}
6263

6364
@Test
@@ -67,6 +68,7 @@ public void testLogRecordMsgFlattened() {
6768
RequestRecord lrec = new RequestRecord(LOG_PROVIDER);
6869
String lmsg = lrec.toString();
6970
assertThat(formatMsg(jmc, lmsg), is(lmsg.substring(1, lmsg.length() - 1)));
71+
lrec.close();
7072
}
7173

7274
@Test

‎cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/TestJsonMessageConverter.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public void testLogRecordMsgNotFlattened() {
5656
RequestRecord lrec = new RequestRecord(LOG_PROVIDER);
5757
String lmsg = lrec.toString();
5858
assertThat(format(jmc, makeEvent(lmsg, NO_ARGS)), is(lmsg));
59+
lrec.close();
5960
}
6061

6162
@Test
@@ -64,6 +65,7 @@ public void testLogRecordMsgFlattened() {
6465
RequestRecord lrec = new RequestRecord(LOG_PROVIDER);
6566
String lmsg = lrec.toString();
6667
assertThat(format(jmc, makeEvent(lmsg, NO_ARGS)), is(lmsg.substring(1, lmsg.length() - 1)));
68+
lrec.close();
6769
}
6870

6971
@Test

‎cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/perf/PerfTestRequestRecord.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,13 @@ public void write(int b) throws IOException {
3838
}
3939
}));
4040
for (int i = 0; i < iterations; i++) {
41-
RequestRecord rrec = new RequestRecord(PerfTestRequestRecord.class.getName());
42-
LOGGER.info(Markers.REQUEST_MARKER, rrec.toString());
41+
RequestRecord rrec = null;
42+
try {
43+
rrec = new RequestRecord(PerfTestRequestRecord.class.getName());
44+
LOGGER.info(Markers.REQUEST_MARKER, rrec.toString());
45+
} finally {
46+
rrec.close();
47+
}
4348
}
4449
double delta = (System.nanoTime() - start) / 1000000.0;
4550
System.setOut(defOut);

‎cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logback/converter/TestJsonMessageConverter.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public void testLogRecordMsgNotFlattened() {
6666
RequestRecord lrec = new RequestRecord(LOG_PROVIDER);
6767
String lmsg = lrec.toString();
6868
assertThat(jmc.convert(makeEvent(lmsg, NO_ARGS)), is(lmsg));
69+
lrec.close();
6970
}
7071

7172
@Test
@@ -76,6 +77,7 @@ public void testLogRecordMsgFlattened() {
7677
RequestRecord lrec = new RequestRecord(LOG_PROVIDER);
7778
String lmsg = lrec.toString();
7879
assertThat(jmc.convert(makeEvent(lmsg, NO_ARGS)), is(lmsg.substring(1, lmsg.length() - 1)));
80+
lrec.close();
7981
}
8082

8183
@Test

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

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -74,59 +74,63 @@ private void doFilterRequest(HttpServletRequest httpRequest, HttpServletResponse
7474
*/
7575
LogContext.initializeContext(getCorrelationIdFromHeader(httpRequest));
7676

77-
RequestRecord rr = new RequestRecord(LOG_PROVIDER);
78-
ContentLengthTrackingResponseWrapper responseWrapper = null;
79-
ContentLengthTrackingRequestWrapper requestWrapper = null;
80-
81-
/*
82-
* -- we essentially do three things here: -- a) we create a log record
83-
* using our library and log it via STDOUT -- b) keep track of certain
84-
* header fields so that they are available in later processing steps --
85-
* b) inject a response wrapper to keep track of content length
86-
* (hopefully)
87-
*/
88-
if (wrapResponse) {
89-
responseWrapper = new ContentLengthTrackingResponseWrapper(httpResponse);
90-
}
91-
if (wrapRequest) {
77+
RequestRecord rr = null;
78+
try {
79+
rr = new RequestRecord(LOG_PROVIDER);
80+
ContentLengthTrackingResponseWrapper responseWrapper = null;
81+
ContentLengthTrackingRequestWrapper requestWrapper = null;
82+
83+
/*
84+
* -- we essentially do three things here: -- a) we create a log
85+
* record using our library and log it via STDOUT -- b) keep track
86+
* of certain header fields so that they are available in later
87+
* processing steps -- b) inject a response wrapper to keep track of
88+
* content length (hopefully)
89+
*/
90+
if (wrapResponse) {
91+
responseWrapper = new ContentLengthTrackingResponseWrapper(httpResponse);
92+
}
93+
if (wrapRequest) {
9294

93-
requestWrapper = new ContentLengthTrackingRequestWrapper(httpRequest);
94-
}
95+
requestWrapper = new ContentLengthTrackingRequestWrapper(httpRequest);
96+
}
9597

96-
addHeaders(httpRequest, rr);
98+
addHeaders(httpRequest, rr);
9799

98-
/* -- start measuring right before calling up the filter chain -- */
99-
rr.start();
100-
if (chain != null) {
101-
chain.doFilter(requestWrapper != null ? requestWrapper : httpRequest, responseWrapper != null
102-
? responseWrapper
103-
: httpResponse);
104-
}
105-
rr.stop();
100+
/* -- start measuring right before calling up the filter chain -- */
101+
rr.start();
102+
if (chain != null) {
103+
chain.doFilter(requestWrapper != null ? requestWrapper : httpRequest, responseWrapper != null
104+
? responseWrapper
105+
: httpResponse);
106+
}
107+
rr.stop();
106108

107-
if (requestWrapper != null) {
108-
rr.addValue(Fields.REQUEST_SIZE_B, new LongValue(requestWrapper.getContentLength()));
109-
} else {
110-
rr.addValue(Fields.REQUEST_SIZE_B, new LongValue(httpRequest.getContentLength()));
111-
}
112-
String headerValue = httpResponse.getHeader(HttpHeaders.CONTENT_LENGTH);
113-
if (headerValue != null) {
114-
rr.addValue(Fields.RESPONSE_SIZE_B, new LongValue(Long.valueOf(headerValue)));
115-
} else {
116-
if (responseWrapper != null) {
117-
rr.addValue(Fields.RESPONSE_SIZE_B, new LongValue(responseWrapper.getContentLength()));
109+
if (requestWrapper != null) {
110+
rr.addValue(Fields.REQUEST_SIZE_B, new LongValue(requestWrapper.getContentLength()));
111+
} else {
112+
rr.addValue(Fields.REQUEST_SIZE_B, new LongValue(httpRequest.getContentLength()));
113+
}
114+
String headerValue = httpResponse.getHeader(HttpHeaders.CONTENT_LENGTH);
115+
if (headerValue != null) {
116+
rr.addValue(Fields.RESPONSE_SIZE_B, new LongValue(Long.valueOf(headerValue)));
117+
} else {
118+
if (responseWrapper != null) {
119+
rr.addValue(Fields.RESPONSE_SIZE_B, new LongValue(responseWrapper.getContentLength()));
120+
}
118121
}
122+
rr.addTag(Fields.RESPONSE_CONTENT_TYPE, getValue(httpResponse.getHeader(HttpHeaders.CONTENT_TYPE)));
123+
rr.addValue(Fields.RESPONSE_STATUS, new LongValue(httpResponse.getStatus()));
124+
/*
125+
* -- log info
126+
*/
127+
logger.info(Markers.REQUEST_MARKER, rr.toString());
128+
/*
129+
* -- close this
130+
*/
131+
} finally {
132+
rr.close();
119133
}
120-
rr.addTag(Fields.RESPONSE_CONTENT_TYPE, getValue(httpResponse.getHeader(HttpHeaders.CONTENT_TYPE)));
121-
rr.addValue(Fields.RESPONSE_STATUS, new LongValue(httpResponse.getStatus()));
122-
/*
123-
* -- log info
124-
*/
125-
logger.info(Markers.REQUEST_MARKER, rr.toString());
126-
/*
127-
* -- close this
128-
*/
129-
rr.close();
130134
}
131135

132136
private String getCorrelationIdFromHeader(HttpServletRequest httpRequest) {

0 commit comments

Comments
 (0)