Remove HttpServerTest#extraAttributes() method (open-telemetry#5176)

* Remove HttpServerTest#extraAttributes() method * fix ktor tests * fix ratpack and restlet tests * fix servlet2 tests * Fix webflux and vertx tests
RashmiRam · May 23, 2022 · bcafbcb · bcafbcb
1 parent 33054a1
commit bcafbcb
Show file tree

Hide file tree

Showing 40 changed files with 170 additions and 268 deletions.
diff --git a/...on/akka/akka-http-10.0/javaagent/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy b/...on/akka/akka-http-10.0/javaagent/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy
@@ -6,7 +6,6 @@
 import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
-import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 
 abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<Object> implements AgentTestTrait {
 
@@ -33,9 +32,7 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<Object>
 
   @Override
   Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
-    def attributes = super.httpAttributes(endpoint)
-    attributes.remove(SemanticAttributes.HTTP_ROUTE)
-    attributes
+    []
   }
 }
 

diff --git a/...groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy b/...groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaHttpServerTest.groovy
@@ -52,14 +52,13 @@ abstract class AbstractArmeriaHttpServerTest extends HttpServerTest<Server> {
   }
 
   @Override
-  List<AttributeKey<?>> extraAttributes() {
-    [
+  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
+    Set<AttributeKey<?>> extra = [
       SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH,
       SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH,
-      SemanticAttributes.HTTP_SERVER_NAME,
-      SemanticAttributes.NET_PEER_NAME,
-      SemanticAttributes.NET_TRANSPORT
+      SemanticAttributes.HTTP_SERVER_NAME
     ]
+    super.httpAttributes(endpoint) + extra
   }
 
   @Override

diff --git a/instrumentation/dropwizard/dropwizard-testing/src/test/groovy/DropwizardTest.groovy b/instrumentation/dropwizard/dropwizard-testing/src/test/groovy/DropwizardTest.groovy
@@ -63,12 +63,11 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
   }
 
   @Override
-  List<AttributeKey<?>> extraAttributes() {
-    [
-      SemanticAttributes.HTTP_SERVER_NAME,
-      SemanticAttributes.NET_PEER_NAME,
-      SemanticAttributes.NET_TRANSPORT
+  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
+    Set<AttributeKey<?>> extra = [
+      SemanticAttributes.HTTP_SERVER_NAME
     ]
+    super.httpAttributes(endpoint) + extra
   }
 
   // this override is needed because dropwizard reports peer ip as the client ip

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
@@ -57,12 +57,11 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
   }
 
   @Override
-  List<AttributeKey<?>> extraAttributes() {
-    [
-      SemanticAttributes.HTTP_SERVER_NAME,
-      SemanticAttributes.NET_PEER_NAME,
-      SemanticAttributes.NET_TRANSPORT
+  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
+    Set<AttributeKey<?>> extra = [
+      SemanticAttributes.HTTP_SERVER_NAME
     ]
+    super.httpAttributes(endpoint) + extra
   }
 
   @Override

diff --git a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyFilterchainServerTest.groovy
@@ -67,6 +67,7 @@ class GrizzlyFilterchainServerTest extends HttpServerTest<HttpServer> implements
   Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
     def attributes = super.httpAttributes(endpoint)
     attributes.remove(SemanticAttributes.HTTP_ROUTE)
+    attributes.remove(SemanticAttributes.NET_TRANSPORT)
     attributes
   }
 

diff --git a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyTest.groovy b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyTest.groovy
@@ -48,6 +48,7 @@ class GrizzlyTest extends HttpServerTest<HttpServer> implements AgentTestTrait {
   Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
     def attributes = super.httpAttributes(endpoint)
     attributes.remove(SemanticAttributes.HTTP_ROUTE)
+    attributes.remove(SemanticAttributes.NET_TRANSPORT)
     attributes
   }
 

diff --git a/...ation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/testing/src/main/groovy/JaxRsHttpServerTest.groovy b/...ation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/testing/src/main/groovy/JaxRsHttpServerTest.groovy
@@ -190,11 +190,11 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
   }
 
   @Override
-  List<AttributeKey<?>> extraAttributes() {
-    [
-      SemanticAttributes.HTTP_SERVER_NAME,
-      SemanticAttributes.NET_TRANSPORT
+  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
+    Set<AttributeKey<?>> extra = [
+      SemanticAttributes.HTTP_SERVER_NAME
     ]
+    super.httpAttributes(endpoint) + extra
   }
 
   @Override

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
@@ -63,11 +63,16 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
   }
 
   @Override
-  List<AttributeKey<?>> extraAttributes() {
-    [
-      SemanticAttributes.HTTP_SERVER_NAME,
-      SemanticAttributes.NET_TRANSPORT
-    ]
+  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
+    def attributes = super.httpAttributes(endpoint)
+    attributes.remove(SemanticAttributes.HTTP_ROUTE)
+    attributes.add(SemanticAttributes.HTTP_SERVER_NAME)
+    attributes
+  }
+
+  @Override
+  String expectedServerSpanName(ServerEndpoint endpoint) {
+    "HTTP GET"
   }
 
   @Override
