From 4688ae7d5a44da0fcec16bcc72653cd32409e224 Mon Sep 17 00:00:00 2001 From: Ray Tsang Date: Tue, 10 Sep 2019 23:03:14 -0700 Subject: [PATCH] Set Stackdriver Span displayName to unknown is Zipkin Span name is empty (#144) --- .../stackdriver/ITStackdriverSender.java | 40 +++++++++++++++++++ .../stackdriver/SpanTranslator.java | 2 +- .../stackdriver/SpanTranslatorTest.java | 24 ++++++++++- 3 files changed, 63 insertions(+), 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 index 4a03cda9..0dc9d56d 100644 --- 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 index 095a72b3..3ee79c6a 100644 --- 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/translation-stackdriver/src/test/java/zipkin2/translation/stackdriver/SpanTranslatorTest.java b/translation-stackdriver/src/test/java/zipkin2/translation/stackdriver/SpanTranslatorTest.java index 3412eed0..d26d4a94 100644 --- a/translation-stackdriver/src/test/java/zipkin2/translation/stackdriver/SpanTranslatorTest.java +++ b/translation-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 spans = Arrays.asList(span1, span2, span3); + List spans = asList(span1, span2, span3); List 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 spans = asList(spanNullName, spanEmptyName, spanNonEmptyName); + List 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"); + } }