From f84f83d45b279857f63c38bb858757937df10091 Mon Sep 17 00:00:00 2001 From: Shashank Patidar Date: Fri, 29 Jan 2021 18:59:16 +0530 Subject: [PATCH 1/7] adding AbstractHttpClientTest and an echo target in TestHttpServer --- .../agent/testing/AbstractHttpClientTest.java | 105 ++++++++++++++++++ .../agent/testing/TestHttpServer.java | 28 +++++ 2 files changed, 133 insertions(+) create mode 100644 testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java new file mode 100644 index 000000000..1f3722e91 --- /dev/null +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java @@ -0,0 +1,105 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.hypertrace.agent.testing; + +import io.opentelemetry.sdk.trace.data.SpanData; +import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; + +public abstract class AbstractHttpClientTest extends AbstractInstrumenterTest { + + protected static final TestHttpServer testHttpServer = new TestHttpServer(); + + public static final String GET_NO_CONTENT_PATH = "http://localhost:%d/get_no_content"; + + public static final String GET_JSON_PATH = "http://localhost:%d/get_json"; + + public static final String POST_PATH = "http://localhost:%d/post"; + + public static final String POST_REDIRECT_PATH = + "http://localhost:%d/post_redirect_to_get_no_content"; + + public static final String ECHO_PATH = "http://localhost:%d/echo"; + + @BeforeAll + public static void startServer() throws Exception { + testHttpServer.start(); + } + + @AfterAll + public static void closeServer() throws Exception { + testHttpServer.close(); + } + + /** + * Test for post request with json body. Use ECHO_PATH as url string + */ + public abstract void echoJson() throws Exception; + + /** + * Test for post request with url encoded form body. Use ECHO_PATH as url string + */ + public abstract void echoUrlEncoded() throws Exception; + + /** + * Test for post request with plain text body. Use ECHO_PATH as url string + */ + public abstract void echoPlainText() throws Exception; + + public void assertResponseHeaders(SpanData spanData) { + Assertions.assertEquals( + TestHttpServer.RESPONSE_HEADER_VALUE, + spanData + .getAttributes() + .get( + HypertraceSemanticAttributes.httpResponseHeader( + TestHttpServer.RESPONSE_HEADER_NAME))); + } + + public void assertGetJsonResponseBody(SpanData spanData) { + Assertions.assertEquals( + TestHttpServer.GetJsonHandler.RESPONSE_BODY, + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + } + + public void assertEchoJson(SpanData spanData) { + assertResponseHeaders(spanData); + Assertions.assertNotNull(spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + Assertions.assertEquals( + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY), + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + } + + public void assertEchoUrlEncoded(SpanData spanData) { + assertResponseHeaders(spanData); + Assertions.assertNotNull(spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + Assertions.assertEquals( + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY), + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + } + + public void assertEchoPlainText(SpanData spanData) { + assertResponseHeaders(spanData); + Assertions.assertNull( + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + Assertions.assertNull( + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + } + +} diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java b/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java index f254a72c2..5fb575657 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java @@ -41,6 +41,7 @@ public void start() throws Exception { handlerList.addHandler(new GetJsonHandler()); handlerList.addHandler(new PostHandler()); handlerList.addHandler(new PostRedirect()); + handlerList.addHandler(new EchoHandler()); server.setHandler(handlerList); server.start(); } @@ -152,4 +153,31 @@ public void handle( } } } + + static class EchoHandler extends ResponseTestHeadersHandler { + @Override + public void handle( + String target, + Request baseRequest, + HttpServletRequest request, + HttpServletResponse response) + throws IOException { + super.handle(target, baseRequest, request, response); + + if (target.equals("/echo") + && "post".equalsIgnoreCase(request.getMethod())) { + ServletInputStream inputStream = request.getInputStream(); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + int nRead; + while ((nRead = inputStream.read()) != -1) { + buffer.write((byte) nRead); + } + + response.setStatus(200); + response.setContentType(request.getContentType()); + response.getWriter().print(buffer.toString()); + baseRequest.setHandled(true); + } + } + } } From 029e2dd1303f5d0d1b108a5336be75b9430f1d33 Mon Sep 17 00:00:00 2001 From: Shashank Patidar Date: Mon, 1 Feb 2021 13:11:52 +0530 Subject: [PATCH 2/7] updating AbstractHttpClientTest --- .../agent/testing/AbstractHttpClientTest.java | 110 ++++++++++++------ 1 file changed, 77 insertions(+), 33 deletions(-) diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java index 1f3722e91..2e2c1ec4f 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java @@ -21,20 +21,18 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeoutException; public abstract class AbstractHttpClientTest extends AbstractInstrumenterTest { protected static final TestHttpServer testHttpServer = new TestHttpServer(); - public static final String GET_NO_CONTENT_PATH = "http://localhost:%d/get_no_content"; - - public static final String GET_JSON_PATH = "http://localhost:%d/get_json"; - - public static final String POST_PATH = "http://localhost:%d/post"; - - public static final String POST_REDIRECT_PATH = - "http://localhost:%d/post_redirect_to_get_no_content"; - public static final String ECHO_PATH = "http://localhost:%d/echo"; @BeforeAll @@ -48,19 +46,75 @@ public static void closeServer() throws Exception { } /** - * Test for post request with json body. Use ECHO_PATH as url string + * Make request using client and return response status code + * @param uri URI to send request to + * @param headers Request headers + * @param body Request body if applicable on given method + * @param contentType Content-type of request body + * @return status code of response */ - public abstract void echoJson() throws Exception; + public abstract int doPostRequest(String uri, Map headers, String body, String contentType) throws IOException; - /** - * Test for post request with url encoded form body. Use ECHO_PATH as url string - */ - public abstract void echoUrlEncoded() throws Exception; + @Test + public void echoJson() throws TimeoutException, InterruptedException, IOException { + String body = "{\"foo\": \"bar\"}"; + Map headers = new HashMap<>(); + headers.put("headerName", "headerValue"); + String uri = String.format(ECHO_PATH, testHttpServer.port()); + + int status = doPostRequest(uri, headers, body, "application/json"); + + Assertions.assertEquals(200, status); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + assertEchoJson(clientSpan, body); + } + + @Test + public void echoUrlEncoded() throws TimeoutException, InterruptedException, IOException { + String body = "key1=value1&key2=value2"; + Map headers = new HashMap<>(); + headers.put("headerName", "headerValue"); + String uri = String.format(ECHO_PATH, testHttpServer.port()); + + int status = doPostRequest(uri, headers, body, "application/x-www-form-urlencoded"); + + Assertions.assertEquals(200, status); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + assertEchoUrlEncoded(clientSpan, body); + } + + @Test + public void echoPlainText() throws TimeoutException, InterruptedException, IOException { + String body = "foobar"; + Map headers = new HashMap<>(); + headers.put("headerName", "headerValue"); + String uri = String.format(ECHO_PATH, testHttpServer.port()); + + int status = doPostRequest(uri, headers, body, "text/plain"); + + Assertions.assertEquals(200, status); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + assertEchoPlainText(clientSpan); + } - /** - * Test for post request with plain text body. Use ECHO_PATH as url string - */ - public abstract void echoPlainText() throws Exception; public void assertResponseHeaders(SpanData spanData) { Assertions.assertEquals( @@ -72,26 +126,16 @@ public void assertResponseHeaders(SpanData spanData) { TestHttpServer.RESPONSE_HEADER_NAME))); } - public void assertGetJsonResponseBody(SpanData spanData) { - Assertions.assertEquals( - TestHttpServer.GetJsonHandler.RESPONSE_BODY, - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); - } - - public void assertEchoJson(SpanData spanData) { + public void assertEchoJson(SpanData spanData, String requestBody) { assertResponseHeaders(spanData); - Assertions.assertNotNull(spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + Assertions.assertEquals(requestBody, spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertEquals( spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY), spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); } - public void assertEchoUrlEncoded(SpanData spanData) { - assertResponseHeaders(spanData); - Assertions.assertNotNull(spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); - Assertions.assertEquals( - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY), - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + public void assertEchoUrlEncoded(SpanData spanData, String requestBody) { + assertEchoJson(spanData, requestBody); } public void assertEchoPlainText(SpanData spanData) { From d77570677b750111a5cbc68518c9635af31f85a6 Mon Sep 17 00:00:00 2001 From: Shashank Patidar Date: Mon, 1 Feb 2021 17:46:58 +0530 Subject: [PATCH 3/7] updating AbstractHttpClientTest --- .../agent/testing/AbstractHttpClientTest.java | 198 ++++++++++++++---- .../agent/testing/TestHttpServer.java | 13 +- 2 files changed, 159 insertions(+), 52 deletions(-) diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java index 2e2c1ec4f..a37840666 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java @@ -16,24 +16,41 @@ package org.hypertrace.agent.testing; +import io.opentelemetry.javaagent.instrumentation.api.Pair; import io.opentelemetry.sdk.trace.data.SpanData; -import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; - import java.io.IOException; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeoutException; +import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; public abstract class AbstractHttpClientTest extends AbstractInstrumenterTest { protected static final TestHttpServer testHttpServer = new TestHttpServer(); - public static final String ECHO_PATH = "http://localhost:%d/echo"; + private final boolean hasResponseBodySpan; + + public AbstractHttpClientTest(boolean hasResponseBodySpan) { + this.hasResponseBodySpan = hasResponseBodySpan; + } + + private static final String ECHO_PATH_FORMAT = "http://localhost:%d/echo"; + private static final String GET_NO_CONTENT_PATH_FORMAT = "http://localhost:%d/get_no_content"; + private static final String GET_JSON_PATH_FORMAT = "http://localhost:%d/get_json"; + + private static final Map headers; + private static final String HEADER_NAME = "headerName"; + private static final String HEADER_VALUE = "headerValue"; + + static { + headers = new HashMap<>(); + headers.put("headerName", "headerValue"); + } @BeforeAll public static void startServer() throws Exception { @@ -46,65 +63,89 @@ public static void closeServer() throws Exception { } /** - * Make request using client and return response status code + * Make request using client and return response status code and body + * * @param uri URI to send request to * @param headers Request headers - * @param body Request body if applicable on given method + * @param body Request body * @param contentType Content-type of request body - * @return status code of response + * @return status code and body of response + */ + public abstract Pair doPostRequest( + String uri, Map headers, String body, String contentType) throws IOException; + + /** + * Make request using client and return response status code and body + * + * @param uri URI to send request to + * @param headers Request headers + * @return status code and body of response */ - public abstract int doPostRequest(String uri, Map headers, String body, String contentType) throws IOException; + public abstract Pair doGetRequest(String uri, Map headers) + throws IOException; @Test public void echoJson() throws TimeoutException, InterruptedException, IOException { String body = "{\"foo\": \"bar\"}"; - Map headers = new HashMap<>(); - headers.put("headerName", "headerValue"); - String uri = String.format(ECHO_PATH, testHttpServer.port()); + String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); - int status = doPostRequest(uri, headers, body, "application/json"); + Pair statusBodyPair = doPostRequest(uri, headers, body, "application/json"); - Assertions.assertEquals(200, status); + Assertions.assertEquals(200, statusBodyPair.getLeft()); + Assertions.assertEquals(body, statusBodyPair.getRight()); TEST_WRITER.waitForTraces(1); List> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - assertEchoJson(clientSpan, body); + if (hasResponseBodySpan) { + Assertions.assertEquals(2, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + SpanData responseBodySpan = traces.get(0).get(1); + assertEchoJsonSpans(clientSpan, responseBodySpan, body); + } else { + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + assertEchoJson(clientSpan, body); + } } @Test public void echoUrlEncoded() throws TimeoutException, InterruptedException, IOException { String body = "key1=value1&key2=value2"; - Map headers = new HashMap<>(); - headers.put("headerName", "headerValue"); - String uri = String.format(ECHO_PATH, testHttpServer.port()); + String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); - int status = doPostRequest(uri, headers, body, "application/x-www-form-urlencoded"); + Pair statusBodyPair = + doPostRequest(uri, headers, body, "application/x-www-form-urlencoded"); - Assertions.assertEquals(200, status); + Assertions.assertEquals(200, statusBodyPair.getLeft()); + Assertions.assertEquals(body, statusBodyPair.getRight()); TEST_WRITER.waitForTraces(1); List> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - assertEchoUrlEncoded(clientSpan, body); + if (hasResponseBodySpan) { + Assertions.assertEquals(2, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + SpanData responseBodySpan = traces.get(0).get(1); + assertEchoUrlEncodedSpans(clientSpan, responseBodySpan, body); + } else { + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + assertEchoUrlEncoded(clientSpan, body); + } } @Test public void echoPlainText() throws TimeoutException, InterruptedException, IOException { String body = "foobar"; - Map headers = new HashMap<>(); - headers.put("headerName", "headerValue"); - String uri = String.format(ECHO_PATH, testHttpServer.port()); + String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); - int status = doPostRequest(uri, headers, body, "text/plain"); + Pair statusBodyPair = doPostRequest(uri, headers, body, "text/plain"); - Assertions.assertEquals(200, status); + Assertions.assertEquals(200, statusBodyPair.getLeft()); + Assertions.assertEquals(body, statusBodyPair.getRight()); TEST_WRITER.waitForTraces(1); List> traces = TEST_WRITER.getTraces(); @@ -112,11 +153,61 @@ public void echoPlainText() throws TimeoutException, InterruptedException, IOExc Assertions.assertEquals(1, traces.get(0).size()); SpanData clientSpan = traces.get(0).get(0); - assertEchoPlainText(clientSpan); + assertNullBodies(clientSpan); } + @Test + public void getNoContent() throws IOException, TimeoutException, InterruptedException { + String uri = String.format(GET_NO_CONTENT_PATH_FORMAT, testHttpServer.port()); + + Pair statusBodyPair = doGetRequest(uri, headers); + + Assertions.assertEquals(204, statusBodyPair.getLeft()); + Assertions.assertNull(statusBodyPair.getRight()); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + assertNullBodies(clientSpan); + } + + @Test + public void getJson() throws IOException, TimeoutException, InterruptedException { + String uri = String.format(GET_JSON_PATH_FORMAT, testHttpServer.port()); + + Pair statusBodyPair = doGetRequest(uri, headers); + + Assertions.assertEquals(200, statusBodyPair.getLeft()); + Assertions.assertEquals(TestHttpServer.GetJsonHandler.RESPONSE_BODY, statusBodyPair.getRight()); - public void assertResponseHeaders(SpanData spanData) { + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + + if (hasResponseBodySpan) { + Assertions.assertEquals(2, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + SpanData responseBodySpan = traces.get(0).get(1); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + Assertions.assertEquals( + TestHttpServer.GetJsonHandler.RESPONSE_BODY, + responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + } else { + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + Assertions.assertEquals( + TestHttpServer.GetJsonHandler.RESPONSE_BODY, + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + } + } + + private void assertHeaders(SpanData spanData) { Assertions.assertEquals( TestHttpServer.RESPONSE_HEADER_VALUE, spanData @@ -124,26 +215,43 @@ public void assertResponseHeaders(SpanData spanData) { .get( HypertraceSemanticAttributes.httpResponseHeader( TestHttpServer.RESPONSE_HEADER_NAME))); + Assertions.assertEquals( + HEADER_VALUE, + spanData.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader(HEADER_NAME))); } - public void assertEchoJson(SpanData spanData, String requestBody) { - assertResponseHeaders(spanData); - Assertions.assertEquals(requestBody, spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + private void assertEchoJson(SpanData spanData, String requestBody) { + Assertions.assertEquals( + requestBody, spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertEquals( - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY), - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY), + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); } - public void assertEchoUrlEncoded(SpanData spanData, String requestBody) { + private void assertEchoJsonSpans( + SpanData clientSpan, SpanData responseBodySpan, String requestBody) { + Assertions.assertEquals( + requestBody, + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + Assertions.assertEquals( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY), + responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + } + + private void assertEchoUrlEncoded(SpanData spanData, String requestBody) { assertEchoJson(spanData, requestBody); } - public void assertEchoPlainText(SpanData spanData) { - assertResponseHeaders(spanData); + private void assertEchoUrlEncodedSpans( + SpanData clientSpan, SpanData responseBodySpan, String requestBody) { + assertEchoJsonSpans(clientSpan, responseBodySpan, requestBody); + } + + private void assertNullBodies(SpanData spanData) { + assertHeaders(spanData); Assertions.assertNull( - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); Assertions.assertNull( - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); } - } diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java b/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java index 5fb575657..2ebb0f70b 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/TestHttpServer.java @@ -157,15 +157,14 @@ public void handle( static class EchoHandler extends ResponseTestHeadersHandler { @Override public void handle( - String target, - Request baseRequest, - HttpServletRequest request, - HttpServletResponse response) - throws IOException { + String target, + Request baseRequest, + HttpServletRequest request, + HttpServletResponse response) + throws IOException { super.handle(target, baseRequest, request, response); - if (target.equals("/echo") - && "post".equalsIgnoreCase(request.getMethod())) { + if (target.equals("/echo") && "post".equalsIgnoreCase(request.getMethod())) { ServletInputStream inputStream = request.getInputStream(); ByteArrayOutputStream buffer = new ByteArrayOutputStream(); int nRead; From b7190b2d2702a4d3f949e9db46d99b6fd82eea53 Mon Sep 17 00:00:00 2001 From: Shashank Patidar Date: Tue, 2 Feb 2021 12:27:09 +0530 Subject: [PATCH 4/7] updating okHttp and apache http client tests --- ...eAsyncClientInstrumentationModuleTest.java | 168 +++++---------- .../ApacheHttpClientInstrumentationTest.java | 197 ++++-------------- .../v3_0/OkHttpTracingInterceptorTest.java | 164 +++------------ .../agent/testing/AbstractHttpClientTest.java | 21 +- 4 files changed, 132 insertions(+), 418 deletions(-) diff --git a/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java b/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java index fbc92cbb1..40ace0432 100644 --- a/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java +++ b/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java @@ -16,138 +16,88 @@ package io.opentelemetry.instrumentation.hypertrace.apachehttpasyncclient; -import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.javaagent.instrumentation.api.Pair; import java.io.BufferedReader; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.io.UnsupportedEncodingException; import java.net.URI; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; -import java.util.concurrent.TimeoutException; -import org.apache.http.Header; -import org.apache.http.HttpEntity; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.concurrent.FutureCallback; -import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; import org.apache.http.impl.nio.client.HttpAsyncClients; -import org.apache.http.message.BasicHeader; -import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; -import org.hypertrace.agent.testing.AbstractInstrumenterTest; -import org.hypertrace.agent.testing.TestHttpServer; -import org.hypertrace.agent.testing.TestHttpServer.GetJsonHandler; +import org.hypertrace.agent.testing.AbstractHttpClientTest; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; -class ApacheAsyncClientInstrumentationModuleTest extends AbstractInstrumenterTest { - - private static final String JSON = "{\"id\":1,\"name\":\"John\"}"; - private static final TestHttpServer testHttpServer = new TestHttpServer(); +class ApacheAsyncClientInstrumentationModuleTest extends AbstractHttpClientTest { private static final CloseableHttpAsyncClient client = HttpAsyncClients.createDefault(); + public ApacheAsyncClientInstrumentationModuleTest() { + super(true); + } + @BeforeAll - public static void startServer() throws Exception { - testHttpServer.start(); + public static void startClient() throws Exception { client.start(); } @AfterAll - public static void closeServer() throws Exception { - testHttpServer.close(); + public static void closeClient() throws IOException { + client.close(); } - @Test - public void getJson() - throws ExecutionException, InterruptedException, TimeoutException, IOException { - HttpGet getRequest = - new HttpGet(String.format("http://localhost:%s/get_json", testHttpServer.port())); - getRequest.addHeader("foo", "bar"); - Future futureResponse = client.execute(getRequest, new NoopFutureCallback()); - - HttpResponse response = futureResponse.get(); - Assertions.assertEquals(200, response.getStatusLine().getStatusCode()); + @Override + public Pair doPostRequest( + String uri, Map headers, String body, String contentType) + throws IOException, ExecutionException, InterruptedException { + HttpPost request = new HttpPost(); + for (String key : headers.keySet()) { + request.addHeader(key, headers.get(key)); + } + request.setURI(URI.create(uri)); + StringEntity entity = new StringEntity(body); + entity.setContentType(contentType); + request.setEntity(entity); + request.addHeader("Content-type", contentType); + Future responseFuture = client.execute(request, new NoopFutureCallback()); + HttpResponse response = responseFuture.get(); + Thread.sleep(200); + if (response.getEntity() == null || response.getEntity().getContentLength() <= 0) { + return Pair.of(response.getStatusLine().getStatusCode(), null); + } String responseBody = readInputStream(response.getEntity().getContent()); - Assertions.assertEquals(GetJsonHandler.RESPONSE_BODY, responseBody); - - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(2, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertEquals( - "bar", - clientSpan.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("foo"))); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - SpanData responseBodySpan = traces.get(0).get(1); - Assertions.assertEquals( - GetJsonHandler.RESPONSE_BODY, - responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + Assertions.assertFalse(Span.current().isRecording()); + return Pair.of(response.getStatusLine().getStatusCode(), responseBody); } - @Test - public void postJson() - throws IOException, TimeoutException, InterruptedException, ExecutionException { - StringEntity entity = - new StringEntity(JSON, ContentType.create(ContentType.APPLICATION_JSON.getMimeType())); - postJsonEntity(entity); - } - - @Test - public void postJsonNonRepeatableEntity() - throws IOException, TimeoutException, InterruptedException, ExecutionException { - StringEntity entity = new NonRepeatableStringEntity(JSON); - postJsonEntity(entity); - } - - public void postJsonEntity(HttpEntity entity) - throws TimeoutException, InterruptedException, IOException, ExecutionException { - HttpPost postRequest = new HttpPost(); - postRequest.setEntity(entity); - postRequest.setHeader("Content-type", "application/json"); - postRequest.setURI( - URI.create(String.format("http://localhost:%d/post", testHttpServer.port()))); - - Future responseFuture = client.execute(postRequest, new NoopFutureCallback()); - + @Override + public Pair doGetRequest(String uri, Map headers) + throws IOException, ExecutionException, InterruptedException { + HttpGet request = new HttpGet(uri); + for (String key : headers.keySet()) { + request.addHeader(key, headers.get(key)); + } + Future responseFuture = client.execute(request, new NoopFutureCallback()); HttpResponse response = responseFuture.get(); - Assertions.assertEquals(204, response.getStatusLine().getStatusCode()); - - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - - String requestBody = readInputStream(entity.getContent()); - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertEquals( - requestBody, - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + Thread.sleep(200); + if (response.getEntity() == null || response.getEntity().getContentLength() <= 0) { + return Pair.of(response.getStatusLine().getStatusCode(), null); + } + String responseBody = readInputStream(response.getEntity().getContent()); + Assertions.assertFalse(Span.current().isRecording()); + return Pair.of(response.getStatusLine().getStatusCode(), responseBody); } private static String readInputStream(InputStream inputStream) throws IOException { @@ -164,28 +114,6 @@ private static String readInputStream(InputStream inputStream) throws IOExceptio return textBuilder.toString(); } - class NonRepeatableStringEntity extends StringEntity { - - public NonRepeatableStringEntity(String s) throws UnsupportedEncodingException { - super(s); - } - - @Override - public Header getContentType() { - return new BasicHeader("Content-Type", "json"); - } - - @Override - public boolean isRepeatable() { - return false; - } - - @Override - public InputStream getContent() { - return new ByteArrayInputStream(this.content); - } - } - static class NoopFutureCallback implements FutureCallback { @Override public void completed(HttpResponse result) {} diff --git a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java index 2c13d71cd..f1fbd7046 100644 --- a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java +++ b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java @@ -16,179 +16,66 @@ package io.opentelemetry.javaagent.instrumentation.hypertrace.apachehttpclient.v4_0; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.sdk.trace.data.SpanData; +import io.opentelemetry.javaagent.instrumentation.api.Pair; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.io.UnsupportedEncodingException; import java.net.URI; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.TimeoutException; -import org.apache.http.HttpEntity; +import java.util.Map; import org.apache.http.HttpResponse; -import org.apache.http.NameValuePair; import org.apache.http.client.HttpClient; -import org.apache.http.client.entity.UrlEncodedFormEntity; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.DefaultHttpClient; -import org.apache.http.message.BasicNameValuePair; -import org.apache.http.protocol.HTTP; -import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; -import org.hypertrace.agent.testing.AbstractInstrumenterTest; -import org.hypertrace.agent.testing.TestHttpServer; -import org.hypertrace.agent.testing.TestHttpServer.GetJsonHandler; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; +import org.hypertrace.agent.testing.AbstractHttpClientTest; -public class ApacheHttpClientInstrumentationTest extends AbstractInstrumenterTest { - - private static final String JSON = "{\"id\":1,\"name\":\"John\"}"; - private static final TestHttpServer testHttpServer = new TestHttpServer(); +public class ApacheHttpClientInstrumentationTest extends AbstractHttpClientTest { private final HttpClient client = new DefaultHttpClient(); - @BeforeAll - public static void startServer() throws Exception { - testHttpServer.start(); + public ApacheHttpClientInstrumentationTest() { + super(true); } - @AfterAll - public static void closeServer() throws Exception { - testHttpServer.close(); - } + @Override + public Pair doPostRequest( + String uri, Map headers, String body, String contentType) + throws IOException, InterruptedException { - @Test - public void getJson() throws IOException, TimeoutException, InterruptedException { - HttpGet getRequest = new HttpGet(); - getRequest.addHeader("foo", "bar"); - getRequest.setURI( - URI.create(String.format("http://localhost:%d/get_json", testHttpServer.port()))); - HttpResponse response = client.execute(getRequest); - Assertions.assertEquals(200, response.getStatusLine().getStatusCode()); + HttpPost request = new HttpPost(); + for (String key : headers.keySet()) { + request.addHeader(key, headers.get(key)); + } + request.setURI(URI.create(uri)); + StringEntity entity = new StringEntity(body); + entity.setContentType(contentType); + request.setEntity(entity); + request.addHeader("Content-type", contentType); + HttpResponse response = client.execute(request); + Thread.sleep(200); InputStream inputStream = response.getEntity().getContent(); - Assertions.assertEquals(GetJsonHandler.RESPONSE_BODY, readInputStream(inputStream)); - - Assertions.assertEquals(false, Span.current().isRecording()); - - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(2, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertEquals( - "bar", - clientSpan.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("foo"))); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - SpanData responseBodySpan = traces.get(0).get(1); - Assertions.assertEquals( - GetJsonHandler.RESPONSE_BODY, - responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); - } - - @Test - public void postUrlEncoded() throws IOException, TimeoutException, InterruptedException { - List nvps = new ArrayList<>(); - nvps.add(new BasicNameValuePair("key1", "value1")); - nvps.add(new BasicNameValuePair("key2", "value2")); - - HttpPost postRequest = new HttpPost(); - postRequest.setEntity(new UrlEncodedFormEntity(nvps, HTTP.UTF_8)); - postRequest.setURI( - URI.create(String.format("http://localhost:%d/post", testHttpServer.port()))); - HttpResponse response = client.execute(postRequest); - Assertions.assertEquals(204, response.getStatusLine().getStatusCode()); - - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertEquals( - "key1=value1&key2=value2", - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + return Pair.of(response.getStatusLine().getStatusCode(), readInputStream(inputStream)); } - @Test - public void postJson() throws IOException, TimeoutException, InterruptedException { - StringEntity entity = new StringEntity(JSON); - entity.setContentType("application/json"); - postJsonEntity(entity); - } - - @Test - public void postJsonNonRepeatableEntity() - throws IOException, TimeoutException, InterruptedException { - StringEntity entity = new NonRepeatableStringEntity(JSON); - entity.setContentType("application/json"); - postJsonEntity(entity); - } - - public void postJsonEntity(HttpEntity entity) - throws TimeoutException, InterruptedException, IOException { - HttpPost postRequest = new HttpPost(); - postRequest.setEntity(entity); - postRequest.setHeader("Content-type", "application/json"); - postRequest.setURI( - URI.create(String.format("http://localhost:%d/post", testHttpServer.port()))); - HttpResponse response = client.execute(postRequest); - Assertions.assertEquals(204, response.getStatusLine().getStatusCode()); - - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertEquals( - readInputStream(entity.getContent()), - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); - } - - @Test - public void getContent_throws_exception() throws IOException { - HttpClient client = new DefaultHttpClient(); - HttpGet getRequest = new HttpGet(); - getRequest.setURI( - URI.create(String.format("http://localhost:%d/get_json", testHttpServer.port()))); - HttpResponse response = client.execute(getRequest); - HttpEntity entity = response.getEntity(); - Assertions.assertNotNull(entity.getContent()); - try { - entity.getContent(); - } catch (Exception ex) { - Assertions.assertEquals(IllegalStateException.class, ex.getClass()); + @Override + public Pair doGetRequest(String uri, Map headers) + throws IOException, InterruptedException { + HttpGet request = new HttpGet(); + for (String key : headers.keySet()) { + request.addHeader(key, headers.get(key)); + } + request.setURI(URI.create(uri)); + HttpResponse response = client.execute(request); + Thread.sleep(200); + if (response.getEntity() == null || response.getEntity().getContentLength() <= 0) { + return Pair.of(response.getStatusLine().getStatusCode(), null); } + InputStream inputStream = response.getEntity().getContent(); + return Pair.of(response.getStatusLine().getStatusCode(), readInputStream(inputStream)); } private static String readInputStream(InputStream inputStream) throws IOException { @@ -204,16 +91,4 @@ private static String readInputStream(InputStream inputStream) throws IOExceptio } return textBuilder.toString(); } - - class NonRepeatableStringEntity extends StringEntity { - - public NonRepeatableStringEntity(String s) throws UnsupportedEncodingException { - super(s); - } - - @Override - public boolean isRepeatable() { - return false; - } - } } diff --git a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java index d1adb0bf2..7f7e3992a 100644 --- a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java +++ b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java @@ -16,159 +16,63 @@ package io.opentelemetry.javaagent.instrumentation.hypertrace.okhttp.v3_0; -import io.opentelemetry.sdk.trace.data.SpanData; -import java.util.List; -import okhttp3.FormBody; +import io.opentelemetry.javaagent.instrumentation.api.Pair; +import java.io.IOException; +import java.util.Map; +import okhttp3.Headers; import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Request; -import okhttp3.Request.Builder; import okhttp3.RequestBody; import okhttp3.Response; -import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; -import org.hypertrace.agent.testing.AbstractInstrumenterTest; -import org.hypertrace.agent.testing.TestHttpServer; -import org.hypertrace.agent.testing.TestHttpServer.GetJsonHandler; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; +import org.hypertrace.agent.testing.AbstractHttpClientTest; -public class OkHttpTracingInterceptorTest extends AbstractInstrumenterTest { - - private static final TestHttpServer testHttpServer = new TestHttpServer(); +public class OkHttpTracingInterceptorTest extends AbstractHttpClientTest { private final OkHttpClient client = new OkHttpClient.Builder().followRedirects(true).build(); - @BeforeAll - public static void startServer() throws Exception { - testHttpServer.start(); - } - - @AfterAll - public static void closeServer() throws Exception { - testHttpServer.close(); + public OkHttpTracingInterceptorTest() { + super(false); } - @Test - public void getNoContent() throws Exception { - Request request = - new Builder() - .url(String.format("http://localhost:%d/get_no_content", testHttpServer.port())) - .header("test-request-header", "test-value") - .get() - .build(); - - Response response = client.newCall(request).execute(); - response.close(); + @Override + public Pair doPostRequest( + String uri, Map headersMap, String body, String contentType) + throws IOException { - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpRequestHeader("test-request-header"))); - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); - } + Headers.Builder headers = new Headers.Builder(); + for (String key : headersMap.keySet()) { + headers.add(key, headersMap.get(key)); + } - @Test - public void getJson() throws Exception { Request request = - new Builder() - .url(String.format("http://localhost:%d/get_json", testHttpServer.port())) - .get() + new Request.Builder() + .url(uri) + .post(RequestBody.create(body, MediaType.get(contentType))) + .headers(headers.build()) .build(); Response response = client.newCall(request).execute(); - response.close(); - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - Assertions.assertEquals( - GetJsonHandler.RESPONSE_BODY, - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + return Pair.of(response.code(), response.body() != null ? response.body().string() : null); } - @Test - public void postUrlEncoded() throws Exception { - FormBody formBody = new FormBody.Builder().add("key1", "value1").add("key2", "value2").build(); - Request request = - new Builder() - .url(String.format("http://localhost:%d/post", testHttpServer.port())) - .post(formBody) - .build(); - - Response response = client.newCall(request).execute(); - response.close(); + @Override + public Pair doGetRequest(String uri, Map headersMap) + throws IOException { + Headers.Builder headers = new Headers.Builder(); + for (String key : headersMap.keySet()) { + headers.add(key, headersMap.get(key)); + } - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertEquals( - "key1=value1&key2=value2", - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); - } - - @Test - public void postRedirect() throws Exception { - String requestBodyStr = "{\"foo\": \"bar\"}"; - RequestBody requestBody = RequestBody.create(requestBodyStr, MediaType.get("application/json")); - Request request = - new Builder() - .url( - String.format( - "http://localhost:%d/post_redirect_to_get_no_content", testHttpServer.port())) - .post(requestBody) - .build(); + Request request = new Request.Builder().url(uri).headers(headers.build()).get().build(); Response response = client.newCall(request).execute(); - response.close(); - TEST_WRITER.waitForTraces(1); - List> traces = TEST_WRITER.getTraces(); - Assertions.assertEquals(1, traces.size()); - Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - Assertions.assertEquals( - "test-value", - clientSpan - .getAttributes() - .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); - Assertions.assertEquals( - requestBodyStr, - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); + return Pair.of( + response.code(), + (response.body() != null && response.body().contentLength() > 0) + ? response.body().string() + : null); } } diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java index a37840666..09a2ff236 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java @@ -22,6 +22,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; import org.junit.jupiter.api.AfterAll; @@ -72,7 +73,8 @@ public static void closeServer() throws Exception { * @return status code and body of response */ public abstract Pair doPostRequest( - String uri, Map headers, String body, String contentType) throws IOException; + String uri, Map headers, String body, String contentType) + throws IOException, ExecutionException, InterruptedException; /** * Make request using client and return response status code and body @@ -82,10 +84,11 @@ public abstract Pair doPostRequest( * @return status code and body of response */ public abstract Pair doGetRequest(String uri, Map headers) - throws IOException; + throws IOException, ExecutionException, InterruptedException; @Test - public void echoJson() throws TimeoutException, InterruptedException, IOException { + public void echoJson() + throws TimeoutException, InterruptedException, IOException, ExecutionException { String body = "{\"foo\": \"bar\"}"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); @@ -111,7 +114,8 @@ public void echoJson() throws TimeoutException, InterruptedException, IOExceptio } @Test - public void echoUrlEncoded() throws TimeoutException, InterruptedException, IOException { + public void echoUrlEncoded() + throws TimeoutException, InterruptedException, IOException, ExecutionException { String body = "key1=value1&key2=value2"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); @@ -138,7 +142,8 @@ public void echoUrlEncoded() throws TimeoutException, InterruptedException, IOEx } @Test - public void echoPlainText() throws TimeoutException, InterruptedException, IOException { + public void echoPlainText() + throws TimeoutException, InterruptedException, IOException, ExecutionException { String body = "foobar"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); @@ -157,7 +162,8 @@ public void echoPlainText() throws TimeoutException, InterruptedException, IOExc } @Test - public void getNoContent() throws IOException, TimeoutException, InterruptedException { + public void getNoContent() + throws IOException, TimeoutException, InterruptedException, ExecutionException { String uri = String.format(GET_NO_CONTENT_PATH_FORMAT, testHttpServer.port()); Pair statusBodyPair = doGetRequest(uri, headers); @@ -175,7 +181,8 @@ public void getNoContent() throws IOException, TimeoutException, InterruptedExce } @Test - public void getJson() throws IOException, TimeoutException, InterruptedException { + public void getJson() + throws IOException, TimeoutException, InterruptedException, ExecutionException { String uri = String.format(GET_JSON_PATH_FORMAT, testHttpServer.port()); Pair statusBodyPair = doGetRequest(uri, headers); From 7cbcb85723512c33c81021b19448d9ef9934f2c5 Mon Sep 17 00:00:00 2001 From: Shashank Patidar Date: Tue, 2 Feb 2021 13:59:45 +0530 Subject: [PATCH 5/7] review comments, refactoring --- .../agent/testing/AbstractHttpClientTest.java | 62 ++++++++----------- 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java index 09a2ff236..15309c562 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java @@ -19,6 +19,7 @@ import io.opentelemetry.javaagent.instrumentation.api.Pair; import io.opentelemetry.sdk.trace.data.SpanData; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -36,10 +37,6 @@ public abstract class AbstractHttpClientTest extends AbstractInstrumenterTest { private final boolean hasResponseBodySpan; - public AbstractHttpClientTest(boolean hasResponseBodySpan) { - this.hasResponseBodySpan = hasResponseBodySpan; - } - private static final String ECHO_PATH_FORMAT = "http://localhost:%d/echo"; private static final String GET_NO_CONTENT_PATH_FORMAT = "http://localhost:%d/get_no_content"; private static final String GET_JSON_PATH_FORMAT = "http://localhost:%d/get_json"; @@ -49,8 +46,13 @@ public AbstractHttpClientTest(boolean hasResponseBodySpan) { private static final String HEADER_VALUE = "headerValue"; static { - headers = new HashMap<>(); - headers.put("headerName", "headerValue"); + Map headersMap = new HashMap<>(); + headersMap.put(HEADER_NAME, HEADER_VALUE); + headers = Collections.unmodifiableMap(headersMap); + } + + public AbstractHttpClientTest(boolean hasResponseBodySpan) { + this.hasResponseBodySpan = hasResponseBodySpan; } @BeforeAll @@ -87,7 +89,7 @@ public abstract Pair doGetRequest(String uri, Map> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); + SpanData clientSpan = traces.get(0).get(0); if (hasResponseBodySpan) { Assertions.assertEquals(2, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); SpanData responseBodySpan = traces.get(0).get(1); - assertEchoJsonSpans(clientSpan, responseBodySpan, body); + assertEchoBodyInSpans(clientSpan, responseBodySpan, body); } else { Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - assertEchoJson(clientSpan, body); + assertEchoBodyInSpan(clientSpan, body); } } @Test - public void echoUrlEncoded() + public void postUrlEncoded_echo() throws TimeoutException, InterruptedException, IOException, ExecutionException { String body = "key1=value1&key2=value2"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); @@ -128,21 +129,20 @@ public void echoUrlEncoded() TEST_WRITER.waitForTraces(1); List> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); + SpanData clientSpan = traces.get(0).get(0); if (hasResponseBodySpan) { Assertions.assertEquals(2, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); SpanData responseBodySpan = traces.get(0).get(1); - assertEchoUrlEncodedSpans(clientSpan, responseBodySpan, body); + assertEchoBodyInSpans(clientSpan, responseBodySpan, body); } else { Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - assertEchoUrlEncoded(clientSpan, body); + assertEchoBodyInSpan(clientSpan, body); } } @Test - public void echoPlainText() + public void postPlainText_echo() throws TimeoutException, InterruptedException, IOException, ExecutionException { String body = "foobar"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); @@ -158,7 +158,7 @@ public void echoPlainText() Assertions.assertEquals(1, traces.get(0).size()); SpanData clientSpan = traces.get(0).get(0); - assertNullBodies(clientSpan); + assertNoBodies(clientSpan); } @Test @@ -177,7 +177,7 @@ public void getNoContent() Assertions.assertEquals(1, traces.get(0).size()); SpanData clientSpan = traces.get(0).get(0); - assertNullBodies(clientSpan); + assertNoBodies(clientSpan); } @Test @@ -193,21 +193,18 @@ public void getJson() TEST_WRITER.waitForTraces(1); List> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); + SpanData clientSpan = traces.get(0).get(0); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); if (hasResponseBodySpan) { Assertions.assertEquals(2, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); SpanData responseBodySpan = traces.get(0).get(1); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertEquals( TestHttpServer.GetJsonHandler.RESPONSE_BODY, responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } else { Assertions.assertEquals(1, traces.get(0).size()); - SpanData clientSpan = traces.get(0).get(0); - Assertions.assertNull( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertEquals( TestHttpServer.GetJsonHandler.RESPONSE_BODY, clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); @@ -227,7 +224,7 @@ private void assertHeaders(SpanData spanData) { spanData.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader(HEADER_NAME))); } - private void assertEchoJson(SpanData spanData, String requestBody) { + private void assertEchoBodyInSpan(SpanData spanData, String requestBody) { Assertions.assertEquals( requestBody, spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertEquals( @@ -235,7 +232,7 @@ private void assertEchoJson(SpanData spanData, String requestBody) { spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); } - private void assertEchoJsonSpans( + private void assertEchoBodyInSpans( SpanData clientSpan, SpanData responseBodySpan, String requestBody) { Assertions.assertEquals( requestBody, @@ -245,16 +242,7 @@ private void assertEchoJsonSpans( responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } - private void assertEchoUrlEncoded(SpanData spanData, String requestBody) { - assertEchoJson(spanData, requestBody); - } - - private void assertEchoUrlEncodedSpans( - SpanData clientSpan, SpanData responseBodySpan, String requestBody) { - assertEchoJsonSpans(clientSpan, responseBodySpan, requestBody); - } - - private void assertNullBodies(SpanData spanData) { + private void assertNoBodies(SpanData spanData) { assertHeaders(spanData); Assertions.assertNull( spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); From a405879f83448a36425950d6b2c72414b9becb4e Mon Sep 17 00:00:00 2001 From: Shashank Patidar Date: Tue, 2 Feb 2021 17:59:31 +0530 Subject: [PATCH 6/7] review comments and removing apache async http client changes --- ...eAsyncClientInstrumentationModuleTest.java | 168 +++++++++++++----- .../ApacheHttpClientInstrumentationTest.java | 35 +--- .../v3_0/OkHttpTracingInterceptorTest.java | 20 +-- .../agent/testing/AbstractHttpClientTest.java | 88 ++++++--- 4 files changed, 197 insertions(+), 114 deletions(-) diff --git a/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java b/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java index 40ace0432..fbc92cbb1 100644 --- a/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java +++ b/instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java @@ -16,88 +16,138 @@ package io.opentelemetry.instrumentation.hypertrace.apachehttpasyncclient; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.javaagent.instrumentation.api.Pair; +import io.opentelemetry.sdk.trace.data.SpanData; import java.io.BufferedReader; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.UnsupportedEncodingException; import java.net.URI; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.Map; +import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.TimeoutException; +import org.apache.http.Header; +import org.apache.http.HttpEntity; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.concurrent.FutureCallback; +import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; import org.apache.http.impl.nio.client.HttpAsyncClients; -import org.hypertrace.agent.testing.AbstractHttpClientTest; +import org.apache.http.message.BasicHeader; +import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes; +import org.hypertrace.agent.testing.AbstractInstrumenterTest; +import org.hypertrace.agent.testing.TestHttpServer; +import org.hypertrace.agent.testing.TestHttpServer.GetJsonHandler; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; -class ApacheAsyncClientInstrumentationModuleTest extends AbstractHttpClientTest { +class ApacheAsyncClientInstrumentationModuleTest extends AbstractInstrumenterTest { - private static final CloseableHttpAsyncClient client = HttpAsyncClients.createDefault(); + private static final String JSON = "{\"id\":1,\"name\":\"John\"}"; + private static final TestHttpServer testHttpServer = new TestHttpServer(); - public ApacheAsyncClientInstrumentationModuleTest() { - super(true); - } + private static final CloseableHttpAsyncClient client = HttpAsyncClients.createDefault(); @BeforeAll - public static void startClient() throws Exception { + public static void startServer() throws Exception { + testHttpServer.start(); client.start(); } @AfterAll - public static void closeClient() throws IOException { - client.close(); + public static void closeServer() throws Exception { + testHttpServer.close(); } - @Override - public Pair doPostRequest( - String uri, Map headers, String body, String contentType) - throws IOException, ExecutionException, InterruptedException { - HttpPost request = new HttpPost(); - for (String key : headers.keySet()) { - request.addHeader(key, headers.get(key)); - } - request.setURI(URI.create(uri)); - StringEntity entity = new StringEntity(body); - entity.setContentType(contentType); - request.setEntity(entity); - request.addHeader("Content-type", contentType); - Future responseFuture = client.execute(request, new NoopFutureCallback()); - HttpResponse response = responseFuture.get(); - Thread.sleep(200); - if (response.getEntity() == null || response.getEntity().getContentLength() <= 0) { - return Pair.of(response.getStatusLine().getStatusCode(), null); - } + @Test + public void getJson() + throws ExecutionException, InterruptedException, TimeoutException, IOException { + HttpGet getRequest = + new HttpGet(String.format("http://localhost:%s/get_json", testHttpServer.port())); + getRequest.addHeader("foo", "bar"); + Future futureResponse = client.execute(getRequest, new NoopFutureCallback()); + + HttpResponse response = futureResponse.get(); + Assertions.assertEquals(200, response.getStatusLine().getStatusCode()); String responseBody = readInputStream(response.getEntity().getContent()); - Assertions.assertFalse(Span.current().isRecording()); - return Pair.of(response.getStatusLine().getStatusCode(), responseBody); + Assertions.assertEquals(GetJsonHandler.RESPONSE_BODY, responseBody); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(2, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + Assertions.assertEquals( + "test-value", + clientSpan + .getAttributes() + .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); + Assertions.assertEquals( + "bar", + clientSpan.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("foo"))); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + SpanData responseBodySpan = traces.get(0).get(1); + Assertions.assertEquals( + GetJsonHandler.RESPONSE_BODY, + responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } - @Override - public Pair doGetRequest(String uri, Map headers) - throws IOException, ExecutionException, InterruptedException { - HttpGet request = new HttpGet(uri); - for (String key : headers.keySet()) { - request.addHeader(key, headers.get(key)); - } - Future responseFuture = client.execute(request, new NoopFutureCallback()); + @Test + public void postJson() + throws IOException, TimeoutException, InterruptedException, ExecutionException { + StringEntity entity = + new StringEntity(JSON, ContentType.create(ContentType.APPLICATION_JSON.getMimeType())); + postJsonEntity(entity); + } + + @Test + public void postJsonNonRepeatableEntity() + throws IOException, TimeoutException, InterruptedException, ExecutionException { + StringEntity entity = new NonRepeatableStringEntity(JSON); + postJsonEntity(entity); + } + + public void postJsonEntity(HttpEntity entity) + throws TimeoutException, InterruptedException, IOException, ExecutionException { + HttpPost postRequest = new HttpPost(); + postRequest.setEntity(entity); + postRequest.setHeader("Content-type", "application/json"); + postRequest.setURI( + URI.create(String.format("http://localhost:%d/post", testHttpServer.port()))); + + Future responseFuture = client.execute(postRequest, new NoopFutureCallback()); + HttpResponse response = responseFuture.get(); - Thread.sleep(200); - if (response.getEntity() == null || response.getEntity().getContentLength() <= 0) { - return Pair.of(response.getStatusLine().getStatusCode(), null); - } - String responseBody = readInputStream(response.getEntity().getContent()); - Assertions.assertFalse(Span.current().isRecording()); - return Pair.of(response.getStatusLine().getStatusCode(), responseBody); + Assertions.assertEquals(204, response.getStatusLine().getStatusCode()); + + TEST_WRITER.waitForTraces(1); + List> traces = TEST_WRITER.getTraces(); + Assertions.assertEquals(1, traces.size()); + Assertions.assertEquals(1, traces.get(0).size()); + SpanData clientSpan = traces.get(0).get(0); + + String requestBody = readInputStream(entity.getContent()); + Assertions.assertEquals( + "test-value", + clientSpan + .getAttributes() + .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header"))); + Assertions.assertEquals( + requestBody, + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + Assertions.assertNull( + clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } private static String readInputStream(InputStream inputStream) throws IOException { @@ -114,6 +164,28 @@ private static String readInputStream(InputStream inputStream) throws IOExceptio return textBuilder.toString(); } + class NonRepeatableStringEntity extends StringEntity { + + public NonRepeatableStringEntity(String s) throws UnsupportedEncodingException { + super(s); + } + + @Override + public Header getContentType() { + return new BasicHeader("Content-Type", "json"); + } + + @Override + public boolean isRepeatable() { + return false; + } + + @Override + public InputStream getContent() { + return new ByteArrayInputStream(this.content); + } + } + static class NoopFutureCallback implements FutureCallback { @Override public void completed(HttpResponse result) {} diff --git a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java index f1fbd7046..f5f92d46e 100644 --- a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java +++ b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java @@ -16,14 +16,9 @@ package io.opentelemetry.javaagent.instrumentation.hypertrace.apachehttpclient.v4_0; -import io.opentelemetry.javaagent.instrumentation.api.Pair; -import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; import java.net.URI; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.util.Map; import org.apache.http.HttpResponse; import org.apache.http.client.HttpClient; @@ -42,9 +37,8 @@ public ApacheHttpClientInstrumentationTest() { } @Override - public Pair doPostRequest( - String uri, Map headers, String body, String contentType) - throws IOException, InterruptedException { + public Response doPostRequest( + String uri, Map headers, String body, String contentType) throws IOException { HttpPost request = new HttpPost(); for (String key : headers.keySet()) { @@ -56,39 +50,22 @@ public Pair doPostRequest( request.setEntity(entity); request.addHeader("Content-type", contentType); HttpResponse response = client.execute(request); - Thread.sleep(200); InputStream inputStream = response.getEntity().getContent(); - return Pair.of(response.getStatusLine().getStatusCode(), readInputStream(inputStream)); + return new Response(readInputStream(inputStream), response.getStatusLine().getStatusCode()); } @Override - public Pair doGetRequest(String uri, Map headers) - throws IOException, InterruptedException { + public Response doGetRequest(String uri, Map headers) throws IOException { HttpGet request = new HttpGet(); for (String key : headers.keySet()) { request.addHeader(key, headers.get(key)); } request.setURI(URI.create(uri)); HttpResponse response = client.execute(request); - Thread.sleep(200); if (response.getEntity() == null || response.getEntity().getContentLength() <= 0) { - return Pair.of(response.getStatusLine().getStatusCode(), null); + return new Response(null, response.getStatusLine().getStatusCode()); } InputStream inputStream = response.getEntity().getContent(); - return Pair.of(response.getStatusLine().getStatusCode(), readInputStream(inputStream)); - } - - private static String readInputStream(InputStream inputStream) throws IOException { - StringBuilder textBuilder = new StringBuilder(); - - try (BufferedReader reader = - new BufferedReader( - new InputStreamReader(inputStream, Charset.forName(StandardCharsets.UTF_8.name())))) { - int c; - while ((c = reader.read()) != -1) { - textBuilder.append((char) c); - } - } - return textBuilder.toString(); + return new Response(readInputStream(inputStream), response.getStatusLine().getStatusCode()); } } diff --git a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java index 7f7e3992a..aa58daf22 100644 --- a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java +++ b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java @@ -16,7 +16,6 @@ package io.opentelemetry.javaagent.instrumentation.hypertrace.okhttp.v3_0; -import io.opentelemetry.javaagent.instrumentation.api.Pair; import java.io.IOException; import java.util.Map; import okhttp3.Headers; @@ -24,7 +23,6 @@ import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.RequestBody; -import okhttp3.Response; import org.hypertrace.agent.testing.AbstractHttpClientTest; public class OkHttpTracingInterceptorTest extends AbstractHttpClientTest { @@ -36,7 +34,7 @@ public OkHttpTracingInterceptorTest() { } @Override - public Pair doPostRequest( + public Response doPostRequest( String uri, Map headersMap, String body, String contentType) throws IOException { @@ -52,14 +50,14 @@ public Pair doPostRequest( .headers(headers.build()) .build(); - Response response = client.newCall(request).execute(); + okhttp3.Response response = client.newCall(request).execute(); - return Pair.of(response.code(), response.body() != null ? response.body().string() : null); + String responseBody = response.body() != null ? response.body().string() : null; + return new Response(responseBody, response.code()); } @Override - public Pair doGetRequest(String uri, Map headersMap) - throws IOException { + public Response doGetRequest(String uri, Map headersMap) throws IOException { Headers.Builder headers = new Headers.Builder(); for (String key : headersMap.keySet()) { headers.add(key, headersMap.get(key)); @@ -67,12 +65,12 @@ public Pair doGetRequest(String uri, Map header Request request = new Request.Builder().url(uri).headers(headers.build()).get().build(); - Response response = client.newCall(request).execute(); + okhttp3.Response response = client.newCall(request).execute(); - return Pair.of( - response.code(), + String responseBody = (response.body() != null && response.body().contentLength() > 0) ? response.body().string() - : null); + : null; + return new Response(responseBody, response.code()); } } diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java index 15309c562..d68492c45 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java @@ -16,9 +16,13 @@ package org.hypertrace.agent.testing; -import io.opentelemetry.javaagent.instrumentation.api.Pair; import io.opentelemetry.sdk.trace.data.SpanData; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -33,17 +37,15 @@ public abstract class AbstractHttpClientTest extends AbstractInstrumenterTest { - protected static final TestHttpServer testHttpServer = new TestHttpServer(); - - private final boolean hasResponseBodySpan; - private static final String ECHO_PATH_FORMAT = "http://localhost:%d/echo"; private static final String GET_NO_CONTENT_PATH_FORMAT = "http://localhost:%d/get_no_content"; private static final String GET_JSON_PATH_FORMAT = "http://localhost:%d/get_json"; - private static final Map headers; private static final String HEADER_NAME = "headerName"; private static final String HEADER_VALUE = "headerValue"; + private static final Map headers; + + protected static final TestHttpServer testHttpServer = new TestHttpServer(); static { Map headersMap = new HashMap<>(); @@ -51,6 +53,8 @@ public abstract class AbstractHttpClientTest extends AbstractInstrumenterTest { headers = Collections.unmodifiableMap(headersMap); } + private final boolean hasResponseBodySpan; + public AbstractHttpClientTest(boolean hasResponseBodySpan) { this.hasResponseBodySpan = hasResponseBodySpan; } @@ -74,9 +78,9 @@ public static void closeServer() throws Exception { * @param contentType Content-type of request body * @return status code and body of response */ - public abstract Pair doPostRequest( + public abstract Response doPostRequest( String uri, Map headers, String body, String contentType) - throws IOException, ExecutionException, InterruptedException; + throws IOException, ExecutionException, InterruptedException, TimeoutException; /** * Make request using client and return response status code and body @@ -85,8 +89,8 @@ public abstract Pair doPostRequest( * @param headers Request headers * @return status code and body of response */ - public abstract Pair doGetRequest(String uri, Map headers) - throws IOException, ExecutionException, InterruptedException; + public abstract Response doGetRequest(String uri, Map headers) + throws IOException, ExecutionException, InterruptedException, TimeoutException; @Test public void postJson_echo() @@ -94,12 +98,15 @@ public void postJson_echo() String body = "{\"foo\": \"bar\"}"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); - Pair statusBodyPair = doPostRequest(uri, headers, body, "application/json"); + Response response = doPostRequest(uri, headers, body, "application/json"); - Assertions.assertEquals(200, statusBodyPair.getLeft()); - Assertions.assertEquals(body, statusBodyPair.getRight()); + Assertions.assertEquals(200, response.statusCode); + Assertions.assertEquals(body, response.body); TEST_WRITER.waitForTraces(1); + if (hasResponseBodySpan) { + TEST_WRITER.waitForSpans(2); + } List> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); SpanData clientSpan = traces.get(0).get(0); @@ -120,13 +127,15 @@ public void postUrlEncoded_echo() String body = "key1=value1&key2=value2"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); - Pair statusBodyPair = - doPostRequest(uri, headers, body, "application/x-www-form-urlencoded"); + Response response = doPostRequest(uri, headers, body, "application/x-www-form-urlencoded"); - Assertions.assertEquals(200, statusBodyPair.getLeft()); - Assertions.assertEquals(body, statusBodyPair.getRight()); + Assertions.assertEquals(200, response.statusCode); + Assertions.assertEquals(body, response.body); TEST_WRITER.waitForTraces(1); + if (hasResponseBodySpan) { + TEST_WRITER.waitForSpans(2); + } List> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); SpanData clientSpan = traces.get(0).get(0); @@ -147,10 +156,10 @@ public void postPlainText_echo() String body = "foobar"; String uri = String.format(ECHO_PATH_FORMAT, testHttpServer.port()); - Pair statusBodyPair = doPostRequest(uri, headers, body, "text/plain"); + Response response = doPostRequest(uri, headers, body, "text/plain"); - Assertions.assertEquals(200, statusBodyPair.getLeft()); - Assertions.assertEquals(body, statusBodyPair.getRight()); + Assertions.assertEquals(200, response.statusCode); + Assertions.assertEquals(body, response.body); TEST_WRITER.waitForTraces(1); List> traces = TEST_WRITER.getTraces(); @@ -166,10 +175,10 @@ public void getNoContent() throws IOException, TimeoutException, InterruptedException, ExecutionException { String uri = String.format(GET_NO_CONTENT_PATH_FORMAT, testHttpServer.port()); - Pair statusBodyPair = doGetRequest(uri, headers); + Response response = doGetRequest(uri, headers); - Assertions.assertEquals(204, statusBodyPair.getLeft()); - Assertions.assertNull(statusBodyPair.getRight()); + Assertions.assertEquals(204, response.statusCode); + Assertions.assertNull(response.body); TEST_WRITER.waitForTraces(1); List> traces = TEST_WRITER.getTraces(); @@ -185,12 +194,15 @@ public void getJson() throws IOException, TimeoutException, InterruptedException, ExecutionException { String uri = String.format(GET_JSON_PATH_FORMAT, testHttpServer.port()); - Pair statusBodyPair = doGetRequest(uri, headers); + Response response = doGetRequest(uri, headers); - Assertions.assertEquals(200, statusBodyPair.getLeft()); - Assertions.assertEquals(TestHttpServer.GetJsonHandler.RESPONSE_BODY, statusBodyPair.getRight()); + Assertions.assertEquals(200, response.statusCode); + Assertions.assertEquals(TestHttpServer.GetJsonHandler.RESPONSE_BODY, response.body); TEST_WRITER.waitForTraces(1); + if (hasResponseBodySpan) { + TEST_WRITER.waitForSpans(2); + } List> traces = TEST_WRITER.getTraces(); Assertions.assertEquals(1, traces.size()); SpanData clientSpan = traces.get(0).get(0); @@ -249,4 +261,28 @@ private void assertNoBodies(SpanData spanData) { Assertions.assertNull( spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); } + + protected static String readInputStream(InputStream inputStream) throws IOException { + StringBuilder textBuilder = new StringBuilder(); + + try (BufferedReader reader = + new BufferedReader( + new InputStreamReader(inputStream, Charset.forName(StandardCharsets.UTF_8.name())))) { + int c; + while ((c = reader.read()) != -1) { + textBuilder.append((char) c); + } + } + return textBuilder.toString(); + } + + public class Response { + String body; + int statusCode; + + public Response(String body, int statusCode) { + this.body = body; + this.statusCode = statusCode; + } + } } From e163156ac09123c50426be9617f138d87fa203c0 Mon Sep 17 00:00:00 2001 From: Shashank Patidar Date: Wed, 3 Feb 2021 10:38:07 +0530 Subject: [PATCH 7/7] review comments --- .../ApacheHttpClientInstrumentationTest.java | 8 +++--- .../v3_0/OkHttpTracingInterceptorTest.java | 8 +++--- .../agent/testing/AbstractHttpClientTest.java | 28 ++++++++++--------- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java index f5f92d46e..e8f8dbd8a 100644 --- a/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java +++ b/instrumentation/apache-httpclient-4.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/apachehttpclient/v4_0/ApacheHttpClientInstrumentationTest.java @@ -41,8 +41,8 @@ public Response doPostRequest( String uri, Map headers, String body, String contentType) throws IOException { HttpPost request = new HttpPost(); - for (String key : headers.keySet()) { - request.addHeader(key, headers.get(key)); + for (Map.Entry entry : headers.entrySet()) { + request.addHeader(entry.getKey(), entry.getValue()); } request.setURI(URI.create(uri)); StringEntity entity = new StringEntity(body); @@ -57,8 +57,8 @@ public Response doPostRequest( @Override public Response doGetRequest(String uri, Map headers) throws IOException { HttpGet request = new HttpGet(); - for (String key : headers.keySet()) { - request.addHeader(key, headers.get(key)); + for (Map.Entry entry : headers.entrySet()) { + request.addHeader(entry.getKey(), entry.getValue()); } request.setURI(URI.create(uri)); HttpResponse response = client.execute(request); diff --git a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java index aa58daf22..7e0f13fd8 100644 --- a/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java +++ b/instrumentation/okhttp/okhttp-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/okhttp/v3_0/OkHttpTracingInterceptorTest.java @@ -39,8 +39,8 @@ public Response doPostRequest( throws IOException { Headers.Builder headers = new Headers.Builder(); - for (String key : headersMap.keySet()) { - headers.add(key, headersMap.get(key)); + for (Map.Entry entry : headersMap.entrySet()) { + headers.add(entry.getKey(), entry.getValue()); } Request request = @@ -59,8 +59,8 @@ public Response doPostRequest( @Override public Response doGetRequest(String uri, Map headersMap) throws IOException { Headers.Builder headers = new Headers.Builder(); - for (String key : headersMap.keySet()) { - headers.add(key, headersMap.get(key)); + for (Map.Entry entry : headersMap.entrySet()) { + headers.add(entry.getKey(), entry.getValue()); } Request request = new Request.Builder().url(uri).headers(headers.build()).get().build(); diff --git a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java index d68492c45..e6fb8c411 100644 --- a/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/org/hypertrace/agent/testing/AbstractHttpClientTest.java @@ -114,10 +114,10 @@ public void postJson_echo() if (hasResponseBodySpan) { Assertions.assertEquals(2, traces.get(0).size()); SpanData responseBodySpan = traces.get(0).get(1); - assertEchoBodyInSpans(clientSpan, responseBodySpan, body); + assertBodies(clientSpan, responseBodySpan, body, body); } else { Assertions.assertEquals(1, traces.get(0).size()); - assertEchoBodyInSpan(clientSpan, body); + assertRequestAndResponseBody(clientSpan, body, body); } } @@ -143,10 +143,10 @@ public void postUrlEncoded_echo() if (hasResponseBodySpan) { Assertions.assertEquals(2, traces.get(0).size()); SpanData responseBodySpan = traces.get(0).get(1); - assertEchoBodyInSpans(clientSpan, responseBodySpan, body); + assertBodies(clientSpan, responseBodySpan, body, body); } else { Assertions.assertEquals(1, traces.get(0).size()); - assertEchoBodyInSpan(clientSpan, body); + assertRequestAndResponseBody(clientSpan, body, body); } } @@ -167,6 +167,7 @@ public void postPlainText_echo() Assertions.assertEquals(1, traces.get(0).size()); SpanData clientSpan = traces.get(0).get(0); + assertHeaders(clientSpan); assertNoBodies(clientSpan); } @@ -186,6 +187,7 @@ public void getNoContent() Assertions.assertEquals(1, traces.get(0).size()); SpanData clientSpan = traces.get(0).get(0); + assertHeaders(clientSpan); assertNoBodies(clientSpan); } @@ -236,33 +238,33 @@ private void assertHeaders(SpanData spanData) { spanData.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader(HEADER_NAME))); } - private void assertEchoBodyInSpan(SpanData spanData, String requestBody) { + private void assertRequestAndResponseBody( + SpanData spanData, String requestBody, String responseBody) { Assertions.assertEquals( requestBody, spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertEquals( - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY), - spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); + responseBody, + spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } - private void assertEchoBodyInSpans( - SpanData clientSpan, SpanData responseBodySpan, String requestBody) { + private void assertBodies( + SpanData clientSpan, SpanData responseBodySpan, String requestBody, String responseBody) { Assertions.assertEquals( requestBody, clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); Assertions.assertEquals( - clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY), + responseBody, responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); } private void assertNoBodies(SpanData spanData) { - assertHeaders(spanData); Assertions.assertNull( spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY)); Assertions.assertNull( spanData.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY)); } - protected static String readInputStream(InputStream inputStream) throws IOException { + public static String readInputStream(InputStream inputStream) throws IOException { StringBuilder textBuilder = new StringBuilder(); try (BufferedReader reader = @@ -276,7 +278,7 @@ protected static String readInputStream(InputStream inputStream) throws IOExcept return textBuilder.toString(); } - public class Response { + public static class Response { String body; int statusCode;