sort spans by start time (parents before children as tiebreaker) to a…

…void common causes for flaky tests
open-telemetry · Jan 5, 2023 · d0be87e · d0be87e
1 parent b091536
commit d0be87e
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 15 deletions.
diff --git a/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/TraceUtil.java b/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/TraceUtil.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.sdk.testing.assertj;
+
+import io.opentelemetry.api.trace.SpanContext;
+import io.opentelemetry.sdk.trace.data.SpanData;
+import java.util.Comparator;
+
+public class TraceUtil {
+
+  /** Compare spans by start time, placing parents before their children as a tiebreaker. */
+  static final Comparator<SpanData> SPAN_DATA_COMPARATOR =
+      Comparator.comparing(SpanData::getStartEpochNanos)
+          .thenComparing(
+              (span1, span2) -> {
+                SpanContext parent1 = span1.getParentSpanContext();
+                if (parent1.isValid() && parent1.getSpanId().equals(span2.getSpanId())) {
+                  return 1;
+                }
+                SpanContext parent2 = span2.getParentSpanContext();
+                if (parent2.isValid() && parent2.getSpanId().equals(span1.getSpanId())) {
+                  return -1;
+                }
+                return 0;
+              });
+
+  private TraceUtil() {}
+}
diff --git a/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/TracesAssert.java b/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/TracesAssert.java
@@ -5,14 +5,18 @@
 
 package io.opentelemetry.sdk.testing.assertj;
 
+import static io.opentelemetry.sdk.testing.assertj.TraceUtil.SPAN_DATA_COMPARATOR;
 import static java.util.stream.Collectors.toList;
 
 import io.opentelemetry.sdk.trace.data.SpanData;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 import org.assertj.core.api.AbstractIterableAssert;
 
@@ -21,6 +25,24 @@ public final class TracesAssert
     extends AbstractIterableAssert<
         TracesAssert, List<List<SpanData>>, List<SpanData>, TraceAssert> {
 
+  /**
+   * Returns an assertion for a list of traces. The provided spans will be grouped into traces by
+   * their trace ID.
+   */
+  public static TracesAssert assertThat(List<SpanData> spanData) {
+    Map<String, List<SpanData>> traces =
+        spanData.stream()
+            .collect(
+                Collectors.groupingBy(
+                    SpanData::getTraceId,
+                    LinkedHashMap::new,
+                    Collectors.toCollection(ArrayList::new)));
+    for (List<SpanData> trace : traces.values()) {
+      trace.sort(SPAN_DATA_COMPARATOR);
+    }
+    return assertThat(traces.values());
+  }
+
   /**
    * Returns an assertion for a list of traces. The traces must already be grouped into {@code
    * List<SpanData>} where each list has spans with the same trace ID.

diff --git a/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtension.java b/sdk/testing/src/main/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtension.java
@@ -23,11 +23,7 @@
 import io.opentelemetry.sdk.trace.data.SpanData;
 import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
 import java.util.ArrayList;
-import java.util.Comparator;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
 import org.junit.jupiter.api.extension.AfterAllCallback;
 import org.junit.jupiter.api.extension.BeforeAllCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
@@ -124,15 +120,7 @@ public List<MetricData> getMetrics() {
    * requires AssertJ to be on the classpath.
    */
   public TracesAssert assertTraces() {
-    Map<String, List<SpanData>> traces =
-        getSpans().stream()
-            .collect(
-                Collectors.groupingBy(
-                    SpanData::getTraceId, LinkedHashMap::new, Collectors.toList()));
-    for (List<SpanData> trace : traces.values()) {
-      trace.sort(Comparator.comparing(SpanData::getStartEpochNanos));
-    }
-    return assertThat(traces.values());
+    return assertThat(spanExporter.getFinishedSpanItems());
   }
 
   /**

diff --git a/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/TraceUtilTest.java b/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/TraceUtilTest.java
@@ -0,0 +1,75 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.sdk.testing.assertj;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.opentelemetry.api.trace.SpanContext;
+import io.opentelemetry.api.trace.SpanId;
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.api.trace.TraceFlags;
+import io.opentelemetry.api.trace.TraceId;
+import io.opentelemetry.api.trace.TraceState;
+import io.opentelemetry.sdk.testing.trace.TestSpanData;
+import io.opentelemetry.sdk.trace.data.StatusData;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+
+class TraceUtilTest {
+  @Test
+  void spanDataComparator() {
+    TestSpanData before = createBasicSpanBuilder().setStartEpochNanos(9).build();
+
+    String traceId = TraceId.fromLongs(1, 2);
+    TestSpanData parent =
+        createBasicSpanBuilder()
+            .setName("parent")
+            .setSpanContext(
+                SpanContext.create(
+                    traceId, SpanId.fromLong(1), TraceFlags.getDefault(), TraceState.getDefault()))
+            .setStartEpochNanos(10)
+            .build();
+    TestSpanData child =
+        createBasicSpanBuilder()
+            .setName("child")
+            .setSpanContext(
+                SpanContext.create(
+                    traceId, SpanId.fromLong(2), TraceFlags.getDefault(), TraceState.getDefault()))
+            .setStartEpochNanos(10)
+            .setParentSpanContext(parent.getSpanContext())
+            .build();
+
+    TestSpanData sameTime1 =
+        createBasicSpanBuilder().setName("sameTime1").setStartEpochNanos(11).build();
+    TestSpanData sameTime2 =
+        createBasicSpanBuilder().setName("sameTime2").setStartEpochNanos(11).build();
+
+    assertSort(Arrays.asList(child, parent, before), before, parent, child);
+    assertSort(Arrays.asList(parent, child, before), before, parent, child);
+
+    assertSort(Arrays.asList(sameTime1, sameTime2, before), before, sameTime1, sameTime2);
+    assertSort(Arrays.asList(sameTime2, sameTime1, before), before, sameTime2, sameTime1);
+  }
+
+  private static void assertSort(List<TestSpanData> spanData, TestSpanData... expected) {
+    ArrayList<TestSpanData> list = new ArrayList<>(spanData);
+    list.sort(TraceUtil.SPAN_DATA_COMPARATOR);
+    assertThat(list).containsExactly(expected);
+  }
+
+  private static TestSpanData.Builder createBasicSpanBuilder() {
+    return TestSpanData.builder()
+        .setHasEnded(true)
+        .setName("spanName")
+        .setEndEpochNanos(100)
+        .setKind(SpanKind.SERVER)
+        .setStatus(StatusData.ok())
+        .setTotalRecordedEvents(0)
+        .setTotalRecordedLinks(0);
+  }
+}
diff --git a/...testing/src/test/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtensionTest.java b/...testing/src/test/java/io/opentelemetry/sdk/testing/junit5/OpenTelemetryExtensionTest.java
@@ -17,6 +17,7 @@
 import io.opentelemetry.sdk.trace.data.SpanData;
 import java.util.Arrays;
 import java.util.LinkedHashMap;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -58,9 +59,9 @@ public void getSpansAgain() {
 
   @Test
   public void assertTraces() {
-    Span span = tracer.spanBuilder("testa1").startSpan();
+    Span span = tracer.spanBuilder("testa1").setStartTimestamp(1000, TimeUnit.SECONDS).startSpan();
     try (Scope ignored = span.makeCurrent()) {
-      tracer.spanBuilder("testa2").startSpan().end();
+      tracer.spanBuilder("testa2").setStartTimestamp(1000, TimeUnit.SECONDS).startSpan().end();
     } finally {
       span.end();
     }