From 177478bb2555a9e5d5e5769ebec622674b53a0f1 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 4 Jan 2018 15:07:33 +1000 Subject: [PATCH] Remove deprecated yaml config In favor of configuration via env vars and sys props. --- dd-trace/dd-trace.gradle | 8 +- .../trace/integration/URLAsResourceName.java | 11 +- .../trace/resolver/DDDecoratorsFactory.java | 9 +- .../trace/resolver/DDSpanDecoratorConfig.java | 67 --------- .../trace/resolver/DDTracerFactory.java | 120 ----------------- .../trace/resolver/DDTracerResolver.java | 12 +- .../trace/resolver/DecoratorsConfig.java | 92 +++++++++++++ .../trace/resolver/FactoryUtils.java | 63 --------- .../trace/resolver/TracerConfig.java | 127 ------------------ .../datadoghq/trace/DDTraceConfigTest.groovy | 2 + 10 files changed, 109 insertions(+), 402 deletions(-) delete mode 100644 dd-trace/src/main/java/com/datadoghq/trace/resolver/DDSpanDecoratorConfig.java delete mode 100644 dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerFactory.java create mode 100644 dd-trace/src/main/java/com/datadoghq/trace/resolver/DecoratorsConfig.java delete mode 100644 dd-trace/src/main/java/com/datadoghq/trace/resolver/TracerConfig.java diff --git a/dd-trace/dd-trace.gradle b/dd-trace/dd-trace.gradle index b71047281b..698b33fd17 100644 --- a/dd-trace/dd-trace.gradle +++ b/dd-trace/dd-trace.gradle @@ -10,11 +10,9 @@ apply from: "${rootDir}/gradle/jacoco.gradle" minimumBranchCoverage = 0.3 minimumInstructionCoverage = 0.5 whitelistedInstructionClasses += whitelistedBranchClasses += [ - "com.datadoghq.trace.integration.*", - 'com.datadoghq.trace.resolver.TracerConfig', - 'com.datadoghq.trace.resolver.WriterConfig', - 'com.datadoghq.trace.resolver.DDTracerFactory', - 'com.datadoghq.trace.resolver.SamplerConfig', + 'com.datadoghq.trace.integration.*', + 'com.datadoghq.trace.resolver.DecoratorsConfig', + 'com.datadoghq.trace.writer.ListWriter', 'com.datadoghq.trace.DDTags', 'com.datadoghq.trace.DDTraceInfo', 'com.datadoghq.trace.util.Clock', diff --git a/dd-trace/src/main/java/com/datadoghq/trace/integration/URLAsResourceName.java b/dd-trace/src/main/java/com/datadoghq/trace/integration/URLAsResourceName.java index 5273c6fc35..7f06c910c7 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/integration/URLAsResourceName.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/integration/URLAsResourceName.java @@ -2,9 +2,8 @@ package com.datadoghq.trace.integration; import com.datadoghq.trace.DDSpanContext; import com.datadoghq.trace.DDTags; -import com.datadoghq.trace.resolver.DDTracerFactory; import com.datadoghq.trace.resolver.FactoryUtils; -import com.datadoghq.trace.resolver.TracerConfig; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import io.opentracing.tag.Tags; import java.net.MalformedURLException; import java.util.ArrayList; @@ -13,13 +12,14 @@ import java.util.Objects; /** Decorator for servlet contrib */ public class URLAsResourceName extends AbstractDecorator { + public static final String CONFIG_PATH = "dd-trace"; public static final Config.Rule RULE_QPARAM = new Config.Rule("\\?.*$", ""); public static final Config.Rule RULE_DIGIT = new Config.Rule("\\d+", "?"); private List patterns = new ArrayList<>(); public URLAsResourceName() { - this(DDTracerFactory.CONFIG_PATH); + this(CONFIG_PATH); } public URLAsResourceName(final String configPath) { @@ -102,8 +102,9 @@ public class URLAsResourceName extends AbstractDecorator { this.patterns = patterns; } - /** Additional properties concerning the UrlAsResourceDecorator in the YAML config */ - public static class Config extends TracerConfig { + /** Properties concerning the UrlAsResourceDecorator in the YAML config */ + @JsonIgnoreProperties(ignoreUnknown = true) + public static class Config { List urlResourcePatterns; diff --git a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDDecoratorsFactory.java b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDDecoratorsFactory.java index bc6e361a95..363fe08540 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDDecoratorsFactory.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDDecoratorsFactory.java @@ -18,9 +18,10 @@ public class DDDecoratorsFactory { * @param decoratorsConfig * @return the list of instanciated and configured decorators */ - public static List create(final List decoratorsConfig) { + public static List create( + final List decoratorsConfig) { final List decorators = new ArrayList<>(); - for (final DDSpanDecoratorConfig decoratorConfig : decoratorsConfig) { + for (final DecoratorsConfig.DDSpanDecoratorConfig decoratorConfig : decoratorsConfig) { if (decoratorConfig.getType() == null) { log.warn("Cannot create decorator without type from configuration {}", decoratorConfig); continue; @@ -68,8 +69,8 @@ public class DDDecoratorsFactory { public static List createFromResources() { List result = new ArrayList<>(); - final TracerConfig config = - FactoryUtils.loadConfigFromResource(CONFIG_PATH, TracerConfig.class); + final DecoratorsConfig config = + FactoryUtils.loadConfigFromResource(CONFIG_PATH, DecoratorsConfig.class); if (config != null) { result = DDDecoratorsFactory.create(config.getDecorators()); } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDSpanDecoratorConfig.java b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDSpanDecoratorConfig.java deleted file mode 100644 index bd159c9eb7..0000000000 --- a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDSpanDecoratorConfig.java +++ /dev/null @@ -1,67 +0,0 @@ -package com.datadoghq.trace.resolver; - -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; - -public class DDSpanDecoratorConfig { - - private String type; - - private String matchingTag; - - private String matchingValue; - - private String setTag; - - private String setValue; - - public String getMatchingTag() { - return matchingTag; - } - - public void setMatchingTag(String matchingTag) { - this.matchingTag = matchingTag; - } - - public String getMatchingValue() { - return matchingValue; - } - - public void setMatchingValue(String matchingValue) { - this.matchingValue = matchingValue; - } - - public String getSetTag() { - return setTag; - } - - public void setSetTag(String setTag) { - this.setTag = setTag; - } - - public String getSetValue() { - return setValue; - } - - public void setSetValue(String setValue) { - this.setValue = setValue; - } - - public void setType(String type) { - this.type = type; - } - - public String getType() { - return type; - } - - @Override - public String toString() { - try { - return new ObjectMapper(new YAMLFactory()).writeValueAsString(this); - } catch (JsonProcessingException e) { - return null; - } - } -} diff --git a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerFactory.java b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerFactory.java deleted file mode 100644 index 620af20fcf..0000000000 --- a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerFactory.java +++ /dev/null @@ -1,120 +0,0 @@ -package com.datadoghq.trace.resolver; - -import com.datadoghq.trace.DDTracer; -import com.datadoghq.trace.integration.AbstractDecorator; -import com.datadoghq.trace.sampling.AbstractSampler; -import com.datadoghq.trace.sampling.AllSampler; -import com.datadoghq.trace.sampling.RateSampler; -import com.datadoghq.trace.sampling.Sampler; -import com.datadoghq.trace.writer.DDAgentWriter; -import com.datadoghq.trace.writer.DDApi; -import com.datadoghq.trace.writer.LoggingWriter; -import com.datadoghq.trace.writer.Writer; -import java.util.List; -import java.util.Map; -import java.util.regex.Pattern; -import lombok.extern.slf4j.Slf4j; - -/** Create a tracer from a configuration file */ -@Slf4j -public class DDTracerFactory { - - public static final String SYSTEM_PROPERTY_CONFIG_PATH = "dd.trace.configurationFile"; - public static final String CONFIG_PATH = "dd-trace"; - - private static final String DD_AGENT_WRITER_TYPE = DDAgentWriter.class.getSimpleName(); - private static final String LOGGING_WRITER_TYPE = LoggingWriter.class.getSimpleName(); - private static final String ALL_SAMPLER_TYPE = AllSampler.class.getSimpleName(); - private static final String RATE_SAMPLER_TYPE = RateSampler.class.getSimpleName(); - - /** - * Create a tracer from a TracerConfig object - * - * @param config - * @return the corresponding tracer - */ - static DDTracer create(final TracerConfig config) { - final String defaultServiceName = - config.getDefaultServiceName() != null - ? config.getDefaultServiceName() - : DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME; - - // Create writer - final Writer writer; - - if (config.getWriter() != null) { - final WriterConfig c = config.getWriter(); - if (DD_AGENT_WRITER_TYPE.equals(c.getType())) { - writer = - new DDAgentWriter( - new DDApi( - c.getHost(DDAgentWriter.DEFAULT_HOSTNAME), - c.getPort(DDAgentWriter.DEFAULT_PORT))); - } else if (LOGGING_WRITER_TYPE.equals(c.getType())) { - writer = new LoggingWriter(); - } else { - writer = DDTracer.UNASSIGNED_WRITER; - } - } else { - writer = DDTracer.UNASSIGNED_WRITER; - } - - // Create sampler - final Sampler sampler; - - if (config.getSampler() != null) { - if (RATE_SAMPLER_TYPE.equals(config.getSampler().getType())) { - sampler = new RateSampler(config.getSampler().getRate()); - } else if (ALL_SAMPLER_TYPE.equals(config.getSampler().getType())) { - sampler = new AllSampler(); - } else { - sampler = DDTracer.UNASSIGNED_SAMPLER; - } - - } else { - sampler = DDTracer.UNASSIGNED_SAMPLER; - } - - // Add sampled tags - final Map skipTagsPatterns = config.getSampler().getSkipTagsPatterns(); - if (skipTagsPatterns != null && sampler instanceof AbstractSampler) { - final AbstractSampler aSampler = (AbstractSampler) sampler; - for (final Map.Entry entry : skipTagsPatterns.entrySet()) { - aSampler.addSkipTagPattern(entry.getKey(), Pattern.compile(entry.getValue())); - } - } - - // Create tracer - return new DDTracer(defaultServiceName, writer, sampler); - } - - @Deprecated - public static DDTracer createFromConfigurationFile() { - final TracerConfig tracerConfig = - FactoryUtils.loadConfigFromFilePropertyOrResource( - SYSTEM_PROPERTY_CONFIG_PATH, CONFIG_PATH, TracerConfig.class); - - log.warn("DDTracerFactory is deprecated and will be removed in the next release."); - log.warn("Use the constructor on DDTrace directly and env vars/sys props for config."); - - DDTracer tracer = null; - log.trace("Tracer configuration: \n{}", tracerConfig); - if (tracerConfig == null) { - log.info("No valid configuration file {} found. Loading default tracer.", CONFIG_PATH); - tracer = new DDTracer(); - } else { - log.debug("Create a tracer instance from the configuration"); - - tracer = DDTracerFactory.create(tracerConfig); - } - - // Create decorators from resource files - final List decorators = DDDecoratorsFactory.createFromResources(); - for (final AbstractDecorator decorator : decorators) { - log.debug("Loading decorator: {}", decorator.getClass().getSimpleName()); - tracer.addDecorator(decorator); - } - - return tracer; - } -} diff --git a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerResolver.java b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerResolver.java index 875865e8e3..c57ebbc22f 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerResolver.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DDTracerResolver.java @@ -29,16 +29,6 @@ public class DDTracerResolver extends TracerResolver { protected Tracer resolve() { log.info("Creating the Datadog Tracer from the resolver"); - final TracerConfig tracerConfig = - FactoryUtils.loadConfigFromFilePropertyOrResource( - DDTracerFactory.SYSTEM_PROPERTY_CONFIG_PATH, - DDTracerFactory.CONFIG_PATH, - TracerConfig.class); - - if (tracerConfig != null) { - return DDTracerFactory.createFromConfigurationFile(); - } else { - return new DDTracer(); - } + return new DDTracer(); } } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/resolver/DecoratorsConfig.java b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DecoratorsConfig.java new file mode 100644 index 0000000000..815747726b --- /dev/null +++ b/dd-trace/src/main/java/com/datadoghq/trace/resolver/DecoratorsConfig.java @@ -0,0 +1,92 @@ +package com.datadoghq.trace.resolver; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import java.util.List; + +@JsonIgnoreProperties(ignoreUnknown = true) +class DecoratorsConfig { + private List decorators; + + public List getDecorators() { + return decorators; + } + + public void setDecorators(final List decorators) { + this.decorators = decorators; + } + + @Override + public String toString() { + try { + return new ObjectMapper(new YAMLFactory()).writeValueAsString(this); + } catch (final JsonProcessingException e) { + // FIXME better toString() while config object stabilized + return null; + } + } + + static class DDSpanDecoratorConfig { + + private String type; + + private String matchingTag; + + private String matchingValue; + + private String setTag; + + private String setValue; + + public String getMatchingTag() { + return matchingTag; + } + + public void setMatchingTag(final String matchingTag) { + this.matchingTag = matchingTag; + } + + public String getMatchingValue() { + return matchingValue; + } + + public void setMatchingValue(final String matchingValue) { + this.matchingValue = matchingValue; + } + + public String getSetTag() { + return setTag; + } + + public void setSetTag(final String setTag) { + this.setTag = setTag; + } + + public String getSetValue() { + return setValue; + } + + public void setSetValue(final String setValue) { + this.setValue = setValue; + } + + public void setType(final String type) { + this.type = type; + } + + public String getType() { + return type; + } + + @Override + public String toString() { + try { + return new ObjectMapper(new YAMLFactory()).writeValueAsString(this); + } catch (final JsonProcessingException e) { + return null; + } + } + } +} diff --git a/dd-trace/src/main/java/com/datadoghq/trace/resolver/FactoryUtils.java b/dd-trace/src/main/java/com/datadoghq/trace/resolver/FactoryUtils.java index 288573cd9e..af86f24e49 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/resolver/FactoryUtils.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/resolver/FactoryUtils.java @@ -1,9 +1,7 @@ package com.datadoghq.trace.resolver; -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; -import java.io.File; import java.io.IOException; import java.net.URL; import lombok.extern.slf4j.Slf4j; @@ -13,38 +11,6 @@ public class FactoryUtils { private static final ObjectMapper objectMapper = new ObjectMapper(new YAMLFactory()); - public static A loadConfigFromFilePropertyOrResource( - final String systemProperty, final String resourceName, final Class targetClass) { - final String filePath = System.getProperty(systemProperty); - if (filePath != null) { - try { - log.info("Loading config from file " + filePath); - return objectMapper.readValue(new File(filePath), targetClass); - } catch (final Exception e) { - log.error( - "Cannot read provided configuration file " + filePath + ". Using the default one.", e); - } - } - - return loadConfigFromResource(resourceName, targetClass); - } - - public static A loadConfigFromFilePropertyOrResource( - final String systemProperty, final String resourceName, final TypeReference type) { - final String filePath = System.getProperty(systemProperty); - if (filePath != null) { - try { - log.info("Loading config from file " + filePath); - return objectMapper.readValue(new File(filePath), type); - } catch (final Exception e) { - log.error( - "Cannot read provided configuration file " + filePath + ". Using the default one.", e); - } - } - - return loadConfigFromResource(resourceName, type); - } - public static A loadConfigFromResource( final String resourceName, final Class targetClass) { A config = null; @@ -73,33 +39,4 @@ public class FactoryUtils { } return config; } - - public static A loadConfigFromResource( - final String resourceName, final TypeReference type) { - A config = null; - - // Try loading both suffixes - if (!resourceName.endsWith(".yaml") && !resourceName.endsWith(".yml")) { - config = loadConfigFromResource(resourceName + ".yaml", type); - if (config == null) { - config = loadConfigFromResource(resourceName + ".yml", type); - } - if (config != null) { - return config; - } - } - - try { - final ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); - final URL resource = classLoader.getResource(resourceName); - if (resource != null) { - log.info("Loading config from resource " + resource); - config = objectMapper.readValue(resource.openStream(), type); - } - } catch (final IOException e) { - log.warn("Could not load configuration file {}.", resourceName); - log.error("Error when loading config file", e); - } - return config; - } } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/resolver/TracerConfig.java b/dd-trace/src/main/java/com/datadoghq/trace/resolver/TracerConfig.java deleted file mode 100644 index 4fdad9d1c7..0000000000 --- a/dd-trace/src/main/java/com/datadoghq/trace/resolver/TracerConfig.java +++ /dev/null @@ -1,127 +0,0 @@ -package com.datadoghq.trace.resolver; - -import com.datadoghq.trace.writer.DDAgentWriter; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; -import java.util.List; -import java.util.Map; - -/** Tracer configuration */ -@JsonIgnoreProperties(ignoreUnknown = true) -public class TracerConfig { - - private String defaultServiceName; - private WriterConfig writer = new WriterConfig(); - private SamplerConfig sampler = new SamplerConfig(); - private List decorators; - - public String getDefaultServiceName() { - return defaultServiceName; - } - - public void setDefaultServiceName(final String defaultServiceName) { - this.defaultServiceName = defaultServiceName; - } - - public WriterConfig getWriter() { - return writer; - } - - public void setWriter(final WriterConfig writer) { - this.writer = writer; - } - - public SamplerConfig getSampler() { - return sampler; - } - - public void setSampler(final SamplerConfig sampler) { - this.sampler = sampler; - } - - public List getDecorators() { - return decorators; - } - - public void setDecorators(final List decorators) { - this.decorators = decorators; - } - - @Override - public String toString() { - try { - return new ObjectMapper(new YAMLFactory()).writeValueAsString(this); - } catch (final JsonProcessingException e) { - // FIXME better toString() while config object stabilized - return null; - } - } -} - -class SamplerConfig { - - private Double rate; - private String type = "AllSampler"; - private Map skipTagsPatterns; - - public String getType() { - return type; - } - - public Double getRate() { - return rate; - } - - public void setRate(final Double rate) { - this.rate = rate; - } - - public void setType(final String type) { - this.type = type; - } - - public Map getSkipTagsPatterns() { - return skipTagsPatterns; - } -} - -class WriterConfig { - - private String host = "localhost"; - private Integer port = 8126; - private String type = DDAgentWriter.class.getSimpleName(); - - public void setHost(final String host) { - this.host = host; - } - - public void setPort(final Integer port) { - this.port = port; - } - - public void setType(final String type) { - this.type = type; - } - - public String getHost() { - return host; - } - - public Integer getPort() { - return port; - } - - public String getType() { - return type; - } - - public String getHost(final String defaultHostname) { - return host == null ? defaultHostname : host; - } - - public Integer getPort(final int defaultPort) { - return port == null ? defaultPort : port; - } -} diff --git a/dd-trace/src/test/groovy/com/datadoghq/trace/DDTraceConfigTest.groovy b/dd-trace/src/test/groovy/com/datadoghq/trace/DDTraceConfigTest.groovy index f6b7c82aad..2f0c2b1a31 100644 --- a/dd-trace/src/test/groovy/com/datadoghq/trace/DDTraceConfigTest.groovy +++ b/dd-trace/src/test/groovy/com/datadoghq/trace/DDTraceConfigTest.groovy @@ -141,5 +141,7 @@ class DDTraceConfigTest extends Specification { tracer.serviceName == "unnamed-java-app" tracer.sampler instanceof AllSampler tracer.writer.toString() == "DDAgentWriter { api=DDApi { tracesEndpoint=http://localhost:8126/v0.3/traces } }" + + tracer.spanContextDecorators.size() == 2 } }