Lack of timestamp in server-only spans implies shared (openzipkin#1690)

adriancole · web-flow · commit 9a8e7d568bfa · 2017-08-13T17:45:28.000+08:00
When "sr" is present without a timestamp, we assume this is a shared span. Moving this signal to the converter allows stackdriver-zipkin to reuse this when converting to their single-host types. See openzipkin/openzipkin.github.io#49
diff --git a/zipkin/src/main/java/zipkin/internal/Span2Converter.java b/zipkin/src/main/java/zipkin/internal/Span2Converter.java
@@ -131,6 +131,12 @@ void processAnnotations(Span source) {
         }
       }
 
+      // Span v1 format did not have a shared flag. By convention, span.timestamp being absent
+      // implied shared. When we only see the server-side, carry this signal over.
+      if (cs == null && (sr != null && source.timestamp == null)) {
+        forEndpoint(source, sr.endpoint).shared(true);
+      }
+
       // ms and mr are not supposed to be in the same span, but in case they are..
       if (ms != null && mr != null) {
         // special-case loopback: We need to make sure on loopback there are two span2s
diff --git a/zipkin/src/test/java/zipkin/internal/Span2ConverterTest.java b/zipkin/src/test/java/zipkin/internal/Span2ConverterTest.java
@@ -361,6 +361,37 @@ public class Span2ConverterTest {
       .containsExactly(client, server);
   }
 
+  /**
+   * The old span format had no means of saying it is shared or not. This uses lack of timestamp as
+   * a signal
+   */
+  @Test public void assumesServerWithoutTimestampIsShared() {
+    Span span = Span.builder()
+      .traceIdHigh(Util.lowerHexToUnsignedLong("7180c278b62e8f6a"))
+      .traceId(Util.lowerHexToUnsignedLong("216a2aea45d08fc9"))
+      .parentId(Util.lowerHexToUnsignedLong("6b221d5bc9e6496c"))
+      .id(Util.lowerHexToUnsignedLong("5b4185666d50f68b"))
+      .name("get")
+      .addAnnotation(Annotation.create(1472470996250000L, Constants.SERVER_RECV, backend))
+      .addAnnotation(Annotation.create(1472470996350000L, Constants.SERVER_SEND, backend))
+      .build();
+
+    Span2 span2 = Span2.builder()
+      .traceId("7180c278b62e8f6a216a2aea45d08fc9")
+      .parentId("6b221d5bc9e6496c")
+      .id("5b4185666d50f68b")
+      .name("get")
+      .kind(Kind.SERVER)
+      .shared(true)
+      .localEndpoint(backend)
+      .timestamp(1472470996250000L)
+      .duration(100000L)
+      .build();
+
+    assertThat(Span2Converter.fromSpan(span))
+      .containsExactly(span2);
+  }
+
   @Test public void clientAndServer_loopback() {
     Span shared = Span.builder()
       .traceIdHigh(Util.lowerHexToUnsignedLong("7180c278b62e8f6a"))

Original file line number	Diff line number	Diff line change
`@@ -131,6 +131,12 @@ void processAnnotations(Span source) {`
`131`	`131`	`}`
`132`	`132`	`}`
`133`	`133`
	`134`	`+ // Span v1 format did not have a shared flag. By convention, span.timestamp being absent`
	`135`	`+ // implied shared. When we only see the server-side, carry this signal over.`
	`136`	`+ if (cs == null && (sr != null && source.timestamp == null)) {`
	`137`	`+ forEndpoint(source, sr.endpoint).shared(true);`
	`138`	`+ }`
	`139`	`+`
`134`	`140`	`// ms and mr are not supposed to be in the same span, but in case they are..`
`135`	`141`	`if (ms != null && mr != null) {`
`136`	`142`	`// special-case loopback: We need to make sure on loopback there are two span2s`