From ec27930d263ab236be19f0a9e222128385f5f789 Mon Sep 17 00:00:00 2001
From: dougqh <dougqh@gmail.com>
Date: Tue, 3 Dec 2019 16:59:36 -0500
Subject: [PATCH] Ordering inconistency with DDSpanBuilder.withTag

Pior to this change, DDSpanBuilder.withTag behaves different than DDSpan.setTag when decorators are triggered.

Specifically builder.withTag(a, val).withTag(b, val) can behave differently than span.setTag(a, val); span.setTag(b, val)

This change makes a small step towards determinism by changing the HashMap inside DDSpanBuilder into a LinkedHashMap.  That guarantees the tags from withTags translated to the same sequence of setTag calls.

NOTE: Even with this change, there are inconsistencies when tags are removed or set to an empty value.
---
 .../java/datadog/opentracing/DDTracer.java    |  3 +-
 .../decorators/SpanDecoratorTest.groovy       | 75 +++++++++++++++++++
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java
index 4255393a91..85908feb28 100644
--- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java
+++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java
@@ -34,6 +34,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -489,7 +490,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace
     private final String operationName;
 
     // Builder attributes
-    private final Map<String, Object> tags = new HashMap<String, Object>(defaultSpanTags);
+    private final Map<String, Object> tags = new LinkedHashMap<String, Object>(defaultSpanTags);
     private long timestampMicro;
     private SpanContext parent;
     private String serviceName;
diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy
index fbe40b791b..64bffaeed7 100644
--- a/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy
+++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy
@@ -171,6 +171,81 @@ class SpanDecoratorTest extends DDSpecification {
     mapping = [(serviceName): "new-service"]
   }
 
+  static createSplittingTracer(tag) {
+    def tracer = new DDTracer(
+      "my-service",
+      new LoggingWriter(),
+      new AllSampler(),
+      "some-runtime-id",
+      emptyMap(),
+      emptyMap(),
+      emptyMap(),
+      emptyMap()
+    )
+    // equivalent to split-by-tags: tag
+    tracer.addDecorator(new ServiceNameDecorator(tag, true))
+
+    return tracer
+  }
+
+  def "peer.service then split-by-tags via builder"() {
+    setup:
+    tracer = createSplittingTracer(Tags.MESSAGE_BUS_DESTINATION.key)
+
+    when:
+    def span = tracer.buildSpan("some span")
+      .withTag(Tags.PEER_SERVICE.key, "peer-service")
+      .withTag(Tags.MESSAGE_BUS_DESTINATION.key, "some-queue")
+      .start()
+    span.finish()
+
+    then:
+    span.serviceName == "some-queue"
+  }
+
+  def "peer.service then split-by-tags via setTag"() {
+    setup:
+    tracer = createSplittingTracer(Tags.MESSAGE_BUS_DESTINATION.key)
+
+    when:
+    def span = tracer.buildSpan("some span").start()
+    span.setTag(Tags.PEER_SERVICE.key, "peer-service")
+    span.setTag(Tags.MESSAGE_BUS_DESTINATION.key, "some-queue")
+    span.finish()
+
+    then:
+    span.serviceName == "some-queue"
+  }
+
+  def "split-by-tags then peer-service via builder"() {
+    setup:
+    tracer = createSplittingTracer(Tags.MESSAGE_BUS_DESTINATION.key)
+
+    when:
+    def span = tracer.buildSpan("some span")
+      .withTag(Tags.MESSAGE_BUS_DESTINATION.key, "some-queue")
+      .withTag(Tags.PEER_SERVICE.key, "peer-service")
+      .start()
+    span.finish()
+
+    then:
+    span.serviceName == "peer-service"
+  }
+
+  def "split-by-tags then peer-service via setTag"() {
+    setup:
+    tracer = createSplittingTracer(Tags.MESSAGE_BUS_DESTINATION.key)
+
+    when:
+    def span = tracer.buildSpan("some span").start()
+    span.setTag(Tags.MESSAGE_BUS_DESTINATION.key, "some-queue")
+    span.setTag(Tags.PEER_SERVICE.key, "peer-service")
+    span.finish()
+
+    then:
+    span.serviceName == "peer-service"
+  }
+
   def "set operation name"() {
     when:
     Tags.COMPONENT.set(span, component)