Fix sporadic test failures (#82)

* Fix sporadic test failure * Remove RetryOnFailure from Elasticsearch tests * Remove retry from Hystrix tests * Improve test verification * Fix sporadic span order not found failures * Add RetryOnFailure to tests with sporadic failures
2020-01-23 11:37:38 -08:00 · 2020-01-23 11:37:38 -08:00 · ee58416dae
parent f53167063e
commit ee58416dae
31 changed files with 100 additions and 169 deletions
--- a/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy
+++ b/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy
@ -4,6 +4,7 @@ import io.opentelemetry.auto.instrumentation.akkahttp.AkkaHttpServerDecorator
 import io.opentelemetry.auto.instrumentation.api.Tags
 import io.opentelemetry.auto.test.asserts.TraceAssert
 import io.opentelemetry.auto.test.base.HttpServerTest
+import spock.lang.Retry

 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS
@ -78,6 +79,7 @@ class AkkaHttpServerInstrumentationTestSync extends AkkaHttpServerInstrumentatio
  }
 }

+@Retry(mode = Retry.Mode.SETUP_FEATURE_CLEANUP)
 class AkkaHttpServerInstrumentationTestAsync extends AkkaHttpServerInstrumentationTest {
  @Override
  def startServer(int port) {
--- a/instrumentation/dropwizard/dropwizard-views/src/test/groovy/ViewRenderTest.groovy
+++ b/instrumentation/dropwizard/dropwizard-views/src/test/groovy/ViewRenderTest.groovy
@ -7,7 +7,6 @@ import io.opentelemetry.auto.test.AgentTestRunner

 import java.nio.charset.StandardCharsets

-import static io.opentelemetry.auto.test.asserts.ListWriterAssert.assertTraces
 import static io.opentelemetry.auto.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

@ -24,7 +23,7 @@ class ViewRenderTest extends AgentTestRunner {

    then:
    outputStream.toString().contains("This is an example of a view")
-    assertTraces(TEST_WRITER, 1) {
+    assertTraces(1) {
      trace(0, 2) {
        basicSpan(it, 0, "parent")
        span(1) {
--- a/instrumentation/elasticsearch/rest-5/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy
+++ b/instrumentation/elasticsearch/rest-5/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import groovy.json.JsonSlurper
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
@ -18,7 +17,6 @@ import org.elasticsearch.node.Node
 import org.elasticsearch.transport.Netty4Plugin
 import spock.lang.Shared

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch6RestClientTest extends AgentTestRunner {
  @Shared
  int httpPort
--- a/instrumentation/elasticsearch/rest-5/src/test/groovy/Elasticsearch5RestClientTest.groovy
+++ b/instrumentation/elasticsearch/rest-5/src/test/groovy/Elasticsearch5RestClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import groovy.json.JsonSlurper
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
@ -21,7 +20,6 @@ import spock.lang.Shared

 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch5RestClientTest extends AgentTestRunner {
  @Shared
  int httpPort
--- a/instrumentation/elasticsearch/rest-6.4/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy
+++ b/instrumentation/elasticsearch/rest-6.4/src/latestDepTest/groovy/Elasticsearch6RestClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import groovy.json.JsonSlurper
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
@ -21,7 +20,6 @@ import org.elasticsearch.plugins.Plugin
 import org.elasticsearch.transport.Netty4Plugin
 import spock.lang.Shared

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch6RestClientTest extends AgentTestRunner {
  @Shared
  int httpPort
--- a/instrumentation/elasticsearch/rest-6.4/src/test/groovy/Elasticsearch6RestClientTest.groovy
+++ b/instrumentation/elasticsearch/rest-6.4/src/test/groovy/Elasticsearch6RestClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import groovy.json.JsonSlurper
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
@ -18,7 +17,6 @@ import org.elasticsearch.node.Node
 import org.elasticsearch.transport.Netty4Plugin
 import spock.lang.Shared

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch6RestClientTest extends AgentTestRunner {
  @Shared
  int httpPort
--- a/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/Elasticsearch2NodeClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/Elasticsearch2NodeClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -14,7 +13,6 @@ import spock.lang.Shared

 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2NodeClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/Elasticsearch2TransportClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/Elasticsearch2TransportClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -17,7 +16,6 @@ import spock.lang.Shared

 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2TransportClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringRepositoryTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringRepositoryTest.groovy
@ -1,6 +1,5 @@
 package springdata

-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -11,7 +10,6 @@ import spock.lang.Shared

 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2SpringRepositoryTest extends AgentTestRunner {
  @Shared
  ApplicationContext applicationContext = new AnnotationConfigApplicationContext(Config)
@ -69,32 +67,13 @@ class Elasticsearch2SpringRepositoryTest extends AgentTestRunner {
    repo.index(doc) == doc

    and:
-    assertTraces(3) {
-      sortTraces {
-        // IndexAction and PutMappingAction run in separate threads and so their order is not always the same
-        if (traces[0][0].attributes[MoreTags.RESOURCE_NAME].stringValue == "IndexAction") {
-          def tmp = traces[0]
-          traces[0] = traces[1]
-          traces[1] = tmp
-        }
-      }
+    def excludes = {
+      // sometimes PutMappingAction is present and sometimes it is not
+      // (only seems to happen with Elasticsearch 2.x, later versions seem to always have PutMappingAction)
+      it[0].attributes[MoreTags.RESOURCE_NAME].stringValue == "PutMappingAction"
+    }
+    assertTracesWithFilter(2, excludes) {
      trace(0, 1) {
-        span(0) {
-          operationName "elasticsearch.query"
-          tags {
-            "$MoreTags.SERVICE_NAME" "elasticsearch"
-            "$MoreTags.RESOURCE_NAME" "PutMappingAction"
-            "$MoreTags.SPAN_TYPE" SpanTypes.ELASTICSEARCH
-            "$Tags.COMPONENT" "elasticsearch-java"
-            "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
-            "$Tags.DB_TYPE" "elasticsearch"
-            "elasticsearch.action" "PutMappingAction"
-            "elasticsearch.request" "PutMappingRequest"
-            "elasticsearch.request.indices" indexName
-          }
-        }
-      }
-      trace(1, 1) {
        span(0) {
          operationName "elasticsearch.query"
          tags {
@ -111,7 +90,7 @@ class Elasticsearch2SpringRepositoryTest extends AgentTestRunner {
          }
        }
      }
-      trace(2, 1) {
+      trace(1, 1) {
        span(0) {
          operationName "elasticsearch.query"
          tags {
--- a/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/latestDepTest/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy
@ -1,6 +1,5 @@
 package springdata

-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -23,7 +22,6 @@ import spock.lang.Shared

 import java.util.concurrent.atomic.AtomicLong

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2SpringTemplateTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-2/src/test/groovy/Elasticsearch2NodeClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/test/groovy/Elasticsearch2NodeClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -14,7 +13,6 @@ import spock.lang.Shared

 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2NodeClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-2/src/test/groovy/Elasticsearch2TransportClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/test/groovy/Elasticsearch2TransportClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -17,7 +16,6 @@ import spock.lang.Shared

 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2TransportClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-2/src/test/groovy/springdata/Elasticsearch2SpringRepositoryTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/test/groovy/springdata/Elasticsearch2SpringRepositoryTest.groovy
@ -1,6 +1,5 @@
 package springdata

-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -11,7 +10,6 @@ import spock.lang.Shared

 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2SpringRepositoryTest extends AgentTestRunner {
  @Shared
  ApplicationContext applicationContext = new AnnotationConfigApplicationContext(Config)
@ -69,32 +67,13 @@ class Elasticsearch2SpringRepositoryTest extends AgentTestRunner {
    repo.index(doc) == doc

    and:
-    assertTraces(3) {
-      sortTraces {
-        // IndexAction and PutMappingAction run in separate threads and so their order is not always the same
-        if (traces[0][0].attributes[MoreTags.RESOURCE_NAME].stringValue == "IndexAction") {
-          def tmp = traces[0]
-          traces[0] = traces[1]
-          traces[1] = tmp
-        }
-      }
+    def excludes = {
+      // sometimes PutMappingAction is present and sometimes it is not
+      // (only seems to happen with Elasticsearch 2.x, later versions seem to always have PutMappingAction)
+      it[0].attributes[MoreTags.RESOURCE_NAME].stringValue == "PutMappingAction"
+    }
+    assertTracesWithFilter(2, excludes) {
      trace(0, 1) {
-        span(0) {
-          operationName "elasticsearch.query"
-          tags {
-            "$MoreTags.SERVICE_NAME" "elasticsearch"
-            "$MoreTags.RESOURCE_NAME" "PutMappingAction"
-            "$MoreTags.SPAN_TYPE" SpanTypes.ELASTICSEARCH
-            "$Tags.COMPONENT" "elasticsearch-java"
-            "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
-            "$Tags.DB_TYPE" "elasticsearch"
-            "elasticsearch.action" "PutMappingAction"
-            "elasticsearch.request" "PutMappingRequest"
-            "elasticsearch.request.indices" indexName
-          }
-        }
-      }
-      trace(1, 1) {
        span(0) {
          operationName "elasticsearch.query"
          tags {
@ -114,7 +93,7 @@ class Elasticsearch2SpringRepositoryTest extends AgentTestRunner {
          }
        }
      }
-      trace(2, 1) {
+      trace(1, 1) {
        span(0) {
          operationName "elasticsearch.query"
          tags {
--- a/instrumentation/elasticsearch/transport-2/src/test/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy
+++ b/instrumentation/elasticsearch/transport-2/src/test/groovy/springdata/Elasticsearch2SpringTemplateTest.groovy
@ -1,6 +1,5 @@
 package springdata

-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -23,7 +22,6 @@ import spock.lang.Shared

 import java.util.concurrent.atomic.AtomicLong

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch2SpringTemplateTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-5.3/src/test/groovy/Elasticsearch53NodeClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-5.3/src/test/groovy/Elasticsearch53NodeClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -17,7 +16,6 @@ import spock.lang.Shared
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch53NodeClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-5.3/src/test/groovy/Elasticsearch53TransportClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-5.3/src/test/groovy/Elasticsearch53TransportClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -21,7 +20,6 @@ import spock.lang.Shared
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch53TransportClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy
+++ b/instrumentation/elasticsearch/transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringRepositoryTest.groovy
@ -1,6 +1,5 @@
 package springdata

-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -15,7 +14,6 @@ import java.lang.reflect.Proxy

 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch53SpringRepositoryTest extends AgentTestRunner {
  // Setting up appContext & repo with @Shared doesn't allow
  // spring-data instrumentation to applied.
--- a/instrumentation/elasticsearch/transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringTemplateTest.groovy
+++ b/instrumentation/elasticsearch/transport-5.3/src/test/groovy/springdata/Elasticsearch53SpringTemplateTest.groovy
@ -1,6 +1,5 @@
 package springdata

-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import com.google.common.collect.ImmutableSet
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
@ -29,7 +28,6 @@ import java.util.concurrent.atomic.AtomicLong
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch53SpringTemplateTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

@ -150,11 +148,10 @@ class Elasticsearch53SpringTemplateTest extends AgentTestRunner {
    template.queryForList(query, Doc) == [new Doc()]

    and:
-    // FIXME: it looks like proper approach is to provide TEST_WRITER with an API to filter traces as they are written
-    TEST_WRITER.waitForTraces(7)
-    filterIgnoredActions()
-
-    assertTraces(7) {
+    def excludes = {
+      trace -> IGNORED_ACTIONS.contains(trace[0].attributes[MoreTags.RESOURCE_NAME].stringValue)
+    }
+    assertTracesWithFilter(7, excludes) {
      sortTraces {
        // IndexAction and PutMappingAction run in separate threads and so their order is not always the same
        if (traces[3][0].attributes[MoreTags.RESOURCE_NAME].stringValue == "IndexAction") {
@ -372,10 +369,4 @@ class Elasticsearch53SpringTemplateTest extends AgentTestRunner {
    where:
    indexName = "test-index-extract"
  }
-
-  void filterIgnoredActions() {
-    TEST_WRITER.filterTraces({
-      trace -> IGNORED_ACTIONS.contains(trace[0].attributes[MoreTags.RESOURCE_NAME].stringValue)
-    })
-  }
 }
--- a/instrumentation/elasticsearch/transport-5/src/test/groovy/Elasticsearch5NodeClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-5/src/test/groovy/Elasticsearch5NodeClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -17,7 +16,6 @@ import spock.lang.Shared
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch5NodeClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-5/src/test/groovy/Elasticsearch5TransportClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-5/src/test/groovy/Elasticsearch5TransportClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -21,7 +20,6 @@ import spock.lang.Shared
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch5TransportClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-6/src/test/groovy/Elasticsearch6NodeClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-6/src/test/groovy/Elasticsearch6NodeClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -16,7 +15,6 @@ import spock.lang.Shared
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch6NodeClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/elasticsearch/transport-6/src/test/groovy/Elasticsearch6TransportClientTest.groovy
+++ b/instrumentation/elasticsearch/transport-6/src/test/groovy/Elasticsearch6TransportClientTest.groovy
@ -1,4 +1,3 @@
-import com.anotherchrisberry.spock.extensions.retry.RetryOnFailure
 import io.opentelemetry.auto.api.MoreTags
 import io.opentelemetry.auto.api.SpanTypes
 import io.opentelemetry.auto.instrumentation.api.Tags
@ -20,7 +19,6 @@ import spock.lang.Shared
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace
 import static org.elasticsearch.cluster.ClusterName.CLUSTER_NAME_SETTING

-@RetryOnFailure(times = 3, delaySeconds = 1)
 class Elasticsearch6TransportClientTest extends AgentTestRunner {
  public static final long TIMEOUT = 10000; // 10 seconds

--- a/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableChainTest.groovy
+++ b/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableChainTest.groovy
@ -5,14 +5,10 @@ import io.opentelemetry.auto.instrumentation.api.Tags
 import io.opentelemetry.auto.test.AgentTestRunner
 import rx.Observable
 import rx.schedulers.Schedulers
-import spock.lang.Retry
-import spock.lang.Timeout

 import static com.netflix.hystrix.HystrixCommandGroupKey.Factory.asKey
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@Retry
-@Timeout(5)
 class HystrixObservableChainTest extends AgentTestRunner {
  static {
    // Disable so failure testing below doesn't inadvertently change the behavior.
--- a/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy
+++ b/instrumentation/hystrix-1.4/src/test/groovy/HystrixObservableTest.groovy
@ -7,8 +7,6 @@ import io.opentelemetry.auto.instrumentation.api.Tags
 import io.opentelemetry.auto.test.AgentTestRunner
 import rx.Observable
 import rx.schedulers.Schedulers
-import spock.lang.Retry
-import spock.lang.Timeout

 import java.util.concurrent.BlockingQueue
 import java.util.concurrent.LinkedBlockingQueue
@ -16,8 +14,6 @@ import java.util.concurrent.LinkedBlockingQueue
 import static com.netflix.hystrix.HystrixCommandGroupKey.Factory.asKey
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

-@Retry
-@Timeout(5)
 class HystrixObservableTest extends AgentTestRunner {
  static {
    // Disable so failure testing below doesn't inadvertently change the behavior.
--- a/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy
+++ b/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy
@ -6,6 +6,7 @@ import play.libs.concurrent.HttpExecution
 import play.mvc.Results
 import play.routing.RoutingDsl
 import play.server.Server
+import spock.lang.Retry
 import spock.lang.Shared

 import java.util.concurrent.CompletableFuture
@ -18,6 +19,7 @@ import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.QUER
 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS

+@Retry(mode = Retry.Mode.SETUP_FEATURE_CLEANUP)
 class PlayAsyncServerTest extends PlayServerTest {
  @Shared
  def executor = Executors.newCachedThreadPool()
--- a/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy
+++ b/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy
@ -13,6 +13,7 @@ import play.Mode
 import play.mvc.Results
 import play.routing.RoutingDsl
 import play.server.Server
+import spock.lang.Retry

 import java.util.function.Supplier

@ -22,6 +23,7 @@ import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.QUER
 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS

+@Retry(mode = Retry.Mode.SETUP_FEATURE_CLEANUP)
 class PlayServerTest extends HttpServerTest<Server, AkkaHttpServerDecorator> {
  @Override
  Server startServer(int port) {
--- a/instrumentation/rmi/src/test/groovy/RmiTest.groovy
+++ b/instrumentation/rmi/src/test/groovy/RmiTest.groovy
@ -10,7 +10,6 @@ import rmi.app.ServerLegacy
 import java.rmi.registry.LocateRegistry
 import java.rmi.server.UnicastRemoteObject

-import static io.opentelemetry.auto.test.asserts.ListWriterAssert.assertTraces
 import static io.opentelemetry.auto.test.utils.TraceUtils.basicSpan
 import static io.opentelemetry.auto.test.utils.TraceUtils.runUnderTrace

@ -36,7 +35,7 @@ class RmiTest extends AgentTestRunner {

    then:
    response.contains("Hello you")
-    assertTraces(TEST_WRITER, 1) {
+    assertTraces(1) {
      trace(0, 4) {
        basicSpan(it, 0, "parent")
        span(1) {
@ -90,7 +89,7 @@ class RmiTest extends AgentTestRunner {
    server.getClass()

    then:
-    assertTraces(TEST_WRITER, 0) {}
+    assertTraces(0) {}

    cleanup:
    serverRegistry.unbind("Server")
@ -109,7 +108,7 @@ class RmiTest extends AgentTestRunner {

    then:
    def thrownException = thrown(RuntimeException)
-    assertTraces(TEST_WRITER, 1) {
+    assertTraces(1) {
      trace(0, 3) {
        basicSpan(it, 0, "parent", null, null, thrownException)
        span(1) {
@ -157,7 +156,7 @@ class RmiTest extends AgentTestRunner {

    then:
    response.contains("Hello you")
-    assertTraces(TEST_WRITER, 1) {
+    assertTraces(1) {
      trace(0, 3) {
        basicSpan(it, 0, "parent")
        span(1) {
--- a/testing/src/main/groovy/io/opentelemetry/auto/test/AgentTestRunner.java
+++ b/testing/src/main/groovy/io/opentelemetry/auto/test/AgentTestRunner.java
@ -2,6 +2,8 @@ package io.opentelemetry.auto.test;

 import ch.qos.logback.classic.Level;
 import ch.qos.logback.classic.Logger;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.collect.Sets;
 import groovy.lang.Closure;
 import groovy.lang.DelegatesTo;
@ -15,6 +17,7 @@ import io.opentelemetry.auto.tooling.AgentTracerImpl;
 import io.opentelemetry.auto.tooling.Instrumenter;
 import io.opentelemetry.auto.util.test.AgentSpecification;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.sdk.trace.SpanData;
 import io.opentelemetry.trace.Tracer;
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
@ -184,7 +187,19 @@ public abstract class AgentTestRunner extends AgentSpecification {
              options = "io.opentelemetry.auto.test.asserts.ListWriterAssert")
          @DelegatesTo(value = ListWriterAssert.class, strategy = Closure.DELEGATE_FIRST)
          final Closure spec) {
-    ListWriterAssert.assertTraces(TEST_WRITER, size, spec);
+    ListWriterAssert.assertTraces(
+        TEST_WRITER, size, Predicates.<List<SpanData>>alwaysFalse(), spec);
+  }
+
+  public static void assertTracesWithFilter(
+      final int size,
+      final Predicate<List<SpanData>> excludes,
+      @ClosureParams(
+              value = SimpleType.class,
+              options = "io.opentelemetry.auto.test.asserts.ListWriterAssert")
+          @DelegatesTo(value = ListWriterAssert.class, strategy = Closure.DELEGATE_FIRST)
+          final Closure spec) {
+    ListWriterAssert.assertTraces(TEST_WRITER, size, excludes, spec);
  }

  public static class TestRunnerListener implements AgentBuilder.Listener {
--- a/testing/src/main/groovy/io/opentelemetry/auto/test/ListWriter.java
+++ b/testing/src/main/groovy/io/opentelemetry/auto/test/ListWriter.java
@ -1,6 +1,7 @@
 package io.opentelemetry.auto.test;

 import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.TreeTraverser;
@ -16,7 +17,6 @@ import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@ -50,7 +50,9 @@ public class ListWriter implements SpanProcessor {
        sd.getSpanId().toLowerBase16(),
        sd.getTraceId().toLowerBase16(),
        sd.getParentSpanId().toLowerBase16());
-    spanOrders.put(readableSpan.getSpanContext().getSpanId(), nextSpanOrder.getAndIncrement());
+    synchronized (tracesLock) {
+      spanOrders.put(readableSpan.getSpanContext().getSpanId(), nextSpanOrder.getAndIncrement());
+    }
  }

  @Override
@ -64,6 +66,12 @@ public class ListWriter implements SpanProcessor {
        sd.getParentSpanId().toLowerBase16());
    final SpanData span = readableSpan.toSpanData();
    synchronized (tracesLock) {
+      if (!spanOrders.containsKey(span.getSpanId())) {
+        // this happens on some tests where there are sporadic background traces,
+        // e.g. Elasticsearch "RefreshAction"
+        log.debug("span ended that was started prior to ListWriter clear(): {}", span);
+        return;
+      }
      boolean found = false;
      for (final List<SpanData> trace : traces) {
        if (trace.get(0).getTraceId().equals(span.getTraceId())) {
@ -83,16 +91,6 @@ public class ListWriter implements SpanProcessor {
    }
  }

-  public void filterTraces(final Predicate<List<SpanData>> filter) {
-    synchronized (tracesLock) {
-      for (final Iterator<List<SpanData>> i = traces.iterator(); i.hasNext(); ) {
-        if (filter.apply(i.next())) {
-          i.remove();
-        }
-      }
-    }
-  }
-
  public List<List<SpanData>> getTraces() {
    synchronized (tracesLock) {
      // important not to sort trace or span lists in place so that any tests that are currently
@ -112,7 +110,7 @@ public class ListWriter implements SpanProcessor {
      }
      // always return a copy so that future structural changes cannot cause race conditions during
      // test verification
-      final List<List<SpanData>> copy = new ArrayList<>();
+      final List<List<SpanData>> copy = new ArrayList<>(traces.size());
      for (final List<SpanData> trace : traces) {
        copy.add(new ArrayList<>(trace));
      }
@ -121,63 +119,57 @@ public class ListWriter implements SpanProcessor {
  }

  public void waitForTraces(final int number) throws InterruptedException, TimeoutException {
+    waitForTraces(number, Predicates.<List<SpanData>>alwaysFalse());
+  }
+
+  public List<List<SpanData>> waitForTraces(
+      final int number, final Predicate<List<SpanData>> excludes)
+      throws InterruptedException, TimeoutException {
    synchronized (tracesLock) {
      long remainingWaitMillis = TimeUnit.SECONDS.toMillis(20);
-      while (completedTraceCount() < number && remainingWaitMillis > 0) {
+      List<List<SpanData>> traces = getCompletedAndFilteredTraces(excludes);
+      while (traces.size() < number && remainingWaitMillis > 0) {
        final Stopwatch stopwatch = Stopwatch.createStarted();
        tracesLock.wait(remainingWaitMillis);
        remainingWaitMillis -= stopwatch.elapsed(TimeUnit.MILLISECONDS);
+        traces = getCompletedAndFilteredTraces(excludes);
      }
-      final int completedTraceCount = completedTraceCount();
-      if (completedTraceCount < number) {
+      if (traces.size() < number) {
        throw new TimeoutException(
            "Timeout waiting for "
                + number
-                + " completed trace(s), found "
-                + completedTraceCount
-                + " completed trace(s) and "
+                + " completed/filtered trace(s), found "
+                + traces.size()
+                + " completed/filtered trace(s) and "
                + traces.size()
                + " total trace(s): "
                + traces);
      }
+      return traces;
    }
  }

+  private List<List<SpanData>> getCompletedAndFilteredTraces(
+      final Predicate<List<SpanData>> excludes) {
+    final List<List<SpanData>> traces = new ArrayList<>();
+    for (final List<SpanData> trace : getTraces()) {
+      if (isCompleted(trace) && !excludes.apply(trace)) {
+        traces.add(trace);
+      }
+    }
+    return traces;
+  }
+
  public void clear() {
    synchronized (tracesLock) {
      traces.clear();
+      spanOrders.clear();
    }
-    spanOrders.clear();
  }

  @Override
  public void shutdown() {}

-  // must be called under tracesLock
-  private int completedTraceCount() {
-    int count = 0;
-    for (final List<SpanData> trace : traces) {
-      if (isCompleted(trace)) {
-        count++;
-      }
-    }
-    return count;
-  }
-
-  // trace is completed if root span is present
-  private boolean isCompleted(final List<SpanData> trace) {
-    for (final SpanData span : trace) {
-      if (!span.getParentSpanId().isValid()) {
-        return true;
-      }
-      if (span.getParentSpanId().toLowerBase16().equals("0000000000000456")) {
-        // this is a special parent id that some tests use
-        return true;
-      }
-    }
-    return false;
-  }
-
  // must be called under tracesLock
  private void sortTraces() {
    Collections.sort(
@ -264,6 +256,20 @@ public class ListWriter implements SpanProcessor {
    return order;
  }

+  // trace is completed if root span is present
+  private static boolean isCompleted(final List<SpanData> trace) {
+    for (final SpanData span : trace) {
+      if (!span.getParentSpanId().isValid()) {
+        return true;
+      }
+      if (span.getParentSpanId().toLowerBase16().equals("0000000000000456")) {
+        // this is a special parent id that some tests use
+        return true;
+      }
+    }
+    return false;
+  }
+
  private static class Node {

    private final SpanData span;
--- a/testing/src/main/groovy/io/opentelemetry/auto/test/asserts/ListWriterAssert.groovy
+++ b/testing/src/main/groovy/io/opentelemetry/auto/test/asserts/ListWriterAssert.groovy
@ -1,5 +1,6 @@
 package io.opentelemetry.auto.test.asserts

+import com.google.common.base.Predicate
 import groovy.transform.stc.ClosureParams
 import groovy.transform.stc.SimpleType
 import io.opentelemetry.auto.test.ListWriter
@ -23,11 +24,11 @@ class ListWriterAssert {
  }

  static void assertTraces(ListWriter writer, int expectedSize,
+                           final Predicate<List<SpanData>> excludes,
                           @ClosureParams(value = SimpleType, options = ['io.opentelemetry.auto.test.asserts.ListWriterAssert'])
                           @DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST) Closure spec) {
    try {
-      writer.waitForTraces(expectedSize)
-      def traces = new ArrayList<>(writer.traces)
+      def traces = writer.waitForTraces(expectedSize, excludes)
      assert traces.size() == expectedSize
      def asserter = new ListWriterAssert(traces, writer)
      def clone = (Closure) spec.clone()
--- a/testing/src/test/groovy/server/ServerTest.groovy
+++ b/testing/src/test/groovy/server/ServerTest.groovy
@ -1,8 +1,6 @@
 package server

-
 import io.opentelemetry.auto.test.AgentTestRunner
-import io.opentelemetry.auto.test.asserts.ListWriterAssert
 import io.opentelemetry.auto.test.utils.OkHttpUtils
 import okhttp3.MultipartBody
 import okhttp3.Request
@ -312,7 +310,7 @@ class ServerTest extends AgentTestRunner {
    response.code() == 200
    response.body().string().trim() == "done"

-    ListWriterAssert.assertTraces(TEST_WRITER, 1) {
+    assertTraces(1) {
      server.distributedRequestTrace(it, 0)
    }