Move serviceNameMappings processing to DDSpanContext

dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java

@@ -72,6 +72,8 @@ public class DDSpanContext implements io.opentracing.SpanContext {
   private final String threadName = Thread.currentThread().getName();
   private final long threadId = Thread.currentThread().getId();
 
+  private final Map<String, String> serviceNameMappings;
+
   public DDSpanContext(
       final BigInteger traceId,
       final BigInteger spanId,
@@ -86,7 +88,8 @@ public DDSpanContext(
       final String spanType,
       final Map<String, Object> tags,
       final PendingTrace trace,
-      final DDTracer tracer) {
+      final DDTracer tracer,
+      final Map<String, String> serviceNameMappings) {
 
     assert tracer != null;
     assert trace != null;
@@ -110,7 +113,8 @@ public DDSpanContext(
       this.tags.putAll(tags);
     }
 
-    this.serviceName = serviceName;
+    this.serviceNameMappings = serviceNameMappings;
+    setServiceName(serviceName);
     this.operationName = operationName;
     this.resourceName = resourceName;
     this.errorFlag = errorFlag;
@@ -155,7 +159,11 @@ public String getServiceName() {
   }
 
   public void setServiceName(final String serviceName) {
-    this.serviceName = serviceName;
+    if (serviceNameMappings.containsKey(serviceName)) {
+      this.serviceName = serviceNameMappings.get(serviceName);
+    } else {
+      this.serviceName = serviceName;
+    }
   }
 
   public String getResourceName() {
@@ -292,6 +300,7 @@ public PendingTrace getTrace() {
     return trace;
   }
 
+  @Deprecated
   public DDTracer getTracer() {
     return tracer;
   }

dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java

@@ -775,7 +775,7 @@ private DDSpanContext buildSpanContext() {
 
         tags.putAll(localRootSpanTags);
 
-        parentTrace = new PendingTrace(DDTracer.this, traceId, serviceNameMappings);
+        parentTrace = new PendingTrace(DDTracer.this, traceId);
       }
 
       if (serviceName == null) {
@@ -800,7 +800,8 @@ private DDSpanContext buildSpanContext() {
               spanType,
               tags,
               parentTrace,
-              DDTracer.this);
+              DDTracer.this,
+              serviceNameMappings);
 
       // Apply Decorators to handle any tags that may have been set via the builder.
       for (final Map.Entry<String, Object> tag : tags.entrySet()) {

dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java

@@ -12,7 +12,6 @@
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedDeque;
@@ -28,7 +27,6 @@ public class PendingTrace extends ConcurrentLinkedDeque<DDSpan> {
 
   private final DDTracer tracer;
   private final BigInteger traceId;
-  private final Map<String, String> serviceNameMappings;
 
   // TODO: consider moving these time fields into DDTracer to ensure that traces have precise
   // relative time
@@ -60,13 +58,9 @@ public class PendingTrace extends ConcurrentLinkedDeque<DDSpan> {
   /** Ensure a trace is never written multiple times */
   private final AtomicBoolean isWritten = new AtomicBoolean(false);
 
-  PendingTrace(
-      final DDTracer tracer,
-      final BigInteger traceId,
-      final Map<String, String> serviceNameMappings) {
+  PendingTrace(final DDTracer tracer, final BigInteger traceId) {
     this.tracer = tracer;
     this.traceId = traceId;
-    this.serviceNameMappings = serviceNameMappings;
 
     startTimeNano = Clock.currentNanoTime();
     startNanoTicks = Clock.currentNanoTicks();
@@ -150,10 +144,6 @@ public void addSpan(final DDSpan span) {
     }
 
     if (!isWritten.get()) {
-      if (serviceNameMappings.containsKey(span.getServiceName())) {
-        span.setServiceName(serviceNameMappings.get(span.getServiceName()));
-      }
-
       addFirst(span);
     } else {
       log.debug("{} - finished after trace reported.", span);

dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy

@@ -156,7 +156,7 @@ class DDSpanBuilderTest extends DDSpecification {
     1 * mockedContext.getSpanId() >> spanId
     _ * mockedContext.getServiceName() >> "foo"
     1 * mockedContext.getBaggageItems() >> [:]
-    1 * mockedContext.getTrace() >> new PendingTrace(tracer, 1G, [:])
+    1 * mockedContext.getTrace() >> new PendingTrace(tracer, 1G)
 
     final String expectedName = "fakeName"
 

dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanSerializationTest.groovy

@@ -60,8 +60,9 @@ class DDSpanSerializationTest extends DDSpecification {
         false,
         spanType,
         ["k1": "v1"],
-        new PendingTrace(tracer, 1G, [:]),
-        tracer)
+        new PendingTrace(tracer, 1G),
+        tracer,
+        [:])
 
     DDSpan span = new DDSpan(100L, context)
 
@@ -95,8 +96,9 @@ class DDSpanSerializationTest extends DDSpecification {
       false,
       spanType,
       Collections.emptyMap(),
-      new PendingTrace(tracer, 1G, [:]),
-      tracer)
+      new PendingTrace(tracer, 1G),
+      tracer,
+      [:])
     def span = new DDSpan(0, context)
     def buffer = new ArrayBufferOutput()
     def packer = MessagePack.newDefaultPacker(buffer)

dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanTest.groovy

@@ -35,8 +35,9 @@ class DDSpanTest extends DDSpecification {
         false,
         "fakeType",
         null,
-        new PendingTrace(tracer, 1G, [:]),
-        tracer)
+        new PendingTrace(tracer, 1G),
+        tracer,
+        [:])
 
     final DDSpan span = new DDSpan(1L, context)
 

dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy

@@ -28,7 +28,7 @@ class PendingTraceTest extends DDSpecification {
   BigInteger traceId = BigInteger.valueOf(System.identityHashCode(this))
 
   @Subject
-  PendingTrace trace = new PendingTrace(tracer, traceId, [:])
+  PendingTrace trace = new PendingTrace(tracer, traceId)
 
   DDSpan rootSpan = SpanFactory.newSpanOf(trace)
 
@@ -147,7 +147,7 @@ class PendingTraceTest extends DDSpecification {
 
   def "register span to wrong trace fails"() {
     setup:
-    def otherTrace = new PendingTrace(tracer, traceId - 10, [:])
+    def otherTrace = new PendingTrace(tracer, traceId - 10)
     otherTrace.registerSpan(new DDSpan(0, rootSpan.context()))
 
     expect:
@@ -158,7 +158,7 @@ class PendingTraceTest extends DDSpecification {
 
   def "add span to wrong trace fails"() {
     setup:
-    def otherTrace = new PendingTrace(tracer, traceId - 10, [:])
+    def otherTrace = new PendingTrace(tracer, traceId - 10)
     rootSpan.finish()
     otherTrace.addSpan(rootSpan)
 
@@ -196,7 +196,7 @@ class PendingTraceTest extends DDSpecification {
     properties.setProperty(PARTIAL_FLUSH_MIN_SPANS, "1")
     def config = Config.get(properties)
     def tracer = DDTracer.builder().config(config).writer(writer).build()
-    def trace = new PendingTrace(tracer, traceId, [:])
+    def trace = new PendingTrace(tracer, traceId)
     def rootSpan = SpanFactory.newSpanOf(trace)
     def child1 = tracer.buildSpan("child1").asChildOf(rootSpan).start()
     def child2 = tracer.buildSpan("child2").asChildOf(rootSpan).start()
@@ -242,7 +242,7 @@ class PendingTraceTest extends DDSpecification {
     properties.setProperty(PARTIAL_FLUSH_MIN_SPANS, "1")
     def config = Config.get(properties)
     def tracer = DDTracer.builder().config(config).writer(writer).build()
-    def trace = new PendingTrace(tracer, traceId, [:])
+    def trace = new PendingTrace(tracer, traceId)
     def rootSpan = SpanFactory.newSpanOf(trace)
     def child1 = tracer.buildSpan("child1").asChildOf(rootSpan).start()
     def child2 = tracer.buildSpan("child2").asChildOf(rootSpan).start()

dd-trace-ot/src/test/groovy/datadog/opentracing/SpanFactory.groovy

@@ -24,8 +24,8 @@ class SpanFactory {
       false,
       "fakeType",
       Collections.emptyMap(),
-      new PendingTrace(tracer, 1G, [:]),
-      tracer)
+      new PendingTrace(tracer, 1G),
+      tracer, [:])
     Thread.currentThread().setName(currentThreadName)
     return new DDSpan(timestampMicro, context)
   }
@@ -44,8 +44,8 @@ class SpanFactory {
       false,
       "fakeType",
       Collections.emptyMap(),
-      new PendingTrace(tracer, 1G, [:]),
-      tracer)
+      new PendingTrace(tracer, 1G),
+      tracer, [:])
     return new DDSpan(1, context)
   }
 
@@ -64,7 +64,7 @@ class SpanFactory {
       "fakeType",
       Collections.emptyMap(),
       trace,
-      trace.tracer)
+      trace.tracer, [:])
     return new DDSpan(1, context)
   }
 
@@ -84,8 +84,9 @@ class SpanFactory {
       false,
       "fakeType",
       Collections.emptyMap(),
-      new PendingTrace(tracer, 1G, [:]),
-      tracer)
+      new PendingTrace(tracer, 1G),
+      tracer,
+      [:])
     context.setTag("env", envName)
     return new DDSpan(0l, context)
   }

dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/SpanDecoratorTest.groovy

@@ -127,7 +127,7 @@ class SpanDecoratorTest extends DDSpecification {
     "other-context" | "my-service"         | "my-service"
   }
 
-  def "set service name from servlet.context with context '#context' for service #serviceName"() {
+  def "mapping causes servlet.context to not change service name"() {
     setup:
     tracer = DDTracer.builder()
       .serviceName(serviceName)
@@ -142,18 +142,12 @@ class SpanDecoratorTest extends DDSpecification {
     span.finish()
 
     then:
-    span.serviceName == expected
+    span.serviceName == "new-service"
 
     where:
-    context         | serviceName          | expected
-    "/"             | DEFAULT_SERVICE_NAME | "new-service"
-    ""              | DEFAULT_SERVICE_NAME | "new-service"
-    "/some-context" | DEFAULT_SERVICE_NAME | "some-context"
-    "other-context" | DEFAULT_SERVICE_NAME | "other-context"
-    "/"             | "my-service"         | "new-service"
-    ""              | "my-service"         | "new-service"
-    "/some-context" | "my-service"         | "new-service"
-    "other-context" | "my-service"         | "new-service"
+    context         | serviceName
+    "/some-context" | DEFAULT_SERVICE_NAME
+    "/some-context" | "my-service"
 
     mapping = [(serviceName): "new-service"]
   }

dd-trace-ot/src/test/groovy/datadog/opentracing/decorators/URLAsResourceNameTest.groovy

@@ -115,12 +115,13 @@ class URLAsResourceNameTest extends DDSpecification {
         "fakeResource",
         PrioritySampling.UNSET,
         null,
-        Collections.<String, String> emptyMap(),
+        [:],
         false,
         "fakeType",
         tags,
-        new PendingTrace(tracer, 1G, [:]),
-        tracer)
+        new PendingTrace(tracer, 1G),
+        tracer,
+        [:])
 
     then:
     decorator.shouldSetTag(context, Tags.HTTP_URL.getKey(), value)

dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/B3HttpInjectorTest.groovy

@@ -40,8 +40,9 @@ class B3HttpInjectorTest extends DDSpecification {
         false,
         "fakeType",
         null,
-        new PendingTrace(tracer, 1G, [:]),
-        tracer)
+        new PendingTrace(tracer, 1G),
+        tracer,
+        [:])
 
     final Map<String, String> carrier = Mock()
 

dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/DatadogHttpInjectorTest.groovy

@@ -42,8 +42,9 @@ class DatadogHttpInjectorTest extends DDSpecification {
         false,
         "fakeType",
         null,
-        new PendingTrace(tracer, 1G, [:]),
-        tracer)
+        new PendingTrace(tracer, 1G),
+        tracer,
+        [:])
 
     final Map<String, String> carrier = Mock()
 

dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HaystackHttpInjectorTest.groovy

@@ -40,8 +40,9 @@ class HaystackHttpInjectorTest extends DDSpecification {
         false,
         "fakeType",
         null,
-        new PendingTrace(tracer, 1G, [:]),
-        tracer)
+        new PendingTrace(tracer, 1G),
+        tracer,
+        [:])
 
     final Map<String, String> carrier = Mock()
 

dd-trace-ot/src/test/groovy/datadog/opentracing/propagation/HttpInjectorTest.groovy

@@ -45,8 +45,9 @@ class HttpInjectorTest extends DDSpecification {
         false,
         "fakeType",
         null,
-        new PendingTrace(tracer, 1G, [:]),
-        tracer)
+        new PendingTrace(tracer, 1G),
+        tracer,
+        [:])
 
     final Map<String, String> carrier = Mock()
 

dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy

@@ -201,12 +201,13 @@ class DDAgentWriterTest extends DDSpecification {
       "",
       PrioritySampling.UNSET,
       "",
-      Collections.emptyMap(),
+      [:],
       false,
       "",
-      Collections.emptyMap(),
+      [:],
       Mock(PendingTrace),
-      Mock(DDTracer))
+      Mock(DDTracer),
+      [:])
     minimalSpan = new DDSpan(0, minimalContext)
     minimalTrace = [minimalSpan]
     traceSize = calculateSize(minimalTrace)
@@ -262,12 +263,13 @@ class DDAgentWriterTest extends DDSpecification {
       "",
       PrioritySampling.UNSET,
       "",
-      Collections.emptyMap(),
+      [:],
       false,
       "",
-      Collections.emptyMap(),
+      [:],
       Mock(PendingTrace),
-      Mock(DDTracer))
+      Mock(DDTracer),
+      [:])
     def minimalSpan = new DDSpan(0, minimalContext)
     def minimalTrace = [minimalSpan]
 

dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy

@@ -32,12 +32,13 @@ class DDApiIntegrationTest {
       "fakeResource",
       PrioritySampling.UNSET,
       null,
-      Collections.emptyMap(),
+      [:],
       false,
       "fakeType",
-      Collections.emptyMap(),
-      new PendingTrace(TRACER, 1G, [:]),
-      TRACER)
+      [:],
+      new PendingTrace(TRACER, 1G),
+      TRACER,
+      [:])
 
     // Looks like okHttp needs to resolve this, even for connection over socket
     static final SOMEHOST = "datadoghq.com"