Merge pull request #513 from newrelic/aonuki/http-status-code

Update http Response code attribute names
newrelic · Nov 17, 2021 · bf6cb39 · bf6cb39
2 parents 7980165 + 876591d
commit bf6cb39
Show file tree

Hide file tree

Showing 60 changed files with 838 additions and 137 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
@@ -90,13 +90,16 @@ public void httpMethod() throws IOException {
 
         Set<String> 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"));
 
         Map<String, Integer> 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)

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
@@ -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
@@ -28,5 +28,9 @@ public interface SpanEvent {
 
     String getTransactionId();
 
+    Integer getStatusCode();
+
+    String getStatusText();
+
     Map<String, Object> getAgentAttributes();
 }
diff --git a/...ntation-test/src/main/java/com/newrelic/agent/introspec/internal/ExternalRequestImpl.java b/...ntation-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 {
 
@@ -21,17 +23,21 @@ 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 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,
+    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
@@ -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<String, Object> getAgentAttributes() {
         return spanEvent.getAgentAttributes();

diff --git a/...ient-2.0.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java b/...ient-2.0.0/src/main/java/com/nr/agent/instrumentation/asynchttpclient/NRAsyncHandler.java
@@ -45,13 +45,16 @@ public class NRAsyncHandler<T> {
     public URI uri;
     @NewField
     private InboundWrapper inboundHeaders;
+    @NewField
+    private HttpResponseStatus responseStatus;
 
     public AsyncHandler.State onStatusReceived(HttpResponseStatus responseStatus) throws Exception {
         AsyncHandler.State userState = Weaver.callOriginal();
         if (userState == AsyncHandler.State.ABORT) {
             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/...src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_0_0Tests.java b/...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/...ync-http-client-2.1.0/src/main/java/org/asynchttpclient/AsyncHandler_Instrumentation.java b/...ync-http-client-2.1.0/src/main/java/org/asynchttpclient/AsyncHandler_Instrumentation.java
@@ -43,6 +43,8 @@ public class AsyncHandler_Instrumentation<T> {
     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;
+    }
 }
diff --git a/...src/test/java/com/nr/agent/instrumentation/asynchttpclient/AsyncHttpClient2_1_0Tests.java b/...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

diff --git a/...2.13/src/main/java/com/agent/instrumentation/awsjavasdk1330/services/s3/S3MetricUtil.java b/...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}.
- *
- * <p>
- * 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());
+        }
     }
 
 }