Skip to content

Commit a9e67d0

Browse files
authored
Merge pull request #186 from ExpediaDotCom/network-latency-improvements
Add missing infrastructure tags for all spans from a service, if foun…
2 parents 94cd7ff + 2ac937b commit a9e67d0

File tree

6 files changed

+138
-3
lines changed

6 files changed

+138
-3
lines changed

reader/src/main/resources/config/base.conf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ trace {
6969
sequence = [
7070
"com.expedia.www.haystack.trace.reader.readers.transformers.DeDuplicateSpanTransformer"
7171
"com.expedia.www.haystack.trace.reader.readers.transformers.ClientServerEventLogTransformer"
72+
"com.expedia.www.haystack.trace.reader.readers.transformers.InfrastructureTagTransformer"
7273
]
7374
}
7475
post {

reader/src/main/scala/com/expedia/www/haystack/trace/reader/readers/transformers/DeDuplicateSpanTransformer.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
1+
/*
2+
* Copyright 2017 Expedia, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
117
package com.expedia.www.haystack.trace.reader.readers.transformers
218

319
import com.expedia.open.tracing.Span
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright 2017 Expedia, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.expedia.www.haystack.trace.reader.readers.transformers
18+
19+
import com.expedia.open.tracing.{Span, Tag}
20+
import com.expedia.www.haystack.trace.reader.readers.utils.AuxiliaryTags
21+
22+
import scala.collection.JavaConverters._
23+
import scala.collection.mutable
24+
25+
/**
26+
* add the infrastructure tag in all the spans coming for a service, if any of its span(client or server) contains it.
27+
* Many services send the infrastructure tag only in the server span for ease and saving transfer cost.
28+
* This transformer refill the infrastructure tags if required.
29+
*/
30+
class InfrastructureTagTransformer extends TraceTransformer {
31+
32+
override def transform(spans: Seq[Span]): Seq[Span] = {
33+
val serviceWithInfraTags = mutable.HashMap[String, mutable.ListBuffer[Tag]]()
34+
val spansWithoutInfraTags = mutable.HashSet[Span]()
35+
36+
spans.foreach { span =>
37+
var infraTagsPresent = false
38+
span.getTagsList.asScala.foreach { tag =>
39+
if (tag.getKey == AuxiliaryTags.INFRASTRUCTURE_PROVIDER || tag.getKey == AuxiliaryTags.INFRASTRUCTURE_LOCATION) {
40+
val tags = serviceWithInfraTags.getOrElseUpdate(span.getServiceName, mutable.ListBuffer[Tag]())
41+
tags.append(tag)
42+
infraTagsPresent = true
43+
}
44+
}
45+
46+
if (!infraTagsPresent) {
47+
spansWithoutInfraTags += span
48+
}
49+
}
50+
51+
spans.map { span =>
52+
if (serviceWithInfraTags.contains(span.getServiceName) && spansWithoutInfraTags.contains(span)) {
53+
span.toBuilder.addAllTags(serviceWithInfraTags(span.getServiceName).asJava).build()
54+
} else {
55+
span
56+
}
57+
}
58+
}
59+
}

reader/src/test/resources/config/base.conf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ trace {
7878
sequence = [
7979
"com.expedia.www.haystack.trace.reader.readers.transformers.DeDuplicateSpanTransformer"
8080
"com.expedia.www.haystack.trace.reader.readers.transformers.ClientServerEventLogTransformer"
81+
"com.expedia.www.haystack.trace.reader.readers.transformers.InfrastructureTagTransformer"
8182
]
8283
}
8384
post {

reader/src/test/scala/com/expedia/www/haystack/trace/reader/unit/config/ConfigurationLoaderSpec.scala

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package com.expedia.www.haystack.trace.reader.unit.config
1818
import com.expedia.www.haystack.trace.commons.config.entities.KeyspaceConfiguration
1919
import com.expedia.www.haystack.trace.reader.config.ProviderConfiguration
2020
import com.expedia.www.haystack.trace.reader.config.entities.{ServiceConfiguration, TraceTransformersConfiguration}
21-
import com.expedia.www.haystack.trace.reader.readers.transformers.{ClientServerEventLogTransformer, DeDuplicateSpanTransformer, PartialSpanTransformer}
21+
import com.expedia.www.haystack.trace.reader.readers.transformers.{ClientServerEventLogTransformer, DeDuplicateSpanTransformer, InfrastructureTagTransformer, PartialSpanTransformer}
2222
import com.expedia.www.haystack.trace.reader.unit.BaseUnitTestSpec
2323

2424
class ConfigurationLoaderSpec extends BaseUnitTestSpec {
@@ -35,18 +35,20 @@ class ConfigurationLoaderSpec extends BaseUnitTestSpec {
3535
val traceConfig: TraceTransformersConfiguration = new ProviderConfiguration().traceTransformerConfig
3636
traceConfig.postTransformers.length shouldBe 3
3737
traceConfig.postTransformers.head.isInstanceOf[PartialSpanTransformer] shouldBe true
38-
traceConfig.preTransformers.length shouldBe 2
38+
traceConfig.preTransformers.length shouldBe 3
3939
traceConfig.preTransformers.head.isInstanceOf[DeDuplicateSpanTransformer] shouldBe true
4040
traceConfig.preTransformers(1).isInstanceOf[ClientServerEventLogTransformer] shouldBe true
41+
traceConfig.preTransformers(2).isInstanceOf[InfrastructureTagTransformer] shouldBe true
4142
}
4243

4344
it("should load the trace validators") {
4445
val traceConfig: TraceTransformersConfiguration = new ProviderConfiguration().traceTransformerConfig
4546
traceConfig.postTransformers.length shouldBe 3
4647
traceConfig.postTransformers.head.isInstanceOf[PartialSpanTransformer] shouldBe true
47-
traceConfig.preTransformers.length shouldBe 2
48+
traceConfig.preTransformers.length shouldBe 3
4849
traceConfig.preTransformers.head.isInstanceOf[DeDuplicateSpanTransformer] shouldBe true
4950
traceConfig.preTransformers(1).isInstanceOf[ClientServerEventLogTransformer] shouldBe true
51+
traceConfig.preTransformers(2).isInstanceOf[InfrastructureTagTransformer] shouldBe true
5052
}
5153

5254
it("should load service metadata configuration ") {
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package com.expedia.www.haystack.trace.reader.unit.readers.transformers
2+
3+
import com.expedia.open.tracing.{Span, Tag}
4+
import com.expedia.www.haystack.trace.reader.readers.transformers.InfrastructureTagTransformer
5+
import com.expedia.www.haystack.trace.reader.readers.utils.AuxiliaryTags
6+
import com.expedia.www.haystack.trace.reader.unit.BaseUnitTestSpec
7+
8+
import scala.collection.JavaConverters._
9+
10+
class InfrastructureTagTransformerSpec extends BaseUnitTestSpec {
11+
12+
private val infraTags = Seq(
13+
Tag.newBuilder().setKey(AuxiliaryTags.INFRASTRUCTURE_PROVIDER).setVStr("aws").setType(Tag.TagType.STRING).build(),
14+
Tag.newBuilder().setKey(AuxiliaryTags.INFRASTRUCTURE_LOCATION).setVStr("us-west-2").setType(Tag.TagType.STRING).build()
15+
)
16+
17+
private val randomTags = Seq(Tag.newBuilder().setKey("error").setVBool(false).setType(Tag.TagType.BOOL).build())
18+
19+
20+
describe("infrastructure tag transformer") {
21+
it("should add missing infrastructure tags in a service spans if any of it contains so") {
22+
// first tag in the sequence contains infrastructure tags, following span doesn't contain from the same service doesn't contain.
23+
val svc1_span1 = Span.newBuilder().setTraceId("traceId").setSpanId("span_1").setServiceName("service_1").addAllTags(randomTags.asJava).addAllTags(infraTags.asJava).build()
24+
val svc1_span2 = Span.newBuilder().setTraceId("traceId").setSpanId("span_2").setParentSpanId("span_1").setServiceName("service_1").build()
25+
26+
// none of the tags from this service contains infrastructure information
27+
val svc2_span_1 = Span.newBuilder().setTraceId("traceId").setSpanId("span_3").setServiceName("service_2").addAllTags(randomTags.asJava).build()
28+
val svc2_span_2 = Span.newBuilder().setTraceId("traceId").setSpanId("span_4").setParentSpanId("span_3").setServiceName("service_2").build()
29+
30+
31+
// first tag in the sequence doesn't contain infrastructure tags, following span from the same service contains.
32+
val svc3_span1 = Span.newBuilder().setTraceId("traceId").setSpanId("span_5").setServiceName("service_3").build()
33+
val svc3_span2 = Span.newBuilder().setTraceId("traceId").setSpanId("span_6").setParentSpanId("span_5").setServiceName("service_3").addAllTags(randomTags.asJava).addAllTags(infraTags.asJava).build()
34+
35+
val transformedSpans = new InfrastructureTagTransformer()
36+
.transform(Seq(svc1_span1, svc1_span2, svc2_span_1, svc2_span_2, svc3_span1, svc3_span2))
37+
38+
transformedSpans.size shouldBe 6
39+
transformedSpans.find(_.getSpanId == "span_1").get.getTagsList should contain allElementsOf infraTags
40+
transformedSpans.find(_.getSpanId == "span_1").get.getTagsList should contain allElementsOf randomTags
41+
42+
transformedSpans.find(_.getSpanId == "span_2").get.getTagsCount shouldBe infraTags.size
43+
transformedSpans.find(_.getSpanId == "span_2").get.getTagsList should contain allElementsOf infraTags
44+
45+
transformedSpans.find(_.getSpanId == "span_3").get.getTagsCount shouldBe 1
46+
transformedSpans.find(_.getSpanId == "span_3").get.getTagsList should contain allElementsOf randomTags
47+
transformedSpans.find(_.getSpanId == "span_4").get.getTagsCount shouldBe 0
48+
49+
transformedSpans.find(_.getSpanId == "span_5").get.getTagsCount shouldBe infraTags.size
50+
transformedSpans.find(_.getSpanId == "span_5").get.getTagsList should contain allElementsOf infraTags
51+
transformedSpans.find(_.getSpanId == "span_6").get.getTagsList should contain allElementsOf infraTags
52+
transformedSpans.find(_.getSpanId == "span_6").get.getTagsList should contain allElementsOf randomTags
53+
54+
}
55+
}
56+
}

0 commit comments

Comments
 (0)