Merge pull request #97 from eocantu/skip-error-false

Set error tag to false when string value is false

Author: mmenindezsegura

agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java

@@ -156,19 +156,29 @@ private static List<Tag> fromZipkinTag(String key, String value) {
         if ("error".equalsIgnoreCase(key)) {
             // Zipkin error tags are Strings where as in Haystack they're Booleans
             // Since a Zipkin error tag may contain relevant information about the error we expand it into two tags (error + error message)
-            Tag errorTag = Tag.newBuilder()
+            if (!"false".equalsIgnoreCase(value)) {
+                Tag errorTag = Tag.newBuilder()
                     .setKey(key)
                     .setVBool(true)
                     .setType(Tag.TagType.BOOL)
                     .build();
 
-            Tag errorMessageTag = Tag.newBuilder()
+                Tag errorMessageTag = Tag.newBuilder()
                     .setKey("error_msg")
                     .setVStr(value)
                     .setType(Tag.TagType.STRING)
                     .build();
 
-            return Arrays.asList(errorTag, errorMessageTag);
+                return Arrays.asList(errorTag, errorMessageTag);
+            } else {
+                final Tag errorTag = Tag.newBuilder()
+                    .setKey(key)
+                    .setVBool(false)
+                    .setType(Tag.TagType.BOOL)
+                    .build();
+
+                return Collections.singletonList(errorTag);
+            }
         }
 
         final Tag tag = Tag.newBuilder()

agent-providers/span/src/test/scala/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverterSpec.scala

@@ -1,5 +1,6 @@
 package com.expedia.www.haystack.agent.pitchfork.processors
 
+import com.expedia.open.tracing.Tag
 import org.scalatest.{FunSpec, Matchers}
 import org.scalatest.easymock.EasyMockSugar
 import zipkin2.{Endpoint, Span}
@@ -16,17 +17,38 @@ class HaystackDomainConverterSpec extends FunSpec with Matchers with EasyMockSug
       .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") {
+    it("should create span from Zipkin span") {
       val traceId = "bd1068b1bc333ec0"
       val zipkinSpan = zipkinSpanBuilder(traceId).clearTags().build()
       val span = HaystackDomainConverter.fromZipkinV2(zipkinSpan)
 
       span.getTraceId shouldBe traceId
+      span.getTagsList.stream().filter(_.getKey == "error").count() shouldBe 0
+    }
+
+    it("should create span from Zipkin span with error false") {
+      val traceId = "bd1068b1bc333ec0"
+      val zipkinSpan = zipkinSpanBuilder(traceId).putTag("error", "false").build()
+      val span = HaystackDomainConverter.fromZipkinV2(zipkinSpan)
+
+      span.getTraceId shouldBe traceId
+      span.getTagsList.stream().filter(_.getKey == "error").count() shouldBe 1
+      span.getTagsList.stream().filter(tag => tag.getKey == "error" && tag.getType == Tag.TagType.BOOL && !tag.getVBool).count() shouldBe 1
+    }
+
+    it("should create span from Zipkin span with error true") {
+      val traceId = "bd1068b1bc333ec0"
+      val zipkinSpan = zipkinSpanBuilder(traceId).putTag("error", "bad things").build()
+      val span = HaystackDomainConverter.fromZipkinV2(zipkinSpan)
+
+      span.getTraceId shouldBe traceId
+      span.getTagsList.stream().filter(_.getKey == "error").count() shouldBe 1
+      span.getTagsList.stream().filter(tag => tag.getKey == "error" && tag.getType == Tag.TagType.BOOL && tag.getVBool).count() shouldBe 1
+      span.getTagsList.stream().filter(_.getKey == "error_msg").count() shouldBe 1
     }
 
     it("should create span with kind tag") {