Merge pull request #1671 from newrelic/http-attrs

Reintroduce previously deprecated transaction/span attributes
newrelic · Dec 19, 2023 · de83f4c · de83f4c
2 parents 1a95f83 + 52408c3
commit de83f4c
Show file tree

Hide file tree

Showing 22 changed files with 266 additions and 17 deletions.
diff --git a/...http-2.11_2.4.5/src/test/java/com/agent/instrumentation/akka/http/AkkaHttpRoutesTest.java b/...http-2.11_2.4.5/src/test/java/com/agent/instrumentation/akka/http/AkkaHttpRoutesTest.java
@@ -1650,9 +1650,11 @@ private void assertResponseCodeOnTxEvents(Collection<TransactionEvent> transacti
         Assert.assertNotNull(transactionEvents);
         Assert.assertEquals(expectedSize, transactionEvents.size());
         for (TransactionEvent transactionEvent : transactionEvents) {
-            String httpResponseCode = String.valueOf(transactionEvent.getAttributes().get("http.statusCode"));
+            String httpResponseCode = (String) transactionEvent.getAttributes().get("httpResponseCode");
             Assert.assertNotNull(httpResponseCode);
             Assert.assertEquals(expectedResponseCode, httpResponseCode);
+            int statusCode = (Integer) transactionEvent.getAttributes().get("http.statusCode");
+            Assert.assertEquals(Integer.parseInt(expectedResponseCode), statusCode);
         }
     }
 

diff --git a/...ttp-2.13_10.1.8/src/test/java/com/agent/instrumentation/akka/http/AkkaHttpRoutesTest.java b/...ttp-2.13_10.1.8/src/test/java/com/agent/instrumentation/akka/http/AkkaHttpRoutesTest.java
@@ -1646,9 +1646,11 @@ private void assertResponseCodeOnTxEvents(Collection<TransactionEvent> transacti
         Assert.assertNotNull(transactionEvents);
         Assert.assertEquals(expectedSize, transactionEvents.size());
         for (TransactionEvent transactionEvent : transactionEvents) {
-            String httpResponseCode = String.valueOf(transactionEvent.getAttributes().get("http.statusCode"));
+            String httpResponseCode = String.valueOf(transactionEvent.getAttributes().get("httpResponseCode"));
             Assert.assertNotNull(httpResponseCode);
             Assert.assertEquals(expectedResponseCode, httpResponseCode);
+            int statusCode = (Integer) transactionEvent.getAttributes().get("http.statusCode");
+            Assert.assertEquals(Integer.parseInt(expectedResponseCode), statusCode);
         }
     }
 