@@ -114,7 +119,7 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
         case EXCEPTION:
           throw new Exception(endpoint.body)
         case INDEXED_CHILD:
-          INDEXED_CHILD.collectSpanAttributes {name -> request.getParameter(name) }
+          INDEXED_CHILD.collectSpanAttributes { name -> request.getParameter(name) }
           response.status = endpoint.status
           response.writer.print(endpoint.body)
           break
@@ -140,16 +145,4 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
       }
     }
   }
-
-  @Override
-  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
-    def attributes = super.httpAttributes(endpoint)
-    attributes.remove(SemanticAttributes.HTTP_ROUTE)
-    attributes
-  }
-
-  @Override
-  String expectedServerSpanName(ServerEndpoint endpoint) {
-    "HTTP GET"
-  }
 }
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
@@ -64,11 +64,16 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
   }
 
   @Override
-  List<AttributeKey<?>> extraAttributes() {
-    [
-      SemanticAttributes.HTTP_SERVER_NAME,
-      SemanticAttributes.NET_TRANSPORT
-    ]
+  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
+    def attributes = super.httpAttributes(endpoint)
+    attributes.remove(SemanticAttributes.HTTP_ROUTE)
+    attributes.addAll(SemanticAttributes.HTTP_SERVER_NAME)
+    attributes
+  }
+
+  @Override
+  String expectedServerSpanName(ServerEndpoint endpoint) {
+    "HTTP GET"
   }
 
   @Override
@@ -115,7 +120,7 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
         case EXCEPTION:
           throw new Exception(endpoint.body)
         case INDEXED_CHILD:
-          INDEXED_CHILD.collectSpanAttributes {name -> request.getParameter(name) }
+          INDEXED_CHILD.collectSpanAttributes { name -> request.getParameter(name) }
           response.status = endpoint.status
           response.writer.print(endpoint.body)
           break
@@ -141,16 +146,4 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
       }
     }
   }
-
-  @Override
-  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
-    def attributes = super.httpAttributes(endpoint)
-    attributes.remove(SemanticAttributes.HTTP_ROUTE)
-    attributes
-  }
-
-  @Override
-  String expectedServerSpanName(ServerEndpoint endpoint) {
-    "HTTP GET"
-  }
 }
diff --git a/...rary/src/test/groovy/io/opentelemetry/instrumentation/ktor/v1_0/KtorHttpServerTest.groovy b/...rary/src/test/groovy/io/opentelemetry/instrumentation/ktor/v1_0/KtorHttpServerTest.groovy
@@ -39,6 +39,13 @@ class KtorHttpServerTest extends HttpServerTest<ApplicationEngine> implements Li
     true
   }
 
+  @Override
+  Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
+    def attributes = super.httpAttributes(endpoint)
+    attributes.remove(SemanticAttributes.NET_PEER_PORT)
+    attributes
+  }
+
   @Override
   String expectedServerSpanName(ServerEndpoint endpoint) {
     def route = expectedHttpRoute(endpoint)
@@ -54,12 +61,4 @@ class KtorHttpServerTest extends HttpServerTest<ApplicationEngine> implements Li
         return super.expectedHttpRoute(endpoint)
     }
   }
-
-  @Override
-  List<AttributeKey<?>> extraAttributes() {
-    [
-      SemanticAttributes.NET_PEER_NAME,
-      SemanticAttributes.NET_TRANSPORT
-    ]
-  }
 }
diff --git a/...emetry/javaagent/instrumentation/netty/v3_8/server/NettyNetServerAttributesExtractor.java b/...emetry/javaagent/instrumentation/netty/v3_8/server/NettyNetServerAttributesExtractor.java
@@ -5,11 +5,15 @@
 
 package io.opentelemetry.javaagent.instrumentation.netty.v3_8.server;
 
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP;
+
 import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
 import io.opentelemetry.javaagent.instrumentation.netty.v3_8.HttpRequestAndChannel;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import javax.annotation.Nullable;
+import org.jboss.netty.channel.socket.DatagramChannel;
 import org.jboss.netty.handler.codec.http.HttpResponse;
 
 final class NettyNetServerAttributesExtractor
@@ -18,7 +22,7 @@ final class NettyNetServerAttributesExtractor
   @Override
   @Nullable
   public String transport(HttpRequestAndChannel requestAndChannel) {
-    return null;
+    return requestAndChannel.channel() instanceof DatagramChannel ? IP_UDP : IP_TCP;
   }
 
   @Override

diff --git a/...etry/javaagent/instrumentation/netty/common/server/NettyNetServerAttributesExtractor.java b/...etry/javaagent/instrumentation/netty/common/server/NettyNetServerAttributesExtractor.java
@@ -5,6 +5,10 @@
 
 package io.opentelemetry.javaagent.instrumentation.netty.common.server;
 
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP;
+
+import io.netty.channel.socket.DatagramChannel;
 import io.netty.handler.codec.http.HttpResponse;
 import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesExtractor;
 import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
