Set Stackdriver Span displayName to unknown is Zipkin Span name is em…

…pty (#144)
openzipkin · Sep 11, 2019 · 4688ae7 · 4688ae7
1 parent c164b9c
commit 4688ae7
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 3 deletions.
diff --git a/sender-stackdriver/src/test/java/zipkin2/reporter/stackdriver/ITStackdriverSender.java b/sender-stackdriver/src/test/java/zipkin2/reporter/stackdriver/ITStackdriverSender.java
@@ -143,6 +143,46 @@ public void sendSpans() {
     assertThat(trace.getSpans(0).getParentSpanId()).isEqualTo(1);
   }
 
+  @Test
+  public void sendSpanEmptyName() {
+    Random random = new Random();
+    Span span = Span.newBuilder()
+            .traceId(random.nextLong(), random.nextLong())
+            .parentId("1")
+            .id("2")
+            .kind(Span.Kind.CLIENT)
+            .localEndpoint(FRONTEND)
+            .remoteEndpoint(BACKEND)
+            .timestamp((TODAY + 50L) * 1000L)
+            .duration(200000L)
+            .addAnnotation((TODAY + 100L) * 1000L, "foo")
+            .putTag("http.path", "/api")
+            .putTag("clnt/finagle.version", "6.45.0")
+            .build();
+
+    reporter.report(span);
+    reporter.flush();
+
+    Trace trace = await()
+            .atLeast(Duration.ONE_SECOND)
+            .atMost(Duration.TEN_SECONDS)
+            .pollInterval(Duration.ONE_SECOND)
+            .ignoreExceptionsMatching(e ->
+                    e instanceof StatusRuntimeException &&
+                            ((StatusRuntimeException) e).getStatus().getCode() == Status.Code.NOT_FOUND
+            )
+            .until(() -> traceServiceGrpcV1.getTrace(GetTraceRequest.newBuilder()
+                    .setProjectId(projectId)
+                    .setTraceId(span.traceId())
+                    .build()), t -> t.getSpansCount() == 1);
+
+    // In Stackdriver Trace v2 API, Zipkin Span "name" is sent as Stackdriver Span "displayName"
+    // However, in Stackdriver Trace v1 API, to read this back, it's Stackdriver TraceSpan's "name"
+    assertThat(trace.getSpans(0).getName()).isEqualTo("unknown");
+    assertThat(trace.getSpans(0).getSpanId()).isEqualTo(2);
+    assertThat(trace.getSpans(0).getParentSpanId()).isEqualTo(1);
+  }
+
   @Test
   public void healthcheckFailNoPermission() {
     CheckResult result = reporterNoPermission.check();

diff --git a/translation-stackdriver/src/main/java/zipkin2/translation/stackdriver/SpanTranslator.java b/translation-stackdriver/src/main/java/zipkin2/translation/stackdriver/SpanTranslator.java
@@ -99,7 +99,7 @@ public static com.google.devtools.cloudtrace.v2.Span.Builder translate(
     // NOTE: opencensus prefixes Send. and Recv. based on Kind. For now we reproduce our V1 behavior
     // of using the span name as the display name as is.
     spanBuilder.setDisplayName(
-        toTruncatableString(zipkinSpan.name() != null ? zipkinSpan.name() : ""));
+        toTruncatableString((zipkinSpan.name() != null && !zipkinSpan.name().isEmpty()) ? zipkinSpan.name() : "unknown"));
 
     if (zipkinSpan.timestampAsLong() != 0L) {
       spanBuilder.setStartTime(createTimestamp(zipkinSpan.timestampAsLong()));

diff --git a/...slation-stackdriver/src/test/java/zipkin2/translation/stackdriver/SpanTranslatorTest.java b/...slation-stackdriver/src/test/java/zipkin2/translation/stackdriver/SpanTranslatorTest.java
@@ -21,6 +21,7 @@
 import zipkin2.Endpoint;
 import zipkin2.Span;
 
+import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static zipkin2.translation.stackdriver.AttributesExtractor.toAttributeValue;
 import static zipkin2.translation.stackdriver.SpanTranslator.createTimestamp;
@@ -87,7 +88,7 @@ public void translate_missingName() {
     com.google.devtools.cloudtrace.v2.Span translated = SpanTranslator.translate(
         com.google.devtools.cloudtrace.v2.Span.newBuilder(), zipkinSpan).build();
 
-    assertThat(translated.getDisplayName().getValue()).isEmpty();
+    assertThat(translated.getDisplayName().getValue()).isNotEmpty();
   }
 
   @Test
@@ -99,7 +100,7 @@ public void testTranslateSpans() {
     Span span3 =
         Span.newBuilder().id("3").traceId("1").name("/c").timestamp(3L).duration(1L).build();
 
-    List<Span> spans = Arrays.asList(span1, span2, span3);
+    List<Span> spans = asList(span1, span2, span3);
     List<com.google.devtools.cloudtrace.v2.Span> stackdriverSpans =
         new ArrayList<>(SpanTranslator.translate("test-project", spans));
 
@@ -110,4 +111,23 @@ public void testTranslateSpans() {
             "projects/test-project/traces/00000000000000000000000000000002/spans/0000000000000002",
             "projects/test-project/traces/00000000000000000000000000000001/spans/0000000000000003");
   }
+
+  @Test
+  public void testTranslateSpanEmptyName() {
+    Span spanNullName =
+        Span.newBuilder().id("1").traceId("1").timestamp(1L).duration(1L).build();
+    Span spanEmptyName =
+        Span.newBuilder().id("2").traceId("2").name("").timestamp(2L).duration(1L).build();
+    Span spanNonEmptyName =
+        Span.newBuilder().id("2").traceId("2").name("somename").timestamp(2L).duration(1L).build();
+
+    List<Span> spans = asList(spanNullName, spanEmptyName, spanNonEmptyName);
+    List<com.google.devtools.cloudtrace.v2.Span> stackdriverSpans =
+            new ArrayList<>(SpanTranslator.translate("test-project", spans));
+
+    assertThat(stackdriverSpans).hasSize(3);
+    assertThat(stackdriverSpans.get(0).getDisplayName().getValue()).isEqualTo("unknown");
+    assertThat(stackdriverSpans.get(1).getDisplayName().getValue()).isEqualTo("unknown");
+    assertThat(stackdriverSpans.get(2).getDisplayName().getValue()).isEqualTo("somename");
+  }
 }