diff --git a/instrumentation/grpc-1.22.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcConfig.java b/instrumentation/grpc-1.22.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcConfig.java
@@ -15,6 +15,16 @@ public class GrpcConfig {
 
     public static final boolean errorsEnabled = NewRelic.getAgent().getConfig().getValue("grpc.errors.enabled", true);
 
+    public static final boolean HTTP_ATTR_LEGACY;
+    public static final boolean HTTP_ATTR_STANDARD;
+
+    static {
+        String attrMode = NewRelic.getAgent().getConfig().getValue("attributes.http_attribute_mode", "both");
+        // legacy is only disabled when standard is selected.
+        HTTP_ATTR_LEGACY = !"standard".equalsIgnoreCase(attrMode);
+        // standard is only disabled when legacy is selected.
+        HTTP_ATTR_STANDARD = !"legacy".equalsIgnoreCase(attrMode);
+    }
     private GrpcConfig() {
     }
 

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
@@ -39,6 +39,7 @@ public void close(Status status, Metadata trailers) {
 
         if (status != null) {
             int statusCode = status.getCode().value();
+            NewRelic.addCustomParameter("response.status", statusCode);
             NewRelic.addCustomParameter("http.statusCode", statusCode);
             NewRelic.addCustomParameter("http.statusText", status.getDescription());
 
@@ -69,8 +70,15 @@ public void cancel(Status status) {
 
         if (status != null) {
             int statusCode = status.getCode().value();
-            NewRelic.addCustomParameter("http.statusCode", statusCode);
-            NewRelic.addCustomParameter("http.statusText", status.getDescription());
+            String statusMessage = status.getDescription();
+            if (GrpcConfig.HTTP_ATTR_LEGACY) {
+                NewRelic.addCustomParameter("response.status", statusCode);
+                NewRelic.addCustomParameter("response.statusMessage", statusMessage);
+            }
+            if (GrpcConfig.HTTP_ATTR_STANDARD) {
+                NewRelic.addCustomParameter("http.statusCode", statusCode);
+                NewRelic.addCustomParameter("http.statusText", statusMessage);
+            }
             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
@@ -68,6 +68,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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"));
@@ -85,6 +86,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
             assertEquals(name, serverTxEvent.getAttributes().get("sayHelloAfter"));
         }
 
+        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"));
     }
@@ -125,6 +127,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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"));
 
@@ -133,6 +136,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN
         assertEquals(1, serverTxEvents.size());
         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/com/nr/agent/instrumentation/grpc/GrpcConfig.java b/instrumentation/grpc-1.30.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcConfig.java
@@ -15,6 +15,16 @@ public class GrpcConfig {
 
     public static final boolean errorsEnabled = NewRelic.getAgent().getConfig().getValue("grpc.errors.enabled", true);
 
+    public static final boolean HTTP_ATTR_LEGACY;
+    public static final boolean HTTP_ATTR_STANDARD;
+
+    static {
+        String attrMode = NewRelic.getAgent().getConfig().getValue("attributes.http_attribute_mode", "both");
+        // legacy is only disabled when standard is selected.
+        HTTP_ATTR_LEGACY = !"standard".equalsIgnoreCase(attrMode);
+        // standard is only disabled when legacy is selected.
+        HTTP_ATTR_STANDARD = !"legacy".equalsIgnoreCase(attrMode);
+    }
     private GrpcConfig() {
     }
 

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
@@ -39,8 +39,15 @@ public void close(Status status, Metadata trailers) {
 
         if (status != null) {
             int statusCode = status.getCode().value();
-            NewRelic.addCustomParameter("http.statusCode", statusCode);
-            NewRelic.addCustomParameter("http.statusText", status.getDescription());
+            String statusMessage = status.getDescription();
+            if (GrpcConfig.HTTP_ATTR_LEGACY) {
+                NewRelic.addCustomParameter("response.status", statusCode);
+                NewRelic.addCustomParameter("response.statusMessage", statusMessage);
+            }
+            if (GrpcConfig.HTTP_ATTR_STANDARD) {
+                NewRelic.addCustomParameter("http.statusCode", statusCode);
+                NewRelic.addCustomParameter("http.statusText", statusMessage);
+            }
 
             if (GrpcConfig.errorsEnabled && status.getCause() != null) {
                 // If an error occurred during the close of this server call we should record it
@@ -69,6 +76,7 @@ public void cancel(Status status) {
 
         if (status != null) {
             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) {

diff --git a/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.30.0/src/test/java/ValidationHelper.java
@@ -68,6 +68,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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"));
@@ -85,6 +86,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
             assertEquals(name, serverTxEvent.getAttributes().get("sayHelloAfter"));
         }
 
+        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"));
     }
@@ -125,6 +127,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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"));
 
@@ -133,6 +136,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN
         assertEquals(1, serverTxEvents.size());
         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/com/nr/agent/instrumentation/grpc/GrpcConfig.java b/instrumentation/grpc-1.4.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcConfig.java
@@ -15,6 +15,16 @@ public class GrpcConfig {
 
     public static final boolean errorsEnabled = NewRelic.getAgent().getConfig().getValue("grpc.errors.enabled", true);
 
+    public static final boolean HTTP_ATTR_LEGACY;
+    public static final boolean HTTP_ATTR_STANDARD;
+
+    static {
+        String attrMode = NewRelic.getAgent().getConfig().getValue("attributes.http_attribute_mode", "both");
+        // legacy is only disabled when standard is selected.
+        HTTP_ATTR_LEGACY = !"standard".equalsIgnoreCase(attrMode);
+        // standard is only disabled when legacy is selected.
+        HTTP_ATTR_STANDARD = !"legacy".equalsIgnoreCase(attrMode);
+    }
     private GrpcConfig() {
     }
 

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
@@ -39,8 +39,15 @@ public void close(Status status, Metadata trailers) {
 
         if (status != null) {
             int statusCode = status.getCode().value();
-            NewRelic.addCustomParameter("http.statusCode", statusCode);
-            NewRelic.addCustomParameter("http.statusText", status.getDescription());
+            String statusMessage = status.getDescription();
+            if (GrpcConfig.HTTP_ATTR_LEGACY) {
+                NewRelic.addCustomParameter("response.status", statusCode);
+                NewRelic.addCustomParameter("response.statusMessage", statusMessage);
+            }
+            if (GrpcConfig.HTTP_ATTR_STANDARD) {
+                NewRelic.addCustomParameter("http.statusCode", statusCode);
+                NewRelic.addCustomParameter("http.statusText", statusMessage);
+            }
             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());
@@ -68,6 +75,7 @@ public void cancel(Status status) {
 
         if (status != null) {
             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) {

diff --git a/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.4.0/src/test/java/ValidationHelper.java
@@ -69,6 +69,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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"));
@@ -86,6 +87,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
             assertEquals(name, serverTxEvent.getAttributes().get("sayHelloAfter"));
         }
 
+        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"));
     }
@@ -126,6 +128,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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"));
 
@@ -134,6 +137,7 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN
         assertEquals(1, serverTxEvents.size());
         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"));
     }

diff --git a/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcConfig.java b/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcConfig.java
@@ -15,6 +15,16 @@ public class GrpcConfig {
 
     public static final boolean errorsEnabled = NewRelic.getAgent().getConfig().getValue("grpc.errors.enabled", true);
 
+    public static final boolean HTTP_ATTR_LEGACY;
+    public static final boolean HTTP_ATTR_STANDARD;
+
+    static {
+        String attrMode = NewRelic.getAgent().getConfig().getValue("attributes.http_attribute_mode", "both");
+        // legacy is only disabled when standard is selected.
+        HTTP_ATTR_LEGACY = !"standard".equalsIgnoreCase(attrMode);
+        // standard is only disabled when legacy is selected.
+        HTTP_ATTR_STANDARD = !"legacy".equalsIgnoreCase(attrMode);
+    }
     private GrpcConfig() {
     }
 

diff --git a/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcUtil.java b/instrumentation/grpc-1.40.0/src/main/java/com/nr/agent/instrumentation/grpc/GrpcUtil.java
@@ -34,14 +34,24 @@ public static void finalizeTransaction(Token token, Status status, Metadata meta
     }
 
     /**
-     * Set the http.statusCode attribute and error cause (if applicable) on the transaction
+     * Set the http status code attribute and error cause (if applicable) on the transaction
      * when the ServerStream is closed or cancelled.
      *
      * @param status The {@link Status} of the completed/cancelled operation
      */
     public static void setServerStreamResponseStatus(Status status) {
+
         if (status != null) {
-            NewRelic.addCustomParameter("http.statusCode", status.getCode().value());
+            String statusMessage = status.getDescription();
+            int value = status.getCode().value(); // code should not be null
+            if (GrpcConfig.HTTP_ATTR_LEGACY) {
+                NewRelic.addCustomParameter("response.status", value);
+                NewRelic.addCustomParameter("response.statusMessage", statusMessage);
+            }
+            if (GrpcConfig.HTTP_ATTR_STANDARD) {
+                NewRelic.addCustomParameter("http.statusCode", value);
+                NewRelic.addCustomParameter("http.statusText", statusMessage);
+            }
             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.40.0/src/test/java/ValidationHelper.java b/instrumentation/grpc-1.40.0/src/test/java/ValidationHelper.java
@@ -60,6 +60,8 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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"));
         assertEquals(grpcType, rootSegment.getTracerAttributes().get("grpc.type"));
 
         // Custom attributes (to test tracing into customer code)
@@ -75,6 +77,7 @@ static void validateGrpcInteraction(TestServer server, String clientTxName, Stri
             assertEquals(name, serverTxEvent.getAttributes().get("sayHelloAfter"));
         }
 
+        assertEquals(0, serverTxEvent.getAttributes().get("response.status"));
         assertEquals("grpc://localhost:" + server.getPort() + "/" + fullMethod, serverTxEvent.getAttributes().get("request.uri"));
     }
 
@@ -114,13 +117,17 @@ static void validateExceptionGrpcInteraction(TestServer server, String clientTxN
         assertTrue(rootSegment.getName().endsWith(fullMethod));
         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)
         Collection<TransactionEvent> serverTxEvents = introspector.getTransactionEvents(serverTxName);
         assertEquals(1, serverTxEvents.size());
         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/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeNames.java b/newrelic-agent/src/main/java/com/newrelic/agent/attributes/AttributeNames.java
@@ -36,6 +36,8 @@ public final class AttributeNames {
     public static final String HTTP_METHOD = "http.method";
     public static final String HTTP_STATUS_CODE = "http.statusCode";
     public static final String HTTP_STATUS_TEXT = "http.statusText";
+    public static final String HTTP_STATUS = "httpResponseCode";
+    public static final String HTTP_STATUS_MESSAGE = "httpResponseMessage";
 
     public static final String LOCK_THREAD_NAME = "jvm.lock_thread_name";
     public static final String THREAD_NAME = "jvm.thread_name";