From 204b7cdd4c70179387d9a3b5a345b6a441909550 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 16 May 2019 16:38:19 -0700 Subject: [PATCH 01/16] Update Integrations Core to 6.11.2 --- dd-java-agent/agent-jmxfetch/integrations-core | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/agent-jmxfetch/integrations-core b/dd-java-agent/agent-jmxfetch/integrations-core index 3e38b4e75e..e6a01f9e88 160000 --- a/dd-java-agent/agent-jmxfetch/integrations-core +++ b/dd-java-agent/agent-jmxfetch/integrations-core @@ -1 +1 @@ -Subproject commit 3e38b4e75edcee3ca84f022ea50240b0fc0537f2 +Subproject commit e6a01f9e885ac9b71c0ffec8c28dc75668570b15 From b505c6054382b97dcfc5c9ac1c73fb19fe91d42a Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 16 May 2019 16:28:46 -0700 Subject: [PATCH 02/16] Use jmx AppConfig Builder instead of factory method Expose new options for configuring JMXFetch with standard datadog-agent config files with `jvm_direct: true` set as an instance attribute (this will be ignored by the datadog-agent). For Example: * `dd.agent.conf.d=/opt/datadog-agent/etc/conf.d` * `dd.jmxfetch.configs=activemq.d/conf.yaml,jmx.d/conf.yaml` will load jmx configs in those two files that have `jvm_direct: true` in their `instance` setup. Environment variables can also be used: `DD_AGENT_CONF_D` and `DD_JMXFETCH_CONFIGS` Depends on https://github.com/DataDog/jmxfetch/releases/tag/0.29.0 being released. --- .../agent-jmxfetch/agent-jmxfetch.gradle | 9 +++- .../trace/agent/jmxfetch/JMXFetch.java | 44 ++++++++++++++----- .../src/main/resources/jmxfetch-config.yaml | 6 +-- .../main/java/datadog/trace/api/Config.java | 10 ++++- 4 files changed, 52 insertions(+), 17 deletions(-) diff --git a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle index 3a8c86a423..5f43983704 100644 --- a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle +++ b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle @@ -4,7 +4,10 @@ plugins { apply from: "${rootDir}/gradle/java.gradle" dependencies { - compile 'com.datadoghq:jmxfetch:0.27.0' + compile('com.datadoghq:jmxfetch:0.29.0'){ + exclude group: 'org.slf4j', module: 'slf4j-log4j12' + exclude group: 'log4j', module: 'log4j' + } compile deps.slf4j compile project(':dd-trace-api') } @@ -32,12 +35,16 @@ tasks.register("submodulesUpdate", Exec) { group 'Build Setup' description 'Initializes and updates integrations-core git submodule' commandLine 'git', 'submodule', 'update', '--init', 'integrations-core' + inputs.file file("${project.rootDir}/.git/modules/dd-java-agent/agent-jmxfetch/integrations-core/HEAD") + outputs.dir file("$projectDir/integrations-core") } tasks.register("copyMetricConfigs", Exec) { group 'Build Setup' description 'Copy metrics.yaml files from integrations-core into resources' commandLine './copy-metric-configs.sh', 'integrations-core', sourceSets.main.output.resourcesDir + inputs.dir file("$projectDir/integrations-core") + outputs.dir sourceSets.main.output.resourcesDir doFirst { // Ensure the resources directory is available. file(sourceSets.main.output.resourcesDir).mkdirs() diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index 113b822036..a9f1fafd39 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -1,5 +1,7 @@ package datadog.trace.agent.jmxfetch; +import static org.datadog.jmxfetch.AppConfig.ACTION_COLLECT; + import com.google.common.collect.ImmutableList; import datadog.trace.api.Config; import java.io.IOException; @@ -16,6 +18,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.io.IOUtils; import org.datadog.jmxfetch.App; import org.datadog.jmxfetch.AppConfig; +import org.datadog.jmxfetch.reporter.ReporterFactory; @Slf4j public class JMXFetch { @@ -38,6 +41,14 @@ public class JMXFetch { return; } + if (!log.isDebugEnabled() + && System.getProperty("org.slf4j.simpleLogger.log.org.datadog.jmxfetch") == null) { + // Reduce noisiness of jmxfetch logging. + System.setProperty("org.slf4j.simpleLogger.log.org.datadog.jmxfetch", "warn"); + } + + final String agentConfDPath = config.getAgentConfDPath(); + final List jmxFetchConfigs = config.getJmxFetchConfigs(); final List internalMetricsConfigs = getInternalMetricFiles(); final List metricsConfigs = config.getJmxFetchMetricsConfigs(); final Integer checkPeriod = config.getJmxFetchCheckPeriod(); @@ -48,7 +59,9 @@ public class JMXFetch { final String logLevel = getLogLevel(); log.info( - "JMXFetch config: {} {} {} {} {} {} {} {}", + "JMXFetch config: {} {} {} {} {} {} {} {} {} {}", + agentConfDPath, + jmxFetchConfigs, internalMetricsConfigs, metricsConfigs, checkPeriod, @@ -57,17 +70,24 @@ public class JMXFetch { reporter, logLocation, logLevel); - final AppConfig appConfig = - AppConfig.create( - DEFAULT_CONFIGS, - internalMetricsConfigs, - metricsConfigs, - checkPeriod, - refreshBeansPeriod, - globalTags, - reporter, - logLocation, - logLevel); + + final AppConfig.AppConfigBuilder configBuilder = + AppConfig.builder() + .action(ImmutableList.of(ACTION_COLLECT)) + .confdDirectory(agentConfDPath) + .yamlFileList(jmxFetchConfigs) + .targetDirectInstances(true) + .instanceConfigResources(DEFAULT_CONFIGS) + .metricConfigResources(internalMetricsConfigs) + .metricConfigFiles(metricsConfigs) + .refreshBeansPeriod(refreshBeansPeriod) + .globalTags(globalTags) + .reporter(ReporterFactory.getReporter(reporter)); + + if (checkPeriod != null) { + configBuilder.checkPeriod(checkPeriod); + } + final AppConfig appConfig = configBuilder.build(); final Thread thread = new Thread( diff --git a/dd-java-agent/agent-jmxfetch/src/main/resources/jmxfetch-config.yaml b/dd-java-agent/agent-jmxfetch/src/main/resources/jmxfetch-config.yaml index 0d913aca1f..bbc6d52fa8 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/resources/jmxfetch-config.yaml +++ b/dd-java-agent/agent-jmxfetch/src/main/resources/jmxfetch-config.yaml @@ -3,6 +3,6 @@ init_config: new_gc_metrics: true instances: -- jmx_url: service:jmx:local:/// - conf: - # Intentionally left empty for now + - jvm_direct: true + name: dd-java-agent default + conf: [] # Intentionally left empty for now diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index c9ca8ba257..063c01cc1a 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -64,7 +64,9 @@ public class Config { public static final String PROPAGATION_STYLE_EXTRACT = "propagation.style.extract"; public static final String PROPAGATION_STYLE_INJECT = "propagation.style.inject"; + public static final String AGENT_CONF_D = "agent.conf.d"; public static final String JMX_FETCH_ENABLED = "jmxfetch.enabled"; + public static final String JMX_FETCH_CONFIGS = "jmxfetch.configs"; public static final String JMX_FETCH_METRICS_CONFIGS = "jmxfetch.metrics-configs"; public static final String JMX_FETCH_CHECK_PERIOD = "jmxfetch.check-period"; public static final String JMX_FETCH_REFRESH_BEANS_PERIOD = "jmxfetch.refresh-beans-period"; @@ -146,8 +148,10 @@ public class Config { @Getter private final Set propagationStylesToExtract; @Getter private final Set propagationStylesToInject; + @Getter private final String agentConfDPath; @Getter private final boolean jmxFetchEnabled; - @Getter private final List jmxFetchMetricsConfigs; + @Getter private final List jmxFetchConfigs; + @Deprecated @Getter private final List jmxFetchMetricsConfigs; @Getter private final Integer jmxFetchCheckPeriod; @Getter private final Integer jmxFetchRefreshBeansPeriod; @Getter private final String jmxFetchStatsdHost; @@ -218,8 +222,10 @@ public class Config { PropagationStyle.class, true); + agentConfDPath = getSettingFromEnvironment(AGENT_CONF_D, null); jmxFetchEnabled = getBooleanSettingFromEnvironment(JMX_FETCH_ENABLED, DEFAULT_JMX_FETCH_ENABLED); + jmxFetchConfigs = getListSettingFromEnvironment(JMX_FETCH_CONFIGS, null); jmxFetchMetricsConfigs = getListSettingFromEnvironment(JMX_FETCH_METRICS_CONFIGS, null); jmxFetchCheckPeriod = getIntegerSettingFromEnvironment(JMX_FETCH_CHECK_PERIOD, null); jmxFetchRefreshBeansPeriod = @@ -298,8 +304,10 @@ public class Config { ? parent.propagationStylesToInject : parsedPropagationStylesToInject; + agentConfDPath = properties.getProperty(AGENT_CONF_D, parent.agentConfDPath); jmxFetchEnabled = getPropertyBooleanValue(properties, JMX_FETCH_ENABLED, parent.jmxFetchEnabled); + jmxFetchConfigs = getPropertyListValue(properties, JMX_FETCH_CONFIGS, parent.jmxFetchConfigs); jmxFetchMetricsConfigs = getPropertyListValue(properties, JMX_FETCH_METRICS_CONFIGS, parent.jmxFetchMetricsConfigs); jmxFetchCheckPeriod = From 5f8e186dcb4b45a595f45a33a0f275235a755f03 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 22 May 2019 16:00:20 -0700 Subject: [PATCH 03/16] Fix task inputs/outputs. --- dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle index 5f43983704..c075f69f72 100644 --- a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle +++ b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle @@ -35,8 +35,15 @@ tasks.register("submodulesUpdate", Exec) { group 'Build Setup' description 'Initializes and updates integrations-core git submodule' commandLine 'git', 'submodule', 'update', '--init', 'integrations-core' - inputs.file file("${project.rootDir}/.git/modules/dd-java-agent/agent-jmxfetch/integrations-core/HEAD") - outputs.dir file("$projectDir/integrations-core") + def submoduleHead = file("${project.rootDir}/.git/modules/dd-java-agent/agent-jmxfetch/integrations-core/HEAD") + if (submoduleHead.exists()) { + inputs.file "${project.rootDir}/.git/modules/dd-java-agent/agent-jmxfetch/integrations-core/HEAD" + } + def integrationsCore = file("$projectDir/integrations-core") + outputs.dir integrationsCore + if (integrationsCore.list().length == 0) { + outputs.upToDateWhen { false } + } } tasks.register("copyMetricConfigs", Exec) { From b09aa1a59da46fc734e5511284c1604479961524 Mon Sep 17 00:00:00 2001 From: Luca Abbati Date: Wed, 8 May 2019 16:49:47 +0200 Subject: [PATCH 04/16] Enable JMXFetch by default --- .../src/main/java/datadog/trace/api/Config.java | 2 +- .../test/groovy/datadog/trace/api/ConfigTest.groovy | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index 063c01cc1a..b3ca942fb1 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -102,7 +102,7 @@ public class Config { 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(); - private static final boolean DEFAULT_JMX_FETCH_ENABLED = false; + private static final boolean DEFAULT_JMX_FETCH_ENABLED = true; public static final int DEFAULT_JMX_FETCH_STATSD_PORT = 8125; diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index d62eb9d47d..c6f127fe50 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -84,7 +84,7 @@ class ConfigTest extends Specification { config.runtimeContextFieldInjection == true config.propagationStylesToExtract.toList() == [Config.PropagationStyle.DATADOG] config.propagationStylesToInject.toList() == [Config.PropagationStyle.DATADOG] - config.jmxFetchEnabled == false + config.jmxFetchEnabled == true config.jmxFetchMetricsConfigs == [] config.jmxFetchCheckPeriod == null config.jmxFetchRefreshBeansPeriod == null @@ -125,7 +125,7 @@ class ConfigTest extends Specification { prop.setProperty(RUNTIME_CONTEXT_FIELD_INJECTION, "false") prop.setProperty(PROPAGATION_STYLE_EXTRACT, "Datadog, B3") prop.setProperty(PROPAGATION_STYLE_INJECT, "B3, Datadog") - prop.setProperty(JMX_FETCH_ENABLED, "true") + prop.setProperty(JMX_FETCH_ENABLED, "false") prop.setProperty(JMX_FETCH_METRICS_CONFIGS, "/foo.yaml,/bar.yaml") prop.setProperty(JMX_FETCH_CHECK_PERIOD, "100") prop.setProperty(JMX_FETCH_REFRESH_BEANS_PERIOD, "200") @@ -156,7 +156,7 @@ class ConfigTest extends Specification { config.runtimeContextFieldInjection == false config.propagationStylesToExtract.toList() == [Config.PropagationStyle.DATADOG, Config.PropagationStyle.B3] config.propagationStylesToInject.toList() == [Config.PropagationStyle.B3, Config.PropagationStyle.DATADOG] - config.jmxFetchEnabled == true + config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 config.jmxFetchRefreshBeansPeriod == 200 @@ -188,7 +188,7 @@ class ConfigTest extends Specification { System.setProperty(PREFIX + RUNTIME_CONTEXT_FIELD_INJECTION, "false") System.setProperty(PREFIX + PROPAGATION_STYLE_EXTRACT, "Datadog, B3") System.setProperty(PREFIX + PROPAGATION_STYLE_INJECT, "B3, Datadog") - System.setProperty(PREFIX + JMX_FETCH_ENABLED, "true") + System.setProperty(PREFIX + JMX_FETCH_ENABLED, "false") System.setProperty(PREFIX + JMX_FETCH_METRICS_CONFIGS, "/foo.yaml,/bar.yaml") System.setProperty(PREFIX + JMX_FETCH_CHECK_PERIOD, "100") System.setProperty(PREFIX + JMX_FETCH_REFRESH_BEANS_PERIOD, "200") @@ -219,7 +219,7 @@ class ConfigTest extends Specification { config.runtimeContextFieldInjection == false config.propagationStylesToExtract.toList() == [Config.PropagationStyle.DATADOG, Config.PropagationStyle.B3] config.propagationStylesToInject.toList() == [Config.PropagationStyle.B3, Config.PropagationStyle.DATADOG] - config.jmxFetchEnabled == true + config.jmxFetchEnabled == false config.jmxFetchMetricsConfigs == ["/foo.yaml", "/bar.yaml"] config.jmxFetchCheckPeriod == 100 config.jmxFetchRefreshBeansPeriod == 200 From 4d01f52b67307638f3f5b82bcff4680cac8907b6 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 23 May 2019 14:25:19 -0700 Subject: [PATCH 05/16] Change JMXFetch config keys `agent.conf.d` -> `jmxfetch.config.dir` `jmxfetch.configs` -> `jmxfetch.config` Enabling bundled JMXFetch configs: `dd.integration..enabled=true` -> `dd.jmxfetch..enabled=true` --- .../trace/agent/jmxfetch/JMXFetch.java | 8 ++-- .../datadog/trace/agent/JMXFetchTest.groovy | 2 +- .../main/java/datadog/trace/api/Config.java | 35 ++++++++++++----- .../datadog/trace/api/ConfigTest.groovy | 38 ++++++++++++++++++- 4 files changed, 67 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index a9f1fafd39..462e9a9192 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -47,7 +47,7 @@ public class JMXFetch { System.setProperty("org.slf4j.simpleLogger.log.org.datadog.jmxfetch", "warn"); } - final String agentConfDPath = config.getAgentConfDPath(); + final String jmxFetchConfigDir = config.getJmxFetchConfigDir(); final List jmxFetchConfigs = config.getJmxFetchConfigs(); final List internalMetricsConfigs = getInternalMetricFiles(); final List metricsConfigs = config.getJmxFetchMetricsConfigs(); @@ -60,7 +60,7 @@ public class JMXFetch { log.info( "JMXFetch config: {} {} {} {} {} {} {} {} {} {}", - agentConfDPath, + jmxFetchConfigDir, jmxFetchConfigs, internalMetricsConfigs, metricsConfigs, @@ -74,7 +74,7 @@ public class JMXFetch { final AppConfig.AppConfigBuilder configBuilder = AppConfig.builder() .action(ImmutableList.of(ACTION_COLLECT)) - .confdDirectory(agentConfDPath) + .confdDirectory(jmxFetchConfigDir) .yamlFileList(jmxFetchConfigs) .targetDirectInstances(true) .instanceConfigResources(DEFAULT_CONFIGS) @@ -151,7 +151,7 @@ public class JMXFetch { for (final String config : split) { integrationName.clear(); integrationName.add(config.replace(".yaml", "")); - if (Config.integrationEnabled(integrationName, false)) { + if (Config.jmxFetchIntegrationEnabled(integrationName, false)) { final URL resource = JMXFetch.class.getResource("metricconfigs/" + config); result.add(resource.getPath().split("\\.jar!/")[1]); } diff --git a/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy b/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy index e7ce2349fd..1b70815081 100644 --- a/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy +++ b/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy @@ -52,7 +52,7 @@ class JMXFetchTest extends Specification { def "test jmxfetch config"() { setup: names.each { - System.setProperty("dd.integration.${it}.enabled", "$enable") + System.setProperty("dd.jmxfetch.${it}.enabled", "$enable") } def classLoader = IntegrationTestUtils.getJmxFetchClassLoader() // Have to set this so JMXFetch knows where to find resources diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index b3ca942fb1..ffda653d25 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -64,9 +64,9 @@ public class Config { public static final String PROPAGATION_STYLE_EXTRACT = "propagation.style.extract"; public static final String PROPAGATION_STYLE_INJECT = "propagation.style.inject"; - public static final String AGENT_CONF_D = "agent.conf.d"; public static final String JMX_FETCH_ENABLED = "jmxfetch.enabled"; - public static final String JMX_FETCH_CONFIGS = "jmxfetch.configs"; + public static final String JMX_FETCH_CONFIG_DIR = "jmxfetch.config.dir"; + public static final String JMX_FETCH_CONFIG = "jmxfetch.config"; public static final String JMX_FETCH_METRICS_CONFIGS = "jmxfetch.metrics-configs"; public static final String JMX_FETCH_CHECK_PERIOD = "jmxfetch.check-period"; public static final String JMX_FETCH_REFRESH_BEANS_PERIOD = "jmxfetch.refresh-beans-period"; @@ -148,8 +148,8 @@ public class Config { @Getter private final Set propagationStylesToExtract; @Getter private final Set propagationStylesToInject; - @Getter private final String agentConfDPath; @Getter private final boolean jmxFetchEnabled; + @Getter private final String jmxFetchConfigDir; @Getter private final List jmxFetchConfigs; @Deprecated @Getter private final List jmxFetchMetricsConfigs; @Getter private final Integer jmxFetchCheckPeriod; @@ -222,10 +222,10 @@ public class Config { PropagationStyle.class, true); - agentConfDPath = getSettingFromEnvironment(AGENT_CONF_D, null); jmxFetchEnabled = getBooleanSettingFromEnvironment(JMX_FETCH_ENABLED, DEFAULT_JMX_FETCH_ENABLED); - jmxFetchConfigs = getListSettingFromEnvironment(JMX_FETCH_CONFIGS, null); + jmxFetchConfigDir = getSettingFromEnvironment(JMX_FETCH_CONFIG_DIR, null); + jmxFetchConfigs = getListSettingFromEnvironment(JMX_FETCH_CONFIG, null); jmxFetchMetricsConfigs = getListSettingFromEnvironment(JMX_FETCH_METRICS_CONFIGS, null); jmxFetchCheckPeriod = getIntegerSettingFromEnvironment(JMX_FETCH_CHECK_PERIOD, null); jmxFetchRefreshBeansPeriod = @@ -304,10 +304,10 @@ public class Config { ? parent.propagationStylesToInject : parsedPropagationStylesToInject; - agentConfDPath = properties.getProperty(AGENT_CONF_D, parent.agentConfDPath); jmxFetchEnabled = getPropertyBooleanValue(properties, JMX_FETCH_ENABLED, parent.jmxFetchEnabled); - jmxFetchConfigs = getPropertyListValue(properties, JMX_FETCH_CONFIGS, parent.jmxFetchConfigs); + jmxFetchConfigDir = properties.getProperty(JMX_FETCH_CONFIG_DIR, parent.jmxFetchConfigDir); + jmxFetchConfigs = getPropertyListValue(properties, JMX_FETCH_CONFIG, parent.jmxFetchConfigs); jmxFetchMetricsConfigs = getPropertyListValue(properties, JMX_FETCH_METRICS_CONFIGS, parent.jmxFetchMetricsConfigs); jmxFetchCheckPeriod = @@ -334,7 +334,7 @@ public class Config { final Map result = new HashMap<>(runtimeTags); if (reportHostName) { - String hostName = getHostName(); + final String hostName = getHostName(); if (null != hostName && !hostName.isEmpty()) { result.put(INTERNAL_HOST_NAME, hostName); } @@ -399,6 +399,23 @@ public class Config { return anyEnabled; } + public static boolean jmxFetchIntegrationEnabled( + final SortedSet integrationNames, final boolean defaultEnabled) { + // If default is enabled, we want to enable individually, + // if default is disabled, we want to disable individually. + boolean anyEnabled = defaultEnabled; + for (final String name : integrationNames) { + final boolean configEnabled = + getBooleanSettingFromEnvironment("jmxfetch." + name + ".enabled", defaultEnabled); + if (defaultEnabled) { + anyEnabled &= configEnabled; + } else { + anyEnabled |= configEnabled; + } + } + return anyEnabled; + } + public static boolean traceAnalyticsIntegrationEnabled( final SortedSet integrationNames, final boolean defaultEnabled) { // If default is enabled, we want to enable individually, @@ -682,7 +699,7 @@ public class Config { private String getHostName() { try { return InetAddress.getLocalHost().getHostName(); - } catch (UnknownHostException e) { + } catch (final UnknownHostException e) { // If we are not able to detect the hostname we do not throw an exception. } diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index c6f127fe50..5f3c0e754c 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -35,8 +35,8 @@ import static datadog.trace.api.Config.SERVICE_MAPPING import static datadog.trace.api.Config.SERVICE_NAME import static datadog.trace.api.Config.SPAN_TAGS import static datadog.trace.api.Config.TRACE_AGENT_PORT -import static datadog.trace.api.Config.TRACE_REPORT_HOSTNAME import static datadog.trace.api.Config.TRACE_ENABLED +import static datadog.trace.api.Config.TRACE_REPORT_HOSTNAME import static datadog.trace.api.Config.TRACE_RESOLVER_ENABLED import static datadog.trace.api.Config.WRITER_TYPE @@ -380,7 +380,7 @@ class ConfigTest extends Specification { properties.setProperty(JMX_FETCH_REFRESH_BEANS_PERIOD, "200") properties.setProperty(JMX_FETCH_STATSD_HOST, "statsd host") properties.setProperty(JMX_FETCH_STATSD_PORT, "321") - + when: def config = Config.get(properties) @@ -478,6 +478,40 @@ class ConfigTest extends Specification { integrationNames = new TreeSet<>(names) } + def "verify integration jmxfetch config"() { + setup: + environmentVariables.set("DD_JMXFETCH_ORDER_ENABLED", "false") + environmentVariables.set("DD_JMXFETCH_TEST_ENV_ENABLED", "true") + environmentVariables.set("DD_JMXFETCH_DISABLED_ENV_ENABLED", "false") + + System.setProperty("dd.jmxfetch.order.enabled", "true") + System.setProperty("dd.jmxfetch.test-prop.enabled", "true") + System.setProperty("dd.jmxfetch.disabled-prop.enabled", "false") + + expect: + Config.jmxFetchIntegrationEnabled(integrationNames, defaultEnabled) == expected + + where: + names | defaultEnabled | expected + [] | true | true + [] | false | false + ["invalid"] | true | true + ["invalid"] | false | false + ["test-prop"] | false | true + ["test-env"] | false | true + ["disabled-prop"] | true | false + ["disabled-env"] | true | false + ["other", "test-prop"] | false | true + ["other", "test-env"] | false | true + ["order"] | false | true + ["test-prop", "disabled-prop"] | false | true + ["disabled-env", "test-env"] | false | true + ["test-prop", "disabled-prop"] | true | false + ["disabled-env", "test-env"] | true | false + + integrationNames = new TreeSet<>(names) + } + def "verify integration trace analytics config"() { setup: environmentVariables.set("DD_ORDER_ANALYTICS_ENABLED", "false") From fd43210e0787175c44db7cf3cec7962cc2a734a7 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 23 May 2019 16:27:53 -0700 Subject: [PATCH 06/16] Add task timeout for tests. --- dd-java-agent/dd-java-agent.gradle | 3 ++- .../agent/test/IntegrationTestUtils.java | 26 +++++++++++++++++-- gradle/java.gradle | 7 ++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/dd-java-agent.gradle b/dd-java-agent/dd-java-agent.gradle index 43c3c0482a..4a8567f3fa 100644 --- a/dd-java-agent/dd-java-agent.gradle +++ b/dd-java-agent/dd-java-agent.gradle @@ -107,7 +107,8 @@ dependencies { } tasks.withType(Test).configureEach { - jvmArgs "-Ddd.writer.type=LogWriter", "-Ddd.service.name=java-app" + jvmArgs "-Ddd.service.name=java-agent-tests" + jvmArgs "-Ddd.writer.type=LoggingWriter" jvmArgs "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=debug" jvmArgs "-Dorg.slf4j.simpleLogger.defaultLogLevel=debug" diff --git a/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java b/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java index c106179ead..1a85694388 100644 --- a/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java +++ b/dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java @@ -17,6 +17,8 @@ import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.jar.Attributes; import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; @@ -201,7 +203,8 @@ public class IntegrationTestUtils { final ProcessBuilder processBuilder = new ProcessBuilder(commands.toArray(new String[0])); processBuilder.environment().putAll(envVars); final Process process = processBuilder.start(); - final int result = process.waitFor(); + + waitFor(process, 30, TimeUnit.SECONDS); if (printOutputStreams) { final BufferedReader stdInput = @@ -221,6 +224,25 @@ public class IntegrationTestUtils { } System.out.println("--- stderr end ---"); } - return result; + return process.exitValue(); + } + + private static void waitFor(final Process process, final long timeout, final TimeUnit unit) + throws InterruptedException, TimeoutException { + final long startTime = System.nanoTime(); + long rem = unit.toNanos(timeout); + + do { + try { + process.exitValue(); + return; + } catch (final IllegalThreadStateException ex) { + if (rem > 0) { + Thread.sleep(Math.min(TimeUnit.NANOSECONDS.toMillis(rem) + 1, 100)); + } + } + rem = unit.toNanos(timeout) - (System.nanoTime() - startTime); + } while (rem > 0); + throw new TimeoutException(); } } diff --git a/gradle/java.gradle b/gradle/java.gradle index a0c1af6cf9..9f6b50da8f 100644 --- a/gradle/java.gradle +++ b/gradle/java.gradle @@ -1,3 +1,5 @@ +import java.time.Duration + apply plugin: 'java' apply plugin: 'groovy' @@ -301,8 +303,11 @@ for (def env : System.getenv().entrySet()) { } } -// Disable all tests if skipTests property was specified tasks.withType(Test).configureEach { + // All tests must complete within 2 minutes. + timeout = Duration.ofMinutes(2) + + // Disable all tests if skipTests property was specified onlyIf { !project.rootProject.hasProperty("skipTests") } } From 80a5cc60259955c56b83b3870d6d6c80f7d14421 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 23 May 2019 20:07:57 -0400 Subject: [PATCH 07/16] Set kafka client service name to application default service name. So that spans inheriting from that client span have application service name rather than 'kafka' --- .../kafka_clients/KafkaDecorator.java | 20 ++++++++++++++----- .../src/test/groovy/KafkaClientTest.groovy | 2 +- .../src/test/groovy/KafkaStreamsTest.groovy | 16 +++++++-------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaDecorator.java b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaDecorator.java index cbd222383a..8d02bc7d0d 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaDecorator.java +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaDecorator.java @@ -12,6 +12,11 @@ import org.apache.kafka.clients.producer.ProducerRecord; public abstract class KafkaDecorator extends ClientDecorator { public static final KafkaDecorator PRODUCER_DECORATE = new KafkaDecorator() { + @Override + protected String service() { + return "kafka"; + } + @Override protected String spanKind() { return Tags.SPAN_KIND_PRODUCER; @@ -25,6 +30,16 @@ public abstract class KafkaDecorator extends ClientDecorator { public static final KafkaDecorator CONSUMER_DECORATE = new KafkaDecorator() { + @Override + protected String service() { + /* + Use default service name. Common use-case here is to have consumer span parent + children spans in instrumented application. Since service name is inherited it makes + sense to default that to application service name rather than 'kafka'. + */ + return null; + } + @Override protected String spanKind() { return Tags.SPAN_KIND_CONSUMER; @@ -41,11 +56,6 @@ public abstract class KafkaDecorator extends ClientDecorator { return new String[] {"kafka"}; } - @Override - protected String service() { - return "kafka"; - } - @Override protected String component() { return "java-kafka"; diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/KafkaClientTest.groovy b/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/KafkaClientTest.groovy index b22b6c30e4..49238f8357 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/KafkaClientTest.groovy +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/test/groovy/KafkaClientTest.groovy @@ -94,7 +94,7 @@ class KafkaClientTest extends AgentTestRunner { trace(1, 1) { // CONSUMER span 0 span(0) { - serviceName "kafka" + serviceName "unnamed-java-app" operationName "kafka.consume" resourceName "Consume Topic $SHARED_TOPIC" spanType "queue" diff --git a/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy b/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy index d74433ec11..cc8a7ce2c1 100644 --- a/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy +++ b/dd-java-agent/instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy @@ -80,13 +80,13 @@ class KafkaStreamsTest extends AgentTestRunner { KStream textLines = builder.stream(STREAM_PENDING) def values = textLines .mapValues(new ValueMapper() { - @Override - String apply(String textLine) { - TEST_WRITER.waitForTraces(1) // ensure consistent ordering of traces - getTestTracer().activeSpan().setTag("asdf", "testing") - return textLine.toLowerCase() - } - }) + @Override + String apply(String textLine) { + TEST_WRITER.waitForTraces(1) // ensure consistent ordering of traces + getTestTracer().activeSpan().setTag("asdf", "testing") + return textLine.toLowerCase() + } + }) KafkaStreams streams try { @@ -172,7 +172,7 @@ class KafkaStreamsTest extends AgentTestRunner { trace(2, 1) { // CONSUMER span 0 span(0) { - serviceName "kafka" + serviceName "unnamed-java-app" operationName "kafka.consume" resourceName "Consume Topic $STREAM_PROCESSED" spanType "queue" From 91cdef1a3581cd244f8d54fe81a0a38de80de86b Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 23 May 2019 17:59:54 -0700 Subject: [PATCH 08/16] Reduce logging to avoid risk of deadlocks. dd-jmx-collector Stack Trace is: java.lang.Thread.State: WAITING (parking) at jdk.internal.misc.Unsafe.park(java.base@10.0.1/Native Method) - parking to wait for <0x00000007a9ce5448> (a java.util.concurrent.locks.ReentrantLock$NonfairSync) at java.util.concurrent.locks.LockSupport.park(java.base@10.0.1/LockSupport.java:194) at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(java.base@10.0.1/AbstractQueuedSynchronizer.java:883) at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(java.base@10.0.1/AbstractQueuedSynchronizer.java:915) at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(java.base@10.0.1/AbstractQueuedSynchronizer.java:1238) at java.util.concurrent.locks.ReentrantLock.lock(java.base@10.0.1/ReentrantLock.java:267) at org.gradle.internal.remote.internal.hub.MessageHub$ChannelDispatch.dispatch(MessageHub.java:361) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93) at com.sun.proxy.$Proxy11.sendOutputEvent(Unknown Source) at org.gradle.process.internal.worker.child.WorkerLogEventListener.onOutput(WorkerLogEventListener.java:36) at jdk.internal.reflect.GeneratedMethodAccessor17.invoke(Unknown Source) at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@10.0.1/DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(java.base@10.0.1/Method.java:564) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:42) at org.gradle.internal.event.BroadcastDispatch$SingletonDispatch.dispatch(BroadcastDispatch.java:230) at org.gradle.internal.event.BroadcastDispatch$SingletonDispatch.dispatch(BroadcastDispatch.java:149) at org.gradle.internal.event.ListenerBroadcast.dispatch(ListenerBroadcast.java:140) at org.gradle.internal.event.ListenerBroadcast.dispatch(ListenerBroadcast.java:37) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93) at com.sun.proxy.$Proxy10.onOutput(Unknown Source) at org.gradle.internal.logging.sink.OutputEventTransformer.onOutput(OutputEventTransformer.java:104) at org.gradle.internal.logging.sink.OutputEventRenderer.onOutput(OutputEventRenderer.java:420) - locked <0x00000007aa223fc0> (a java.lang.Object) at org.gradle.internal.logging.sink.OutputEventListenerManager$1.onOutput(OutputEventListenerManager.java:36) at org.gradle.internal.logging.services.TextStreamOutputEventListener.onTextEvent(TextStreamOutputEventListener.java:57) at org.gradle.internal.logging.services.TextStreamOutputEventListener.onOutput(TextStreamOutputEventListener.java:41) at org.gradle.internal.logging.source.PrintStreamLoggingSystem$OutputEventDestination.onOutput(PrintStreamLoggingSystem.java:167) at org.gradle.internal.logging.source.PrintStreamLoggingSystem$1.text(PrintStreamLoggingSystem.java:44) at org.gradle.internal.io.LineBufferingOutputStream.flush(LineBufferingOutputStream.java:95) at org.gradle.internal.io.LineBufferingOutputStream.write(LineBufferingOutputStream.java:80) at java.io.OutputStream.write(java.base@10.0.1/OutputStream.java:113) at java.io.PrintStream.write(java.base@10.0.1/PrintStream.java:559) - locked <0x00000007afd031a0> (a java.io.PrintStream) at sun.nio.cs.StreamEncoder.writeBytes(java.base@10.0.1/StreamEncoder.java:233) at sun.nio.cs.StreamEncoder.implFlushBuffer(java.base@10.0.1/StreamEncoder.java:312) at sun.nio.cs.StreamEncoder.flushBuffer(java.base@10.0.1/StreamEncoder.java:104) - locked <0x00000007afd03ac0> (a java.io.OutputStreamWriter) at java.io.OutputStreamWriter.flushBuffer(java.base@10.0.1/OutputStreamWriter.java:184) at java.io.PrintStream.newLine(java.base@10.0.1/PrintStream.java:625) - locked <0x00000007afd031a0> (a java.io.PrintStream) at java.io.PrintStream.println(java.base@10.0.1/PrintStream.java:883) - locked <0x00000007afd031a0> (a java.io.PrintStream) at org.gradle.internal.io.LinePerThreadBufferingOutputStream.println(LinePerThreadBufferingOutputStream.java:203) at datadog.slf4j.impl.SimpleLogger.write(SimpleLogger.java:318) at datadog.slf4j.impl.SimpleLogger.log(SimpleLogger.java:295) at datadog.slf4j.impl.SimpleLogger.info(SimpleLogger.java:480) at org.datadog.jmxfetch.reporter.ConsoleReporter.doSendServiceCheck(ConsoleReporter.java:52) at org.datadog.jmxfetch.reporter.Reporter.sendServiceCheck(Reporter.java:166) at org.datadog.jmxfetch.App.sendServiceCheck(App.java:767) at org.datadog.jmxfetch.App.processCollectionStatus(App.java:1161) at org.datadog.jmxfetch.App.doIteration(App.java:491) at org.datadog.jmxfetch.App.start(App.java:424) at org.datadog.jmxfetch.App.run(App.java:219) at datadog.trace.agent.jmxfetch.JMXFetch$1.run(JMXFetch.java:99) at java.lang.Thread.run(java.base@10.0.1/Thread.java:844) main Stack Trace is: java.lang.Thread.State: BLOCKED (on object monitor) at org.gradle.internal.logging.sink.OutputEventRenderer.onOutput(OutputEventRenderer.java:420) - waiting to lock <0x00000007aa223fc0> (a java.lang.Object) at org.gradle.internal.logging.sink.OutputEventListenerManager$1.onOutput(OutputEventListenerManager.java:36) at org.gradle.internal.logging.services.TextStreamOutputEventListener.onTextEvent(TextStreamOutputEventListener.java:57) at org.gradle.internal.logging.services.TextStreamOutputEventListener.onOutput(TextStreamOutputEventListener.java:41) at org.gradle.internal.logging.source.PrintStreamLoggingSystem$OutputEventDestination.onOutput(PrintStreamLoggingSystem.java:167) at org.gradle.internal.logging.source.PrintStreamLoggingSystem$1.text(PrintStreamLoggingSystem.java:44) at org.gradle.internal.io.LineBufferingOutputStream.flush(LineBufferingOutputStream.java:95) at org.gradle.internal.io.LineBufferingOutputStream.write(LineBufferingOutputStream.java:80) at java.io.OutputStream.write(java.base@10.0.1/OutputStream.java:113) at java.io.PrintStream.write(java.base@10.0.1/PrintStream.java:559) - locked <0x00000007a9c6b060> (a java.io.PrintStream) at sun.nio.cs.StreamEncoder.writeBytes(java.base@10.0.1/StreamEncoder.java:233) at sun.nio.cs.StreamEncoder.implFlushBuffer(java.base@10.0.1/StreamEncoder.java:312) at sun.nio.cs.StreamEncoder.flushBuffer(java.base@10.0.1/StreamEncoder.java:104) - locked <0x00000007a9c6b990> (a java.io.OutputStreamWriter) at java.io.OutputStreamWriter.flushBuffer(java.base@10.0.1/OutputStreamWriter.java:184) at java.io.PrintStream.newLine(java.base@10.0.1/PrintStream.java:625) - locked <0x00000007a9c6b060> (a java.io.PrintStream) at java.io.PrintStream.println(java.base@10.0.1/PrintStream.java:883) - locked <0x00000007a9c6b060> (a java.io.PrintStream) at org.gradle.internal.io.LinePerThreadBufferingOutputStream.println(LinePerThreadBufferingOutputStream.java:203) at datadog.slf4j.impl.SimpleLogger.write(SimpleLogger.java:318) at datadog.slf4j.impl.SimpleLogger.log(SimpleLogger.java:295) at datadog.slf4j.impl.SimpleLogger.formatAndLog(SimpleLogger.java:355) at datadog.slf4j.impl.SimpleLogger.debug(SimpleLogger.java:446) at datadog.trace.agent.tooling.context.FieldBackedProvider$1$1$1.visitMethodInsn(FieldBackedProvider.java:202) at net.bytebuddy.jar.asm.MethodVisitor.visitMethodInsn(MethodVisitor.java:427) at net.bytebuddy.utility.visitor.ExceptionTableSensitiveMethodVisitor.onVisitMethodInsn(ExceptionTableSensitiveMethodVisitor.java:180) at net.bytebuddy.utility.visitor.ExceptionTableSensitiveMethodVisitor.visitMethodInsn(ExceptionTableSensitiveMethodVisitor.java:167) at net.bytebuddy.jar.asm.MethodVisitor.visitMethodInsn(MethodVisitor.java:427) at net.bytebuddy.utility.visitor.StackAwareMethodVisitor.visitMethodInsn(StackAwareMethodVisitor.java:302) at net.bytebuddy.jar.asm.MethodVisitor.visitMethodInsn(MethodVisitor.java:427) at net.bytebuddy.jar.asm.MethodVisitor.visitMethodInsn(MethodVisitor.java:427) at net.bytebuddy.jar.asm.ClassReader.readCode(ClassReader.java:2214) at net.bytebuddy.jar.asm.ClassReader.readMethod(ClassReader.java:1283) at net.bytebuddy.jar.asm.ClassReader.accept(ClassReader.java:688) at net.bytebuddy.jar.asm.ClassReader.accept(ClassReader.java:400) at net.bytebuddy.asm.Advice$Dispatcher$Inlining$Resolved$AdviceMethodInliner.apply(Advice.java:7379) at net.bytebuddy.asm.Advice$AdviceVisitor.onAfterExceptionTable(Advice.java:9427) at net.bytebuddy.utility.visitor.ExceptionTableSensitiveMethodVisitor.considerEndOfExceptionTable(ExceptionTableSensitiveMethodVisitor.java:49) at net.bytebuddy.utility.visitor.ExceptionTableSensitiveMethodVisitor.visitLabel(ExceptionTableSensitiveMethodVisitor.java:62) at net.bytebuddy.jar.asm.Label.accept(Label.java:357) at net.bytebuddy.jar.asm.ClassReader.readCode(ClassReader.java:1823) at net.bytebuddy.jar.asm.ClassReader.readMethod(ClassReader.java:1283) at net.bytebuddy.jar.asm.ClassReader.accept(ClassReader.java:688) at net.bytebuddy.jar.asm.ClassReader.accept(ClassReader.java:400) at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForInlining.create(TypeWriter.java:3393) at net.bytebuddy.dynamic.scaffold.TypeWriter$Default.make(TypeWriter.java:1930) at net.bytebuddy.dynamic.scaffold.inline.RedefinitionDynamicTypeBuilder.make(RedefinitionDynamicTypeBuilder.java:217) at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Simple$Resolution.apply(AgentBuilder.java:10132) at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.doTransform(AgentBuilder.java:10551) at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:10514) at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.access$1500(AgentBuilder.java:10280) at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer$Java9CapableVmDispatcher.run(AgentBuilder.java:10964) at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer$Java9CapableVmDispatcher.run(AgentBuilder.java:10902) at java.security.AccessController.doPrivileged(java.base@10.0.1/Native Method) at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:10470) at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer$ByteBuddy$ModuleSupport.transform(Unknown Source) at sun.instrument.TransformerManager.transform(java.instrument@10.0.1/TransformerManager.java:188) at sun.instrument.InstrumentationImpl.transform(java.instrument@10.0.1/InstrumentationImpl.java:560) at java.lang.ClassLoader.defineClass1(java.base@10.0.1/Native Method) at java.lang.ClassLoader.defineClass(java.base@10.0.1/ClassLoader.java:1009) at java.security.SecureClassLoader.defineClass(java.base@10.0.1/SecureClassLoader.java:174) at java.net.URLClassLoader.defineClass(java.base@10.0.1/URLClassLoader.java:545) at java.net.URLClassLoader.access$100(java.base@10.0.1/URLClassLoader.java:83) at java.net.URLClassLoader$1.run(java.base@10.0.1/URLClassLoader.java:453) at java.net.URLClassLoader$1.run(java.base@10.0.1/URLClassLoader.java:447) at java.security.AccessController.doPrivileged(java.base@10.0.1/Native Method) at java.net.URLClassLoader.findClass(java.base@10.0.1/URLClassLoader.java:446) at java.lang.ClassLoader.loadClass(java.base@10.0.1/ClassLoader.java:566) - locked <0x00000007afb4bc50> (a java.lang.Object) at java.lang.ClassLoader.loadClass(java.base@10.0.1/ClassLoader.java:499) at org.gradle.internal.remote.internal.hub.MessageHub.addHandler(MessageHub.java:131) at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection.addIncoming(MessageHubBackedObjectConnection.java:98) at org.gradle.api.internal.tasks.testing.worker.TestWorker.startReceivingTests(TestWorker.java:104) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:68) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:46) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:93) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:36) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:125) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:68) at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:62) at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:67) --- dd-java-agent/dd-java-agent.gradle | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/dd-java-agent.gradle b/dd-java-agent/dd-java-agent.gradle index 4a8567f3fa..dda95c047e 100644 --- a/dd-java-agent/dd-java-agent.gradle +++ b/dd-java-agent/dd-java-agent.gradle @@ -109,8 +109,9 @@ dependencies { tasks.withType(Test).configureEach { jvmArgs "-Ddd.service.name=java-agent-tests" jvmArgs "-Ddd.writer.type=LoggingWriter" - jvmArgs "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=debug" - jvmArgs "-Dorg.slf4j.simpleLogger.defaultLogLevel=debug" + // Multi-threaded logging seems to be causing deadlocks with Gradle's log capture. +// jvmArgs "-Ddatadog.slf4j.simpleLogger.defaultLogLevel=debug" +// jvmArgs "-Dorg.slf4j.simpleLogger.defaultLogLevel=debug" doFirst { // Defining here to allow jacoco to be first on the command line. From a7271ed2b21b985beb49e8fbee1175855e1225eb Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Thu, 23 May 2019 20:24:01 -0400 Subject: [PATCH 09/16] Use default service name for RabbitMQ consumer So spans parented by consumer span had reasonable service name --- .../instrumentation/rabbitmq/amqp/RabbitDecorator.java | 10 ++++++++++ .../src/test/groovy/RabbitMQTest.groovy | 9 ++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitDecorator.java b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitDecorator.java index 66b3122f5b..10160b6488 100644 --- a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitDecorator.java +++ b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitDecorator.java @@ -27,6 +27,16 @@ public class RabbitDecorator extends ClientDecorator { public static final RabbitDecorator CONSUMER_DECORATE = new RabbitDecorator() { + @Override + protected String service() { + /* + Use default service name. Common use-case here is to have consumer span parent + children spans in instrumented application. Since service name is inherited it makes + sense to default that to application service name rather than 'rabbitmq'. + */ + return null; + } + @Override protected String spanKind() { return Tags.SPAN_KIND_CONSUMER; diff --git a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/test/groovy/RabbitMQTest.groovy b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/test/groovy/RabbitMQTest.groovy index fe1db12e0b..558520c17a 100644 --- a/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/test/groovy/RabbitMQTest.groovy +++ b/dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/test/groovy/RabbitMQTest.groovy @@ -341,7 +341,14 @@ class RabbitMQTest extends AgentTestRunner { String errorMsg = null ) { trace.span(index) { - serviceName "rabbitmq" + switch (span.tags["amqp.command"]) { + case "basic.get": + case "basic.deliver": + serviceName "unnamed-java-app" + break + default: + serviceName "rabbitmq" + } operationName "amqp.command" resourceName resource switch (span.tags["amqp.command"]) { From 25e06b496501e1e8613254e6748426f2a1012e6a Mon Sep 17 00:00:00 2001 From: Luca Abbati Date: Fri, 24 May 2019 17:42:06 +0200 Subject: [PATCH 10/16] Make spring controller spans to handle async --- .../springweb/HandlerAdapterInstrumentation.java | 4 ++++ .../src/main/java/datadog/trace/context/TraceScope.java | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java index b9881a993e..8ada084ccc 100644 --- a/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-web/src/main/java/datadog/trace/instrumentation/springweb/HandlerAdapterInstrumentation.java @@ -14,6 +14,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.context.TraceScope; import io.opentracing.Scope; import io.opentracing.util.GlobalTracer; import java.lang.reflect.Method; @@ -107,6 +108,9 @@ public final class HandlerAdapterInstrumentation extends Instrumenter.Default { final String operationName = DECORATE.spanNameForClass(clazz) + "." + methodName; final Scope scope = GlobalTracer.get().buildSpan(operationName).startActive(true); + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } DECORATE.afterStart(scope); return scope; } diff --git a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java index 709301d8fb..1b3542a87b 100644 --- a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java +++ b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java @@ -2,7 +2,7 @@ package datadog.trace.context; import java.io.Closeable; -/** An object when can propagate a datadog trace across multiple threads. */ +/** An object that can propagate a datadog trace across multiple threads. */ public interface TraceScope extends Closeable { /** * Prevent the trace attached to this TraceScope from reporting until the returned Continuation From 35cd97b2ad2704bac8dcb680ac945c98f18aa814 Mon Sep 17 00:00:00 2001 From: Luca Abbati Date: Fri, 24 May 2019 17:43:42 +0200 Subject: [PATCH 11/16] Make spring controller spans to handle async (followup) --- .../src/main/java/datadog/trace/context/TraceScope.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java index 1b3542a87b..709301d8fb 100644 --- a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java +++ b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java @@ -2,7 +2,7 @@ package datadog.trace.context; import java.io.Closeable; -/** An object that can propagate a datadog trace across multiple threads. */ +/** An object when can propagate a datadog trace across multiple threads. */ public interface TraceScope extends Closeable { /** * Prevent the trace attached to this TraceScope from reporting until the returned Continuation From 21c22d6985bd9fc6b3fca888aabf3d512c3194b7 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 10 May 2019 10:30:18 -0700 Subject: [PATCH 12/16] Fix log message and add ignores Also add CODEOWNERS --- .github/CODEOWNERS | 4 ++++ .../datadog/trace/agent/tooling/AgentInstaller.java | 10 ++++++++++ .../trace/agent/tooling/ClassLoaderMatcher.java | 1 + dd-trace-java.gradle | 1 + .../main/java/datadog/trace/common/writer/DDApi.java | 2 +- 5 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000000..401d6036fd --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,4 @@ +# Automatically assign the team as a reviewer. +# https://help.github.com/en/articles/about-code-owners + +* @DataDog/apm-java diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java index ee8250c1b8..ba39b39e5f 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java @@ -121,6 +121,16 @@ public class AgentInstaller { .or(nameStartsWith("com.intellij.rt.debugger.")) .or(nameStartsWith("com.p6spy.")) .or(nameStartsWith("com.newrelic.")) + .or(nameStartsWith("com.dynatrace.")) + .or(nameStartsWith("com.jloadtrace.")) + .or(nameStartsWith("com.appdynamics.")) + .or(nameStartsWith("com.singularity.")) + .or(nameStartsWith("com.jinspired.")) + .or(nameStartsWith("org.jinspired.")) + .or(nameStartsWith("org.apache.log4j.")) + .or(nameStartsWith("org.slf4j.").and(not(named("org.slf4j.MDC")))) + .or(nameContains("$JaxbAccessor")) + .or(nameContains("CGLIB$$")) .or(nameContains("javassist")) .or(nameContains(".asm.")) .or(nameMatches("com\\.mchange\\.v2\\.c3p0\\..*Proxy")) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java index 81015fd654..59c05548f9 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ClassLoaderMatcher.java @@ -53,6 +53,7 @@ public class ClassLoaderMatcher { classesToSkip.add("sun.reflect.DelegatingClassLoader"); classesToSkip.add("jdk.internal.reflect.DelegatingClassLoader"); classesToSkip.add("clojure.lang.DynamicClassLoader"); + classesToSkip.add("org.apache.cxf.common.util.ASMHelper$TypeHelperClassLoader"); classesToSkip.add(DatadogClassLoader.class.getName()); CLASSLOADER_CLASSES_TO_SKIP = Collections.unmodifiableSet(classesToSkip); } diff --git a/dd-trace-java.gradle b/dd-trace-java.gradle index 974002cf1d..86d9282c91 100644 --- a/dd-trace-java.gradle +++ b/dd-trace-java.gradle @@ -3,6 +3,7 @@ plugins { id 'com.jfrog.artifactory' version '4.8.1' id 'com.jfrog.bintray' version '1.8.4' id 'org.unbroken-dome.test-sets' version '2.1.1' + id 'com.github.ben-manes.versions' version '0.21.0' id 'com.gradle.build-scan' version '2.2.1' // Not applying google java format by default because it gets confused by stray java build diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java index 5c42284f5f..553c699b33 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java @@ -159,7 +159,7 @@ public class DDApi { } else if (nextAllowedLogTime < System.currentTimeMillis()) { nextAllowedLogTime = System.currentTimeMillis() + MILLISECONDS_BETWEEN_ERROR_LOG; log.warn( - "Error while sending {} of {} traces to the DD agent. Status: {} (going silent for {} minutes)", + "Error while sending {} of {} traces to the DD agent. Status: {} {} (going silent for {} minutes)", traces.size(), representativeCount, response.code(), From 0b85f048d14b4804f900898124a77e5453738306 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Fri, 24 May 2019 16:27:28 -0400 Subject: [PATCH 13/16] Handle Scope in Kafka producer properly Holding onto scope in `Callback` is bad because that code may run on different thread. --- .../KafkaProducerInstrumentation.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java index a0c8d7f4c1..f555215c4b 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java @@ -10,6 +10,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import io.opentracing.Scope; +import io.opentracing.Span; import io.opentracing.propagation.Format; import io.opentracing.util.GlobalTracer; import java.util.Map; @@ -70,7 +71,7 @@ public final class KafkaProducerInstrumentation extends Instrumenter.Default { PRODUCER_DECORATE.afterStart(scope); PRODUCER_DECORATE.onProduce(scope, record); - callback = new ProducerCallback(callback, scope); + callback = new ProducerCallback(callback, scope.span()); // Do not inject headers for batch versions below 2 // This is how similar check is being done in Kafka client itself: @@ -115,24 +116,25 @@ public final class KafkaProducerInstrumentation extends Instrumenter.Default { public static class ProducerCallback implements Callback { private final Callback callback; - private final Scope scope; + private final Span span; - public ProducerCallback(final Callback callback, final Scope scope) { + public ProducerCallback(final Callback callback, final Span span) { this.callback = callback; - this.scope = scope; + this.span = span; } @Override public void onCompletion(final RecordMetadata metadata, final Exception exception) { - PRODUCER_DECORATE.onError(scope, exception); - try { - if (callback != null) { - callback.onCompletion(metadata, exception); + try (final Scope scope = GlobalTracer.get().scopeManager().activate(span, false)) { + PRODUCER_DECORATE.onError(span, exception); + try { + if (callback != null) { + callback.onCompletion(metadata, exception); + } + } finally { + PRODUCER_DECORATE.beforeFinish(span); + span.finish(); } - } finally { - PRODUCER_DECORATE.beforeFinish(scope); - scope.span().finish(); - scope.close(); } } } From a3a325868c5647329e8cc847886f8154abf3a988 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Fri, 24 May 2019 16:36:07 -0400 Subject: [PATCH 14/16] Add some rudimetrary tests for CompletableFuture --- .../groovy/ExecutorInstrumentationTest.groovy | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy index 9825b95813..aeb20de523 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy @@ -11,6 +11,7 @@ import java.lang.reflect.InvocationTargetException import java.util.concurrent.AbstractExecutorService import java.util.concurrent.ArrayBlockingQueue import java.util.concurrent.Callable +import java.util.concurrent.CompletableFuture import java.util.concurrent.ExecutionException import java.util.concurrent.ForkJoinPool import java.util.concurrent.ForkJoinTask @@ -21,6 +22,8 @@ import java.util.concurrent.ScheduledThreadPoolExecutor import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException +import java.util.function.Function +import java.util.function.Supplier class ExecutorInstrumentationTest extends AgentTestRunner { @@ -234,6 +237,74 @@ class ExecutorInstrumentationTest extends AgentTestRunner { "submit Callable" | submitCallable | new ForkJoinPool() } + def "CompletableFuture test"() { + setup: + def pool = new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) + def differentPool = new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) + def supplier = new Supplier() { + @Override + @Trace(operationName = "supplier") + String get() { + sleep(1000) + return "a" + } + } + + def function = new Function() { + @Override + @Trace(operationName = "function") + String apply(String s) { + return s + "c" + } + } + + def future = new Supplier>() { + @Override + @Trace(operationName = "parent") + CompletableFuture get() { + ((ContinuableScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true) + return CompletableFuture.supplyAsync(supplier, pool) + .thenCompose({ s -> CompletableFuture.supplyAsync(new AppendingSupplier(s), differentPool) }) + .thenApply(function) + } + }.get() + + def result = future.get() + + TEST_WRITER.waitForTraces(1) + List trace = TEST_WRITER.get(0) + + expect: + result == "abc" + + TEST_WRITER.size() == 1 + trace.size() == 4 + trace.get(0).operationName == "parent" + trace.get(1).operationName == "function" + trace.get(1).parentId == trace.get(0).spanId + trace.get(2).operationName == "appendingSupplier" + trace.get(2).parentId == trace.get(0).spanId + trace.get(3).operationName == "supplier" + trace.get(3).parentId == trace.get(0).spanId + + cleanup: + pool?.shutdown() + } + + class AppendingSupplier implements Supplier { + String letter + + AppendingSupplier(String letter) { + this.letter = letter + } + + @Override + @Trace(operationName = "appendingSupplier") + String get() { + return letter + "b" + } + } + static class CustomThreadPoolExecutor extends AbstractExecutorService { volatile running = true def workQueue = new LinkedBlockingQueue(10) From ef94e2fb79a64708f37c0a628e8819ebe9f8643e Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Fri, 24 May 2019 16:37:27 -0400 Subject: [PATCH 15/16] Add note about Kafka consumer iterator thread safety --- .../trace/instrumentation/kafka_clients/TracingIterable.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterable.java b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterable.java index 01135fea39..3757fd12ac 100644 --- a/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterable.java +++ b/dd-java-agent/instrumentation/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/TracingIterable.java @@ -33,6 +33,10 @@ public class TracingIterable implements Iterable { private final String operationName; private final KafkaDecorator decorator; + /** + * Note: this may potentially create problems if this iterator is used from different threads. + * But at the moment we cannot do much about this. + */ private Scope currentScope; public TracingIterator( From 70fa97f8c26caa3673849e389f0c1d572530b5fb Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Fri, 24 May 2019 16:52:57 -0400 Subject: [PATCH 16/16] Move code to make java7 happy --- .../groovy/CompletableFutureTest.groovy | 89 +++++++++++++++++++ .../groovy/ExecutorInstrumentationTest.groovy | 71 --------------- 2 files changed, 89 insertions(+), 71 deletions(-) create mode 100644 dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy diff --git a/dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy b/dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy new file mode 100644 index 0000000000..40793b84c0 --- /dev/null +++ b/dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/CompletableFutureTest.groovy @@ -0,0 +1,89 @@ +import datadog.opentracing.DDSpan +import datadog.opentracing.scopemanager.ContinuableScope +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.Trace +import io.opentracing.util.GlobalTracer + +import java.util.concurrent.ArrayBlockingQueue +import java.util.concurrent.CompletableFuture +import java.util.concurrent.ThreadPoolExecutor +import java.util.concurrent.TimeUnit +import java.util.function.Function +import java.util.function.Supplier + +/** + * Note: ideally this should live with the rest of ExecutorInstrumentationTest, + * but this code needs java8 so we put it here for now. + */ +class CompletableFutureTest extends AgentTestRunner { + + def "CompletableFuture test"() { + setup: + def pool = new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) + def differentPool = new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) + def supplier = new Supplier() { + @Override + @Trace(operationName = "supplier") + String get() { + sleep(1000) + return "a" + } + } + + def function = new Function() { + @Override + @Trace(operationName = "function") + String apply(String s) { + return s + "c" + } + } + + def future = new Supplier>() { + @Override + @Trace(operationName = "parent") + CompletableFuture get() { + ((ContinuableScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true) + return CompletableFuture.supplyAsync(supplier, pool) + .thenCompose({ s -> CompletableFuture.supplyAsync(new AppendingSupplier(s), differentPool) }) + .thenApply(function) + } + }.get() + + def result = future.get() + + TEST_WRITER.waitForTraces(1) + List trace = TEST_WRITER.get(0) + + expect: + result == "abc" + + TEST_WRITER.size() == 1 + trace.size() == 4 + trace.get(0).operationName == "parent" + trace.get(1).operationName == "function" + trace.get(1).parentId == trace.get(0).spanId + trace.get(2).operationName == "appendingSupplier" + trace.get(2).parentId == trace.get(0).spanId + trace.get(3).operationName == "supplier" + trace.get(3).parentId == trace.get(0).spanId + + cleanup: + pool?.shutdown() + differentPool?.shutdown() + } + + class AppendingSupplier implements Supplier { + String letter + + AppendingSupplier(String letter) { + this.letter = letter + } + + @Override + @Trace(operationName = "appendingSupplier") + String get() { + return letter + "b" + } + } + +} diff --git a/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy index aeb20de523..9825b95813 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy @@ -11,7 +11,6 @@ import java.lang.reflect.InvocationTargetException import java.util.concurrent.AbstractExecutorService import java.util.concurrent.ArrayBlockingQueue import java.util.concurrent.Callable -import java.util.concurrent.CompletableFuture import java.util.concurrent.ExecutionException import java.util.concurrent.ForkJoinPool import java.util.concurrent.ForkJoinTask @@ -22,8 +21,6 @@ import java.util.concurrent.ScheduledThreadPoolExecutor import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException -import java.util.function.Function -import java.util.function.Supplier class ExecutorInstrumentationTest extends AgentTestRunner { @@ -237,74 +234,6 @@ class ExecutorInstrumentationTest extends AgentTestRunner { "submit Callable" | submitCallable | new ForkJoinPool() } - def "CompletableFuture test"() { - setup: - def pool = new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) - def differentPool = new ThreadPoolExecutor(1, 1, 1000, TimeUnit.NANOSECONDS, new ArrayBlockingQueue(1)) - def supplier = new Supplier() { - @Override - @Trace(operationName = "supplier") - String get() { - sleep(1000) - return "a" - } - } - - def function = new Function() { - @Override - @Trace(operationName = "function") - String apply(String s) { - return s + "c" - } - } - - def future = new Supplier>() { - @Override - @Trace(operationName = "parent") - CompletableFuture get() { - ((ContinuableScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true) - return CompletableFuture.supplyAsync(supplier, pool) - .thenCompose({ s -> CompletableFuture.supplyAsync(new AppendingSupplier(s), differentPool) }) - .thenApply(function) - } - }.get() - - def result = future.get() - - TEST_WRITER.waitForTraces(1) - List trace = TEST_WRITER.get(0) - - expect: - result == "abc" - - TEST_WRITER.size() == 1 - trace.size() == 4 - trace.get(0).operationName == "parent" - trace.get(1).operationName == "function" - trace.get(1).parentId == trace.get(0).spanId - trace.get(2).operationName == "appendingSupplier" - trace.get(2).parentId == trace.get(0).spanId - trace.get(3).operationName == "supplier" - trace.get(3).parentId == trace.get(0).spanId - - cleanup: - pool?.shutdown() - } - - class AppendingSupplier implements Supplier { - String letter - - AppendingSupplier(String letter) { - this.letter = letter - } - - @Override - @Trace(operationName = "appendingSupplier") - String get() { - return letter + "b" - } - } - static class CustomThreadPoolExecutor extends AbstractExecutorService { volatile running = true def workQueue = new LinkedBlockingQueue(10)