More consistent custom trace sorting in tests (#2728)

instrumentation/elasticsearch/elasticsearch-transport-5.0/javaagent/src/test/groovy/Elasticsearch5TransportClientTest.groovy

@@ -176,14 +176,9 @@ class Elasticsearch5TransportClientTest extends AgentInstrumentationSpecificatio
 
     and:
     assertTraces(5) {
-      sortTraces {
+      // PutMappingAction and IndexAction run in separate threads so their order can vary
-        // IndexAction and PutMappingAction run in separate threads and so their order is not always the same
+      traces.subList(2, 4).sort(orderByRootSpanName("PutMappingAction", "IndexAction"))
-        if (traces[2][0].name == "IndexAction") {
+
-          def tmp = traces[2]
-          traces[2] = traces[3]
-          traces[3] = tmp
-        }
-      }
       trace(0, 1) {
         span(0) {
           name "CreateIndexAction"

instrumentation/elasticsearch/elasticsearch-transport-5.3/javaagent/src/test/groovy/Elasticsearch53TransportClientTest.groovy

@@ -181,14 +181,9 @@ class Elasticsearch53TransportClientTest extends AgentInstrumentationSpecificati
 
     and:
     assertTraces(5) {
-      sortTraces {
+      // PutMappingAction and IndexAction run in separate threads so their order can vary
-        // IndexAction and PutMappingAction run in separate threads and so their order is not always the same
+      traces.subList(2, 4).sort(orderByRootSpanName("PutMappingAction", "IndexAction"))
-        if (traces[2][0].name == "IndexAction") {
+
-          def tmp = traces[2]
-          traces[2] = traces[3]
-          traces[3] = tmp
-        }
-      }
       trace(0, 1) {
         span(0) {
           name "CreateIndexAction"

instrumentation/elasticsearch/elasticsearch-transport-6.0/javaagent/src/test/groovy/Elasticsearch6TransportClientTest.groovy

@@ -174,14 +174,12 @@ class Elasticsearch6TransportClientTest extends AgentInstrumentationSpecificatio
 
     and:
     assertTraces(5) {
-      sortTraces {
+      // PutMappingAction and IndexAction run in separate threads so their order can vary
-        // IndexAction and PutMappingAction run in separate threads and so their order is not always the same
+      traces.subList(2, 4).sort(orderByRootSpanName(
-        if (traces[2][0].name == "IndexAction") {
+        "PutMappingAction", // elasticsearch < 7
-          def tmp = traces[2]
+        "AutoPutMappingAction", // elasticsearch >= 7
-          traces[2] = traces[3]
+        "IndexAction"))
-          traces[3] = tmp
+
-        }
-      }
       trace(0, 1) {
         span(0) {
           name "CreateIndexAction"

instrumentation/jms-1.1/javaagent/src/jms2Test/groovy/SpringTemplateJms2Test.groovy

@@ -119,16 +119,11 @@ class SpringTemplateJms2Test extends AgentInstrumentationSpecification {
     expect:
     receivedMessage.text == "responded!"
     assertTraces(4) {
-      sortTraces {
+      traces.sort(orderByRootSpanName(
-        def expectedOrder = ["$destinationName receive",
+        "$destinationName receive",
         "$destinationName send",
         "(temporary) receive",
-                             "(temporary) send"]
+        "(temporary) send"))
-        // ensure that traces appear in expected order
-        traces.sort {a,b ->
-          expectedOrder.indexOf(a[0].name) - expectedOrder.indexOf(b[0].name)
-        }
-      }
 
       trace(0, 1) {
         consumerSpan(it, 0, destinationType, destinationName, msgId.get(), null, "receive")

instrumentation/jms-1.1/javaagent/src/test/groovy/SpringListenerJms1Test.groovy

@@ -5,6 +5,7 @@
 
 import static Jms1Test.consumerSpan
 import static Jms1Test.producerSpan
+import static io.opentelemetry.api.trace.SpanKind.CONSUMER
 import static io.opentelemetry.api.trace.SpanKind.PRODUCER
 
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
@@ -25,14 +26,7 @@ class SpringListenerJms1Test extends AgentInstrumentationSpecification {
 
     expect:
     assertTraces(2) {
-      sortTraces {
+      traces.sort(orderByRootSpanKind(CONSUMER, PRODUCER))
-        // ensure that traces appear in expected order
-        if (traces[0][0].kind == PRODUCER) {
-          def tmp = traces[0]
-          traces[0] = traces[1]
-          traces[1] = tmp
-        }
-      }
 
       trace(0, 1) {
         consumerSpan(it, 0, "queue", "SpringListenerJms1", "", null, "receive")

instrumentation/jms-1.1/javaagent/src/test/groovy/SpringTemplateJms1Test.groovy

@@ -97,16 +97,11 @@ class SpringTemplateJms1Test extends AgentInstrumentationSpecification {
     expect:
     receivedMessage.text == "responded!"
     assertTraces(4) {
-      sortTraces {
+      traces.sort(orderByRootSpanName(
-        def expectedOrder = ["$destinationName receive",
+        "$destinationName receive",
         "$destinationName send",
         "(temporary) receive",
-                             "(temporary) send"]
+        "(temporary) send"))
-        // ensure that traces appear in expected order
-        traces.sort {a,b ->
-          expectedOrder.indexOf(a[0].name) - expectedOrder.indexOf(b[0].name)
-        }
-      }
 
       trace(0, 1) {
         consumerSpan(it, 0, destinationType, destinationName, msgId.get(), null, "receive")

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

@@ -9,15 +9,15 @@ import static TraceAssert.assertTrace
 
 import groovy.transform.stc.ClosureParams
 import groovy.transform.stc.SimpleType
+import io.opentelemetry.api.trace.SpanKind
 import io.opentelemetry.instrumentation.testing.util.TelemetryDataUtil
 import io.opentelemetry.sdk.trace.data.SpanData
+import java.util.function.Supplier
 import org.codehaus.groovy.runtime.powerassert.PowerAssertionError
 import org.spockframework.runtime.Condition
 import org.spockframework.runtime.ConditionNotSatisfiedError
 import org.spockframework.runtime.model.TextPosition
 
-import java.util.function.Supplier
-
 class InMemoryExporterAssert {
   private final List<List<SpanData>> traces
   private final Supplier<List<SpanData>> spanSupplier
@@ -75,9 +75,14 @@ class InMemoryExporterAssert {
     assertTrace(spanSupplier, traces[index][0].traceId, expectedSize, spec)
   }
 
-  // this doesn't provide any functionality, just a self-documenting marker
+  static Comparator<List<SpanData>> orderByRootSpanName(String... names) {
-  static void sortTraces(Closure callback) {
+    def list = Arrays.asList(names)
-    callback.call()
+    return Comparator.comparing { item -> list.indexOf(item[0].name) }
+  }
+
+  static Comparator<List<SpanData>> orderByRootSpanKind(SpanKind... spanKinds) {
+    def list = Arrays.asList(spanKinds)
+    return Comparator.comparing { item -> list.indexOf(item[0].kind) }
   }
 
   void assertTracesAllVerified() {

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

@@ -209,6 +209,8 @@ abstract class HttpClientTest extends InstrumentationSpecification {
     // there should be 2 separate traces since the nested CLIENT span is suppressed
     // (and the span context propagation along with it)
     assertTraces(2) {
+      traces.sort(orderByRootSpanKind(CLIENT, SERVER))
+
       trace(0, 1) {
         basicClientSpan(it, 0, "parent-client-span")
       }