From 0124b7223412d13c79611c2ab4f2f1df7e105ffd Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Fri, 9 Apr 2021 15:10:17 -0400 Subject: [PATCH 01/16] Adding http status code/text to transaction spans --- .../com/newrelic/agent/dispatchers/WebRequestDispatcher.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/dispatchers/WebRequestDispatcher.java b/newrelic-agent/src/main/java/com/newrelic/agent/dispatchers/WebRequestDispatcher.java index 8d36a3e6aa..e6d75bf945 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/dispatchers/WebRequestDispatcher.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/dispatchers/WebRequestDispatcher.java @@ -122,10 +122,13 @@ public void transactionActivityWithResponseFinished() { if (getStatus() > 0) { // http status is now being recorded as a string getTransaction().getAgentAttributes().put(AttributeNames.HTTP_STATUS, String.valueOf(getStatus())); + // http.statusCode is supposed to be an int + getTransaction().getAgentAttributes().put(AttributeNames.HTTP_STATUS_CODE, getStatus()); } if (getStatusMessage() != null) { getTransaction().getAgentAttributes().put(AttributeNames.HTTP_STATUS_MESSAGE, getStatusMessage()); + getTransaction().getAgentAttributes().put(AttributeNames.HTTP_STATUS_TEXT, getStatusMessage()); } // adding request.uri here includes it in the Transaction event, which also propagates to any Transaction error events From 2bfcb7b81eef5c68da594b0e4e5636f3283fb41d Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Fri, 9 Apr 2021 15:09:36 -0400 Subject: [PATCH 02/16] Adding http status code/text to external spans --- .../org/apache/http/client/HttpClient.java | 2 + .../agent/attributes/AttributeNames.java | 3 ++ .../service/analytics/SpanEventFactory.java | 19 +++++++ .../analytics/SpanEventFactoryTest.java | 45 ++++++++++++++++ .../newrelic/api/agent/HttpParameters.java | 53 +++++++++++++++++-- 5 files changed, 118 insertions(+), 4 deletions(-) diff --git a/instrumentation/httpclient-4.0/src/main/java/org/apache/http/client/HttpClient.java b/instrumentation/httpclient-4.0/src/main/java/org/apache/http/client/HttpClient.java index a78848a36b..dfd9ac3b11 100644 --- a/instrumentation/httpclient-4.0/src/main/java/org/apache/http/client/HttpClient.java +++ b/instrumentation/httpclient-4.0/src/main/java/org/apache/http/client/HttpClient.java @@ -63,6 +63,7 @@ private static void processResponse(URI requestURI, HttpResponse response) { .uri(requestURI) .procedure(PROCEDURE) .inboundHeaders(inboundCatWrapper) + .status(response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()) .build()); } @@ -219,6 +220,7 @@ private static void processResponse(URI requestURI, HttpResponse response) { .uri(requestURI) .procedure(PROCEDURE) .inboundHeaders(inboundCatWrapper) + .status(response.getStatusLine().getStatusCode(), response.getStatusLine().getReasonPhrase()) .build()); } } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeNames.java b/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeNames.java index 0fa7453565..b35ae0bc83 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeNames.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeNames.java @@ -32,6 +32,9 @@ public final class AttributeNames { public static final String HTTP_STATUS = "httpResponseCode"; public static final String HTTP_STATUS_MESSAGE = "httpResponseMessage"; + public static final String HTTP_STATUS_CODE = "http.statusCode"; + public static final String HTTP_STATUS_TEXT = "http.statusText"; + public static final String LOCK_THREAD_NAME = "jvm.lock_thread_name"; public static final String THREAD_NAME = "jvm.thread_name"; diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java index 505b3ea6fd..20f5663166 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java @@ -8,6 +8,9 @@ package com.newrelic.agent.service.analytics; import com.google.common.base.Joiner; +import com.newrelic.agent.Transaction; +import com.newrelic.agent.TransactionData; +import com.newrelic.agent.attributes.AttributeNames; import com.newrelic.agent.attributes.AttributeValidator; import com.newrelic.agent.config.AgentConfig; import com.newrelic.agent.database.SqlObfuscator; @@ -193,6 +196,20 @@ public SpanEventFactory setHttpComponent(String component) { return this; } + public SpanEventFactory setHttpStatusCode(Integer statusCode) { + if (filter.shouldIncludeAgentAttribute(appName, AttributeNames.HTTP_STATUS_CODE)) { + builder.putAgentAttribute(AttributeNames.HTTP_STATUS_CODE, statusCode); + } + return this; + } + + public SpanEventFactory setHttpStatusText(String statusText) { + if (filter.shouldIncludeAgentAttribute(appName, AttributeNames.HTTP_STATUS_TEXT)) { + builder.putAgentAttribute(AttributeNames.HTTP_STATUS_TEXT, statusText); + } + return this; + } + // datastore parameter public SpanEventFactory setDatabaseName(String databaseName) { builder.putIntrinsic("db.instance", databaseName); @@ -301,6 +318,8 @@ public SpanEventFactory setExternalParameterAttributes(ExternalParameters parame setCategory(SpanCategory.http); setUri(httpParameters.getUri()); setHttpMethod(httpParameters.getProcedure()); + setHttpStatusCode(httpParameters.getStatusCode()); + setHttpStatusText(httpParameters.getStatusText()); setHttpComponent((httpParameters).getLibrary()); setKindFromUserAttributes(); } else if (parameters instanceof DatastoreParameters) { diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/SpanEventFactoryTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/SpanEventFactoryTest.java index 672784ab99..fcdff75b3d 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/SpanEventFactoryTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/analytics/SpanEventFactoryTest.java @@ -21,6 +21,7 @@ import static com.newrelic.agent.service.analytics.SpanEventFactory.DEFAULT_SYSTEM_TIMESTAMP_SUPPLIER; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -121,6 +122,50 @@ public void doesNotSetHttpAgentAttributesWhenFiltering() { assertNull(spanEvent.getAgentAttributes().get("http.method")); } + @Test + public void shouldSetStatusCode() { + SpanEvent spanEvent = spanEventFactory.setHttpStatusCode(418).build(); + + assertEquals(418, spanEvent.getAgentAttributes().get("http.statusCode")); + } + + @Test + public void shouldNotSetStatusCodeWhenFiltering() { + SpanEventFactory factory = new SpanEventFactory("blerb", new PassNothingAttributeFilter(), DEFAULT_SYSTEM_TIMESTAMP_SUPPLIER); + SpanEvent spanEvent = factory.setHttpStatusCode(418).build(); + + assertNull(spanEvent.getAgentAttributes().get("http.statusCode")); + } + + @Test + public void shouldNotSetNullStatusCode() { + SpanEvent spanEvent = spanEventFactory.setHttpStatusCode(null).build(); + + assertFalse(spanEvent.getAgentAttributes().containsKey("http.statusCode")); + } + + @Test + public void shouldSetStatusText() { + SpanEvent spanEvent = spanEventFactory.setHttpStatusText("I'm a teapot.").build(); + + assertEquals("I'm a teapot.", spanEvent.getAgentAttributes().get("http.statusText")); + } + + @Test + public void shouldNotSetStatusTextWhenFiltering() { + SpanEventFactory factory = new SpanEventFactory("blerb", new PassNothingAttributeFilter(), DEFAULT_SYSTEM_TIMESTAMP_SUPPLIER); + SpanEvent spanEvent = factory.setHttpStatusText("I'm a teapot.").build(); + + assertNull(spanEvent.getAgentAttributes().get("http.statusText")); + } + + @Test + public void shouldNotSetNullStatusText() { + SpanEvent spanEvent = spanEventFactory.setHttpStatusText(null).build(); + + assertFalse(spanEvent.getAgentAttributes().containsKey("http.statusText")); + } + @Test public void shouldSetDataStoreParameters() { DatastoreParameters mockParameters = mock(DatastoreParameters.class); diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java b/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java index e29b0456eb..c119ed32bb 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java @@ -31,6 +31,10 @@ public class HttpParameters implements ExternalParameters { */ private final String procedure; + private final Integer statusCode; + + private final String statusText; + /** * The headers from the response of the external call. */ @@ -42,13 +46,16 @@ public class HttpParameters implements ExternalParameters { private final ExtendedInboundHeaders extendedInboundResponseHeaders; protected HttpParameters(String library, URI uri, String procedure, InboundHeaders inboundHeaders) { - this(library, uri, procedure, inboundHeaders, null); + this(library, uri, procedure, null, null, inboundHeaders, null); } - protected HttpParameters(String library, URI uri, String procedure, InboundHeaders inboundHeaders, ExtendedInboundHeaders extendedInboundHeaders) { + protected HttpParameters(String library, URI uri, String procedure, Integer statusCode, String statusText, InboundHeaders inboundHeaders, + ExtendedInboundHeaders extendedInboundHeaders) { this.library = library; this.uri = uri; this.procedure = procedure; + this.statusCode = statusCode; + this.statusText = statusText; this.inboundResponseHeaders = inboundHeaders; this.extendedInboundResponseHeaders = extendedInboundHeaders; } @@ -57,6 +64,8 @@ protected HttpParameters(HttpParameters httpParameters) { this.library = httpParameters.library; this.uri = httpParameters.uri; this.procedure = httpParameters.procedure; + this.statusCode = httpParameters.statusCode; + this.statusText = httpParameters.statusText; this.inboundResponseHeaders = httpParameters.inboundResponseHeaders; this.extendedInboundResponseHeaders = null; } @@ -91,6 +100,26 @@ public String getProcedure() { return procedure; } + /** + * Returns the HTTP status code for the call. + * + * @return the status code for the call, null if not available + * @since 6.5.0 + */ + public Integer getStatusCode() { + return statusCode; + } + + /** + * Returns the HTTP reason message for the call. + * + * @return the text of the reason message, null if not available + * @since 6.5.0 + */ + public String getStatusText() { + return statusText; + } + /** * Returns the headers from the response of the external call. * @@ -115,6 +144,8 @@ protected static class Builder implements UriParameter, ProcedureParameter, Inbo private String library; private URI uri; private String procedure; + private Integer statusCode; + private String statusText; private InboundHeaders inboundHeaders; private ExtendedInboundHeaders extendedInboundHeaders; @@ -147,8 +178,17 @@ public Build noInboundHeaders() { return this; } + @Override + public Build status(Integer statusCode, String statusText) { + this.statusCode = statusCode; + if (statusText != null && !statusText.isEmpty()) { + this.statusText = statusText; + } + return this; + } + public HttpParameters build() { - return new HttpParameters(library, uri, procedure, inboundHeaders, extendedInboundHeaders); + return new HttpParameters(library, uri, procedure, statusCode, statusText, inboundHeaders, extendedInboundHeaders); } } @@ -201,7 +241,6 @@ public interface InboundHeadersParameter { * @return the completed HttpParameters object */ Build extendedInboundHeaders(ExtendedInboundHeaders extendedInboundHeaders); - /** * No inbound headers. * @@ -212,6 +251,12 @@ public interface InboundHeadersParameter { public interface Build { + /** + * Set the status code/text of the HTTP response. + * @return the builder in a buildable state. + */ + Build status(Integer statusCode, String statusText); + /** * Build the final {@link HttpParameters} for the API call. * From 2b11504aa34fdb1d8a4a755b10eaa63bd59b5965 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Tue, 13 Apr 2021 15:31:44 -0400 Subject: [PATCH 03/16] Reverts constructor changes to maintain backward compatibility Creates a new constructor that receives the Builder, so no changes on the constructor are needed when Builder is updated --- .../newrelic/api/agent/HttpParameters.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java b/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java index c119ed32bb..877267216f 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java @@ -46,16 +46,15 @@ public class HttpParameters implements ExternalParameters { private final ExtendedInboundHeaders extendedInboundResponseHeaders; protected HttpParameters(String library, URI uri, String procedure, InboundHeaders inboundHeaders) { - this(library, uri, procedure, null, null, inboundHeaders, null); + this(library, uri, procedure, inboundHeaders, null); } - protected HttpParameters(String library, URI uri, String procedure, Integer statusCode, String statusText, InboundHeaders inboundHeaders, - ExtendedInboundHeaders extendedInboundHeaders) { + protected HttpParameters(String library, URI uri, String procedure, InboundHeaders inboundHeaders, ExtendedInboundHeaders extendedInboundHeaders) { this.library = library; this.uri = uri; this.procedure = procedure; - this.statusCode = statusCode; - this.statusText = statusText; + this.statusCode = null; + this.statusText = null; this.inboundResponseHeaders = inboundHeaders; this.extendedInboundResponseHeaders = extendedInboundHeaders; } @@ -70,6 +69,16 @@ protected HttpParameters(HttpParameters httpParameters) { this.extendedInboundResponseHeaders = null; } + protected HttpParameters(Builder builder) { + this.library = builder.library; + this.uri = builder.uri; + this.procedure = builder.procedure; + this.statusCode = builder.statusCode; + this.statusText = builder.statusText; + this.inboundResponseHeaders = builder.inboundHeaders; + this.extendedInboundResponseHeaders = builder.extendedInboundHeaders; + } + /** * Returns the name of the framework used to make the connection. * @@ -188,7 +197,7 @@ public Build status(Integer statusCode, String statusText) { } public HttpParameters build() { - return new HttpParameters(library, uri, procedure, statusCode, statusText, inboundHeaders, extendedInboundHeaders); + return new HttpParameters(this); } } From 9f767bbc20a9b7877c17ca0c8f3ee496c92106a4 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Tue, 13 Apr 2021 15:34:01 -0400 Subject: [PATCH 04/16] Common changes for testing --- .../agent/introspec/ExternalRequest.java | 4 +++ .../newrelic/agent/introspec/SpanEvent.java | 4 +++ .../internal/ExternalRequestImpl.java | 31 +++++++++++++++++-- .../introspec/internal/SpanEventImpl.java | 11 +++++++ 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/ExternalRequest.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/ExternalRequest.java index cae3a438d5..c56ec65a68 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/ExternalRequest.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/ExternalRequest.java @@ -17,6 +17,10 @@ public interface ExternalRequest { String getLibrary(); + Integer getStatusCode(); + + String getStatusText(); + String getOperation(); String getTransactionGuild(); diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/SpanEvent.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/SpanEvent.java index 9a3fd0f31a..3e30b34fc4 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/SpanEvent.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/SpanEvent.java @@ -28,5 +28,9 @@ public interface SpanEvent { String getTransactionId(); + Integer getStatusCode(); + + String getStatusText(); + Map getAgentAttributes(); } diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java index ef65cf3cfa..c6092d71e1 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java @@ -12,6 +12,8 @@ import com.newrelic.agent.introspec.ExternalRequest; import com.newrelic.agent.tracers.Tracer; +import com.newrelic.api.agent.ExternalParameters; +import com.newrelic.api.agent.HttpParameters; class ExternalRequestImpl extends RequestImpl implements ExternalRequest { @@ -23,15 +25,19 @@ class ExternalRequestImpl extends RequestImpl implements ExternalRequest { private String library; private String operation; + private Integer statusCode; + private String statusText; private String segmentName; private String catTransactionGuid; - private ExternalRequestImpl(String originalMetric, String segmentName, String host, String lib, String operation, + private ExternalRequestImpl(String originalMetric, String segmentName, String host, String lib, String operation, Integer statusCode, String statusText, String catTransactionGuid) { super(originalMetric, host); this.library = lib; this.segmentName = segmentName; this.operation = operation; + this.statusCode = statusCode; + this.statusText = statusText; this.catTransactionGuid = catTransactionGuid; } @@ -41,6 +47,15 @@ public static ExternalRequestImpl checkAndMakeExternal(Tracer transactionSegment String metricName = transactionSegment.getMetricName(); Matcher segMatcher = EXTERNAL_SEGMENT.matcher(transactionSegmentName); Matcher metricMatcher = EXTERNAL_METRIC.matcher(metricName); + + Integer statusCode = null; + String statusText = null; + if (transactionSegment.getExternalParameters() instanceof HttpParameters) { + HttpParameters httpParams = (HttpParameters) transactionSegment.getExternalParameters(); + statusCode = httpParams.getStatusCode(); + statusText = httpParams.getStatusText(); + } + if (segMatcher.matches() && (segMatcher.groupCount() == 2 || segMatcher.groupCount() == 4) && metricMatcher.matches() && metricMatcher.groupCount() == 2) { String host = segMatcher.group(1); @@ -49,7 +64,7 @@ public static ExternalRequestImpl checkAndMakeExternal(Tracer transactionSegment if (segMatcher.groupCount() == 4) { op = segMatcher.group(4); } - return new ExternalRequestImpl(metricName, transactionSegmentName, host, lib, op, + return new ExternalRequestImpl(metricName, transactionSegmentName, host, lib, op, statusCode, statusText, (String) transactionSegment.getAgentAttribute("transaction_guid")); } else { segMatcher = EXTERNAL_TX_METRIC.matcher(transactionSegmentName); @@ -59,7 +74,7 @@ public static ExternalRequestImpl checkAndMakeExternal(Tracer transactionSegment String host = segMatcher.group(1); String lib = null; String op = null; - return new ExternalRequestImpl(metricName, transactionSegmentName, host, lib, op, + return new ExternalRequestImpl(metricName, transactionSegmentName, host, lib, op, statusCode, statusText, (String) transactionSegment.getAgentAttribute("transaction_guid")); } } @@ -83,6 +98,16 @@ public String getLibrary() { return library; } + @Override + public Integer getStatusCode() { + return statusCode; + } + + @Override + public String getStatusText() { + return statusText; + } + @Override public String getOperation() { return this.operation; diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/SpanEventImpl.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/SpanEventImpl.java index e69e9b6692..fcaa743ee5 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/SpanEventImpl.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/SpanEventImpl.java @@ -64,6 +64,17 @@ public String getTransactionId() { return (String) spanEvent.getIntrinsics().get("transactionId"); } + @Override + public Integer getStatusCode() { + return (Integer) spanEvent.getAgentAttributes().get("http.statusCode"); + } + + @Override + public String getStatusText() { + return (String) spanEvent.getAgentAttributes().get("http.statusText"); + } + + @Override public Map getAgentAttributes() { return spanEvent.getAgentAttributes(); From 7ac14dc8dad3dd319106c6dc6899c2f377f3bbf4 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Tue, 13 Apr 2021 15:40:24 -0400 Subject: [PATCH 05/16] Http status - testing async-http5 --- .../asynchttpclient/NRAsyncHandler.java | 18 ++++++++++++++++++ .../AsyncHttpClient2_0_0Tests.java | 4 ++++ .../AsyncHandler_Instrumentation.java | 18 ++++++++++++++++++ .../AsyncHttpClient2_1_0Tests.java | 4 ++++ 4 files changed, 44 insertions(+) diff --git a/instrumentation/async-http-client-2.0.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java b/instrumentation/async-http-client-2.0.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java index b5590084f9..69fd556fe3 100644 --- a/instrumentation/async-http-client-2.0.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java +++ b/instrumentation/async-http-client-2.0.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java @@ -45,6 +45,8 @@ public class NRAsyncHandler { public URI uri; @NewField private InboundWrapper inboundHeaders; + @NewField + private HttpResponseStatus responseStatus; public AsyncHandler.State onStatusReceived(HttpResponseStatus responseStatus) throws Exception { AsyncHandler.State userState = Weaver.callOriginal(); @@ -52,6 +54,7 @@ public AsyncHandler.State onStatusReceived(HttpResponseStatus responseStatus) th userAbortedOnStatusReceived.set(true); return AsyncHandler.State.CONTINUE; } + this.responseStatus = responseStatus; return userState; } @@ -93,6 +96,7 @@ public T onCompleted() throws Exception { .uri(uri) .procedure("onCompleted") .inboundHeaders(inboundHeaders) + .status(getStatusCode(), getStatusText()) .build()); //This used to be segment.finish(t), but the agent doesn't automatically report t. segment.end(); @@ -102,4 +106,18 @@ public T onCompleted() throws Exception { return Weaver.callOriginal(); } + + private Integer getStatusCode() { + if (responseStatus != null) { + return responseStatus.getStatusCode(); + } + return null; + } + + private String getStatusText() { + if (responseStatus != null) { + return responseStatus.getStatusText(); + } + return null; + } } diff --git a/instrumentation/async-http-client-2.0.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_0_0Tests.java b/instrumentation/async-http-client-2.0.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_0_0Tests.java index ad1bdf82a5..cb0bbc6429 100644 --- a/instrumentation/async-http-client-2.0.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_0_0Tests.java +++ b/instrumentation/async-http-client-2.0.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_0_0Tests.java @@ -96,6 +96,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(endpoint.getHost(), externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @@ -153,6 +155,8 @@ public void testSuccess() throws Exception { assertEquals(host, externalRequest.getHostname()); assertEquals("AsyncHttpClient", externalRequest.getLibrary()); assertEquals("onCompleted", externalRequest.getOperation()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test diff --git a/instrumentation/async-http-client-2.1.0/src/main/java/org/asynchttpclient/AsyncHandler_Instrumentation.java b/instrumentation/async-http-client-2.1.0/src/main/java/org/asynchttpclient/AsyncHandler_Instrumentation.java index e7f81eb863..f21a4b384e 100644 --- a/instrumentation/async-http-client-2.1.0/src/main/java/org/asynchttpclient/AsyncHandler_Instrumentation.java +++ b/instrumentation/async-http-client-2.1.0/src/main/java/org/asynchttpclient/AsyncHandler_Instrumentation.java @@ -43,6 +43,8 @@ public class AsyncHandler_Instrumentation { public URI uri; @NewField private InboundWrapper inboundHeaders; + @NewField + private HttpResponseStatus responseStatus; public AsyncHandler.State onStatusReceived(HttpResponseStatus responseStatus) { AsyncHandler.State userState = Weaver.callOriginal(); @@ -53,6 +55,7 @@ public AsyncHandler.State onStatusReceived(HttpResponseStatus responseStatus) { userAbortedOnStatusReceived.set(true); return AsyncHandler.State.CONTINUE; } + this.responseStatus = responseStatus; return userState; } @@ -94,6 +97,7 @@ public T onCompleted() throws Exception { .uri(uri) .procedure("onCompleted") .inboundHeaders(inboundHeaders) + .status(getStatusCode(), getStatusText()) .build()); //This used to be segment.finish(t), but the agent doesn't automatically report t. segment.end(); @@ -105,4 +109,18 @@ public T onCompleted() throws Exception { return Weaver.callOriginal(); } + + private Integer getStatusCode() { + if (responseStatus != null) { + return responseStatus.getStatusCode(); + } + return null; + } + + private String getStatusText() { + if (responseStatus != null) { + return responseStatus.getStatusText(); + } + return null; + } } \ No newline at end of file diff --git a/instrumentation/async-http-client-2.1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_1_0Tests.java b/instrumentation/async-http-client-2.1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_1_0Tests.java index 96f8a7ef16..508c888b2c 100644 --- a/instrumentation/async-http-client-2.1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_1_0Tests.java +++ b/instrumentation/async-http-client-2.1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_1_0Tests.java @@ -115,6 +115,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test @@ -170,6 +172,8 @@ public void testSuccess() throws Exception { assertEquals(host, externalRequest.getHostname()); assertEquals("AsyncHttpClient", externalRequest.getLibrary()); assertEquals("onCompleted", externalRequest.getOperation()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test From 9e41c0291c2ee493a75828685980127223388e2a Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Tue, 13 Apr 2021 17:17:58 -0400 Subject: [PATCH 06/16] Http status - testing ning-async --- .../asynchttpclient/NRAsyncHandler.java | 19 +++++++++++++++++++ .../NingAsyncHttpClient10Tests.java | 4 ++++ .../asynchttpclient/NRAsyncHandler.java | 19 +++++++++++++++++++ .../NingAsyncHttpClient11Tests.java | 5 ++++- .../asynchttpclient/NRAsyncHandler.java | 19 +++++++++++++++++++ .../NingAsyncHttpClient161Tests.java | 5 ++++- 6 files changed, 69 insertions(+), 2 deletions(-) diff --git a/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java b/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java index 2edf5ef33e..c54f65ec4d 100644 --- a/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java +++ b/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java @@ -48,6 +48,8 @@ public class NRAsyncHandler { public URI uri; @NewField private InboundWrapper inboundHeaders; + @NewField + private HttpResponseStatus responseStatus; public AsyncHandler.STATE onStatusReceived(HttpResponseStatus responseStatus) { AsyncHandler.STATE userState = Weaver.callOriginal(); @@ -58,6 +60,7 @@ public AsyncHandler.STATE onStatusReceived(HttpResponseStatus responseStatus) { userAbortedOnStatusReceived.set(true); return AsyncHandler.STATE.CONTINUE; } + this.responseStatus = responseStatus; return userState; } @@ -108,6 +111,7 @@ public T onCompleted() throws Exception { .uri(uri) .procedure("onCompleted") .inboundHeaders(inboundHeaders) + .status(getStatusCode(), getReasonMessage()) .build()); //This used to be segment.finish(t), but the agent doesn't automatically report t. segment.end(); @@ -119,4 +123,19 @@ public T onCompleted() throws Exception { return Weaver.callOriginal(); } + + private Integer getStatusCode() { + if (responseStatus != null) + { + return responseStatus.getStatusCode(); + } + return null; + } + + private String getReasonMessage() { + if (responseStatus != null) { + return responseStatus.getStatusText(); + } + return null; + } } \ No newline at end of file diff --git a/instrumentation/ning-async-http-client-1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient10Tests.java b/instrumentation/ning-async-http-client-1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient10Tests.java index b769ac8d45..c2edb5f217 100644 --- a/instrumentation/ning-async-http-client-1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient10Tests.java +++ b/instrumentation/ning-async-http-client-1.0/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient10Tests.java @@ -90,6 +90,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test @@ -148,6 +150,8 @@ public void testSuccess() throws Exception { assertEquals("AsyncHttpClient", externalRequest.getLibrary()); assertEquals("onCompleted", externalRequest.getOperation()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); // netty? } diff --git a/instrumentation/ning-async-http-client-1.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java b/instrumentation/ning-async-http-client-1.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java index b50ea6e88e..c4087e889d 100644 --- a/instrumentation/ning-async-http-client-1.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java +++ b/instrumentation/ning-async-http-client-1.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java @@ -45,6 +45,8 @@ public class NRAsyncHandler { public URI uri; @NewField private InboundWrapper inboundHeaders; + @NewField + private HttpResponseStatus responseStatus; public AsyncHandler.STATE onStatusReceived(HttpResponseStatus responseStatus) { AsyncHandler.STATE userState = Weaver.callOriginal(); @@ -55,6 +57,7 @@ public AsyncHandler.STATE onStatusReceived(HttpResponseStatus responseStatus) { userAbortedOnStatusReceived.set(true); return AsyncHandler.STATE.CONTINUE; } + this.responseStatus = responseStatus; return userState; } @@ -98,6 +101,7 @@ public T onCompleted() throws Exception { .uri(uri) .procedure("onCompleted") .inboundHeaders(inboundHeaders) + .status(getStatusCode(), getReasonMessage()) .build()); //This used to be segment.finish(t), but the agent doesn't automatically report t. segment.end(); @@ -109,4 +113,19 @@ public T onCompleted() throws Exception { return Weaver.callOriginal(); } + + private Integer getStatusCode() { + if (responseStatus != null) + { + return responseStatus.getStatusCode(); + } + return null; + } + + private String getReasonMessage() { + if (responseStatus != null) { + return responseStatus.getStatusText(); + } + return null; + } } \ No newline at end of file diff --git a/instrumentation/ning-async-http-client-1.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient11Tests.java b/instrumentation/ning-async-http-client-1.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient11Tests.java index 0c316cc29b..c3f24da85f 100644 --- a/instrumentation/ning-async-http-client-1.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient11Tests.java +++ b/instrumentation/ning-async-http-client-1.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient11Tests.java @@ -88,6 +88,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test @@ -146,7 +148,8 @@ public void testSuccess() throws Exception { assertEquals(host, externalRequest.getHostname()); assertEquals("AsyncHttpClient", externalRequest.getLibrary()); assertEquals("onCompleted", externalRequest.getOperation()); - + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test diff --git a/instrumentation/ning-async-http-client-1.6.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java b/instrumentation/ning-async-http-client-1.6.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java index 63376068d4..d7467249cc 100644 --- a/instrumentation/ning-async-http-client-1.6.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java +++ b/instrumentation/ning-async-http-client-1.6.1/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java @@ -45,6 +45,8 @@ public class NRAsyncHandler { public URI uri; @NewField private InboundWrapper inboundHeaders; + @NewField + private HttpResponseStatus responseStatus; public AsyncHandler.STATE onStatusReceived(HttpResponseStatus responseStatus) { AsyncHandler.STATE userState = Weaver.callOriginal(); @@ -55,6 +57,7 @@ public AsyncHandler.STATE onStatusReceived(HttpResponseStatus responseStatus) { userAbortedOnStatusReceived.set(true); return AsyncHandler.STATE.CONTINUE; } + this.responseStatus = responseStatus; return userState; } @@ -98,6 +101,7 @@ public T onCompleted() throws Exception { .uri(uri) .procedure("onCompleted") .inboundHeaders(inboundHeaders) + .status(getStatusCode(), getReasonMessage()) .build()); // This used to be segment.finish(t), but the agent doesn't automatically report it. segment.end(); @@ -109,4 +113,19 @@ public T onCompleted() throws Exception { return Weaver.callOriginal(); } + + private Integer getStatusCode() { + if (responseStatus != null) + { + return responseStatus.getStatusCode(); + } + return null; + } + + private String getReasonMessage() { + if (responseStatus != null) { + return responseStatus.getStatusText(); + } + return null; + } } \ No newline at end of file diff --git a/instrumentation/ning-async-http-client-1.6.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient161Tests.java b/instrumentation/ning-async-http-client-1.6.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient161Tests.java index f561c783f5..222438ff0e 100644 --- a/instrumentation/ning-async-http-client-1.6.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient161Tests.java +++ b/instrumentation/ning-async-http-client-1.6.1/src/test/java/com/nr/agent/instrumentation/asynchttpclient/NingAsyncHttpClient161Tests.java @@ -96,6 +96,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test @@ -153,7 +155,8 @@ public void testSuccess() throws Exception { assertEquals(host, externalRequest.getHostname()); assertEquals("AsyncHttpClient", externalRequest.getLibrary()); assertEquals("onCompleted", externalRequest.getOperation()); - + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } @Test From ff50611c8c18e1a7c54580034ffccdafc3719baa Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Tue, 13 Apr 2021 17:35:24 -0400 Subject: [PATCH 07/16] Http status - testing okhttp --- .../okhttp30/internal/RealCall_Instrumentation.java | 1 + .../com/nr/agent/instrumentation/okhttp30/OkHttp30Test.java | 4 ++++ .../java/com/nr/agent/instrumentation/okhttp314/OkUtils.java | 1 + .../com/nr/agent/instrumentation/okhttp314/OkHttp314Test.java | 4 ++++ .../okhttp34/internal/RealCall_Instrumentation.java | 1 + .../com/nr/agent/instrumentation/okhttp34/OkHttp34Test.java | 4 ++++ .../java/com/nr/agent/instrumentation/okhttp35/OkUtils.java | 1 + .../com/nr/agent/instrumentation/okhttp35/OkHttp35Test.java | 4 ++++ .../java/com/nr/agent/instrumentation/okhttp36/OkUtils.java | 1 + .../com/nr/agent/instrumentation/okhttp36/OkHttp36Test.java | 4 ++++ .../java/com/nr/agent/instrumentation/okhttp4/OkUtils.java | 1 + .../com/nr/agent/instrumentation/okhttp4/OkHttp4Test.java | 4 ++++ .../java/com/nr/agent/instrumentation/okhttp44/OkUtils.java | 1 + .../com/nr/agent/instrumentation/okhttp44/OkHttp44Test.java | 4 ++++ 14 files changed, 35 insertions(+) diff --git a/instrumentation/okhttp-3.0.0/src/main/java/com/nr/agent/instrumentation/okhttp30/internal/RealCall_Instrumentation.java b/instrumentation/okhttp-3.0.0/src/main/java/com/nr/agent/instrumentation/okhttp30/internal/RealCall_Instrumentation.java index 34f8610cd5..e7b7844d9d 100644 --- a/instrumentation/okhttp-3.0.0/src/main/java/com/nr/agent/instrumentation/okhttp30/internal/RealCall_Instrumentation.java +++ b/instrumentation/okhttp-3.0.0/src/main/java/com/nr/agent/instrumentation/okhttp30/internal/RealCall_Instrumentation.java @@ -64,6 +64,7 @@ private static void processResponse(URI requestUri, Response response) { .uri(requestUri) .procedure(PROCEDURE) .inboundHeaders(new InboundWrapper(response)) + .status(response.code(), response.message()) .build()); } } diff --git a/instrumentation/okhttp-3.0.0/src/test/java/com/nr/agent/instrumentation/okhttp30/OkHttp30Test.java b/instrumentation/okhttp-3.0.0/src/test/java/com/nr/agent/instrumentation/okhttp30/OkHttp30Test.java index 60910160d4..001682e370 100644 --- a/instrumentation/okhttp-3.0.0/src/test/java/com/nr/agent/instrumentation/okhttp30/OkHttp30Test.java +++ b/instrumentation/okhttp-3.0.0/src/test/java/com/nr/agent/instrumentation/okhttp30/OkHttp30Test.java @@ -106,6 +106,8 @@ public void testExternal() throws Exception { Assert.assertEquals(host1, externalRequest.getHostname()); Assert.assertEquals("OkHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } @Test @@ -151,6 +153,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } private void httpClientExternal(String host) throws IOException { diff --git a/instrumentation/okhttp-3.14.0/src/main/java/com/nr/agent/instrumentation/okhttp314/OkUtils.java b/instrumentation/okhttp-3.14.0/src/main/java/com/nr/agent/instrumentation/okhttp314/OkUtils.java index 3a1cbf8166..11176cc19a 100644 --- a/instrumentation/okhttp-3.14.0/src/main/java/com/nr/agent/instrumentation/okhttp314/OkUtils.java +++ b/instrumentation/okhttp-3.14.0/src/main/java/com/nr/agent/instrumentation/okhttp314/OkUtils.java @@ -52,6 +52,7 @@ public static void processResponse(URI requestUri, Response response) { .uri(requestUri) .procedure(PROCEDURE) .inboundHeaders(new InboundWrapper(response)) + .status(response.code(), response.message()) .build()); } } diff --git a/instrumentation/okhttp-3.14.0/src/test/java/com/nr/agent/instrumentation/okhttp314/OkHttp314Test.java b/instrumentation/okhttp-3.14.0/src/test/java/com/nr/agent/instrumentation/okhttp314/OkHttp314Test.java index 6840e84c22..1d12f5d317 100644 --- a/instrumentation/okhttp-3.14.0/src/test/java/com/nr/agent/instrumentation/okhttp314/OkHttp314Test.java +++ b/instrumentation/okhttp-3.14.0/src/test/java/com/nr/agent/instrumentation/okhttp314/OkHttp314Test.java @@ -108,6 +108,8 @@ public void testExternal() throws Exception { Assert.assertEquals(host1, externalRequest.getHostname()); Assert.assertEquals("OkHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } @Test @@ -153,6 +155,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } private void httpClientExternal(String host) throws IOException { diff --git a/instrumentation/okhttp-3.4.0/src/main/java/com/nr/agent/instrumentation/okhttp34/internal/RealCall_Instrumentation.java b/instrumentation/okhttp-3.4.0/src/main/java/com/nr/agent/instrumentation/okhttp34/internal/RealCall_Instrumentation.java index 2bc6438394..2b98fe639d 100644 --- a/instrumentation/okhttp-3.4.0/src/main/java/com/nr/agent/instrumentation/okhttp34/internal/RealCall_Instrumentation.java +++ b/instrumentation/okhttp-3.4.0/src/main/java/com/nr/agent/instrumentation/okhttp34/internal/RealCall_Instrumentation.java @@ -66,6 +66,7 @@ private static void processResponse(URI requestUri, Response response) { .uri(requestUri) .procedure(PROCEDURE) .inboundHeaders(new InboundWrapper(response)) + .status(response.code(), response.message()) .build()); } } diff --git a/instrumentation/okhttp-3.4.0/src/test/java/com/nr/agent/instrumentation/okhttp34/OkHttp34Test.java b/instrumentation/okhttp-3.4.0/src/test/java/com/nr/agent/instrumentation/okhttp34/OkHttp34Test.java index aa91225780..dbe3a93e48 100644 --- a/instrumentation/okhttp-3.4.0/src/test/java/com/nr/agent/instrumentation/okhttp34/OkHttp34Test.java +++ b/instrumentation/okhttp-3.4.0/src/test/java/com/nr/agent/instrumentation/okhttp34/OkHttp34Test.java @@ -105,6 +105,8 @@ public void testExternal() throws Exception { Assert.assertEquals(host1, externalRequest.getHostname()); Assert.assertEquals("OkHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } @Test @@ -150,6 +152,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } private void httpClientExternal(String host) throws IOException { diff --git a/instrumentation/okhttp-3.5.0/src/main/java/com/nr/agent/instrumentation/okhttp35/OkUtils.java b/instrumentation/okhttp-3.5.0/src/main/java/com/nr/agent/instrumentation/okhttp35/OkUtils.java index 14cc95e39f..58642f9ff0 100644 --- a/instrumentation/okhttp-3.5.0/src/main/java/com/nr/agent/instrumentation/okhttp35/OkUtils.java +++ b/instrumentation/okhttp-3.5.0/src/main/java/com/nr/agent/instrumentation/okhttp35/OkUtils.java @@ -52,6 +52,7 @@ public static void processResponse(URI requestUri, Response response) { .uri(requestUri) .procedure(PROCEDURE) .inboundHeaders(new InboundWrapper(response)) + .status(response.code(), response.message()) .build()); } } diff --git a/instrumentation/okhttp-3.5.0/src/test/java/com/nr/agent/instrumentation/okhttp35/OkHttp35Test.java b/instrumentation/okhttp-3.5.0/src/test/java/com/nr/agent/instrumentation/okhttp35/OkHttp35Test.java index 04a1f8d28e..b12876cfb3 100644 --- a/instrumentation/okhttp-3.5.0/src/test/java/com/nr/agent/instrumentation/okhttp35/OkHttp35Test.java +++ b/instrumentation/okhttp-3.5.0/src/test/java/com/nr/agent/instrumentation/okhttp35/OkHttp35Test.java @@ -106,6 +106,8 @@ public void testExternal() throws Exception { Assert.assertEquals(host1, externalRequest.getHostname()); Assert.assertEquals("OkHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } @Test @@ -151,6 +153,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } private void httpClientExternal(String host) throws IOException { diff --git a/instrumentation/okhttp-3.6.0/src/main/java/com/nr/agent/instrumentation/okhttp36/OkUtils.java b/instrumentation/okhttp-3.6.0/src/main/java/com/nr/agent/instrumentation/okhttp36/OkUtils.java index d86a198f36..190038d7b4 100644 --- a/instrumentation/okhttp-3.6.0/src/main/java/com/nr/agent/instrumentation/okhttp36/OkUtils.java +++ b/instrumentation/okhttp-3.6.0/src/main/java/com/nr/agent/instrumentation/okhttp36/OkUtils.java @@ -52,6 +52,7 @@ public static void processResponse(URI requestUri, Response response) { .uri(requestUri) .procedure(PROCEDURE) .inboundHeaders(new InboundWrapper(response)) + .status(response.code(), response.message()) .build()); } } diff --git a/instrumentation/okhttp-3.6.0/src/test/java/com/nr/agent/instrumentation/okhttp36/OkHttp36Test.java b/instrumentation/okhttp-3.6.0/src/test/java/com/nr/agent/instrumentation/okhttp36/OkHttp36Test.java index 2dce233448..8c3846eecc 100644 --- a/instrumentation/okhttp-3.6.0/src/test/java/com/nr/agent/instrumentation/okhttp36/OkHttp36Test.java +++ b/instrumentation/okhttp-3.6.0/src/test/java/com/nr/agent/instrumentation/okhttp36/OkHttp36Test.java @@ -106,6 +106,8 @@ public void testExternal() throws Exception { Assert.assertEquals(host1, externalRequest.getHostname()); Assert.assertEquals("OkHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } @Test @@ -151,6 +153,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } private void httpClientExternal(String host) throws IOException { diff --git a/instrumentation/okhttp-4.0.0/src/main/java/com/nr/agent/instrumentation/okhttp4/OkUtils.java b/instrumentation/okhttp-4.0.0/src/main/java/com/nr/agent/instrumentation/okhttp4/OkUtils.java index 83e0b32ce2..3538104cb6 100644 --- a/instrumentation/okhttp-4.0.0/src/main/java/com/nr/agent/instrumentation/okhttp4/OkUtils.java +++ b/instrumentation/okhttp-4.0.0/src/main/java/com/nr/agent/instrumentation/okhttp4/OkUtils.java @@ -51,6 +51,7 @@ public static void processResponse(URI requestUri, Response response) { .uri(requestUri) .procedure(PROCEDURE) .inboundHeaders(new InboundWrapper(response)) + .status(response.code(), response.message()) .build()); } } diff --git a/instrumentation/okhttp-4.0.0/src/test/java/com/nr/agent/instrumentation/okhttp4/OkHttp4Test.java b/instrumentation/okhttp-4.0.0/src/test/java/com/nr/agent/instrumentation/okhttp4/OkHttp4Test.java index 9bf5ea1df3..6805a2cf7b 100644 --- a/instrumentation/okhttp-4.0.0/src/test/java/com/nr/agent/instrumentation/okhttp4/OkHttp4Test.java +++ b/instrumentation/okhttp-4.0.0/src/test/java/com/nr/agent/instrumentation/okhttp4/OkHttp4Test.java @@ -109,6 +109,8 @@ public void testExternal() throws Exception { Assert.assertEquals(host1, externalRequest.getHostname()); Assert.assertEquals("OkHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } @Test @@ -154,6 +156,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } private void httpClientExternal(String host) throws IOException { diff --git a/instrumentation/okhttp-4.4.0/src/main/java/com/nr/agent/instrumentation/okhttp44/OkUtils.java b/instrumentation/okhttp-4.4.0/src/main/java/com/nr/agent/instrumentation/okhttp44/OkUtils.java index 38dcb28bf9..350a9e8d4e 100644 --- a/instrumentation/okhttp-4.4.0/src/main/java/com/nr/agent/instrumentation/okhttp44/OkUtils.java +++ b/instrumentation/okhttp-4.4.0/src/main/java/com/nr/agent/instrumentation/okhttp44/OkUtils.java @@ -51,6 +51,7 @@ public static void processResponse(URI requestUri, Response response) { .uri(requestUri) .procedure(PROCEDURE) .inboundHeaders(new InboundWrapper(response)) + .status(response.code(), response.message()) .build()); } } diff --git a/instrumentation/okhttp-4.4.0/src/test/java/com/nr/agent/instrumentation/okhttp44/OkHttp44Test.java b/instrumentation/okhttp-4.4.0/src/test/java/com/nr/agent/instrumentation/okhttp44/OkHttp44Test.java index 2e169d8dfe..4e0a0c1c76 100644 --- a/instrumentation/okhttp-4.4.0/src/test/java/com/nr/agent/instrumentation/okhttp44/OkHttp44Test.java +++ b/instrumentation/okhttp-4.4.0/src/test/java/com/nr/agent/instrumentation/okhttp44/OkHttp44Test.java @@ -106,6 +106,8 @@ public void testExternal() throws Exception { Assert.assertEquals(host1, externalRequest.getHostname()); Assert.assertEquals("OkHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } @Test @@ -151,6 +153,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK ", externalRequest.getStatusText()); // the test server does return the trailing space, this client does not trim it } private void httpClientExternal(String host) throws IOException { From 870e4ad26dcfb9ebdab9327dd57116786a808022 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Wed, 14 Apr 2021 12:20:21 -0400 Subject: [PATCH 08/16] Http status - testing S3 --- .../awsjavasdk2/services/s3/S3MetricUtil.java | 16 ++++ .../s3/S3AsyncClient_Instrumentation.java | 48 ++++++----- .../services/s3/S3Client_Instrumentation.java | 80 +++++++++++++++---- .../awsjavasdks3/AmazonS3AsyncApiTest.java | 18 ++--- .../awsjavasdks3/AmazonS3SyncApiTest.java | 20 +++-- .../awsjavasdks3/S3MetricAssertions.java | 21 ++++- 6 files changed, 148 insertions(+), 55 deletions(-) diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java index bec70a7947..5a7a2cb217 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java @@ -13,6 +13,8 @@ import com.newrelic.api.agent.Segment; import com.newrelic.api.agent.TracedMethod; import com.newrelic.api.agent.weaver.Weaver; +import software.amazon.awssdk.services.s3.model.CreateBucketResponse; +import software.amazon.awssdk.services.s3.model.S3Response; import java.net.URI; import java.net.URISyntaxException; @@ -39,4 +41,18 @@ public static void reportExternalMetrics(TracedMethod tracedMethod, String uri, } } + + public static void reportExternalMetrics(TracedMethod tracedMethod, String uri, S3Response s3Response, String operationName) { + try { + HttpParameters httpParameters = HttpParameters.library(SERVICE) + .uri(new URI(uri)) + .procedure(operationName) + .noInboundHeaders() + .status(s3Response.sdkHttpResponse().statusCode(), s3Response.sdkHttpResponse().statusText().orElse(null)) + .build(); + tracedMethod.reportAsExternal(httpParameters); + } catch (URISyntaxException e) { + AgentBridge.instrumentation.noticeInstrumentationError(e, Weaver.getImplementationTitle()); + } + } } diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java index f1d9e4eac5..188868abe5 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java @@ -27,7 +27,6 @@ import software.amazon.awssdk.services.s3.model.GetBucketLocationRequest; import software.amazon.awssdk.services.s3.model.GetBucketLocationResponse; import software.amazon.awssdk.services.s3.model.GetObjectRequest; -import software.amazon.awssdk.services.s3.model.GetObjectResponse; import software.amazon.awssdk.services.s3.model.ListBucketsRequest; import software.amazon.awssdk.services.s3.model.ListBucketsResponse; import software.amazon.awssdk.services.s3.model.ListObjectsRequest; @@ -45,51 +44,46 @@ public class S3AsyncClient_Instrumentation { public CompletableFuture createBucket(CreateBucketRequest createBucketRequest) { String uri = "s3://" + createBucketRequest.bucket(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "createBucket"); - S3MetricUtil.reportExternalMetrics(segment, uri, "createBucket"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "createBucket")); } public CompletableFuture deleteBucket(DeleteBucketRequest deleteBucketRequest) { String uri = "s3://" + deleteBucketRequest.bucket(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "deleteBucket"); - S3MetricUtil.reportExternalMetrics(segment, uri, "deleteBucket"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "deleteBucket")); } public CompletableFuture listBuckets(ListBucketsRequest listBucketsRequest) { String uri = "s3://amazon/"; Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "listBuckets"); - S3MetricUtil.reportExternalMetrics(segment, uri, "listBuckets"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "listBuckets")); } public CompletableFuture getBucketLocation(GetBucketLocationRequest getBucketLocationRequest) { String uri = "s3://" + getBucketLocationRequest.bucket(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "getBucketLocation"); - S3MetricUtil.reportExternalMetrics(segment, uri, "getBucketLocation"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "getBucketLocation")); } public CompletableFuture getObject(GetObjectRequest getObjectRequest, AsyncResponseTransformer asyncResponseTransformer) { String uri = "s3://" + getObjectRequest.bucket() + "/" + getObjectRequest.key(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "getObject"); - S3MetricUtil.reportExternalMetrics(segment, uri, "getObject"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); @@ -100,45 +94,41 @@ public CompletableFuture getObject(GetObjectRequest getObject public CompletableFuture listObjects(ListObjectsRequest listObjectsRequest) { String uri = "s3://" + listObjectsRequest.bucket(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "listObjects"); - S3MetricUtil.reportExternalMetrics(segment, uri, "listObjects"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "listObjects")); } public CompletableFuture putObject(PutObjectRequest putObjectRequest, AsyncRequestBody asyncRequestBody) { String uri = "s3://" + putObjectRequest.bucket() + "/" + putObjectRequest.key(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "putObject"); - S3MetricUtil.reportExternalMetrics(segment, uri, "putObject"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "putObject")); } public CompletableFuture deleteObject(DeleteObjectRequest deleteObjectRequest) { String uri = "s3://" + deleteObjectRequest.bucket() + "/" + deleteObjectRequest.key(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "deleteObject"); - S3MetricUtil.reportExternalMetrics(segment, uri, "deleteObject"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "deleteObject")); } public CompletableFuture deleteObjects(DeleteObjectsRequest deleteObjectsRequest) { String uri = "s3://" + deleteObjectsRequest.bucket(); Segment segment = NewRelic.getAgent().getTransaction().startSegment("S3", "deleteObjects"); - S3MetricUtil.reportExternalMetrics(segment, uri, "deleteObjects"); AgentBridge.getAgent().getTracedMethod().setTrackChildThreads(false); CompletableFuture result = Weaver.callOriginal(); - return result.whenComplete(new ResultWrapper<>(segment)); + return result.whenComplete(new S3ResponseResultWrapper<>(segment, uri, "deleteObjects")); } private class ResultWrapper implements BiConsumer { @@ -157,4 +147,26 @@ public void accept(T t, U u) { } } } + + private class S3ResponseResultWrapper implements BiConsumer { + private Segment segment; + private String uri; + private String operationName; + + public S3ResponseResultWrapper(Segment segment, String uri, String operationName) { + this.segment = segment; + this.uri = uri; + this.operationName = operationName; + } + + @Override + public void accept(T t, U u) { + try { + S3MetricUtil.reportExternalMetrics(segment, uri, operationName); + segment.end(); + } catch (Throwable t1) { + AgentBridge.instrumentation.noticeInstrumentationError(t1, Weaver.getImplementationTitle()); + } + } + } } diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java index 54d160d422..0d2c494b10 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java @@ -40,29 +40,53 @@ public class S3Client_Instrumentation { @Trace public CreateBucketResponse createBucket(CreateBucketRequest createBucketRequest) { String uri = "s3://" + createBucketRequest.bucket(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "createBucket"); - return Weaver.callOriginal(); + try { + CreateBucketResponse createBucketResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, createBucketResponse, "createBucket"); + return createBucketResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "createBucket"); + throw e; + } } @Trace public DeleteBucketResponse deleteBucket(DeleteBucketRequest deleteBucketRequest) { String uri = "s3://" + deleteBucketRequest.bucket(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "deleteBucket"); - return Weaver.callOriginal(); + try { + DeleteBucketResponse deleteBucketResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, deleteBucketResponse, "deleteBucket"); + return deleteBucketResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "deleteBucket"); + throw e; + } } @Trace public ListBucketsResponse listBuckets(ListBucketsRequest listBucketsRequest) { String uri = "s3://amazon/"; - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "listBuckets"); - return Weaver.callOriginal(); + try { + ListBucketsResponse listBucketsResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, listBucketsResponse, "listBuckets"); + return listBucketsResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "listBuckets"); + throw e; + } } @Trace public GetBucketLocationResponse getBucketLocation(GetBucketLocationRequest getBucketLocationRequest) { String uri = "s3://" + getBucketLocationRequest.bucket(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "getBucketLocation"); - return Weaver.callOriginal(); + try { + GetBucketLocationResponse getBucketLocationResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, getBucketLocationResponse, "getBucketLocation"); + return getBucketLocationResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "getBucketLocation"); + throw e; + } } @Trace @@ -75,29 +99,53 @@ public T getObject(GetObjectRequest getObjectRequest, ResponseTransformer Re @Trace public ListObjectsResponse listObjects(ListObjectsRequest listObjectsRequest) { String uri = "s3://" + listObjectsRequest.bucket(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "listObjects"); - return Weaver.callOriginal(); + try { + ListObjectsResponse listObjectsResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, listObjectsResponse, "listObjects"); + return listObjectsResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "listObjects"); + throw e; + } } @Trace public PutObjectResponse putObject(PutObjectRequest putObjectRequest, RequestBody RequestBody) { String uri = "s3://" + putObjectRequest.bucket() + "/" + putObjectRequest.key(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "putObject"); - return Weaver.callOriginal(); + try { + PutObjectResponse putObjectResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, putObjectResponse, "putObject"); + return putObjectResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "putObject"); + throw e; + } } @Trace public DeleteObjectResponse deleteObject(DeleteObjectRequest deleteObjectRequest) { String uri = "s3://" + deleteObjectRequest.bucket() + "/" + deleteObjectRequest.key(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "deleteObject"); - return Weaver.callOriginal(); + try { + DeleteObjectResponse deleteObjectResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, deleteObjectResponse, "deleteObject"); + return deleteObjectResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "deleteObject"); + throw e; + } } @Trace public DeleteObjectsResponse deleteObjects(DeleteObjectsRequest deleteObjectsRequest) { String uri = "s3://" + deleteObjectsRequest.bucket(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "deleteObjects"); - return Weaver.callOriginal(); + try { + DeleteObjectsResponse deleteObjectsResponse = Weaver.callOriginal(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, deleteObjectsResponse, "deleteObjects"); + return deleteObjectsResponse; + } catch (Exception e) { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, "deleteObjects"); + throw e; + } } } diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3AsyncApiTest.java b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3AsyncApiTest.java index 6a5af0fe45..b7d4a29da3 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3AsyncApiTest.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3AsyncApiTest.java @@ -86,14 +86,14 @@ public static void afterClass() { @Test public void testCreateBucket() { createBucket(); - assertMetrics("createBucket"); + assertMetrics("createBucket", 200); } @Test public void testListBuckets() { createBucketNoTxn(); listBuckets(); - assertMetrics("listBuckets"); + assertMetrics("listBuckets", 200); } // This test passes but it consistently takes 30s to complete @@ -103,35 +103,35 @@ public void testListBuckets() { // createBucketNoTxn(); // putObjectNoTxn(); // getObject(); -// assertMetrics("getObject"); +// assertMetrics("getObject", null); // } @Test public void testListObjects() { createBucketNoTxn(); listObjects(); - assertMetrics("listObjects"); + assertMetrics("listObjects", 200); } @Test public void testPutObject() { createBucketNoTxn(); putObject(); - assertMetrics("putObject"); + assertMetrics("putObject", 200); } @Test public void testDeleteBucket() { createBucketNoTxn(); deleteBucket(); - assertMetrics("deleteBucket"); + assertMetrics("deleteBucket", 204); } @Test public void testGetBucketLocation() { createBucketNoTxn(); getBucketLocation(); - assertMetrics("getBucketLocation"); + assertMetrics("getBucketLocation", 200); } // This test passes but it consistently takes 60s to complete @@ -141,7 +141,7 @@ public void testGetBucketLocation() { // createBucketNoTxn(); // putObjectNoTxn(); // deleteObject(); -// assertMetrics("deleteObject"); +// assertMetrics("deleteObject", 204); // } // This test passes but it consistently takes 30s to complete @@ -151,7 +151,7 @@ public void testGetBucketLocation() { // createBucketNoTxn(); // putObjectNoTxn(); // deleteObjects(); -// assertMetrics("deleteObjects"); +// assertMetrics("deleteObjects", 200); // } @Trace(dispatcher = true) diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java index 8ae52afa5d..7eabe3e431 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java @@ -19,8 +19,6 @@ import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; -import software.amazon.awssdk.auth.credentials.AwsCredentials; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.s3.S3Client; import software.amazon.awssdk.services.s3.model.CreateBucketRequest; @@ -86,26 +84,26 @@ public static void afterClass() { @Test public void testCreateBucket() { createBucket(); - assertMetrics("createBucket"); + assertMetrics("createBucket", 200); } @Test public void testDeleteBucket() { createBucketNoTxn(); deleteBucket(); - assertMetrics("deleteBucket"); + assertMetrics("deleteBucket", 204); } @Test public void testListBuckets() { listBuckets(); - assertMetrics("listBuckets"); + assertMetrics("listBuckets", 200); } @Test public void testGetBucketLocation() { getBucketLocation(); - assertMetrics("getBucketLocation"); + assertMetrics("getBucketLocation", 200); } @Test @@ -113,21 +111,21 @@ public void testGetObject() { createBucketNoTxn(); putObjectNoTxn(); getObject(); - assertMetrics("getObject"); + assertMetrics("getObject", null); } @Test public void testListObjects() { createBucketNoTxn(); listObjects(); - assertMetrics("listObjects"); + assertMetrics("listObjects", 200); } @Test public void testPutObject() { createBucketNoTxn(); putObject(); - assertMetrics("putObject"); + assertMetrics("putObject", 200); } @Test @@ -135,7 +133,7 @@ public void testDeleteObject() { createBucketNoTxn(); putObjectNoTxn(); deleteObject(); - assertMetrics("deleteObject"); + assertMetrics("deleteObject", 204); } @Test @@ -143,7 +141,7 @@ public void testDeleteObjects() { createBucketNoTxn(); putObjectNoTxn(); deleteObjects(); - assertMetrics("deleteObjects"); + assertMetrics("deleteObjects", 200); } @Trace(dispatcher = true) diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java index 6563fb766b..1b84aee4a1 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java @@ -7,19 +7,29 @@ package com.agent.instrumentation.awsjavasdks3; +import com.newrelic.agent.introspec.ExternalRequest; import com.newrelic.agent.introspec.InstrumentationTestRunner; import com.newrelic.agent.introspec.Introspector; import com.newrelic.agent.introspec.MetricsHelper; import com.newrelic.agent.introspec.TransactionEvent; import java.util.Collection; +import java.util.HashMap; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; class S3MetricAssertions { - static void assertMetrics(String operation) { + private static Map statusCodeText; + static { + statusCodeText = new HashMap<>(); + statusCodeText.put(200, "OK"); + statusCodeText.put(204, "No Content"); + } + + static void assertMetrics(String operation, Integer expectedStatusCode) { Introspector introspector = InstrumentationTestRunner.getIntrospector(); assertEquals(1, introspector.getFinishedTransactionCount(2000)); @@ -68,6 +78,15 @@ static void assertMetrics(String operation) { assertEquals(1, transactionEvent.getExternalCallCount()); assertTrue(transactionEvent.getExternalDurationInSec() > 0); } + + Collection externalRequests = introspector.getExternalRequests(txName); + assertEquals(1, externalRequests.size()); + + ExternalRequest externalRequest = externalRequests.iterator().next(); + assertEquals(expectedStatusCode, externalRequest.getStatusCode()); + + String expectedStatusText = statusCodeText.get(expectedStatusCode); + assertEquals(expectedStatusText, externalRequest.getStatusText()); } } From b304d017bbc1d20b88eae3ab17475f51c69bf4c4 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Wed, 14 Apr 2021 17:20:13 -0400 Subject: [PATCH 09/16] Http status - instrumenting/testing grpc --- .../main/java/io/grpc/ClientCall_Instrumentation.java | 1 + .../io/grpc/internal/ServerStream_Instrumentation.java | 6 +++++- .../grpc-1.22.0/src/test/java/ValidationHelper.java | 10 ++++++++++ .../main/java/io/grpc/ClientCall_Instrumentation.java | 1 + .../io/grpc/internal/ServerStream_Instrumentation.java | 6 +++++- .../grpc-1.30.0/src/test/java/ValidationHelper.java | 10 ++++++++++ .../main/java/io/grpc/ClientCall_Instrumentation.java | 1 + .../io/grpc/internal/ServerStream_Instrumentation.java | 5 ++++- .../grpc-1.4.0/src/test/java/ValidationHelper.java | 10 ++++++++++ 9 files changed, 47 insertions(+), 3 deletions(-) diff --git a/instrumentation/grpc-1.22.0/src/main/java/io/grpc/ClientCall_Instrumentation.java b/instrumentation/grpc-1.22.0/src/main/java/io/grpc/ClientCall_Instrumentation.java index 4365b36336..2861b7bf73 100644 --- a/instrumentation/grpc-1.22.0/src/main/java/io/grpc/ClientCall_Instrumentation.java +++ b/instrumentation/grpc-1.22.0/src/main/java/io/grpc/ClientCall_Instrumentation.java @@ -108,6 +108,7 @@ public void onClose(Status status, Metadata trailers) { .uri(uri) .procedure(methodDescriptor.getFullMethodName()) .inboundHeaders(wrapper) + .status(status.getCode().value(), status.getDescription()) .build(); if (segment != null) { segment.reportAsExternal(params); diff --git a/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java b/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java index 6ed40dba40..f79f78c3ae 100644 --- a/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java +++ b/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java @@ -38,7 +38,11 @@ public void close(Status status, Metadata trailers) { } if (status != null) { - NewRelic.addCustomParameter("response.status", status.getCode().value()); + int statusCode = status.getCode().value(); + NewRelic.addCustomParameter("response.status", statusCode); + NewRelic.addCustomParameter("http.statusCode", statusCode); + NewRelic.addCustomParameter("http.statusText", status.getDescription()); + if (GrpcConfig.errorsEnabled && status.getCause() != null) { // If an error occurred during the close of this server call we should record it NewRelic.noticeError(status.getCause()); diff --git a/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java index f867ede24d..c9e302ec65 100644 --- a/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java +++ b/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java @@ -7,6 +7,7 @@ import app.TestServer; import com.newrelic.agent.introspec.CatHelper; +import com.newrelic.agent.introspec.ExternalRequest; import com.newrelic.agent.introspec.InstrumentationTestRunner; import com.newrelic.agent.introspec.Introspector; import com.newrelic.agent.introspec.TraceSegment; @@ -18,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; class ValidationHelper { @@ -52,6 +54,12 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri } assertTrue("Unable to find client side External/ segment", foundSegment); + Collection externalRequests = introspector.getExternalRequests(clientTxName); + assertEquals(1, externalRequests.size()); + ExternalRequest externalRequest = externalRequests.iterator().next(); + assertEquals(Integer.valueOf(0), externalRequest.getStatusCode()); + assertNull(externalRequest.getStatusText()); + // Server side Collection serverTransactionTrace = introspector.getTransactionTracesForTransaction(serverTxName); assertEquals(1, serverTransactionTrace.size()); @@ -61,6 +69,8 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri assertEquals(1, rootSegment.getCallCount()); assertEquals(fullMethod, rootSegment.getTracerAttributes().get("request.method")); assertEquals(0, rootSegment.getTracerAttributes().get("response.status")); + assertEquals(0, rootSegment.getTracerAttributes().get("http.statusCode")); + assertNull(rootSegment.getTracerAttributes().get("http.statusText")); assertEquals(grpcType, rootSegment.getTracerAttributes().get("grpc.type")); // Custom attributes (to test tracing into customer code) diff --git a/instrumentation/grpc-1.30.0/src/main/java/io/grpc/ClientCall_Instrumentation.java b/instrumentation/grpc-1.30.0/src/main/java/io/grpc/ClientCall_Instrumentation.java index df181d26d9..c39834be6a 100644 --- a/instrumentation/grpc-1.30.0/src/main/java/io/grpc/ClientCall_Instrumentation.java +++ b/instrumentation/grpc-1.30.0/src/main/java/io/grpc/ClientCall_Instrumentation.java @@ -108,6 +108,7 @@ public void onClose(Status status, Metadata trailers) { .uri(uri) .procedure(methodDescriptor.getFullMethodName()) .inboundHeaders(wrapper) + .status(status.getCode().value(), status.getDescription()) .build(); if (segment != null) { segment.reportAsExternal(params); diff --git a/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java b/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java index 6ed40dba40..f79f78c3ae 100644 --- a/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java +++ b/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java @@ -38,7 +38,11 @@ public void close(Status status, Metadata trailers) { } if (status != null) { - NewRelic.addCustomParameter("response.status", status.getCode().value()); + int statusCode = status.getCode().value(); + NewRelic.addCustomParameter("response.status", statusCode); + NewRelic.addCustomParameter("http.statusCode", statusCode); + NewRelic.addCustomParameter("http.statusText", status.getDescription()); + if (GrpcConfig.errorsEnabled && status.getCause() != null) { // If an error occurred during the close of this server call we should record it NewRelic.noticeError(status.getCause()); diff --git a/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java index f867ede24d..c9e302ec65 100644 --- a/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java +++ b/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java @@ -7,6 +7,7 @@ import app.TestServer; import com.newrelic.agent.introspec.CatHelper; +import com.newrelic.agent.introspec.ExternalRequest; import com.newrelic.agent.introspec.InstrumentationTestRunner; import com.newrelic.agent.introspec.Introspector; import com.newrelic.agent.introspec.TraceSegment; @@ -18,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; class ValidationHelper { @@ -52,6 +54,12 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri } assertTrue("Unable to find client side External/ segment", foundSegment); + Collection externalRequests = introspector.getExternalRequests(clientTxName); + assertEquals(1, externalRequests.size()); + ExternalRequest externalRequest = externalRequests.iterator().next(); + assertEquals(Integer.valueOf(0), externalRequest.getStatusCode()); + assertNull(externalRequest.getStatusText()); + // Server side Collection serverTransactionTrace = introspector.getTransactionTracesForTransaction(serverTxName); assertEquals(1, serverTransactionTrace.size()); @@ -61,6 +69,8 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri assertEquals(1, rootSegment.getCallCount()); assertEquals(fullMethod, rootSegment.getTracerAttributes().get("request.method")); assertEquals(0, rootSegment.getTracerAttributes().get("response.status")); + assertEquals(0, rootSegment.getTracerAttributes().get("http.statusCode")); + assertNull(rootSegment.getTracerAttributes().get("http.statusText")); assertEquals(grpcType, rootSegment.getTracerAttributes().get("grpc.type")); // Custom attributes (to test tracing into customer code) diff --git a/instrumentation/grpc-1.4.0/src/main/java/io/grpc/ClientCall_Instrumentation.java b/instrumentation/grpc-1.4.0/src/main/java/io/grpc/ClientCall_Instrumentation.java index 4365b36336..2861b7bf73 100644 --- a/instrumentation/grpc-1.4.0/src/main/java/io/grpc/ClientCall_Instrumentation.java +++ b/instrumentation/grpc-1.4.0/src/main/java/io/grpc/ClientCall_Instrumentation.java @@ -108,6 +108,7 @@ public void onClose(Status status, Metadata trailers) { .uri(uri) .procedure(methodDescriptor.getFullMethodName()) .inboundHeaders(wrapper) + .status(status.getCode().value(), status.getDescription()) .build(); if (segment != null) { segment.reportAsExternal(params); diff --git a/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java b/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java index 6ed40dba40..dcb1a37aab 100644 --- a/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java +++ b/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java @@ -38,7 +38,10 @@ public void close(Status status, Metadata trailers) { } if (status != null) { - NewRelic.addCustomParameter("response.status", status.getCode().value()); + int statusCode = status.getCode().value(); + NewRelic.addCustomParameter("response.status", statusCode); + NewRelic.addCustomParameter("http.statusCode", statusCode); + NewRelic.addCustomParameter("http.statusText", status.getDescription()); if (GrpcConfig.errorsEnabled && status.getCause() != null) { // If an error occurred during the close of this server call we should record it NewRelic.noticeError(status.getCause()); diff --git a/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java index f867ede24d..c9e302ec65 100644 --- a/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java +++ b/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java @@ -7,6 +7,7 @@ import app.TestServer; import com.newrelic.agent.introspec.CatHelper; +import com.newrelic.agent.introspec.ExternalRequest; import com.newrelic.agent.introspec.InstrumentationTestRunner; import com.newrelic.agent.introspec.Introspector; import com.newrelic.agent.introspec.TraceSegment; @@ -18,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; class ValidationHelper { @@ -52,6 +54,12 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri } assertTrue("Unable to find client side External/ segment", foundSegment); + Collection externalRequests = introspector.getExternalRequests(clientTxName); + assertEquals(1, externalRequests.size()); + ExternalRequest externalRequest = externalRequests.iterator().next(); + assertEquals(Integer.valueOf(0), externalRequest.getStatusCode()); + assertNull(externalRequest.getStatusText()); + // Server side Collection serverTransactionTrace = introspector.getTransactionTracesForTransaction(serverTxName); assertEquals(1, serverTransactionTrace.size()); @@ -61,6 +69,8 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri assertEquals(1, rootSegment.getCallCount()); assertEquals(fullMethod, rootSegment.getTracerAttributes().get("request.method")); assertEquals(0, rootSegment.getTracerAttributes().get("response.status")); + assertEquals(0, rootSegment.getTracerAttributes().get("http.statusCode")); + assertNull(rootSegment.getTracerAttributes().get("http.statusText")); assertEquals(grpcType, rootSegment.getTracerAttributes().get("grpc.type")); // Custom attributes (to test tracing into customer code) From 294487c15526603adf4cac2638647f3e1fa9561b Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Thu, 15 Apr 2021 13:35:00 -0400 Subject: [PATCH 10/16] Http status - instrumenting/testing httpclient --- ...AsyncResponseConsumer_Instrumentation.java | 19 +++++++++ .../HttpAsyncClient4Test.java | 4 ++ .../commons/httpclient/HttpMethodBase.java | 14 +++++-- .../httpclient/HttpClient3Test.java | 40 ++++++++++++------- .../commons/httpclient/HttpMethodBase.java | 1 + .../httpclient/HttpClient31Test.java | 6 +++ .../httpclient/HttpClientSpanTests.java | 2 + 7 files changed, 67 insertions(+), 19 deletions(-) diff --git a/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java b/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java index 632fc46280..7f2ff2f0f5 100644 --- a/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java +++ b/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java @@ -35,12 +35,15 @@ public class HttpAsyncResponseConsumer_Instrumentation { public URI uri; @NewField private InboundWrapper inboundHeaders; + @NewField + private HttpResponse httpResponse; /** * Invoked when a HTTP response message is received. */ public void responseReceived(HttpResponse response) throws IOException, HttpException { inboundHeaders = new InboundWrapper(response); + httpResponse = response; Weaver.callOriginal(); } @@ -54,6 +57,7 @@ public void responseCompleted(HttpContext context) { .uri(uri) .procedure("responseCompleted") .inboundHeaders(inboundHeaders) + .status(getStatusCode(), getReasonMessage()) .build()); segment.end(); } @@ -84,4 +88,19 @@ public void failed(Exception ex) { } Weaver.callOriginal(); } + + private Integer getStatusCode() { + if (httpResponse != null && httpResponse.getStatusLine() != null) + { + return httpResponse.getStatusLine().getStatusCode(); + } + return null; + } + + private String getReasonMessage() { + if (httpResponse != null && httpResponse.getStatusLine() != null) { + return httpResponse.getStatusLine().getReasonPhrase(); + } + return null; + } } diff --git a/instrumentation/http-async-client-4/src/test/java/com/nr/agent/instrumentation/httpasyncclient4/HttpAsyncClient4Test.java b/instrumentation/http-async-client-4/src/test/java/com/nr/agent/instrumentation/httpasyncclient4/HttpAsyncClient4Test.java index 8b05e6b583..5ec2990a61 100644 --- a/instrumentation/http-async-client-4/src/test/java/com/nr/agent/instrumentation/httpasyncclient4/HttpAsyncClient4Test.java +++ b/instrumentation/http-async-client-4/src/test/java/com/nr/agent/instrumentation/httpasyncclient4/HttpAsyncClient4Test.java @@ -116,6 +116,8 @@ public void testExternal() throws IOException, InterruptedException, ExecutionEx Assert.assertEquals(host, externalRequest.getHostname()); Assert.assertEquals("HttpAsyncClient", externalRequest.getLibrary()); Assert.assertEquals("responseCompleted", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK", externalRequest.getStatusText()); } @Test @@ -184,6 +186,8 @@ public void testCat() throws IOException, InterruptedException, ExecutionExcepti ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK", externalRequest.getStatusText()); } /** diff --git a/instrumentation/httpclient-3.0/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java b/instrumentation/httpclient-3.0/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java index e6114188b6..2e4b3b8f60 100644 --- a/instrumentation/httpclient-3.0/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java +++ b/instrumentation/httpclient-3.0/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java @@ -13,6 +13,8 @@ import com.newrelic.agent.bridge.external.ExternalMetrics; import com.newrelic.agent.bridge.external.URISupport; import com.newrelic.agent.tracers.IgnoreChildSocketCalls; +import com.newrelic.api.agent.HttpParameters; +import com.newrelic.api.agent.NewRelic; import com.newrelic.api.agent.Trace; import com.newrelic.api.agent.weaver.MatchType; import com.newrelic.api.agent.weaver.NewField; @@ -59,7 +61,7 @@ public int execute(HttpState state, HttpConnection conn) throws HttpException, I } // Set cross process headers for this outbound request - tx.getCrossProcessState().processOutboundRequestHeaders(new OutboundWrapper(this), method); + method.addOutboundRequestHeaders(new OutboundWrapper(this)); } int responseCode = Weaver.callOriginal(); @@ -68,9 +70,13 @@ public int execute(HttpState state, HttpConnection conn) throws HttpException, I try { InboundWrapper inboundHeaders = new InboundWrapper(this); - // Since this method does network I/O we need to pass "true" as the last parameter here - tx.getCrossProcessState().processInboundResponseHeaders(inboundHeaders, method, host, uri, true); - + method.reportAsExternal(HttpParameters + .library(LIBRARY) + .uri(new java.net.URI(uri)) + .procedure("execute") + .inboundHeaders(inboundHeaders) + .status(responseCode, getStatusText()) + .build()); if (inboundHeaders.getHeader("X-NewRelic-App-Data") == null) { // If this wasn't a cross process request, handle External/ metrics for CommonsHttp ExternalMetrics.makeExternalComponentMetric(method, host, LIBRARY, false, uri, "execute"); diff --git a/instrumentation/httpclient-3.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient3Test.java b/instrumentation/httpclient-3.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient3Test.java index 3701fed78e..e1735cb5f9 100644 --- a/instrumentation/httpclient-3.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient3Test.java +++ b/instrumentation/httpclient-3.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient3Test.java @@ -96,8 +96,8 @@ public void testExternal() throws Exception { Assert.assertEquals(2, introspector.getFinishedTransactionCount()); String txOne = "OtherTransaction/Custom/com.nr.agent.instrumentation.httpclient.HttpClient3Test/httpClientExternal"; - Assert.assertEquals(1, MetricsHelper.getScopedMetricCount(txOne, "External/localhost/CommonsHttp")); - Assert.assertEquals(1, MetricsHelper.getUnscopedMetricCount("External/localhost/CommonsHttp")); + Assert.assertEquals(1, MetricsHelper.getScopedMetricCount(txOne, "External/localhost/CommonsHttp/execute")); + Assert.assertEquals(1, MetricsHelper.getUnscopedMetricCount("External/localhost/CommonsHttp/execute")); // external rollups Assert.assertEquals(1, MetricsHelper.getUnscopedMetricCount("External/localhost/all")); @@ -117,6 +117,8 @@ public void testExternal() throws Exception { Assert.assertEquals("localhost", externalRequest.getHostname()); Assert.assertEquals("CommonsHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK", externalRequest.getStatusText()); } @Test @@ -133,10 +135,10 @@ public void testRollups() throws Exception { Assert.assertEquals(6, introspector.getFinishedTransactionCount()); // host rollups - Assert.assertEquals(2, MetricsHelper.getUnscopedMetricCount("External/localhost/CommonsHttp")); + Assert.assertEquals(2, MetricsHelper.getUnscopedMetricCount("External/localhost/CommonsHttp/execute")); Assert.assertEquals(2, MetricsHelper.getUnscopedMetricCount("External/localhost/all")); - Assert.assertEquals(1, MetricsHelper.getUnscopedMetricCount("External/127.0.0.1/CommonsHttp")); + Assert.assertEquals(1, MetricsHelper.getUnscopedMetricCount("External/127.0.0.1/CommonsHttp/execute")); Assert.assertEquals(1, MetricsHelper.getUnscopedMetricCount("External/127.0.0.1/all")); // external rollups @@ -188,6 +190,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } /** @@ -223,8 +227,10 @@ public void httpClient1() throws IOException, URISyntaxException { "OtherTransaction/Custom/com.nr.agent.instrumentation.httpclient.HttpClient3Test/execute1"); Assert.assertTrue(metrics.containsKey("External/" + endpoint.getHost() + "/CommonsHttp")); metrics.get("External/" + endpoint.getHost() + "/CommonsHttp"); - // This is 2 because execute3() calls execute() and releaseConnection(), both are instrumented - Assert.assertEquals(2, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); + + // execute1() calls execute() and releaseConnection(), both are instrumented + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp/execute").getCallCount()); + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); } @Trace(dispatcher = true) @@ -249,8 +255,9 @@ public void httpClient2() throws IOException, URISyntaxException { "OtherTransaction/Custom/com.nr.agent.instrumentation.httpclient.HttpClient3Test/execute2"); Assert.assertTrue(metrics.containsKey("External/" + endpoint.getHost() + "/CommonsHttp")); metrics.get("External/" + endpoint.getHost() + "/CommonsHttp"); - // This is 2 because execute2() calls execute() and releaseConnection(), both are instrumented - Assert.assertEquals(2, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); + // execute2() calls execute() and releaseConnection(), both are instrumented + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp/execute").getCallCount()); + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); } @Trace(dispatcher = true) @@ -276,8 +283,9 @@ public void httpClient3() throws IOException, URISyntaxException { "OtherTransaction/Custom/com.nr.agent.instrumentation.httpclient.HttpClient3Test/execute3"); Assert.assertTrue(metrics.containsKey("External/" + endpoint.getHost() + "/CommonsHttp")); metrics.get("External/" + endpoint.getHost() + "/CommonsHttp"); - // This is 2 because execute3() calls execute() and releaseConnection(), both are instrumented - Assert.assertEquals(2, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); + // execute3() calls execute() and releaseConnection(), both are instrumented + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp/execute").getCallCount()); + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); } @Trace(dispatcher = true) @@ -304,9 +312,10 @@ public void httpClient4() throws IOException, URISyntaxException, InterruptedExc Assert.assertTrue(metrics.containsKey("External/" + endpoint.getHost() + "/CommonsHttp")); metrics.get("External/" + endpoint.getHost() + "/CommonsHttp"); - // This is 3 because execute4() calls execute(), getResponseBody() - // and releaseConnection(). All 3 methods are instrumented - Assert.assertEquals(3, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); + // execute4() calls execute(), getResponseBody() and releaseConnection(). All 3 methods are instrumented + // execute() has a metric of its own, the others are aggregated in the general metric + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp/execute").getCallCount()); + Assert.assertEquals(2, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); } @Trace(dispatcher = true) @@ -331,8 +340,9 @@ public void httpClient5() throws IOException, URISyntaxException { "OtherTransaction/Custom/com.nr.agent.instrumentation.httpclient.HttpClient3Test/execute5"); Assert.assertTrue(metrics.containsKey("External/" + endpoint.getHost() + "/CommonsHttp")); metrics.get("External/" + endpoint.getHost() + "/CommonsHttp"); - // This is 2 because execute5() calls execute() and releaseConnection(), both are instrumented - Assert.assertEquals(2, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); + // execute5() calls execute() and releaseConnection(), both are instrumented + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp/execute").getCallCount()); + Assert.assertEquals(1, metrics.get("External/" + endpoint.getHost() + "/CommonsHttp").getCallCount()); } @Trace(dispatcher = true) diff --git a/instrumentation/httpclient-3.1/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java b/instrumentation/httpclient-3.1/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java index c94d348803..9ae44ab436 100644 --- a/instrumentation/httpclient-3.1/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java +++ b/instrumentation/httpclient-3.1/src/main/java/org/apache/commons/httpclient/HttpMethodBase.java @@ -74,6 +74,7 @@ public int execute(HttpState state, HttpConnection conn) throws IOException { .uri(netURI) .procedure("execute") .inboundHeaders(inboundHeaders) + .status(responseCode, this.getStatusText()) .build()); } catch (Throwable e) { AgentBridge.getAgent().getLogger().log(Level.FINER, e, diff --git a/instrumentation/httpclient-3.1/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient31Test.java b/instrumentation/httpclient-3.1/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient31Test.java index af17e9ee27..e4cefa38a7 100644 --- a/instrumentation/httpclient-3.1/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient31Test.java +++ b/instrumentation/httpclient-3.1/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClient31Test.java @@ -14,6 +14,7 @@ import com.newrelic.agent.introspec.InstrumentationTestRunner; import com.newrelic.agent.introspec.Introspector; import com.newrelic.agent.introspec.MetricsHelper; +import com.newrelic.agent.introspec.TracedMetricData; import com.newrelic.agent.introspec.TransactionEvent; import com.newrelic.agent.introspec.internal.HttpServerLocator; import com.newrelic.agent.introspec.internal.HttpServerRule; @@ -36,6 +37,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -112,6 +114,8 @@ public void testExternal() throws Exception { Assert.assertEquals("localhost", externalRequest.getHostname()); Assert.assertEquals("CommonsHttp", externalRequest.getLibrary()); Assert.assertEquals("execute", externalRequest.getOperation()); + Assert.assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + Assert.assertEquals("OK", externalRequest.getStatusText()); } @Test @@ -203,6 +207,8 @@ public void testCat() throws Exception { ExternalRequest externalRequest = externalRequests.iterator().next(); assertEquals(1, externalRequest.getCount()); assertEquals(host, externalRequest.getHostname()); + assertEquals(Integer.valueOf(200), externalRequest.getStatusCode()); + assertEquals("OK", externalRequest.getStatusText()); } } diff --git a/instrumentation/httpclient-4.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClientSpanTests.java b/instrumentation/httpclient-4.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClientSpanTests.java index 25240069ad..fad381dc61 100644 --- a/instrumentation/httpclient-4.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClientSpanTests.java +++ b/instrumentation/httpclient-4.0/src/test/java/com/nr/agent/instrumentation/httpclient/HttpClientSpanTests.java @@ -57,6 +57,8 @@ public void testExternalSpan() throws Exception { assertEquals(endpoint.toString(), externalSpanEvent.getHttpUrl()); assertEquals("CommonsHttp", externalSpanEvent.getHttpComponent()); assertEquals("execute", externalSpanEvent.getHttpMethod()); + assertEquals(Integer.valueOf(200), externalSpanEvent.getStatusCode()); + assertEquals("OK", externalSpanEvent.getStatusText()); } /** From 647b9bca7ab63ca81c1dd61f6a02e4704c8a1348 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Fri, 16 Apr 2021 16:34:24 -0400 Subject: [PATCH 11/16] Adds status code to S3's getObject --- .../awsjavasdk2/services/s3/S3MetricUtil.java | 14 +++++++++ .../services/s3/S3Client_Instrumentation.java | 30 ++++++++++++++++--- .../awsjavasdks3/AmazonS3SyncApiTest.java | 16 +++++++++- .../awsjavasdks3/S3MetricAssertions.java | 5 +++- 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java index 5a7a2cb217..626acb1586 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java @@ -55,4 +55,18 @@ public static void reportExternalMetrics(TracedMethod tracedMethod, String uri, AgentBridge.instrumentation.noticeInstrumentationError(e, Weaver.getImplementationTitle()); } } + + public static void reportExternalMetrics(TracedMethod tracedMethod, String uri, Integer statusCode, String operationName) { + try { + HttpParameters httpParameters = HttpParameters.library(SERVICE) + .uri(new URI(uri)) + .procedure(operationName) + .noInboundHeaders() + .status(statusCode, null) + .build(); + tracedMethod.reportAsExternal(httpParameters); + } catch (URISyntaxException e) { + AgentBridge.instrumentation.noticeInstrumentationError(e, Weaver.getImplementationTitle()); + } + } } diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java index 0d2c494b10..3c5666e3d4 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java @@ -13,6 +13,8 @@ import com.newrelic.api.agent.weaver.MatchType; import com.newrelic.api.agent.weaver.Weave; import com.newrelic.api.agent.weaver.Weaver; +import software.amazon.awssdk.awscore.exception.AwsServiceException; +import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.sync.RequestBody; import software.amazon.awssdk.core.sync.ResponseTransformer; import software.amazon.awssdk.services.s3.model.CreateBucketRequest; @@ -30,9 +32,10 @@ import software.amazon.awssdk.services.s3.model.ListBucketsResponse; import software.amazon.awssdk.services.s3.model.ListObjectsRequest; import software.amazon.awssdk.services.s3.model.ListObjectsResponse; +import software.amazon.awssdk.services.s3.model.NoSuchKeyException; import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.PutObjectResponse; - +import software.amazon.awssdk.services.s3.model.S3Exception; @Weave(type = MatchType.Interface, originalName = "software.amazon.awssdk.services.s3.S3Client") public class S3Client_Instrumentation { @@ -89,11 +92,30 @@ public GetBucketLocationResponse getBucketLocation(GetBucketLocationRequest getB } } + /* + * This method does not return an S3Response like all the others, so the instrumentation is drastically different. + * If the method returns properly, it is assumed as a 200 status. + * Exceptional cases either use the status on the exception or assume a specific value. + * The original method may throw a SdkClientException, which may happen before the request is made, thus no status code. + * statusCode is an Integer to account for this last case. + */ @Trace - public T getObject(GetObjectRequest getObjectRequest, ResponseTransformer ResponseTransformer) { + public T getObject(GetObjectRequest getObjectRequest, ResponseTransformer responseTransformer) { String uri = "s3://" + getObjectRequest.bucket() + "/" + getObjectRequest.key(); - S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri,"getObject"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + T t = Weaver.callOriginal(); + statusCode = 200; + return t; + } catch (NoSuchKeyException noSuchKeyException) { + statusCode = 404; + throw noSuchKeyException; + } catch (AwsServiceException awsServiceException) { + statusCode = awsServiceException.statusCode(); + throw awsServiceException; + } finally { + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "getObject"); + } } @Trace diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java index 7eabe3e431..1c61129864 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/AmazonS3SyncApiTest.java @@ -12,6 +12,7 @@ import com.newrelic.api.agent.Trace; import io.findify.s3mock.S3Mock; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; @@ -29,6 +30,7 @@ import software.amazon.awssdk.services.s3.model.GetBucketLocationRequest; import software.amazon.awssdk.services.s3.model.GetObjectRequest; import software.amazon.awssdk.services.s3.model.ListObjectsRequest; +import software.amazon.awssdk.services.s3.model.NoSuchKeyException; import software.amazon.awssdk.services.s3.model.PutObjectRequest; import java.io.File; @@ -111,7 +113,19 @@ public void testGetObject() { createBucketNoTxn(); putObjectNoTxn(); getObject(); - assertMetrics("getObject", null); + assertMetrics("getObject", 200); + } + + @Test + public void testGetObjectNonexistent() { + createBucketNoTxn(); + try { + getObject(); + Assert.fail("Exception was expected."); + } catch (NoSuchKeyException exception) { + // this exception was expected, let the assertions begin + } + assertMetrics("getObject", 404); } @Test diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java index 1b84aee4a1..eb7ea784e1 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/test/java/com/agent/instrumentation/awsjavasdks3/S3MetricAssertions.java @@ -86,7 +86,10 @@ static void assertMetrics(String operation, Integer expectedStatusCode) { assertEquals(expectedStatusCode, externalRequest.getStatusCode()); String expectedStatusText = statusCodeText.get(expectedStatusCode); - assertEquals(expectedStatusText, externalRequest.getStatusText()); + + if (!"getObject".equals(operation)) { + assertEquals(expectedStatusText, externalRequest.getStatusText()); + } } } From b99243a3e974bc27d96609ea1df1e105ed2b6432 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Mon, 3 May 2021 16:49:54 -0400 Subject: [PATCH 12/16] AWS S3 SDK instrumentation --- .../services/s3/S3MetricUtil.java | 31 ++-- .../services/s3/AmazonS3_Instrumentation.java | 137 ++++++++++++++---- .../awsjavasdks3/package-info.java | 16 ++ .../awsjavasdk2/services/s3/S3MetricUtil.java | 24 ++- .../s3/S3AsyncClient_Instrumentation.java | 5 +- .../services/s3/S3Client_Instrumentation.java | 8 +- 6 files changed, 172 insertions(+), 49 deletions(-) create mode 100644 instrumentation/aws-java-sdk-s3-1.2.13/src/test/java/com/agent/instrumentation/awsjavasdks3/package-info.java diff --git a/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/agent/instrumentation/awsjavasdk1330/services/s3/S3MetricUtil.java b/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/agent/instrumentation/awsjavasdk1330/services/s3/S3MetricUtil.java index c868e1cd33..77012a9c6a 100644 --- a/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/agent/instrumentation/awsjavasdk1330/services/s3/S3MetricUtil.java +++ b/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/agent/instrumentation/awsjavasdk1330/services/s3/S3MetricUtil.java @@ -7,26 +7,35 @@ package com.agent.instrumentation.awsjavasdk1330.services.s3; -import com.newrelic.agent.bridge.TracedMethod; -import com.newrelic.agent.bridge.Transaction; -import com.newrelic.agent.bridge.external.ExternalMetrics; +import com.newrelic.agent.bridge.AgentBridge; import com.newrelic.api.agent.ExternalParameters; +import com.newrelic.api.agent.HttpParameters; +import com.newrelic.api.agent.TracedMethod; +import com.newrelic.api.agent.weaver.Weaver; + +import java.net.URI; +import java.net.URISyntaxException; /** - * This uses {@link ExternalMetrics} to create external metrics for all S3 calls in + * This uses {@link TracedMethod#reportAsExternal(ExternalParameters)} to create external metrics for all S3 calls in * {@link com.amazonaws.services.s3.AmazonS3_Instrumentation}. - * - *

- * It should be updated to use {@link TracedMethod#reportAsExternal(ExternalParameters)} at some point. */ public abstract class S3MetricUtil { - private static final String HOST = "amazon"; private static final String SERVICE = "S3"; - private static final String URI = ""; - public static void metrics(Transaction transaction, TracedMethod tracedMethod, String operationName) { - ExternalMetrics.makeExternalComponentTrace(transaction, tracedMethod, HOST, SERVICE, false, URI, operationName); + public static void reportExternalMetrics(TracedMethod tracedMethod, String uri, Integer statusCode, String operationName) { + try { + HttpParameters httpParameters = HttpParameters.library(SERVICE) + .uri(new URI(uri)) + .procedure(operationName) + .noInboundHeaders() + .status(statusCode, null) + .build(); + tracedMethod.reportAsExternal(httpParameters); + } catch (URISyntaxException e) { + AgentBridge.instrumentation.noticeInstrumentationError(e, Weaver.getImplementationTitle()); + } } } diff --git a/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/amazonaws/services/s3/AmazonS3_Instrumentation.java b/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/amazonaws/services/s3/AmazonS3_Instrumentation.java index a2cb8b1cae..fab69edf61 100644 --- a/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/amazonaws/services/s3/AmazonS3_Instrumentation.java +++ b/instrumentation/aws-java-sdk-s3-1.2.13/src/main/java/com/amazonaws/services/s3/AmazonS3_Instrumentation.java @@ -12,6 +12,7 @@ import com.amazonaws.AmazonServiceException; import com.amazonaws.services.s3.model.*; import com.newrelic.agent.bridge.AgentBridge; +import com.newrelic.api.agent.NewRelic; import com.newrelic.api.agent.Trace; import com.newrelic.api.agent.weaver.MatchType; import com.newrelic.api.agent.weaver.Weave; @@ -28,64 +29,146 @@ public class AmazonS3_Instrumentation { @Trace public Bucket createBucket(CreateBucketRequest createBucketRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "createBucket"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + Bucket bucket = Weaver.callOriginal(); + statusCode = 200; + return bucket; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + createBucketRequest.getBucketName(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "createBucket"); + } + } @Trace public void deleteBucket(DeleteBucketRequest deleteBucketRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "deleteBucket"); - Weaver.callOriginal(); + Integer statusCode = null; + try { + Weaver.callOriginal(); + statusCode = 200; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + deleteBucketRequest.getBucketName(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "deleteBucket"); + } } @Trace public List listBuckets(ListBucketsRequest listBucketsRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "listBuckets"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + List buckets = Weaver.callOriginal(); + statusCode = 200; + return buckets; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://amazon/"; + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "listBuckets"); + } } @Trace public String getBucketLocation(GetBucketLocationRequest getBucketLocationRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "getBucketLocation"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + String bucketLocation = Weaver.callOriginal(); + statusCode = 200; + return bucketLocation; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + getBucketLocationRequest.getBucketName(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "getBucketLocation"); + } } @Trace public S3Object getObject(GetObjectRequest getObjectRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "getObject"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + S3Object s3Object = Weaver.callOriginal(); + statusCode = 200; + return s3Object; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + getObjectRequest.getBucketName() + "/" + getObjectRequest.getKey(); + System.out.println("URI:" + uri); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "getObject"); + } } @Trace public ObjectListing listObjects(ListObjectsRequest listObjectsRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "listObjects"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + ObjectListing objectListing = Weaver.callOriginal(); + statusCode = 200; + return objectListing; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + listObjectsRequest.getBucketName(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "listObjects"); + } } @Trace public PutObjectResult putObject(PutObjectRequest putObjectRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "putObject"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + PutObjectResult putObjectResult = Weaver.callOriginal(); + statusCode = 200; + return putObjectResult; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + putObjectRequest.getBucketName() + "/" + putObjectRequest.getKey(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "putObject"); + } } @Trace public void deleteObject(DeleteObjectRequest deleteObjectRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "deleteObject"); - Weaver.callOriginal(); + Integer statusCode = null; + try { + Weaver.callOriginal(); + statusCode = 200; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + deleteObjectRequest.getBucketName() + "/" + deleteObjectRequest.getKey(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "deleteObject"); + } } @Trace public DeleteObjectsResult deleteObjects(DeleteObjectsRequest deleteObjectsRequest) { - S3MetricUtil.metrics(AgentBridge.getAgent().getTransaction(), AgentBridge.getAgent().getTracedMethod(), - "deleteObjects"); - return Weaver.callOriginal(); + Integer statusCode = null; + try { + DeleteObjectsResult deleteObjectsResult = Weaver.callOriginal(); + statusCode = 200; + return deleteObjectsResult; + } catch (AmazonServiceException exception) { + statusCode = exception.getStatusCode(); + throw exception; + } finally { + String uri = "s3://" + deleteObjectsRequest.getBucketName(); + S3MetricUtil.reportExternalMetrics(NewRelic.getAgent().getTracedMethod(), uri, statusCode, "deleteObjects"); + } + } } diff --git a/instrumentation/aws-java-sdk-s3-1.2.13/src/test/java/com/agent/instrumentation/awsjavasdks3/package-info.java b/instrumentation/aws-java-sdk-s3-1.2.13/src/test/java/com/agent/instrumentation/awsjavasdks3/package-info.java new file mode 100644 index 0000000000..1d0945a2b3 --- /dev/null +++ b/instrumentation/aws-java-sdk-s3-1.2.13/src/test/java/com/agent/instrumentation/awsjavasdks3/package-info.java @@ -0,0 +1,16 @@ +/* + * + * * Copyright 2021 New Relic Corporation. All rights reserved. + * * SPDX-License-Identifier: Apache-2.0 + * + */ + +/** + * About testing this aws-java-sdk-s3-1.2.13: + *

    + *
  • It is not possible to use an embedded s3mock because it brings conflicting AWS libraries.
    • + *
  • Even if an external s3mock is used, it returns human readable (indented) XML and this version of the S3 sdk only understands single line XML.
    • + *
+ * So to run this test you must use a real S3 bucket. + */ +package com.agent.instrumentation.awsjavasdks3; \ No newline at end of file diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java index 626acb1586..a20a97ce60 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/com/agent/instrumentation/awsjavasdk2/services/s3/S3MetricUtil.java @@ -23,9 +23,21 @@ public abstract class S3MetricUtil { private static final String SERVICE = "S3"; - public static void reportExternalMetrics(Segment segment, String uri, String operationName) { + public static void reportExternalMetrics(Segment segment, String uri, S3Response s3Response, String operationName) { try { - HttpParameters httpParameters = HttpParameters.library(SERVICE).uri(new URI(uri)).procedure(operationName).noInboundHeaders().build(); + + Integer statusCode = null; + String statusText = null; + if (s3Response != null) { + statusCode = s3Response.sdkHttpResponse().statusCode(); + statusText = s3Response.sdkHttpResponse().statusText().orElse(null); + } + HttpParameters httpParameters = HttpParameters.library(SERVICE) + .uri(new URI(uri)) + .procedure(operationName) + .noInboundHeaders() + .status(statusCode, statusText) + .build(); segment.reportAsExternal(httpParameters); } catch (URISyntaxException e) { AgentBridge.instrumentation.noticeInstrumentationError(e, Weaver.getImplementationTitle()); @@ -44,11 +56,17 @@ public static void reportExternalMetrics(TracedMethod tracedMethod, String uri, public static void reportExternalMetrics(TracedMethod tracedMethod, String uri, S3Response s3Response, String operationName) { try { + Integer statusCode = null; + String statusText = null; + if (s3Response != null) { + statusCode = s3Response.sdkHttpResponse().statusCode(); + statusText = s3Response.sdkHttpResponse().statusText().orElse(null); + } HttpParameters httpParameters = HttpParameters.library(SERVICE) .uri(new URI(uri)) .procedure(operationName) .noInboundHeaders() - .status(s3Response.sdkHttpResponse().statusCode(), s3Response.sdkHttpResponse().statusText().orElse(null)) + .status(statusCode, statusText) .build(); tracedMethod.reportAsExternal(httpParameters); } catch (URISyntaxException e) { diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java index 188868abe5..adf6a9bd81 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3AsyncClient_Instrumentation.java @@ -14,6 +14,7 @@ import com.newrelic.api.agent.weaver.MatchType; import com.newrelic.api.agent.weaver.Weave; import com.newrelic.api.agent.weaver.Weaver; +import java.util.Optional; import software.amazon.awssdk.core.async.AsyncRequestBody; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.services.s3.model.CreateBucketRequest; @@ -160,9 +161,9 @@ public S3ResponseResultWrapper(Segment segment, String uri, String operationName } @Override - public void accept(T t, U u) { + public void accept(T s3Response, U u) { try { - S3MetricUtil.reportExternalMetrics(segment, uri, operationName); + S3MetricUtil.reportExternalMetrics(segment, uri, s3Response, operationName); segment.end(); } catch (Throwable t1) { AgentBridge.instrumentation.noticeInstrumentationError(t1, Weaver.getImplementationTitle()); diff --git a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java index 3c5666e3d4..119c59cc31 100644 --- a/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java +++ b/instrumentation/aws-java-sdk-s3-2.0/src/main/java/software/amazon/awssdk/services/s3/S3Client_Instrumentation.java @@ -95,9 +95,8 @@ public GetBucketLocationResponse getBucketLocation(GetBucketLocationRequest getB /* * This method does not return an S3Response like all the others, so the instrumentation is drastically different. * If the method returns properly, it is assumed as a 200 status. - * Exceptional cases either use the status on the exception or assume a specific value. - * The original method may throw a SdkClientException, which may happen before the request is made, thus no status code. - * statusCode is an Integer to account for this last case. + * When AwsServiceException is thrown, the status code is retrieved from the exception. + * Other exceptions may be thrown, but they indicate something happened before the HTTP request, so there would be no status to record. */ @Trace public T getObject(GetObjectRequest getObjectRequest, ResponseTransformer responseTransformer) { @@ -107,9 +106,6 @@ public T getObject(GetObjectRequest getObjectRequest, ResponseTransformer re T t = Weaver.callOriginal(); statusCode = 200; return t; - } catch (NoSuchKeyException noSuchKeyException) { - statusCode = 404; - throw noSuchKeyException; } catch (AwsServiceException awsServiceException) { statusCode = awsServiceException.statusCode(); throw awsServiceException; From 55d6092a52753020be685feb12adcfaa2456cca0 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Mon, 3 May 2021 16:55:49 -0400 Subject: [PATCH 13/16] grpc instrumentation --- .../java/io/grpc/internal/ServerStream_Instrumentation.java | 5 ++++- .../grpc-1.22.0/src/test/java/ValidationHelper.java | 3 +++ .../java/io/grpc/internal/ServerStream_Instrumentation.java | 5 ++++- .../grpc-1.30.0/src/test/java/ValidationHelper.java | 3 +++ .../java/io/grpc/internal/ServerStream_Instrumentation.java | 5 ++++- .../grpc-1.4.0/src/test/java/ValidationHelper.java | 4 ++++ 6 files changed, 22 insertions(+), 3 deletions(-) diff --git a/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java b/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java index f79f78c3ae..e6128f7b64 100644 --- a/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java +++ b/instrumentation/grpc-1.22.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java @@ -69,7 +69,10 @@ public void cancel(Status status) { } if (status != null) { - NewRelic.addCustomParameter("response.status", status.getCode().value()); + int statusCode = status.getCode().value(); + NewRelic.addCustomParameter("response.status", statusCode); + NewRelic.addCustomParameter("http.statusCode", statusCode); + NewRelic.addCustomParameter("http.statusText", status.getDescription()); if (GrpcConfig.errorsEnabled && status.getCause() != null) { // If an error occurred during the close of this server call we should record it NewRelic.noticeError(status.getCause()); diff --git a/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java index c9e302ec65..974bed325e 100644 --- a/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java +++ b/instrumentation/grpc-1.22.0/src/test/java/ValidationHelper.java @@ -87,6 +87,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri } assertEquals(0, serverTxEvent.getAttributes().get("response.status")); + assertEquals(0, serverTxEvent.getAttributes().get("http.statusCode")); assertEquals("grpc://localhost:" + server.getPort() + "/" + fullMethod, serverTxEvent.getAttributes().get("request.uri")); } @@ -127,6 +128,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN assertEquals(1, rootSegment.getCallCount()); assertEquals(fullMethod, rootSegment.getTracerAttributes().get("request.method")); assertEquals(status, rootSegment.getTracerAttributes().get("response.status")); + assertEquals(status, rootSegment.getTracerAttributes().get("http.statusCode")); assertEquals(grpcType, rootSegment.getTracerAttributes().get("grpc.type")); // Custom attributes (to test tracing into customer code) @@ -135,6 +137,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN TransactionEvent serverTxEvent = serverTxEvents.iterator().next(); assertNotNull(serverTxEvent); assertEquals(status, serverTxEvent.getAttributes().get("response.status")); + assertEquals(status, serverTxEvent.getAttributes().get("http.statusCode")); assertEquals("grpc://localhost:" + server.getPort() + "/" + fullMethod, serverTxEvent.getAttributes().get("request.uri")); } diff --git a/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java b/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java index f79f78c3ae..e6128f7b64 100644 --- a/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java +++ b/instrumentation/grpc-1.30.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java @@ -69,7 +69,10 @@ public void cancel(Status status) { } if (status != null) { - NewRelic.addCustomParameter("response.status", status.getCode().value()); + int statusCode = status.getCode().value(); + NewRelic.addCustomParameter("response.status", statusCode); + NewRelic.addCustomParameter("http.statusCode", statusCode); + NewRelic.addCustomParameter("http.statusText", status.getDescription()); if (GrpcConfig.errorsEnabled && status.getCause() != null) { // If an error occurred during the close of this server call we should record it NewRelic.noticeError(status.getCause()); diff --git a/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java index c9e302ec65..974bed325e 100644 --- a/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java +++ b/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java @@ -87,6 +87,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri } assertEquals(0, serverTxEvent.getAttributes().get("response.status")); + assertEquals(0, serverTxEvent.getAttributes().get("http.statusCode")); assertEquals("grpc://localhost:" + server.getPort() + "/" + fullMethod, serverTxEvent.getAttributes().get("request.uri")); } @@ -127,6 +128,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN assertEquals(1, rootSegment.getCallCount()); assertEquals(fullMethod, rootSegment.getTracerAttributes().get("request.method")); assertEquals(status, rootSegment.getTracerAttributes().get("response.status")); + assertEquals(status, rootSegment.getTracerAttributes().get("http.statusCode")); assertEquals(grpcType, rootSegment.getTracerAttributes().get("grpc.type")); // Custom attributes (to test tracing into customer code) @@ -135,6 +137,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN TransactionEvent serverTxEvent = serverTxEvents.iterator().next(); assertNotNull(serverTxEvent); assertEquals(status, serverTxEvent.getAttributes().get("response.status")); + assertEquals(status, serverTxEvent.getAttributes().get("http.statusCode")); assertEquals("grpc://localhost:" + server.getPort() + "/" + fullMethod, serverTxEvent.getAttributes().get("request.uri")); } diff --git a/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java b/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java index dcb1a37aab..0e84e2c28f 100644 --- a/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java +++ b/instrumentation/grpc-1.4.0/src/main/java/io/grpc/internal/ServerStream_Instrumentation.java @@ -68,7 +68,10 @@ public void cancel(Status status) { } if (status != null) { - NewRelic.addCustomParameter("response.status", status.getCode().value()); + int statusCode = status.getCode().value(); + NewRelic.addCustomParameter("response.status", statusCode); + NewRelic.addCustomParameter("http.statusCode", statusCode); + NewRelic.addCustomParameter("http.statusText", status.getDescription()); if (GrpcConfig.errorsEnabled && status.getCause() != null) { // If an error occurred during the close of this server call we should record it NewRelic.noticeError(status.getCause()); diff --git a/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java index c9e302ec65..112a362bc7 100644 --- a/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java +++ b/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java @@ -6,6 +6,7 @@ */ import app.TestServer; +import com.newrelic.agent.attributes.AttributeNames; import com.newrelic.agent.introspec.CatHelper; import com.newrelic.agent.introspec.ExternalRequest; import com.newrelic.agent.introspec.InstrumentationTestRunner; @@ -87,6 +88,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri } assertEquals(0, serverTxEvent.getAttributes().get("response.status")); + assertEquals(0, serverTxEvent.getAttributes().get(AttributeNames.HTTP_STATUS_CODE)); assertEquals("grpc://localhost:" + server.getPort() + "/" + fullMethod, serverTxEvent.getAttributes().get("request.uri")); } @@ -127,6 +129,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN assertEquals(1, rootSegment.getCallCount()); assertEquals(fullMethod, rootSegment.getTracerAttributes().get("request.method")); assertEquals(status, rootSegment.getTracerAttributes().get("response.status")); + assertEquals(status, rootSegment.getTracerAttributes().get(AttributeNames.HTTP_STATUS_CODE)); assertEquals(grpcType, rootSegment.getTracerAttributes().get("grpc.type")); // Custom attributes (to test tracing into customer code) @@ -135,6 +138,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN TransactionEvent serverTxEvent = serverTxEvents.iterator().next(); assertNotNull(serverTxEvent); assertEquals(status, serverTxEvent.getAttributes().get("response.status")); + assertEquals(status, serverTxEvent.getAttributes().get(AttributeNames.HTTP_STATUS_CODE)); assertEquals("grpc://localhost:" + server.getPort() + "/" + fullMethod, serverTxEvent.getAttributes().get("request.uri")); } From 8ca9fa4169cdd39d315bd75486955f243592865e Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Mon, 3 May 2021 18:37:17 -0400 Subject: [PATCH 14/16] Fixing broken http-client functional test --- .../java/test/newrelic/test/agent/HttpCommonsTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java b/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java index 81b4ac7bb7..f63fed0348 100644 --- a/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java +++ b/functional_test/src/test/java/test/newrelic/test/agent/HttpCommonsTest.java @@ -90,6 +90,7 @@ public void httpMethod() throws IOException { Set metrics = AgentHelper.getMetrics(); assertTrue(metrics.toString(), metrics.contains("External/localhost/CommonsHttp")); + assertTrue(metrics.toString(), metrics.contains("External/localhost/CommonsHttp/execute")); assertTrue(metrics.toString(), metrics.contains("External/localhost/all")); assertTrue(metrics.toString(), metrics.contains("External/all")); assertTrue(metrics.toString(), metrics.contains("External/allOther")); @@ -97,6 +98,8 @@ public void httpMethod() throws IOException { Map metricCounts = getMetricCounts( MetricName.create("External/localhost/CommonsHttp", "OtherTransaction/Custom/test.newrelic.test.agent.HttpCommonsTest/httpMethodImpl"), + MetricName.create("External/localhost/CommonsHttp/execute", + "OtherTransaction/Custom/test.newrelic.test.agent.HttpCommonsTest/httpMethodImpl"), MetricName.create("External/localhost/all"), MetricName.create("External/all"), MetricName.create("External/allOther")); @@ -105,9 +108,10 @@ public void httpMethod() throws IOException { assertEquals(3, (int) metricCounts.get("External/all")); assertEquals(3, (int) metricCounts.get("External/allOther")); - // This is 6 because the loop executes 3 times and each loop calls + // This is 3 because the loop executes 3 times and each loop calls // execute() and releaseConnection(), both of which are instrumented - assertEquals(6, (int) metricCounts.get("External/localhost/CommonsHttp")); + assertEquals(3, (int) metricCounts.get("External/localhost/CommonsHttp")); // releaseConnection + assertEquals(3, (int) metricCounts.get("External/localhost/CommonsHttp/execute")); } @Trace(dispatcher = true) From 18ba60f23d8a604696c0c594867be42711f46d39 Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Fri, 5 Nov 2021 10:20:36 -0400 Subject: [PATCH 15/16] Adding http.statusCode to spans --- .../com/newrelic/agent/service/analytics/SpanEventFactory.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java index 20f5663166..ecbc22c9c4 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java @@ -200,6 +200,9 @@ public SpanEventFactory setHttpStatusCode(Integer statusCode) { if (filter.shouldIncludeAgentAttribute(appName, AttributeNames.HTTP_STATUS_CODE)) { builder.putAgentAttribute(AttributeNames.HTTP_STATUS_CODE, statusCode); } + if (filter.shouldIncludeAgentAttribute(appName, AttributeNames.HTTP_STATUS)) { + builder.putAgentAttribute(AttributeNames.HTTP_STATUS, statusCode); + } return this; } From 876591d8e67142182eab3f4c44d8071af1d3e45e Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Tue, 16 Nov 2021 14:35:46 -0500 Subject: [PATCH 16/16] Http Status code - indentation fixes, adding final to some fields --- .../introspec/internal/ExternalRequestImpl.java | 12 ++++++------ .../HttpAsyncResponseConsumer_Instrumentation.java | 3 +-- .../asynchttpclient/NRAsyncHandler.java | 3 +-- .../java/com/newrelic/api/agent/HttpParameters.java | 4 ++-- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java index c6092d71e1..3507fbb7b9 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java @@ -23,12 +23,12 @@ class ExternalRequestImpl extends RequestImpl implements ExternalRequest { private static Pattern EXTERNAL_TX_METRIC = Pattern.compile("ExternalTransaction/([^/]+)/(.+)"); private static Pattern EXTERNAL_TX_SEGMENT = Pattern.compile("ExternalTransaction/([^/]+)/(.+)"); - private String library; - private String operation; - private Integer statusCode; - private String statusText; - private String segmentName; - private String catTransactionGuid; + private final String library; + private final String operation; + private final Integer statusCode; + private final String statusText; + private final String segmentName; + private final String catTransactionGuid; private ExternalRequestImpl(String originalMetric, String segmentName, String host, String lib, String operation, Integer statusCode, String statusText, String catTransactionGuid) { diff --git a/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java b/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java index 7f2ff2f0f5..2058327f97 100644 --- a/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java +++ b/instrumentation/http-async-client-4/src/main/java/org/apache/http/nio/protocol/HttpAsyncResponseConsumer_Instrumentation.java @@ -90,8 +90,7 @@ public void failed(Exception ex) { } private Integer getStatusCode() { - if (httpResponse != null && httpResponse.getStatusLine() != null) - { + if (httpResponse != null && httpResponse.getStatusLine() != null) { return httpResponse.getStatusLine().getStatusCode(); } return null; diff --git a/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java b/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java index c54f65ec4d..a35ced4a4f 100644 --- a/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java +++ b/instrumentation/ning-async-http-client-1.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java @@ -125,8 +125,7 @@ public T onCompleted() throws Exception { } private Integer getStatusCode() { - if (responseStatus != null) - { + if (responseStatus != null) { return responseStatus.getStatusCode(); } return null; diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java b/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java index 877267216f..a4c9791046 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/HttpParameters.java @@ -113,7 +113,7 @@ public String getProcedure() { * Returns the HTTP status code for the call. * * @return the status code for the call, null if not available - * @since 6.5.0 + * @since 7.5.0 */ public Integer getStatusCode() { return statusCode; @@ -123,7 +123,7 @@ public Integer getStatusCode() { * Returns the HTTP reason message for the call. * * @return the text of the reason message, null if not available - * @since 6.5.0 + * @since 7.5.0 */ public String getStatusText() { return statusText;