Verify span kind in all tests (#2715)

instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.api.trace.SpanKind.SERVER
 import static io.opentelemetry.instrumentation.test.utils.ClassUtils.getClassName
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace
 
@@ -51,6 +52,7 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica
       trace(0, 2) {
         span(0) {
           name paramName
+          kind SERVER
           hasNoParent()
           attributes {
           }
@@ -137,6 +139,7 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica
       trace(0, 1) {
         span(0) {
           name "test"
+          kind SERVER
           attributes {
           }
         }

instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JerseyTest.groovy

@@ -4,6 +4,7 @@
  */
 
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
+import static io.opentelemetry.api.trace.SpanKind.SERVER
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace
 
 import io.dropwizard.testing.junit.ResourceTestRule
@@ -37,6 +38,7 @@ class JerseyTest extends AgentInstrumentationSpecification {
       trace(0, 2) {
         span(0) {
           name expectedSpanName
+          kind SERVER
           attributes {
           }
         }
@@ -72,6 +74,7 @@ class JerseyTest extends AgentInstrumentationSpecification {
       trace(0, 2) {
         span(0) {
           name expectedSpanName
+          kind SERVER
           attributes {
           }
         }

instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.api.trace.SpanKind.SERVER
 import static io.opentelemetry.instrumentation.test.utils.ClassUtils.getClassName
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace
 
@@ -51,6 +52,7 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentInstrumentationS
       trace(0, 2) {
         span(0) {
           name paramName
+          kind SERVER
           hasNoParent()
           attributes {
           }
@@ -137,6 +139,7 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentInstrumentationS
       trace(0, 1) {
         span(0) {
           name "test"
+          kind SERVER
           attributes {
           }
         }

instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy

@@ -4,6 +4,7 @@
  */
 
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
+import static io.opentelemetry.api.trace.SpanKind.SERVER
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace
 
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
@@ -71,6 +72,7 @@ abstract class JaxRsFilterTest extends AgentInstrumentationSpecification {
       trace(0, 2) {
         span(0) {
           name parentSpanName != null ? parentSpanName : "test.span"
+          kind SERVER
           if (runsOnServer()) {
             errored abortNormal
           } else {
@@ -123,6 +125,7 @@ abstract class JaxRsFilterTest extends AgentInstrumentationSpecification {
       trace(0, 2) {
         span(0) {
           name parentResourceName
+          kind SERVER
           if (!runsOnServer()) {
             attributes {
             }

instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-wildfly-testing/src/test/groovy/WildflyRestTest.groovy

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.api.trace.SpanKind.SERVER
+
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import javax.ws.rs.client.Client
 import javax.ws.rs.client.WebTarget
@@ -57,6 +59,7 @@ class WildflyRestTest extends AgentInstrumentationSpecification {
       trace(0, 2) {
         span(0) {
           name getContextRoot() + path
+          kind SERVER
           hasNoParent()
         }
         span(1) {
@@ -67,7 +70,7 @@ class WildflyRestTest extends AgentInstrumentationSpecification {
     }
 
     where:
-    path        | className
+    path       | className
     "cdiHello" | "CdiRestResource"
     "ejbHello" | "EjbRestResource"
   }

instrumentation/jdbc/javaagent/src/test/groovy/JdbcInstrumentationTest.groovy

@@ -551,6 +551,7 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification {
 
         span(1) {
           name "${datasource.class.simpleName}.getConnection"
+          kind CLIENT
           childOf span(0)
           attributes {
           }
@@ -558,6 +559,7 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification {
         if (recursive) {
           span(2) {
             name "${datasource.class.simpleName}.getConnection"
+            kind CLIENT
             childOf span(1)
             attributes {
             }

instrumentation/jsf/jsf-testing-common/src/main/groovy/BaseJsfTest.groovy

@@ -4,7 +4,7 @@
  */
 
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
-import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
+import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicServerSpan
 
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
@@ -82,7 +82,7 @@ abstract class BaseJsfTest extends AgentInstrumentationSpecification implements
     and:
     assertTraces(1) {
       trace(0, 1) {
-        basicSpan(it, 0, getContextPath() + "/hello.xhtml", null)
+        basicServerSpan(it, 0, getContextPath() + "/hello.xhtml", null)
       }
     }
 
@@ -105,7 +105,7 @@ abstract class BaseJsfTest extends AgentInstrumentationSpecification implements
     and:
     assertTraces(1) {
       trace(0, 1) {
-        basicSpan(it, 0, getContextPath() + "/greeting.xhtml", null)
+        basicServerSpan(it, 0, getContextPath() + "/greeting.xhtml", null)
       }
     }
     clearExportedData()
@@ -144,7 +144,7 @@ abstract class BaseJsfTest extends AgentInstrumentationSpecification implements
     and:
     assertTraces(1) {
       trace(0, 2) {
-        basicSpan(it, 0, getContextPath() + "/greeting.xhtml", null)
+        basicServerSpan(it, 0, getContextPath() + "/greeting.xhtml", null)
         handlerSpan(it, 1, span(0), "#{greetingForm.submit()}")
       }
     }
@@ -165,7 +165,7 @@ abstract class BaseJsfTest extends AgentInstrumentationSpecification implements
     and:
     assertTraces(1) {
       trace(0, 1) {
-        basicSpan(it, 0, getContextPath() + "/greeting.xhtml", null)
+        basicServerSpan(it, 0, getContextPath() + "/greeting.xhtml", null)
       }
     }
     clearExportedData()
@@ -201,7 +201,7 @@ abstract class BaseJsfTest extends AgentInstrumentationSpecification implements
     and:
     assertTraces(1) {
       trace(0, 2) {
-        basicSpan(it, 0, getContextPath() + "/greeting.xhtml", null, new Exception("submit exception"))
+        basicServerSpan(it, 0, getContextPath() + "/greeting.xhtml", null, new Exception("submit exception"))
         handlerSpan(it, 1, span(0), "#{greetingForm.submit()}", new Exception("submit exception"))
       }
     }

instrumentation/lettuce/lettuce-5.1/testing/src/main/groovy/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceSyncClientTest.groovy

@@ -454,6 +454,7 @@ abstract class AbstractLettuceSyncClientTest extends InstrumentationSpecificatio
       trace(0, 1) {
         span(0) {
           name "DEBUG"
+          kind CLIENT
           // Disconnect not an actual error even though an exception is recorded.
           errored false
           attributes {
@@ -488,6 +489,7 @@ abstract class AbstractLettuceSyncClientTest extends InstrumentationSpecificatio
       trace(0, 1) {
         span(0) {
           name "SHUTDOWN"
+          kind CLIENT
           if (Boolean.getBoolean("testLatestDeps")) {
             // Seems to only be treated as an error with Lettuce 6+
             errored true

instrumentation/netty/netty-3.8/javaagent/src/latestDepTest/groovy/Netty38ClientTest.groovy

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.instrumentation.test.utils.PortUtils.UNUSABLE_PORT
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace
@@ -64,7 +65,10 @@ class Netty38ClientTest extends HttpClientTest implements AgentTestTrait {
     return false
   }
 
-  def "connection error (unopened port)"() {
+  // This is almost identical to "connection error (unopened port)" test from superclass.
+  // But it uses somewhat different span name for the client span.
+  // For now creating a separate test for this, hoping to remove this duplication in the future.
+  def "netty connection error (unopened port)"() {
     given:
     def uri = new URI("http://127.0.0.1:$UNUSABLE_PORT/")
 
@@ -84,6 +88,7 @@ class Netty38ClientTest extends HttpClientTest implements AgentTestTrait {
 
         span(1) {
           name "CONNECT"
+          kind CLIENT
           childOf span(0)
           errored true
           Class errorClass = ConnectException

instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ClientTest.groovy

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.instrumentation.test.utils.PortUtils.UNUSABLE_PORT
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace
@@ -64,7 +65,10 @@ class Netty38ClientTest extends HttpClientTest implements AgentTestTrait {
     return false
   }
 
-  def "connection error (unopened port)"() {
+  // This is almost identical to "connection error (unopened port)" test from superclass.
+  // But it uses somewhat different span name for the client span.
+  // For now creating a separate test for this, hoping to remove this duplication in the future.
+  def "netty connection error (unopened port)"() {
     given:
     def uri = new URI("http://127.0.0.1:$UNUSABLE_PORT/")
 
@@ -84,6 +88,7 @@ class Netty38ClientTest extends HttpClientTest implements AgentTestTrait {
 
         span(1) {
           name "CONNECT"
+          kind CLIENT
           childOf span(0)
           errored true
           Class errorClass = ConnectException

instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ClientTest.groovy

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.instrumentation.test.utils.PortUtils.UNUSABLE_PORT
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace
@@ -82,9 +83,9 @@ class Netty40ClientTest extends HttpClientTest implements AgentTestTrait {
     false
   }
 
-  //This is almost identical to "connection error (unopened port)" test from superclass.
+  // This is almost identical to "connection error (unopened port)" test from superclass.
-  //But it uses somewhat different span name for the client span.
+  // But it uses somewhat different span name for the client span.
-  //For now creating a separate test for this, hoping to remove this duplication in the future.
+  // For now creating a separate test for this, hoping to remove this duplication in the future.
   def "netty connection error (unopened port)"() {
     given:
     def uri = new URI("http://127.0.0.1:$UNUSABLE_PORT/") // Use numeric address to avoid ipv4/ipv6 confusion
@@ -104,6 +105,7 @@ class Netty40ClientTest extends HttpClientTest implements AgentTestTrait {
         basicSpan(it, 0, "parent", null, thrownException)
         span(1) {
           name "CONNECT"
+          kind CLIENT
           childOf span(0)
           errored true
           Class errorClass = ConnectException

instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ClientTest.groovy

@@ -3,6 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.instrumentation.test.utils.PortUtils.UNUSABLE_PORT
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace
@@ -193,7 +194,10 @@ class Netty41ClientTest extends HttpClientTest implements AgentTestTrait {
     group.shutdownGracefully()
   }
 
-  def "connection error (unopened port)"() {
+  // This is almost identical to "connection error (unopened port)" test from superclass.
+  // But it uses somewhat different span name for the client span.
+  // For now creating a separate test for this, hoping to remove this duplication in the future.
+  def "netty connection error (unopened port)"() {
     given:
     def uri = new URI("http://localhost:$UNUSABLE_PORT/")
 
@@ -218,6 +222,7 @@ class Netty41ClientTest extends HttpClientTest implements AgentTestTrait {
         for (def i = 1; i < size; i++) {
           span(i) {
             name "CONNECT"
+            kind CLIENT
             childOf span(0)
             errored true
             errorEvent(thrownException.class, ~/Connection refused:( no further information:)? localhost\/\[?[0-9.:]+\]?:$UNUSABLE_PORT/)

instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy

@@ -7,6 +7,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
+import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicServerSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
 
 import io.opentelemetry.api.trace.SpanKind
@@ -131,7 +132,7 @@ class Struts2ActionSpanTest extends HttpServerTest<Server> implements AgentTestT
     and:
     assertTraces(1) {
       trace(0, 3) {
-        basicSpan(it, 0, getContextPath() + "/dispatch", null)
+        basicServerSpan(it, 0, getContextPath() + "/dispatch", null)
         basicSpan(it, 1, "GreetingAction.dispatch_servlet", span(0))
         basicSpan(it, 2, "Dispatcher.forward", span(0))
       }

instrumentation/wicket-8.0/javaagent/src/test/groovy/WicketTest.groovy

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
+import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicServerSpan
 
 import hello.HelloApplication
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
@@ -66,7 +66,7 @@ class WicketTest extends AgentInstrumentationSpecification implements HttpServer
 
     assertTraces(1) {
       trace(0, 1) {
-        basicSpan(it, 0, getContextPath() + "/wicket-test/hello.HelloPage")
+        basicServerSpan(it, 0, getContextPath() + "/wicket-test/hello.HelloPage")
       }
     }
   }
@@ -82,7 +82,7 @@ class WicketTest extends AgentInstrumentationSpecification implements HttpServer
 
     assertTraces(1) {
       trace(0, 1) {
-        basicSpan(it, 0, getContextPath() + "/wicket-test/org.apache.wicket.markup.html.pages.InternalErrorPage", null, new Exception("test exception"))
+        basicServerSpan(it, 0, getContextPath() + "/wicket-test/org.apache.wicket.markup.html.pages.InternalErrorPage", null, new Exception("test exception"))
       }
     }
   }

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/asserts/SpanAssert.groovy

@@ -160,6 +160,9 @@ class SpanAssert {
     if (!checked.status) {
       errored(false)
     }
+    if (!checked.kind) {
+      kind(SpanKind.INTERNAL)
+    }
   }
 
   void attributes(@ClosureParams(value = SimpleType, options = ['io.opentelemetry.instrumentation.test.asserts.AttributesAssert'])
@@ -173,7 +176,7 @@ class SpanAssert {
 
   private Map<String, Object> toMap(Attributes attributes) {
     def map = new HashMap()
-    attributes.forEach {key, value ->
+    attributes.forEach { key, value ->
       map.put(key.key, value)
     }
     return map

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy

@@ -9,6 +9,7 @@ import static io.opentelemetry.api.trace.SpanKind.CLIENT
 import static io.opentelemetry.api.trace.SpanKind.SERVER
 import static io.opentelemetry.instrumentation.test.server.http.TestHttpServer.httpServer
 import static io.opentelemetry.instrumentation.test.utils.PortUtils.UNUSABLE_PORT
+import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicClientSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderParentClientSpan
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace
@@ -158,7 +159,7 @@ abstract class HttpClientTest extends InstrumentationSpecification {
     // (and the span context propagation along with it)
     assertTraces(2) {
       trace(0, 1) {
-        basicSpan(it, 0, "parent-client-span")
+        basicClientSpan(it, 0, "parent-client-span")
       }
       trace(1, 1) {
         serverSpan(it, 0)

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/server/http/TestHttpServer.groovy

@@ -116,6 +116,7 @@ class TestHttpServer implements AutoCloseable {
   static distributedRequestSpan(TraceAssert trace, int index, SpanData parentSpan = null) {
     trace.span(index) {
       name "test-http-server"
+      kind SERVER
       errored false
       if (parentSpan == null) {
         hasNoParent()

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/utils/TraceUtils.groovy

@@ -59,9 +59,28 @@ class TraceUtils {
     r.call()
   }
 
+  static basicClientSpan(TraceAssert trace, int index, String operation, Object parentSpan = null, Throwable exception = null,
+                         @ClosureParams(value = SimpleType, options = ['io.opentelemetry.instrumentation.test.asserts.AttributesAssert'])
+                         @DelegatesTo(value = AttributesAssert, strategy = Closure.DELEGATE_FIRST) Closure additionAttributesAssert = null) {
+    return basicSpanForKind(trace, index, SpanKind.CLIENT, operation, parentSpan, exception, additionAttributesAssert)
+  }
+
+  static basicServerSpan(TraceAssert trace, int index, String operation, Object parentSpan = null, Throwable exception = null,
+                         @ClosureParams(value = SimpleType, options = ['io.opentelemetry.instrumentation.test.asserts.AttributesAssert'])
+                         @DelegatesTo(value = AttributesAssert, strategy = Closure.DELEGATE_FIRST) Closure additionAttributesAssert = null) {
+    return basicSpanForKind(trace, index, SpanKind.SERVER, operation, parentSpan, exception, additionAttributesAssert)
+  }
+
+  // TODO rename to basicInternalSpan
   static basicSpan(TraceAssert trace, int index, String operation, Object parentSpan = null, Throwable exception = null,
                    @ClosureParams(value = SimpleType, options = ['io.opentelemetry.instrumentation.test.asserts.AttributesAssert'])
                    @DelegatesTo(value = AttributesAssert, strategy = Closure.DELEGATE_FIRST) Closure additionAttributesAssert = null) {
+    return basicSpanForKind(trace, index, SpanKind.INTERNAL, operation, parentSpan, exception, additionAttributesAssert)
+  }
+
+  private static basicSpanForKind(TraceAssert trace, int index, SpanKind spanKind, String operation, Object parentSpan = null, Throwable exception = null,
+                                  @ClosureParams(value = SimpleType, options = ['io.opentelemetry.instrumentation.test.asserts.AttributesAssert'])
+                                  @DelegatesTo(value = AttributesAssert, strategy = Closure.DELEGATE_FIRST) Closure additionAttributesAssert = null) {
     trace.span(index) {
       if (parentSpan == null) {
         hasNoParent()
@@ -69,6 +88,7 @@ class TraceUtils {
         childOf((SpanData) parentSpan)
       }
       name operation
+      kind spanKind
       errored exception != null
       if (exception) {
         errorEvent(exception.class, exception.message)