Merge pull request #881 from DataDog/tyler/instance-name

Add `dd.trace.db.client.split-by-instance` Config

Author: tylerbenson

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/DatabaseClientDecorator.java

@@ -1,5 +1,7 @@
 package datadog.trace.agent.decorator;
 
+import datadog.trace.api.Config;
+import datadog.trace.api.DDTags;
 import io.opentracing.Span;
 import io.opentracing.tag.Tags;
 
@@ -29,7 +31,11 @@ public Span onConnection(final Span span, final CONNECTION connection) {
     assert span != null;
     if (connection != null) {
       Tags.DB_USER.set(span, dbUser(connection));
-      Tags.DB_INSTANCE.set(span, dbInstance(connection));
+      final String instanceName = dbInstance(connection);
+      Tags.DB_INSTANCE.set(span, instanceName);
+      if (instanceName != null && Config.get().isDbClientSplitByInstance()) {
+        span.setTag(DDTags.SERVICE_NAME, instanceName);
+      }
     }
     return span;
   }

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/DatabaseClientDecoratorTest.groovy

@@ -1,9 +1,12 @@
 package datadog.trace.agent.decorator
 
+import datadog.trace.api.Config
 import datadog.trace.api.DDTags
 import io.opentracing.Span
 import io.opentracing.tag.Tags
 
