From fed0559389ca903b9d5600316f6b62c5b7fb3434 Mon Sep 17 00:00:00 2001 From: eocantu Date: Wed, 3 Mar 2021 17:06:20 -0600 Subject: [PATCH 1/2] Skip setting error tag when it's false --- .../agent/pitchfork/processors/HaystackDomainConverter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java b/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java index c9e7113..63a6b10 100644 --- a/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java +++ b/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java @@ -143,7 +143,7 @@ private static Optional getTagForKind(zipkin2.Span.Kind kind) { } private static List fromZipkinTag(String key, String value) { - if ("error".equalsIgnoreCase(key)) { + if ("error".equalsIgnoreCase(key) && !"false".equalsIgnoreCase(value)) { // 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() From 92493fe458281ab4f774440c1874f25e24232075 Mon Sep 17 00:00:00 2001 From: eocantu Date: Wed, 3 Mar 2021 23:50:21 -0600 Subject: [PATCH 2/2] Set error tag to false when tag value is false --- .../processors/HaystackDomainConverter.java | 18 ++++++++++--- .../HaystackDomainConverterSpec.scala | 26 +++++++++++++++++-- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java b/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java index 5fab0e9..988542e 100644 --- a/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java +++ b/agent-providers/span/src/main/java/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverter.java @@ -153,22 +153,32 @@ private static Optional getTagForKind(zipkin2.Span.Kind kind) { } private static List fromZipkinTag(String key, String value) { - if ("error".equalsIgnoreCase(key) && !"false".equalsIgnoreCase(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() diff --git a/agent-providers/span/src/test/scala/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverterSpec.scala b/agent-providers/span/src/test/scala/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverterSpec.scala index bd40232..e295303 100644 --- a/agent-providers/span/src/test/scala/com/expedia/www/haystack/agent/pitchfork/processors/HaystackDomainConverterSpec.scala +++ b/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") {