Collect more attributes from servlet instrumenter (open-telemetry#4356)

* Collect more attributes from servlet instrumenter * spotless
RashmiRam · May 23, 2022 · fc5938b · fc5938b
1 parent 379f9da
commit fc5938b
Show file tree

Hide file tree

Showing 28 changed files with 258 additions and 63 deletions.
diff --git a/...c/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/RestCamelTest.groovy b/...c/test/groovy/io/opentelemetry/javaagent/instrumentation/apachecamel/RestCamelTest.groovy
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.instrumentation.apachecamel
 
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
+
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.RetryOnAddressAlreadyInUseTrait
 import io.opentelemetry.instrumentation.test.utils.PortUtils
@@ -96,6 +98,8 @@ class RestCamelTest extends AgentInstrumentationSpecification implements RetryOn
             "$SemanticAttributes.HTTP_METHOD.key" "GET"
             "$SemanticAttributes.NET_PEER_IP.key" "127.0.0.1"
             "$SemanticAttributes.NET_PEER_PORT.key" Long
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         it.span(3) {

diff --git a/...lemetry/javaagent/instrumentation/apachecamel/TwoServicesWithDirectClientCamelTest.groovy b/...lemetry/javaagent/instrumentation/apachecamel/TwoServicesWithDirectClientCamelTest.groovy
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.javaagent.instrumentation.apachecamel
 
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
+
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.RetryOnAddressAlreadyInUseTrait
 import io.opentelemetry.instrumentation.test.utils.PortUtils
@@ -131,6 +133,9 @@ class TwoServicesWithDirectClientCamelTest extends AgentInstrumentationSpecifica
             "$SemanticAttributes.NET_PEER_IP.key" "127.0.0.1"
             "$SemanticAttributes.HTTP_USER_AGENT.key" "Jakarta Commons-HttpClient/3.1"
             "$SemanticAttributes.HTTP_FLAVOR.key" "1.1"
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
+            "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long
           }
         }
         it.span(5) {

diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardAsyncTest.groovy
@@ -7,7 +7,6 @@ import io.dropwizard.Application
 import io.dropwizard.Configuration
 import io.dropwizard.setup.Bootstrap
 import io.dropwizard.setup.Environment
-
 import javax.ws.rs.GET
 import javax.ws.rs.HeaderParam
 import javax.ws.rs.Path

diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy
@@ -9,6 +9,7 @@ import io.dropwizard.setup.Bootstrap
 import io.dropwizard.setup.Environment
 import io.dropwizard.testing.ConfigOverride
 import io.dropwizard.testing.DropwizardTestSupport
+import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
@@ -25,7 +26,6 @@ import javax.ws.rs.QueryParam
 import javax.ws.rs.core.Response
 
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
-import static io.opentelemetry.api.trace.SpanKind.SERVER
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
@@ -61,6 +61,21 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
     testSupport.after()
   }
 
+  @Override
+  List<AttributeKey<?>> extraAttributes() {
+    [
+      SemanticAttributes.HTTP_SERVER_NAME,
+      SemanticAttributes.NET_PEER_NAME,
+      SemanticAttributes.NET_TRANSPORT
+    ]
+  }
+
+  // this override is needed because dropwizard reports peer ip as the client ip
+  @Override
+  String peerIp(ServerEndpoint endpoint) {
+    TEST_CLIENT_IP
+  }
+
   @Override
   boolean hasHandlerSpan(ServerEndpoint endpoint) {
     endpoint != NOT_FOUND
@@ -96,41 +111,6 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
     }
   }
 
-  // this override is needed because dropwizard reports peer ip as the client ip
-  @Override
-  void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", Long responseContentLength = null, ServerEndpoint endpoint = SUCCESS) {
-    trace.span(index) {
-      name expectedServerSpanName(endpoint)
-      kind SERVER
-      if (endpoint.errored) {
-        status StatusCode.ERROR
-      }
-      if (parentID != null) {
-        traceId traceID
-        parentSpanId parentID
-      } else {
-        hasNoParent()
-      }
-      attributes {
-        // dropwizard reports peer ip as the client ip
-        "${SemanticAttributes.NET_PEER_IP.key}" TEST_CLIENT_IP
-        "${SemanticAttributes.NET_PEER_PORT.key}" Long
-        "${SemanticAttributes.HTTP_SCHEME.key}" "http"
-        "${SemanticAttributes.HTTP_HOST}" "localhost:${port}"
-        "${SemanticAttributes.HTTP_TARGET}" endpoint.resolvePath(address).getPath() + "${endpoint == QUERY_PARAM ? "?${endpoint.body}" : ""}"
-        "${SemanticAttributes.HTTP_METHOD.key}" method
-        "${SemanticAttributes.HTTP_STATUS_CODE.key}" endpoint.status
-        "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
-        "${SemanticAttributes.HTTP_USER_AGENT.key}" TEST_USER_AGENT
-        "${SemanticAttributes.HTTP_CLIENT_IP.key}" TEST_CLIENT_IP
-        if (endpoint == ServerEndpoint.CAPTURE_HEADERS) {
-          "http.request.header.x_test_request" { it == ["test"] }
-          "http.response.header.x_test_response" { it == ["test"] }
-        }
-      }
-    }
-  }
-
   static class TestApp extends Application<Configuration> {
     @Override
     void initialize(Bootstrap<Configuration> bootstrap) {

diff --git a/instrumentation/grails-3.0/javaagent/src/test/groovy/test/GrailsTest.groovy b/instrumentation/grails-3.0/javaagent/src/test/groovy/test/GrailsTest.groovy
@@ -8,11 +8,13 @@ package test
 import grails.boot.GrailsApp
 import grails.boot.config.GrailsAutoConfiguration
 import groovy.transform.CompileStatic
+import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 import io.opentelemetry.sdk.trace.data.SpanData
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import org.springframework.boot.autoconfigure.SpringBootApplication
 import org.springframework.boot.autoconfigure.web.ServerProperties
 import org.springframework.context.ConfigurableApplicationContext
@@ -53,6 +55,15 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
     }
   }
 
