review

Author: amarziali

dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java

@@ -765,7 +765,7 @@ void setAllTags(final Map<String, ?> map) {
     }
   }
 
-  public void unsafeSetTag(final String tag, final Object value) {
+  void unsafeSetTag(final String tag, final Object value) {
     unsafeTags.put(tag, value);
   }
 

internal-api/build.gradle

@@ -122,9 +122,6 @@ excludedClassesCoverage += [
   "datadog.trace.api.civisibility.CiVisibilityWellKnownTags",
   "datadog.trace.api.civisibility.InstrumentationBridge",
   "datadog.trace.api.civisibility.InstrumentationTestBridge",
-  "datadog.trace.api.ClassloaderConfigurationOverrides",
-  "datadog.trace.api.ClassloaderConfigurationOverrides.Lazy",
-  "datadog.trace.api.ClassloaderConfigurationOverrides.ContextualInfo",
   // POJO
   "datadog.trace.api.git.GitInfo",
   "datadog.trace.api.git.GitInfoProvider",

internal-api/src/main/java/datadog/trace/api/ClassloaderConfigurationOverrides.java

@@ -15,7 +15,8 @@
 public class ClassloaderConfigurationOverrides {
   public static final String DATADOG_TAGS_PREFIX = "datadog/tags/";
   public static final String DATADOG_TAGS_JNDI_PREFIX = "java:comp/env/" + DATADOG_TAGS_PREFIX;
-  private static final boolean CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT =
+  // not final for testing purposes
+  static boolean CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT =
       Config.get().isJeeSplitByDeployment() && !Config.get().isServiceNameSetByUser();
 
   private static class Lazy {
@@ -44,7 +45,7 @@ public Map<String, Object> getTags() {
     }
   }
 
-  private static Function<ClassLoader, ContextualInfo> EMPTY_CONTEXTUAL_INFO_ADDER =
+  private static final Function<ClassLoader, ContextualInfo> EMPTY_CONTEXTUAL_INFO_ADDER =
       ignored -> new ContextualInfo(null);
 
   private final WeakHashMap<ClassLoader, ContextualInfo> weakCache = new WeakHashMap<>();

internal-api/src/test/groovy/datadog/trace/api/ClassloaderConfigurationOverridesTest.groovy

@@ -0,0 +1,53 @@
+package datadog.trace.api
+
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan
+import datadog.trace.test.util.DDSpecification
+import spock.lang.Unroll
+
+class ClassloaderConfigurationOverridesTest extends DDSpecification {
+
+  @Unroll
+  def 'service name set = #expected when split-by-deployment enabled is #splitByDeploymentEnabled and service name manually set = #spanServiceName and contextual service name is #contextualServiceName'() {
+    setup:
+    injectSysConfig("dd.service", "test")
+    def span = Mock(AgentSpan)
+    when:
+    ClassloaderConfigurationOverrides.CAN_SPLIT_SERVICE_NAME_BY_DEPLOYMENT = splitByDeploymentEnabled
+    if (splitByDeploymentEnabled) {
+      injectSysConfig("dd.trace.experimental.jee.split-by-deployment", "true")
+    }
+    ClassloaderConfigurationOverrides.addContextualInfo(Thread.currentThread().getContextClassLoader(),
+      new ClassloaderConfigurationOverrides.ContextualInfo(contextualServiceName))
+    ClassloaderConfigurationOverrides.maybeEnrichSpan(span)
+    then:
+    if (splitByDeploymentEnabled && contextualServiceName != null && !contextualServiceName.isEmpty()) {
+      (1.._) * span.getServiceName()
+    }
+    if (expected) {
+      1 * span.setServiceName(contextualServiceName)
+    }
+
+    where:
+    splitByDeploymentEnabled | contextualServiceName | spanServiceName | expected
+    false                    | null                  | null            | false
+    false                    | "test"                | null            | false
+    false                    | null                  | "test"          | false
+    true                     | null                  | null            | false
+    true                     | ""                    | null            | false
+    true                     | "cont"                | null            | true
+    true                     | ""                    | "test"          | false
+    true                     | null                  | "test"          | false
+    true                     | "test"                | "test"          | false
+    true                     | "cont"                | "test"          | true
+  }
+
+  def "enrich should set tags when present"() {
+    def span = Mock(AgentSpan)
+    when:
+    ClassloaderConfigurationOverrides.getOrAddEmpty(Thread.currentThread().getContextClassLoader()).addTag("key", "value")
+    ClassloaderConfigurationOverrides.maybeEnrichSpan(span)
+    then:
+    1 * span.setTag("key", "value")
+    _ * span.getServiceName()
+  }
+}