@@ -18,7 +22,7 @@ final class NettyNetServerAttributesExtractor
   @Override
   @Nullable
   public String transport(HttpRequestAndChannel requestAndChannel) {
-    return null;
+    return requestAndChannel.channel() instanceof DatagramChannel ? IP_UDP : IP_TCP;
   }
 
   @Override

diff --git a/instrumentation/play/play-2.6/javaagent/src/test/groovy/server/PlayServerTest.groovy b/instrumentation/play/play-2.6/javaagent/src/test/groovy/server/PlayServerTest.groovy
@@ -11,7 +11,6 @@ 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 play.BuiltInComponents
 import play.Mode
 import play.mvc.Controller
@@ -103,9 +102,7 @@ class PlayServerTest extends HttpServerTest<Server> implements AgentTestTrait {
 
   @Override
   Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
-    def attributes = super.httpAttributes(endpoint)
-    attributes.remove(SemanticAttributes.HTTP_ROUTE)
-    attributes
+    []
   }
 
   @Override

diff --git a/...n/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy b/...n/groovy/io/opentelemetry/instrumentation/ratpack/server/AbstractRatpackRoutesTest.groovy
@@ -5,7 +5,6 @@
 
 package io.opentelemetry.instrumentation.ratpack.server
 
-import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.instrumentation.test.InstrumentationSpecification
 import io.opentelemetry.instrumentation.test.utils.PortUtils
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
@@ -81,10 +80,6 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification {
 
   abstract boolean hasHandlerSpan()
 
-  List<AttributeKey<?>> extraAttributes() {
-    []
-  }
-
   def "test bindings for #path"() {
     when:
     def resp = client.get(path).aggregate().join()
@@ -93,44 +88,26 @@ abstract class AbstractRatpackRoutesTest extends InstrumentationSpecification {
     resp.status().code() == 200
     resp.contentUtf8() == route
 
-    def extraAttributes = extraAttributes()
-
     assertTraces(1) {
       trace(0, 1 + (hasHandlerSpan() ? 1 : 0)) {
         span(0) {
           name "/$route"
           kind SERVER
           hasNoParent()
           attributes {
-            if (extraAttributes.contains(SemanticAttributes.NET_TRANSPORT)) {
-              "$SemanticAttributes.NET_TRANSPORT" IP_TCP
-            }
+            "$SemanticAttributes.NET_TRANSPORT" IP_TCP
             // net.peer.name resolves to "127.0.0.1" on windows which is same as net.peer.ip so then not captured
             "$SemanticAttributes.NET_PEER_NAME" { it == null || it == "localhost" }
             "$SemanticAttributes.NET_PEER_IP" { it == null || it == "127.0.0.1" }
             "$SemanticAttributes.NET_PEER_PORT" Long
+
             "$SemanticAttributes.HTTP_METHOD" "GET"
             "$SemanticAttributes.HTTP_STATUS_CODE" 200
             "$SemanticAttributes.HTTP_FLAVOR" "1.1"
             "$SemanticAttributes.HTTP_USER_AGENT" String
-
-            if (extraAttributes.contains(SemanticAttributes.HTTP_URL)) {
-              "$SemanticAttributes.HTTP_URL" "http://localhost:${app.bindPort}/${path}"
-            } else {
-              "$SemanticAttributes.HTTP_SCHEME" "http"
-              "$SemanticAttributes.HTTP_HOST" "localhost:${app.bindPort}"
-              "$SemanticAttributes.HTTP_TARGET" "/$path"
-            }
-
-            if (extraAttributes.contains(SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH)) {
-              "$SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH" Long
-            }
-            if (extraAttributes.contains(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH)) {
-              "$SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH" Long
-            }
-            if (extraAttributes.contains(SemanticAttributes.HTTP_SERVER_NAME)) {
-              "$SemanticAttributes.HTTP_SERVER_NAME" String
-            }
+            "$SemanticAttributes.HTTP_SCHEME" "http"
+            "$SemanticAttributes.HTTP_HOST" "localhost:${app.bindPort}"
+            "$SemanticAttributes.HTTP_TARGET" "/$path"
             "$SemanticAttributes.HTTP_ROUTE" "/$route"
           }
         }

diff --git a/.../groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackAsyncHttpServerTest.groovy b/.../groovy/io/opentelemetry/instrumentation/ratpack/server/RatpackAsyncHttpServerTest.groovy
@@ -5,10 +5,8 @@
 
 package io.opentelemetry.instrumentation.ratpack.server
 
-import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.instrumentation.ratpack.RatpackTracing
 import io.opentelemetry.instrumentation.test.LibraryTestTrait
-import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import ratpack.server.RatpackServerSpec
 
 class RatpackAsyncHttpServerTest extends AbstractRatpackAsyncHttpServerTest implements LibraryTestTrait {
@@ -26,12 +24,4 @@ class RatpackAsyncHttpServerTest extends AbstractRatpackAsyncHttpServerTest impl
   boolean hasHandlerSpan(ServerEndpoint endpoint) {
     false
   }
-
-  @Override
-  List<AttributeKey<?>> extraAttributes() {
-    return [
-      SemanticAttributes.HTTP_ROUTE,
-      SemanticAttributes.NET_TRANSPORT
-    ]
-  }
 }