+  @Override
+  List<AttributeKey<?>> extraAttributes() {
+    [
+      SemanticAttributes.HTTP_SERVER_NAME,
+      SemanticAttributes.NET_PEER_NAME,
+      SemanticAttributes.NET_TRANSPORT
+    ]
+  }
+
   @Override
   String expectedServerSpanName(ServerEndpoint endpoint) {
     if (endpoint == PATH_PARAM) {

diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy
@@ -3,6 +3,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
+
+import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
@@ -187,6 +190,14 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
     "failing"    | "throw"   | 500        | { it == "failure" } | true    | "failure"
   }
 
+  @Override
+  List<AttributeKey<?>> extraAttributes() {
+    [
+      SemanticAttributes.HTTP_SERVER_NAME,
+      SemanticAttributes.NET_TRANSPORT
+    ]
+  }
+
   @Override
   boolean hasHandlerSpan(ServerEndpoint endpoint) {
     true
@@ -281,6 +292,9 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
         "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
         "${SemanticAttributes.HTTP_USER_AGENT.key}" TEST_USER_AGENT
         "${SemanticAttributes.HTTP_CLIENT_IP.key}" TEST_CLIENT_IP
+        "${SemanticAttributes.HTTP_SERVER_NAME}" String
+        "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
+        "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH}" { it == null || it instanceof Long } // Optional
         if (fullUrl.getPath().endsWith(ServerEndpoint.CAPTURE_HEADERS.getPath())) {
           "http.request.header.x_test_request" { it == ["test"] }
           "http.response.header.x_test_response" { it == ["test"] }

diff --git a/instrumentation/jetty/jetty-11.0/javaagent/src/test/groovy/JettyHandlerTest.groovy b/instrumentation/jetty/jetty-11.0/javaagent/src/test/groovy/JettyHandlerTest.groovy
@@ -3,9 +3,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import jakarta.servlet.DispatcherType
 import jakarta.servlet.ServletException
 import jakarta.servlet.http.HttpServletRequest
@@ -60,6 +62,14 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
     server.stop()
   }
 
+  @Override
+  List<AttributeKey<?>> extraAttributes() {
+    [
+      SemanticAttributes.HTTP_SERVER_NAME,
+      SemanticAttributes.NET_TRANSPORT
+    ]
+  }
+
   @Override
   boolean hasResponseSpan(ServerEndpoint endpoint) {
     endpoint == REDIRECT || endpoint == ERROR

diff --git a/instrumentation/jetty/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy b/instrumentation/jetty/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy
@@ -3,9 +3,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import org.eclipse.jetty.server.Request
 import org.eclipse.jetty.server.Response
 import org.eclipse.jetty.server.Server
@@ -61,6 +63,14 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
     server.stop()
   }
 
+  @Override
+  List<AttributeKey<?>> extraAttributes() {
+    [
+      SemanticAttributes.HTTP_SERVER_NAME,
+      SemanticAttributes.NET_TRANSPORT
+    ]
+  }
+
   @Override
   boolean hasResponseSpan(ServerEndpoint endpoint) {
     endpoint == REDIRECT || endpoint == ERROR

diff --git a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy
@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
+
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.utils.PortUtils
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
@@ -96,6 +98,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
+            "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" { it == null || it instanceof Long } // Optional
           }
         }
         span(1) {
@@ -148,6 +153,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -196,6 +203,9 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
+            "${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH}" Long
           }
         }
         span(1) {
@@ -253,6 +263,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 500
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -315,6 +327,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -358,6 +372,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -433,6 +449,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 500
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -477,6 +495,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
       }

diff --git a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy
@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
+
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.utils.PortUtils
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
@@ -93,6 +95,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -157,6 +161,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -200,6 +206,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -291,6 +299,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 200
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -368,6 +378,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 500
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {
@@ -424,6 +436,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
             "${SemanticAttributes.HTTP_STATUS_CODE.key}" 404
             "${SemanticAttributes.HTTP_FLAVOR.key}" "1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key}" String
+            "${SemanticAttributes.HTTP_SERVER_NAME}" String
+            "${SemanticAttributes.NET_TRANSPORT}" IP_TCP
           }
         }
         span(1) {