From 4dd4ee0c05a080367d476441196ea988a65f23d1 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 20 Feb 2019 09:48:59 -0800 Subject: [PATCH] Trace Analytics Config Also make the instrumentation names a sorted list so the evaluation order is consistent. --- .../trace/agent/jmxfetch/JMXFetch.java | 8 +++-- .../trace/agent/tooling/Instrumenter.java | 8 ++--- .../main/java/datadog/trace/api/Config.java | 35 ++++++++++++++++-- .../main/java/datadog/trace/api/DDTags.java | 3 +- .../datadog/trace/api/ConfigTest.groovy | 36 ++++++++++++++++++- ...java => AnalyticsSampleRateDecorator.java} | 8 ++--- .../decorators/DDDecoratorsFactory.java | 2 +- 7 files changed, 85 insertions(+), 15 deletions(-) rename dd-trace-ot/src/main/java/datadog/opentracing/decorators/{EventSampleRateDecorator.java => AnalyticsSampleRateDecorator.java} (57%) 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 95ca378a5f..5ae9e905bc 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 @@ -10,6 +10,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.SortedSet; +import java.util.TreeSet; import lombok.extern.slf4j.Slf4j; import org.apache.commons.io.IOUtils; import org.datadog.jmxfetch.App; @@ -116,9 +118,11 @@ public class JMXFetch { final String configs = IOUtils.toString(metricConfigsStream, StandardCharsets.UTF_8); final String[] split = configs.split("\n"); final List result = new ArrayList<>(split.length); + final SortedSet integrationName = new TreeSet<>(); for (final String config : split) { - if (Config.integrationEnabled( - Collections.singleton(config.replace(".yaml", "")), false)) { + integrationName.clear(); + integrationName.add(config.replace(".yaml", "")); + if (Config.integrationEnabled(integrationName, false)) { final URL resource = JMXFetch.class.getResource("metricconfigs/" + config); result.add(resource.getPath().split("\\.jar!/")[1]); } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java index a41a1eefd0..3fc7e93bf4 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java @@ -11,10 +11,10 @@ import datadog.trace.api.Config; import java.security.ProtectionDomain; import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import lombok.extern.slf4j.Slf4j; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.description.method.MethodDescription; @@ -39,7 +39,7 @@ public interface Instrumenter { @Slf4j abstract class Default implements Instrumenter { - private final Set instrumentationNames; + private final SortedSet instrumentationNames; private final String instrumentationPrimaryName; private final InstrumentationContextProvider contextProvider; protected final boolean enabled; @@ -48,7 +48,7 @@ public interface Instrumenter { getClass().getPackage() == null ? "" : getClass().getPackage().getName(); public Default(final String instrumentationName, final String... additionalNames) { - instrumentationNames = new HashSet<>(Arrays.asList(additionalNames)); + instrumentationNames = new TreeSet<>(Arrays.asList(additionalNames)); instrumentationNames.add(instrumentationName); instrumentationPrimaryName = instrumentationName; 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 7ef08f41fa..746367542c 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 @@ -6,7 +6,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; -import java.util.Set; +import java.util.SortedSet; import java.util.UUID; import java.util.regex.Pattern; import lombok.Getter; @@ -44,6 +44,7 @@ public class Config { public static final String GLOBAL_TAGS = "trace.global.tags"; public static final String SPAN_TAGS = "trace.span.tags"; public static final String JMX_TAGS = "trace.jmx.tags"; + public static final String TRACE_ANALYTICS_ENABLED = "trace.analytics.enabled"; public static final String TRACE_ANNOTATIONS = "trace.annotations"; public static final String TRACE_METHODS = "trace.methods"; public static final String HEADER_TAGS = "trace.header.tags"; @@ -260,7 +261,7 @@ public class Config { } public static boolean integrationEnabled( - final Set integrationNames, final boolean defaultEnabled) { + 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; @@ -276,6 +277,24 @@ public class Config { return anyEnabled; } + public static boolean traceAnalyticsIntegrationEnabled( + 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( + "integration." + name + ".analytics.enabled", defaultEnabled); + if (defaultEnabled) { + anyEnabled &= configEnabled; + } else { + anyEnabled |= configEnabled; + } + } + return anyEnabled; + } + /** * Helper method that takes the name, adds a "dd." prefix then checks for System Properties of * that name. If none found, the name is converted to an Environment Variable and used to check @@ -316,6 +335,18 @@ public class Config { return value == null ? defaultValue : Boolean.valueOf(value); } + /** + * Calls {@link #getSettingFromEnvironment(String, String)} and converts the result to a Float. + * + * @param name + * @param defaultValue + * @return + */ + public static Float getFloatSettingFromEnvironment(final String name, final Float defaultValue) { + final String value = getSettingFromEnvironment(name, null); + return value == null ? defaultValue : Float.valueOf(value); + } + private static Integer getIntegerSettingFromEnvironment( final String name, final Integer defaultValue) { final String value = getSettingFromEnvironment(name, null); diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java index 6707a0d006..f0931da771 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/DDTags.java @@ -14,5 +14,6 @@ public class DDTags { public static final String ERROR_TYPE = "error.type"; // string representing the type of the error public static final String ERROR_STACK = "error.stack"; // human readable version of the stack - public static final String EVENT_SAMPLE_RATE = "_dd1.sr.eausr"; + public static final String ANALYTICS_SAMPLE_RATE = "_dd1.sr.eausr"; + @Deprecated public static final String EVENT_SAMPLE_RATE = ANALYTICS_SAMPLE_RATE; } 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 dda81db435..f74e2d1fce 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 @@ -312,7 +312,41 @@ class ConfigTest extends Specification { ["test-prop", "disabled-prop"] | true | false ["disabled-env", "test-env"] | true | false - integrationNames = names.toSet() + integrationNames = new TreeSet<>(names) + } + + def "verify integration trace analytics config"() { + setup: + environmentVariables.set("DD_INTEGRATION_ORDER_ANALYTICS_ENABLED", "false") + environmentVariables.set("DD_INTEGRATION_TEST_ENV_ANALYTICS_ENABLED", "true") + environmentVariables.set("DD_INTEGRATION_DISABLED_ENV_ANALYTICS_ENABLED", "false") + + System.setProperty("dd.integration.order.analytics.enabled", "true") + System.setProperty("dd.integration.test-prop.analytics.enabled", "true") + System.setProperty("dd.integration.disabled-prop.analytics.enabled", "false") + + expect: + Config.traceAnalyticsIntegrationEnabled(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 mapping configs on tracer"() { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/EventSampleRateDecorator.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/AnalyticsSampleRateDecorator.java similarity index 57% rename from dd-trace-ot/src/main/java/datadog/opentracing/decorators/EventSampleRateDecorator.java rename to dd-trace-ot/src/main/java/datadog/opentracing/decorators/AnalyticsSampleRateDecorator.java index cb5c62aaec..f53b115a70 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/EventSampleRateDecorator.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/AnalyticsSampleRateDecorator.java @@ -3,16 +3,16 @@ package datadog.opentracing.decorators; import datadog.opentracing.DDSpanContext; import datadog.trace.api.DDTags; -public class EventSampleRateDecorator extends AbstractDecorator { - public EventSampleRateDecorator() { +public class AnalyticsSampleRateDecorator extends AbstractDecorator { + public AnalyticsSampleRateDecorator() { super(); - setMatchingTag(DDTags.EVENT_SAMPLE_RATE); + setMatchingTag(DDTags.ANALYTICS_SAMPLE_RATE); } @Override public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) { if (value instanceof Number) { - context.setMetric(DDTags.EVENT_SAMPLE_RATE, (Number) value); + context.setMetric(DDTags.ANALYTICS_SAMPLE_RATE, (Number) value); } return false; } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java index c2b522958c..dc87c538c8 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java @@ -11,7 +11,7 @@ public class DDDecoratorsFactory { new DBStatementAsResourceName(), new DBTypeDecorator(), new ErrorFlag(), - new EventSampleRateDecorator(), + new AnalyticsSampleRateDecorator(), new OperationDecorator(), new PeerServiceDecorator(), new ResourceNameDecorator(),