Skip to content

Commit

Permalink
[SRE-30501] Remove logResponse String builder during all http requests (
Browse files Browse the repository at this point in the history 
#312)

* Remove logResponse String builder during all http requests

* Check logging level before string builder

* log exception message in testplanstarter and add OOM exception log

---------

Co-authored-by: zkofiro <[email protected]>
  • Loading branch information
@kevin-mcgoldrick
kevin-mcgoldrick and zkofiro authored Mar 27, 2024
1 parent d061fad commit 3699680
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 94 deletions.
63 changes: 29 additions & 34 deletions agent/agent_common/src/main/java/com/intuit/tank/http/BaseResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,35 @@ public abstract class BaseResponse {
* @return the responseLogMsg
*/
public String getLogMsg() {
try {
StringBuilder sb = new StringBuilder();
sb.append("RESPONSE HTTP CODE: ").append(this.httpCode).append(NEWLINE)
.append("RESPONSE HTTP MSG: ").append(this.rspMessage).append(NEWLINE)
.append("RESPONSE TIME: ").append(responseTime).append(NEWLINE)
.append("RESPONSE SIZE: ").append(getResponseSize()).append(NEWLINE);
for (Entry<String, String> mapEntry : headers.entrySet()) {
sb.append("RESPONSE HEADER: ")
.append((String) mapEntry.getKey()).append(" = ")
.append((String) mapEntry.getValue()).append(NEWLINE);
}
for (Entry<String, String> entry : cookies.entrySet()) {
sb.append("RESPONSE COOKIE: ")
.append(entry.getKey()).append(" = ")
.append(entry.getValue()).append(NEWLINE);
}
if (response != null) {
String contentType = this.headers.get("Content-Type");
if (isDataType(contentType)) {
sb.append("RESPONSE BODY: ").append(this.response).append(NEWLINE);
} else {
sb.append("RESPONSE BODY: not logged because ")
.append(contentType).append(" is not a data type.").append(NEWLINE);
}
}
this.responseLogMsg = sb.toString();
} catch (Exception ex) {
LOG.error("Error processing response: " + ex.getMessage(), ex);
}
return responseLogMsg;
}

Expand Down Expand Up @@ -191,40 +220,6 @@ public Map<String, String> getCookies() {
return cookies;
}

/**
* Log the response object
*/
public void logResponse() {
try {
StringBuilder sb = new StringBuilder();
// System.out.println("******** RESPONSE ***********");
sb.append("RESPONSE HTTP CODE: " + this.httpCode).append(NEWLINE);
sb.append("RESPONSE HTTP MSG: " + this.rspMessage).append(NEWLINE);
sb.append("RESPONSE TIME: " + responseTime).append(NEWLINE);
sb.append("RESPONSE SIZE: " + getResponseSize()).append(NEWLINE);
for (Entry<String, String> mapEntry : headers.entrySet()) {
sb.append("RESPONSE HEADER: " + (String) mapEntry.getKey() + " = " + (String) mapEntry.getValue()).append(NEWLINE);
}
for (Entry<String, String> entry : cookies.entrySet()) {
sb.append("RESPONSE COOKIE: " + entry.getKey() + " = " + entry.getValue()).append(NEWLINE);
}
if (response != null) {
String contentType = this.headers.get("Content-Type");
if (isDataType(contentType)) {
sb.append("RESPONSE BODY: " + this.response).append(NEWLINE);
} else {
sb.append("RESPONSE BODY: not logged because " + contentType + " is not a data type.").append(NEWLINE);
}
}
this.responseLogMsg = sb.toString();
LOG.debug("******** RESPONSE ***********");
LOG.debug(this.responseLogMsg);

} catch (Exception ex) {
LOG.error("Error processing response: " + ex.getMessage(), ex);
}
}

public static final boolean isDataType(String contentType) {
if (!StringUtils.isBlank(contentType)) {
contentType = contentType.toLowerCase();
Expand Down
17 changes: 6 additions & 11 deletions agent/agent_common/src/main/java/com/intuit/tank/http/TankHttpUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,13 @@ public static String getQueryString(Map<String, String> urlVariables) {
if (urlVariables != null && !urlVariables.isEmpty()) {
return "?" + urlVariables.entrySet().stream()
.map(entry -> {
try {
if (StringUtils.isBlank(entry.getValue())) {
return URLEncoder.encode(entry.getKey(), StandardCharsets.UTF_8.name());
} else {
return URLEncoder.encode(entry.getKey(), StandardCharsets.UTF_8.name())
+ "="
+ URLEncoder.encode(entry.getValue(), StandardCharsets.UTF_8.name());
}
} catch (UnsupportedEncodingException ex) {
LOG.warn("Unable to set query string value: " + ex.getMessage());
if (StringUtils.isBlank(entry.getValue())) {
return URLEncoder.encode(entry.getKey(), StandardCharsets.UTF_8);
} else {
return URLEncoder.encode(entry.getKey(), StandardCharsets.UTF_8)
+ "="
+ URLEncoder.encode(entry.getValue(), StandardCharsets.UTF_8);
}
return "";
})
.collect(joining("&"));
}
Expand Down
7 changes: 2 additions & 5 deletions agent/agent_common/src/test/java/com/intuit/tank/http/BaseResponseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,13 +448,11 @@ public void testLogResponse_1()
fixture.responseByteArray = new byte[] {};
fixture.rspMessage = "";
fixture.responseLogMsg = "";

fixture.logResponse();
assertEquals("RESPONSE HTTP CODE: 1\n" +
"RESPONSE HTTP MSG: \n" +
"RESPONSE TIME: 1\n" +
"RESPONSE SIZE: 0\n" +
"RESPONSE BODY: not logged because null is not a data type.\n", fixture.responseLogMsg);
"RESPONSE BODY: not logged because null is not a data type.\n", fixture.getLogMsg());
}

@Test
Expand All @@ -465,14 +463,13 @@ public void testLogResponse_2() {
fixture.cookies = new HashMap<String, String>();
fixture.cookies.put("testHeadersKey", "testHeadersValue");
fixture.response = "testResponse";
fixture.logResponse();
assertEquals("RESPONSE HTTP CODE: -1\n" +
"RESPONSE HTTP MSG: \n" +
"RESPONSE TIME: -1\n" +
"RESPONSE SIZE: 12\n" +
"RESPONSE HEADER: Content-Type = text/html\n" +
"RESPONSE COOKIE: testHeadersKey = testHeadersValue\n" +
"RESPONSE BODY: testResponse\n", fixture.responseLogMsg);
"RESPONSE BODY: testResponse\n", fixture.getLogMsg());
}

}
32 changes: 0 additions & 32 deletions agent/agent_common/src/test/java/com/intuit/tank/http/TankHttpUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,38 +179,6 @@ public void testGetQueryString_3()
assertEquals( "?data=%7B%22val1%22%3A%22val1%22%2C+%22val2%22%3A%22val2%22%2C+%22val3%22%3A%22val3%22%7D", result);
}

/**
* Run the String getQueryString(Map<String,String>) method test.
*
* @throws Exception
*
* @generatedBy CodePro at 12/16/14 3:57 PM
*/
@Test
public void testGetQueryString_4()
throws Exception {
Map<String, String> urlVariables = new HashMap();

String result = TankHttpUtil.getQueryString(urlVariables);
assertNotNull(result);
}

/**
* Run the String getQueryString(Map<String,String>) method test.
*
* @throws Exception
*
* @generatedBy CodePro at 12/16/14 3:57 PM
*/
@Test
public void testGetQueryString_5()
throws Exception {
Map<String, String> urlVariables = new HashMap();

String result = TankHttpUtil.getQueryString(urlVariables);
assertNotNull(result);
}

/**
* Run the String getQueryString(Map<String,String>) method test.
*
Expand Down
4 changes: 2 additions & 2 deletions agent/apiharness/src/main/java/com/intuit/tank/harness/TestPlanStarter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void run() {
}
done = true;
} catch (final Throwable t) {
LOG.error(LogUtil.getLogMessage("Linear - TestPlanStarter Unknown Error:"), t);
LOG.error(LogUtil.getLogMessage("Linear - TestPlanStarter Error:" + t.getMessage()), t);
throwUnchecked(t);
}
} else { // Nonlinear Workload
Expand Down Expand Up @@ -257,7 +257,7 @@ public void run() {
}
done = true;
} catch (final Throwable t) {
LOG.error(LogUtil.getLogMessage("Nonlinear - TestPlanStarter Unknown Error:"), t);
LOG.error(LogUtil.getLogMessage("Nonlinear - TestPlanStarter Error:" + t.getMessage()), t);
throwUnchecked(t);
}
}
Expand Down
5 changes: 5 additions & 0 deletions agent/apiharness/src/main/java/com/intuit/tank/runner/TestPlanRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,11 @@ private void runScriptSteps(HDScriptUseCase hdScriptUseCase) throws KillScriptEx
String validation = TankConstants.HTTP_CASE_FAIL;
try {
validation = tsr.execute();
} catch (Error e) {
if(e instanceof OutOfMemoryError){
throw (OutOfMemoryError) e;
}
throw new Error(e);
} catch (Exception e) {
if (e instanceof RuntimeException) {
throw (RuntimeException) e;
Expand Down
10 changes: 6 additions & 4 deletions agent/http_client_3/src/main/java/com/intuit/tank/httpclient3/TankHttpClient3.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import org.apache.commons.httpclient.methods.multipart.Part;
import org.apache.commons.httpclient.methods.multipart.PartSource;
import org.apache.commons.httpclient.methods.multipart.StringPart;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.http.HttpHeaders;
Expand Down Expand Up @@ -269,7 +268,7 @@ private void sendRequest(BaseRequest request, @Nonnull HttpMethod method, String
// check for no content headers
if (method.getStatusCode() != 203 && method.getStatusCode() != 202 && method.getStatusCode() != 204) {
try ( InputStream is = method.getResponseBodyAsStream() ) {
responseBody = IOUtils.toByteArray(is);
responseBody = is.readAllBytes();
} catch (IOException | NullPointerException e) {
LOG.warn(request.getLogUtil().getLogMessage("could not get response body: " + e));
}
Expand Down Expand Up @@ -358,14 +357,17 @@ private void processResponse(byte[] bResponse, long waitTime, BaseRequest reques

String contentEncoding = response.getHttpHeader("Content-Encoding");
bResponse = StringUtils.equalsIgnoreCase(contentEncoding, "gzip") ?
IOUtils.toByteArray(new GZIPInputStream(new ByteArrayInputStream(bResponse))) :
new GZIPInputStream(new ByteArrayInputStream(bResponse)).readAllBytes() :
bResponse;
response.setResponseBody(bResponse);

} catch (Exception ex) {
LOG.warn("Unable to get response: " + ex.getMessage());
} finally {
response.logResponse();
if (LOG.isDebugEnabled()) {
LOG.debug("******** RESPONSE ***********");
LOG.debug(response.getLogMsg());
}
}
}

Expand Down
8 changes: 5 additions & 3 deletions agent/http_client_4/src/main/java/com/intuit/tank/httpclient4/TankHttpClient4.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import jakarta.annotation.Nonnull;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.http.*;
Expand Down Expand Up @@ -293,7 +292,7 @@ private void sendRequest(BaseRequest request, @Nonnull HttpRequestBase method, S
// check for no content headers
if (response.getStatusLine().getStatusCode() != 203 && response.getStatusLine().getStatusCode() != 202 && response.getStatusLine().getStatusCode() != 204) {
try ( InputStream is = response.getEntity().getContent() ) {
responseBody = IOUtils.toByteArray(is);
responseBody = is.readAllBytes();
} catch (IOException | NullPointerException e) {
LOG.warn(request.getLogUtil().getLogMessage("could not get response body: " + e));
}
Expand Down Expand Up @@ -385,7 +384,10 @@ private void processResponse(byte[] bResponse, long waitTime, BaseRequest reques
} catch (Exception ex) {
LOG.warn("Unable to get response: " + ex.getMessage());
} finally {
response.logResponse();
if (LOG.isDebugEnabled()) {
LOG.debug("******** RESPONSE ***********");
LOG.debug(response.getLogMsg());
}
}
}

Expand Down
8 changes: 5 additions & 3 deletions agent/http_client_5/src/main/java/com/intuit/tank/httpclient5/TankHttpClient5.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

import jakarta.annotation.Nonnull;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.hc.client5.http.UserTokenHandler;
Expand Down Expand Up @@ -393,15 +392,18 @@ private void processResponse(byte[] bResponse, long waitTime, BaseRequest reques

String contentEncoding = response.getHttpHeader("content-ecncoding");
bResponse = StringUtils.equalsIgnoreCase(contentEncoding, "gzip") ?
IOUtils.toByteArray(new GZIPInputStream(new ByteArrayInputStream(bResponse))) :
new GZIPInputStream(new ByteArrayInputStream(bResponse)).readAllBytes() :
bResponse;

response.setResponseBody(bResponse);

} catch (Exception ex) {
LOG.warn(request.getLogUtil().getLogMessage("Unable to get response: " + ex.getMessage()));
} finally {
response.logResponse();
if (LOG.isDebugEnabled()) {
LOG.debug("******** RESPONSE ***********");
LOG.debug(response.getLogMsg());
}
}
}

Expand Down

0 comments on commit 3699680

Please sign in to comment.