From 9be778c5e588a19bef0393331322cc8dd3a0f09f Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Mon, 12 Dec 2016 12:11:42 +0100 Subject: [PATCH] Warn log deprecation warnings received from server (#21895) The warnings get printed out in a single line e.g. WARNING: request [DELETE http://localhost:9200/index/type/_api] returned 3 warnings:[this is warning number 0],[this is warning number 1],[this is warning number 2] --- .../elasticsearch/client/RequestLogger.java | 19 ++++ .../client/RequestLoggerTests.java | 92 +++++++++++-------- 2 files changed, 71 insertions(+), 40 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java b/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java index fd4c3600234..24e09364c22 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RequestLogger.java @@ -37,6 +37,7 @@ import java.io.IOException; import java.io.InputStreamReader; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.util.StringJoiner; /** * Helper class that exposes static methods to unify the way requests are logged. @@ -59,6 +60,12 @@ final class RequestLogger { logger.debug("request [" + request.getMethod() + " " + host + getUri(request.getRequestLine()) + "] returned [" + httpResponse.getStatusLine() + "]"); } + if (logger.isWarnEnabled()) { + Header[] warnings = httpResponse.getHeaders("Warning"); + if (warnings != null && warnings.length > 0) { + logger.warn(buildWarningMessage(request, host, warnings)); + } + } if (tracer.isTraceEnabled()) { String requestLine; try { @@ -97,6 +104,18 @@ final class RequestLogger { } } + static String buildWarningMessage(HttpUriRequest request, HttpHost host, Header[] warnings) { + StringBuilder message = new StringBuilder("request [").append(request.getMethod()).append(" ").append(host) + .append(getUri(request.getRequestLine())).append("] returned ").append(warnings.length).append(" warnings: "); + for (int i = 0; i < warnings.length; i++) { + if (i > 0) { + message.append(","); + } + message.append("[").append(warnings[i].getValue()).append("]"); + } + return message.toString(); + } + /** * Creates curl output for given request */ diff --git a/client/rest/src/test/java/org/elasticsearch/client/RequestLoggerTests.java b/client/rest/src/test/java/org/elasticsearch/client/RequestLoggerTests.java index 17c2a158ea8..b66a7bcea2d 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RequestLoggerTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RequestLoggerTests.java @@ -19,7 +19,7 @@ package org.elasticsearch.client; -import com.carrotsearch.randomizedtesting.generators.RandomNumbers; +import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpEntityEnclosingRequest; import org.apache.http.HttpHost; @@ -29,10 +29,11 @@ import org.apache.http.client.methods.HttpOptions; import org.apache.http.client.methods.HttpPatch; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; -import org.apache.http.client.methods.HttpRequestBase; import org.apache.http.client.methods.HttpTrace; +import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.entity.InputStreamEntity; import org.apache.http.entity.StringEntity; +import org.apache.http.message.BasicHeader; import org.apache.http.message.BasicHttpResponse; import org.apache.http.message.BasicStatusLine; import org.apache.http.nio.entity.NByteArrayEntity; @@ -46,13 +47,13 @@ import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; public class RequestLoggerTests extends RestClientTestCase { public void testTraceRequest() throws IOException, URISyntaxException { - HttpHost host = new HttpHost("localhost", 9200, getRandom().nextBoolean() ? "http" : "https"); - + HttpHost host = new HttpHost("localhost", 9200, randomBoolean() ? "http" : "https"); String expectedEndpoint = "/index/type/_api"; URI uri; if (randomBoolean()) { @@ -60,46 +61,15 @@ public class RequestLoggerTests extends RestClientTestCase { } else { uri = new URI("index/type/_api"); } - - HttpRequestBase request; - int requestType = RandomNumbers.randomIntBetween(getRandom(), 0, 7); - switch(requestType) { - case 0: - request = new HttpGetWithEntity(uri); - break; - case 1: - request = new HttpPost(uri); - break; - case 2: - request = new HttpPut(uri); - break; - case 3: - request = new HttpDeleteWithEntity(uri); - break; - case 4: - request = new HttpHead(uri); - break; - case 5: - request = new HttpTrace(uri); - break; - case 6: - request = new HttpOptions(uri); - break; - case 7: - request = new HttpPatch(uri); - break; - default: - throw new UnsupportedOperationException(); - } - + HttpUriRequest request = randomHttpRequest(uri); String expected = "curl -iX " + request.getMethod() + " '" + host + expectedEndpoint + "'"; - boolean hasBody = request instanceof HttpEntityEnclosingRequest && getRandom().nextBoolean(); + boolean hasBody = request instanceof HttpEntityEnclosingRequest && randomBoolean(); String requestBody = "{ \"field\": \"value\" }"; if (hasBody) { expected += " -d '" + requestBody + "'"; HttpEntityEnclosingRequest enclosingRequest = (HttpEntityEnclosingRequest) request; HttpEntity entity; - switch(RandomNumbers.randomIntBetween(getRandom(), 0, 3)) { + switch(randomIntBetween(0, 3)) { case 0: entity = new StringEntity(requestBody, StandardCharsets.UTF_8); break; @@ -128,12 +98,12 @@ public class RequestLoggerTests extends RestClientTestCase { public void testTraceResponse() throws IOException { ProtocolVersion protocolVersion = new ProtocolVersion("HTTP", 1, 1); - int statusCode = RandomNumbers.randomIntBetween(getRandom(), 200, 599); + int statusCode = randomIntBetween(200, 599); String reasonPhrase = "REASON"; BasicStatusLine statusLine = new BasicStatusLine(protocolVersion, statusCode, reasonPhrase); String expected = "# " + statusLine.toString(); BasicHttpResponse httpResponse = new BasicHttpResponse(statusLine); - int numHeaders = RandomNumbers.randomIntBetween(getRandom(), 0, 3); + int numHeaders = randomIntBetween(0, 3); for (int i = 0; i < numHeaders; i++) { httpResponse.setHeader("header" + i, "value"); expected += "\n# header" + i + ": value"; @@ -162,4 +132,46 @@ public class RequestLoggerTests extends RestClientTestCase { assertThat(body, equalTo(responseBody)); } } + + public void testResponseWarnings() throws Exception { + HttpHost host = new HttpHost("localhost", 9200); + HttpUriRequest request = randomHttpRequest(new URI("/index/type/_api")); + int numWarnings = randomIntBetween(1, 5); + StringBuilder expected = new StringBuilder("request [").append(request.getMethod()).append(" ").append(host) + .append("/index/type/_api] returned ").append(numWarnings).append(" warnings: "); + Header[] warnings = new Header[numWarnings]; + for (int i = 0; i < numWarnings; i++) { + String warning = "this is warning number " + i; + warnings[i] = new BasicHeader("Warning", warning); + if (i > 0) { + expected.append(","); + } + expected.append("[").append(warning).append("]"); + } + assertEquals(expected.toString(), RequestLogger.buildWarningMessage(request, host, warnings)); + } + + private static HttpUriRequest randomHttpRequest(URI uri) { + int requestType = randomIntBetween(0, 7); + switch(requestType) { + case 0: + return new HttpGetWithEntity(uri); + case 1: + return new HttpPost(uri); + case 2: + return new HttpPut(uri); + case 3: + return new HttpDeleteWithEntity(uri); + case 4: + return new HttpHead(uri); + case 5: + return new HttpTrace(uri); + case 6: + return new HttpOptions(uri); + case 7: + return new HttpPatch(uri); + default: + throw new UnsupportedOperationException(); + } + } }