Skip to content

Commit

Permalink
Merge pull request #1719 from newrelic/1715_HttpClient5Fixes
Browse files Browse the repository at this point in the history 
1715 http client5 fixes
  • Loading branch information
@jbedell-newrelic
jbedell-newrelic authored Feb 2, 2024
2 parents 3c123ac + b3943b6 commit 0e8a844
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 22 deletions.
1 change: 1 addition & 0 deletions ...ent-5.0/src/main/java/com/nr/agent/instrumentation/httpclient50/InstrumentationUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public static void handleUnknownHost(Exception e) {
}

public static void processResponse(URI requestURI, HttpResponse response, Segment segment) {
if (response == null) return;
HttpParameters params = createInboundParams(requestURI, response);
if (segment != null) {
segment.reportAsExternal(params);
Expand Down
13 changes: 11 additions & 2 deletions ...nt-5.0/src/main/java/com/nr/agent/instrumentation/httpclient50/WrappedFutureCallback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import com.newrelic.api.agent.Segment;
import org.apache.hc.core5.concurrent.FutureCallback;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.Message;
import org.apache.hc.core5.http.message.BasicHttpResponse;

import java.net.URISyntaxException;
Expand All @@ -33,9 +35,16 @@ public WrappedFutureCallback (HttpRequest request, FutureCallback origCallback,
@Override
public void completed(T response) {
try {
InstrumentationUtils.processResponse(request.getUri(), (BasicHttpResponse)response, segment);
if (response instanceof HttpResponse) {
InstrumentationUtils.processResponse(request.getUri(), (HttpResponse) response, segment);
} else if (response instanceof Message && ((Message)response).getHead() instanceof HttpResponse) {
HttpResponse resp2 = (HttpResponse)(((Message)response).getHead());
InstrumentationUtils.processResponse(request.getUri(), resp2, segment);
} else {
AgentBridge.getAgent().getLogger().log(Level.FINER, "Unhandled response type: {0}", (response == null ? "null" : response.getClass()));
}
} catch (URISyntaxException e) {
AgentBridge.getAgent().getLogger().log(Level.FINER, "Exception with uri: " + e.getMessage());
AgentBridge.getAgent().getLogger().log(Level.FINER, "Exception with uri: {0}", e.getMessage());
}
if (origCallback != null) origCallback.completed(response);
if (segment != null) segment.end();
Expand Down
107 changes: 87 additions & 20 deletions ...httpclient-5.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient5Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,24 @@
import org.apache.hc.client5.http.impl.classic.BasicHttpClientResponseHandler;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.Message;
import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.apache.hc.core5.http.nio.AsyncRequestProducer;
import org.apache.hc.core5.http.nio.entity.AsyncEntityProducers;
import org.apache.hc.core5.http.nio.entity.StringAsyncEntityConsumer;
import org.apache.hc.core5.http.nio.support.BasicRequestProducer;
import org.apache.hc.core5.http.nio.support.BasicResponseConsumer;
import org.apache.hc.core5.io.CloseMode;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.util.Collection;
import java.util.concurrent.ExecutionException;
Expand All @@ -55,19 +63,28 @@ public class HttpClient5Test {

@Test
public void testErrorClassic() throws Exception {
testError(false);
testError(false, false);
}

@Test
public void testErrorAsync() throws Exception {
testError(true);
testError(true, false);
}

private void testError(boolean async) throws Exception {
@Test
public void testErrorAsyncWithMessage() throws Exception {
testError(true, true);
}

private void testError(boolean async, boolean withMessage) throws Exception {
final String host2 = "www.notarealhostbrosef.bro";
try {
if (async) {
httpClientExternalAsync("http://" + host2, true);
if (withMessage) {
httpClientExternalAsyncWithMessage("http://" + host2, true);
} else {
httpClientExternalAsync("http://" + host2, true);
}
} else {
httpClientExternalClassic("http://" + host2, true);
}
Expand All @@ -89,20 +106,29 @@ private void testError(boolean async) throws Exception {

@Test
public void testExternalClassic() throws Exception {
testExternal(false);
testExternal(false, false);
}

@Test
public void testExternalAsync() throws Exception {
testExternal(true);
testExternal(true, false);
}

private void testExternal(boolean async) throws Exception {
@Test
public void testExternalAsyncWithMessage() throws Exception {
testExternal(true, true);
}

private void testExternal(boolean async, boolean withMessage) throws Exception {
URI endpoint = server.getEndPoint();
String host1 = endpoint.getHost();

if (async) {
httpClientExternalAsync(endpoint.toString(), false);
if (withMessage) {
httpClientExternalAsyncWithMessage(endpoint.toString(), false);
} else {
httpClientExternalAsync(endpoint.toString(), false);
}
} else {
httpClientExternalClassic(endpoint.toString(), false);
}
Expand Down Expand Up @@ -141,20 +167,31 @@ private void testExternal(boolean async) throws Exception {

@Test
public void testRollupsClassic() throws Exception {
testRollups(false);
testRollups(false, false);
}

@Test
public void testRollupsAsync() throws Exception {
testRollups(true);
testRollups(true, false);
}

@Test
public void testRollupsAsyncWithMessage() throws Exception {
testRollups(true, true);
}

private void testRollups(boolean async) throws Exception {
private void testRollups(boolean async, boolean withMessage) throws Exception {
// manually set host and port here in order to get 2 unique endpoints
if (async) {
httpClientExternalAsync("http://localhost:" + server.getEndPoint().getPort(), false);
httpClientExternalAsync("http://localhost:" + server.getEndPoint().getPort(), false);
httpClientExternalAsync("http://127.0.0.1:" + server.getEndPoint().getPort(), false);
if (withMessage) {
httpClientExternalAsyncWithMessage("http://localhost:" + server.getEndPoint().getPort(), false);
httpClientExternalAsyncWithMessage("http://localhost:" + server.getEndPoint().getPort(), false);
httpClientExternalAsyncWithMessage("http://127.0.0.1:" + server.getEndPoint().getPort(), false);
} else {
httpClientExternalAsync("http://localhost:" + server.getEndPoint().getPort(), false);
httpClientExternalAsync("http://localhost:" + server.getEndPoint().getPort(), false);
httpClientExternalAsync("http://127.0.0.1:" + server.getEndPoint().getPort(), false);
}
} else {
httpClientExternalClassic("http://localhost:" + server.getEndPoint().getPort(), false);
httpClientExternalClassic("http://localhost:" + server.getEndPoint().getPort(), false);
Expand All @@ -179,23 +216,33 @@ private void testRollups(boolean async) throws Exception {

@Test
public void testCatClassic() throws Exception {
testCat(false);
testCat(false, false);
}

@Test
public void testCatAsync() throws Exception {
testCat(true);
testCat(true, false);
}

private void testCat(boolean async) throws Exception {
@Test
public void testCatAsyncWithMessage() throws Exception {
testCat(true, true);
}

private void testCat(boolean async, boolean withMessage) throws Exception {
Introspector introspector = InstrumentationTestRunner.getIntrospector();
URI endpoint = server.getEndPoint();
String host = endpoint.getHost();

String methodName = "httpClientExternalClassic";
if (async) {
httpClientExternalAsync(endpoint.toURL().toString(), true);
methodName = "httpClientExternalAsync";
if (withMessage) {
httpClientExternalAsyncWithMessage(endpoint.toURL().toString(), true);
methodName = "httpClientExternalAsyncWithMessage";
} else {
httpClientExternalAsync(endpoint.toURL().toString(), true);
methodName = "httpClientExternalAsync";
}
} else {
httpClientExternalClassic(endpoint.toURL().toString(), true);
}
Expand Down Expand Up @@ -277,6 +324,26 @@ private void httpClientExternalAsync(String host, boolean doCat) throws Exceptio
}
}

@Trace(dispatcher = true)
private void httpClientExternalAsyncWithMessage(String host, boolean doCat) throws Exception {
try (CloseableHttpAsyncClient httpAsyncClient = HttpAsyncClients.createDefault()) {
httpAsyncClient.start();
SimpleHttpRequest request = SimpleRequestBuilder.get(host).build();
request.addHeader(HttpTestServer.DO_CAT, String.valueOf(doCat));
final AsyncRequestProducer requestProducer = new BasicRequestProducer(request,
AsyncEntityProducers.create("", ContentType.TEXT_PLAIN));
Future<Message<HttpResponse, String>> response = httpAsyncClient.execute(
requestProducer,
new BasicResponseConsumer<String>(new StringAsyncEntityConsumer()),
null);
Message msg = response.get();

httpAsyncClient.close(CloseMode.GRACEFUL);
} catch (ExecutionException ee) {
if (ee.getCause() instanceof UnknownHostException) throw new UnknownHostException();
}
}

@Trace(dispatcher = true)
public void externalRequest(URI endpoint) throws IOException {
NewRelic.getAgent().getTransaction().setTransactionName(TransactionNamePriority.CUSTOM_HIGH, true, "HttpClientTest", "externalRequest");
Expand Down

0 comments on commit 0e8a844

Please sign in to comment.