+import static datadog.trace.agent.test.utils.ConfigUtils.withConfigOverride
+
 class DatabaseClientDecoratorTest extends ClientDecoratorTest {
 
   def span = Mock(Span)
@@ -35,21 +38,26 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest {
     def decorator = newDecorator()
 
     when:
-    decorator.onConnection(span, session)
+    withConfigOverride(Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "$renameService") {
+      decorator.onConnection(span, session)
+    }
 
     then:
     if (session) {
       1 * span.setTag(Tags.DB_USER.key, session.user)
       1 * span.setTag(Tags.DB_INSTANCE.key, session.instance)
+      if (renameService && session.instance) {
+        1 * span.setTag(DDTags.SERVICE_NAME, session.instance)
+      }
     }
     0 * _
 
     where:
-    session                                        | _
-    null                                           | _
-    [user: "test-user"]                            | _
-    [instance: "test-instance"]                    | _
-    [user: "test-user", instance: "test-instance"] | _
+    renameService | session
+    false         | null
+    true          | [user: "test-user"]
+    false         | [instance: "test-instance"]
+    true          | [user: "test-user", instance: "test-instance"]
   }
 
   def "test onStatement"() {

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/HttpClientDecoratorTest.groovy

@@ -26,12 +26,12 @@ class HttpClientDecoratorTest extends ClientDecoratorTest {
 
     then:
     if (req) {
-      1 * span.setTag(Tags.HTTP_METHOD.key, "test-method")
-      1 * span.setTag(Tags.HTTP_URL.key, "$testUrl")
-      1 * span.setTag(Tags.PEER_HOSTNAME.key, "test-host")
-      1 * span.setTag(Tags.PEER_PORT.key, 555)
+      1 * span.setTag(Tags.HTTP_METHOD.key, req.method)
+      1 * span.setTag(Tags.HTTP_URL.key, "$req.url")
+      1 * span.setTag(Tags.PEER_HOSTNAME.key, req.host)
+      1 * span.setTag(Tags.PEER_PORT.key, req.port)
       if (renameService) {
-        1 * span.setTag(DDTags.SERVICE_NAME, "test-host")
+        1 * span.setTag(DDTags.SERVICE_NAME, req.host)
       }
     }
     0 * _

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/OrmClientDecoratorTest.groovy

@@ -1,12 +1,9 @@
 package datadog.trace.agent.decorator
 
 import datadog.trace.api.DDTags
-import io.opentracing.Span
 
 class OrmClientDecoratorTest extends DatabaseClientDecoratorTest {
 
-  def span = Mock(Span)
-
   def "test onOperation #testName"() {
     setup:
     decorator = newDecorator({ e -> entityName })

dd-trace-api/src/main/java/datadog/trace/api/Config.java

@@ -63,6 +63,7 @@ public class Config {
   public static final String HTTP_SERVER_TAG_QUERY_STRING = "http.server.tag.query-string";
   public static final String HTTP_CLIENT_TAG_QUERY_STRING = "http.client.tag.query-string";
   public static final String HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN = "trace.http.client.split-by-domain";
+  public static final String DB_CLIENT_HOST_SPLIT_BY_INSTANCE = "trace.db.client.split-by-instance";
   public static final String PARTIAL_FLUSH_MIN_SPANS = "trace.partial.flush.min.spans";
   public static final String RUNTIME_CONTEXT_FIELD_INJECTION =
       "trace.runtime.context.field.injection";
@@ -107,6 +108,7 @@ public class Config {
   private static final boolean DEFAULT_HTTP_SERVER_TAG_QUERY_STRING = false;
   private static final boolean DEFAULT_HTTP_CLIENT_TAG_QUERY_STRING = false;
   private static final boolean DEFAULT_HTTP_CLIENT_SPLIT_BY_DOMAIN = false;
+  private static final boolean DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE = false;
   private static final int DEFAULT_PARTIAL_FLUSH_MIN_SPANS = 1000;
   private static final String DEFAULT_PROPAGATION_STYLE_EXTRACT = PropagationStyle.DATADOG.name();
   private static final String DEFAULT_PROPAGATION_STYLE_INJECT = PropagationStyle.DATADOG.name();
@@ -160,6 +162,7 @@ public enum PropagationStyle {
   @Getter private final boolean httpServerTagQueryString;
   @Getter private final boolean httpClientTagQueryString;
   @Getter private final boolean httpClientSplitByDomain;
+  @Getter private final boolean dbClientSplitByInstance;
   @Getter private final Integer partialFlushMinSpans;
   @Getter private final boolean runtimeContextFieldInjection;
   @Getter private final Set<PropagationStyle> propagationStylesToExtract;
@@ -238,6 +241,10 @@ public enum PropagationStyle {
         getBooleanSettingFromEnvironment(
             HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, DEFAULT_HTTP_CLIENT_SPLIT_BY_DOMAIN);
 
+    dbClientSplitByInstance =
+        getBooleanSettingFromEnvironment(
+            DB_CLIENT_HOST_SPLIT_BY_INSTANCE, DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE);
+
     partialFlushMinSpans =
         getIntegerSettingFromEnvironment(PARTIAL_FLUSH_MIN_SPANS, DEFAULT_PARTIAL_FLUSH_MIN_SPANS);
 
@@ -342,6 +349,10 @@ private Config(final Properties properties, final Config parent) {
         getPropertyBooleanValue(
             properties, HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, parent.httpClientSplitByDomain);
 
+    dbClientSplitByInstance =
+        getPropertyBooleanValue(
+            properties, DB_CLIENT_HOST_SPLIT_BY_INSTANCE, parent.dbClientSplitByInstance);
+
     partialFlushMinSpans =
         getPropertyIntegerValue(properties, PARTIAL_FLUSH_MIN_SPANS, parent.partialFlushMinSpans);
 

dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy

@@ -8,6 +8,7 @@ import spock.lang.Specification
 import static datadog.trace.api.Config.AGENT_HOST
 import static datadog.trace.api.Config.AGENT_PORT_LEGACY
 import static datadog.trace.api.Config.AGENT_UNIX_DOMAIN_SOCKET
+import static datadog.trace.api.Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE
 import static datadog.trace.api.Config.DEFAULT_JMX_FETCH_STATSD_PORT
 import static datadog.trace.api.Config.GLOBAL_TAGS
 import static datadog.trace.api.Config.HEADER_TAGS
@@ -79,6 +80,7 @@ class ConfigTest extends Specification {
     config.httpServerErrorStatuses == (500..599).toSet()
     config.httpClientErrorStatuses == (400..499).toSet()
     config.httpClientSplitByDomain == false
+    config.dbClientSplitByInstance == false
     config.partialFlushMinSpans == 1000
     config.reportHostName == false
     config.runtimeContextFieldInjection == true
@@ -120,6 +122,7 @@ class ConfigTest extends Specification {
     prop.setProperty(HTTP_SERVER_ERROR_STATUSES, "123-456,457,124-125,122")
     prop.setProperty(HTTP_CLIENT_ERROR_STATUSES, "111")
     prop.setProperty(HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true")
+    prop.setProperty(DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true")
     prop.setProperty(PARTIAL_FLUSH_MIN_SPANS, "15")
     prop.setProperty(TRACE_REPORT_HOSTNAME, "true")
     prop.setProperty(RUNTIME_CONTEXT_FIELD_INJECTION, "false")
@@ -151,6 +154,7 @@ class ConfigTest extends Specification {
     config.httpServerErrorStatuses == (122..457).toSet()
     config.httpClientErrorStatuses == (111..111).toSet()
     config.httpClientSplitByDomain == true
+    config.dbClientSplitByInstance == true
     config.partialFlushMinSpans == 15
     config.reportHostName == true
     config.runtimeContextFieldInjection == false
@@ -183,6 +187,7 @@ class ConfigTest extends Specification {
     System.setProperty(PREFIX + HTTP_SERVER_ERROR_STATUSES, "123-456,457,124-125,122")
     System.setProperty(PREFIX + HTTP_CLIENT_ERROR_STATUSES, "111")
     System.setProperty(PREFIX + HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true")
+    System.setProperty(PREFIX + DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true")
     System.setProperty(PREFIX + PARTIAL_FLUSH_MIN_SPANS, "25")
     System.setProperty(PREFIX + TRACE_REPORT_HOSTNAME, "true")
     System.setProperty(PREFIX + RUNTIME_CONTEXT_FIELD_INJECTION, "false")
@@ -214,6 +219,7 @@ class ConfigTest extends Specification {
     config.httpServerErrorStatuses == (122..457).toSet()
     config.httpClientErrorStatuses == (111..111).toSet()
     config.httpClientSplitByDomain == true
+    config.dbClientSplitByInstance == true
     config.partialFlushMinSpans == 25
     config.reportHostName == true
     config.runtimeContextFieldInjection == false
@@ -287,6 +293,7 @@ class ConfigTest extends Specification {
     System.setProperty(PREFIX + HTTP_SERVER_ERROR_STATUSES, "1111")
     System.setProperty(PREFIX + HTTP_CLIENT_ERROR_STATUSES, "1:1")
     System.setProperty(PREFIX + HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "invalid")
+    System.setProperty(PREFIX + DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "invalid")
     System.setProperty(PREFIX + PROPAGATION_STYLE_EXTRACT, "some garbage")
     System.setProperty(PREFIX + PROPAGATION_STYLE_INJECT, " ")
 
@@ -307,6 +314,7 @@ class ConfigTest extends Specification {
     config.httpServerErrorStatuses == (500..599).toSet()
     config.httpClientErrorStatuses == (400..499).toSet()
     config.httpClientSplitByDomain == false
+    config.dbClientSplitByInstance == false
     config.propagationStylesToExtract.toList() == [Config.PropagationStyle.DATADOG]
     config.propagationStylesToInject.toList() == [Config.PropagationStyle.DATADOG]
   }
@@ -372,6 +380,7 @@ class ConfigTest extends Specification {
     properties.setProperty(HTTP_SERVER_ERROR_STATUSES, "123-456,457,124-125,122")
     properties.setProperty(HTTP_CLIENT_ERROR_STATUSES, "111")
     properties.setProperty(HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "true")
+    properties.setProperty(DB_CLIENT_HOST_SPLIT_BY_INSTANCE, "true")
     properties.setProperty(PARTIAL_FLUSH_MIN_SPANS, "15")
     properties.setProperty(PROPAGATION_STYLE_EXTRACT, "B3 Datadog")
     properties.setProperty(PROPAGATION_STYLE_INJECT, "Datadog B3")
@@ -400,6 +409,7 @@ class ConfigTest extends Specification {
     config.httpServerErrorStatuses == (122..457).toSet()
     config.httpClientErrorStatuses == (111..111).toSet()
     config.httpClientSplitByDomain == true
+    config.dbClientSplitByInstance == true
     config.partialFlushMinSpans == 15
     config.propagationStylesToExtract.toList() == [Config.PropagationStyle.B3, Config.PropagationStyle.DATADOG]
     config.propagationStylesToInject.toList() == [Config.PropagationStyle.DATADOG, Config.PropagationStyle.B3]