Merge pull request #96 from eocantu/duplicate-span-kind-tags

Prevent duplicate span.kind tags
ExpediaDotCom · Mar 4, 2021 · 0ae2a9e · 0ae2a9e
2 parents 0cc41fb + 01bf00c
commit 0ae2a9e
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 3 deletions.
diff --git a/...ain/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java b/...ain/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java
@@ -36,6 +36,8 @@
 @SuppressWarnings("PMD.UnusedPrivateMethod")
 class HaystackDomainConverter {
 
+    private static final String SPAN_KIND_TAG_KEY = "span.kind";
+
     private HaystackDomainConverter() { }
     /**
      * Accepts a span in {@code Zipkin V2} format and returns a span in {@code Haystack} format.
@@ -53,7 +55,9 @@ static Span fromZipkinV2(final zipkin2.Span zipkin) {
 
         builder.addAllTags(addRemoteEndpointAsTags(zipkin.remoteEndpoint()));
 
-        getTagForKind(zipkin.kind()).ifPresent(builder::addTags);
+        if (!spanKindTagPresent(zipkin)) {
+            getTagForKind(zipkin.kind()).ifPresent(builder::addTags);
+        }
 
         if (zipkin.tags() != null && !zipkin.tags().isEmpty()) {
             zipkin.tags().forEach((key, value) -> {
@@ -111,6 +115,12 @@ private static <T> void doIfNotNull(T nullable, Consumer<T> runnable) {
         }
     }
 
+    private static boolean spanKindTagPresent(zipkin2.Span zipkinSpan) {
+        return zipkinSpan.tags() != null &&
+            !zipkinSpan.tags().isEmpty() &&
+            zipkinSpan.tags().keySet().contains(SPAN_KIND_TAG_KEY);
+    }
+
     private static Optional<Tag> getTagForKind(zipkin2.Span.Kind kind) {
         String value;
 
@@ -133,7 +143,7 @@ private static Optional<Tag> getTagForKind(zipkin2.Span.Kind kind) {
             }
 
             return Optional.of(Tag.newBuilder()
-                    .setKey("span.kind")
+                    .setKey(SPAN_KIND_TAG_KEY)
                     .setVStr(value)
                     .setType(Tag.TagType.STRING)
                     .build());
@@ -169,4 +179,4 @@ private static List<Tag> fromZipkinTag(String key, String value) {
 
         return Collections.singletonList(tag);
     }
-}
+}
diff --git a/...ala/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverterSpec.scala b/...ala/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverterSpec.scala
@@ -0,0 +1,54 @@
+package com.expedia.www.haystack.agent.pitchfork.processors
+
+import org.scalatest.{FunSpec, Matchers}
+import org.scalatest.easymock.EasyMockSugar
+import zipkin2.{Endpoint, Span}
+
+class HaystackDomainConverterSpec extends FunSpec with Matchers with EasyMockSugar {
+
+  private def zipkinSpanBuilder(traceId: String): Span.Builder = {
+    zipkin2.Span.newBuilder()
+      .traceId(traceId)
+      .id(1)
+      .parentId(2)
+      .name("/foo")
+      .localEndpoint(Endpoint.newBuilder().serviceName("foo").build())
+      .remoteEndpoint(Endpoint.newBuilder().serviceName("bar").port(8080).ip("10.10.10.10").build())
+      .timestamp(System.currentTimeMillis() * 1000)
+      .duration(100000l)
+      .putTag("error", "true")
+      .putTag("pos", "1")
+  }
+
+  describe("Haystack Domain Converter") {
+    it("should create span from Zipking span") {
+      val traceId = "bd1068b1bc333ec0"
+      val zipkinSpan = zipkinSpanBuilder(traceId).clearTags().build()
+      val span = HaystackDomainConverter.fromZipkinV2(zipkinSpan)
+
+      span.getTraceId shouldBe traceId
+    }
+
+    it("should create span with kind tag") {
+      val traceId = "edcb04102634b702"
+      val zipkinSpan = zipkinSpanBuilder(traceId)
+        .kind(Span.Kind.SERVER)
+        .clearTags()
+        .build()
+      val span = HaystackDomainConverter.fromZipkinV2(zipkinSpan)
+
+      span.getTraceId shouldBe traceId
+      span.getTagsList.stream().filter(_.getKey == "span.kind").count() shouldBe 1
+    }
+
+    it("should create span without duplicate kind tag") {
+      val traceId = "661e251d4406e110"
+      val zipkinSpan = zipkinSpanBuilder(traceId).kind(Span.Kind.SERVER).putTag("span.kind", "server").build()
+      val span = HaystackDomainConverter.fromZipkinV2(zipkinSpan)
+
+      span.getTraceId shouldBe traceId
+      span.getTagsList.stream().filter(_.getKey == "span.kind").count() shouldBe 1
+    }
+  }
+
+}