From d95309b9cd02114511848eb62713f7e98ecdb4be Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 8 Jan 2018 16:45:24 +1000 Subject: [PATCH] Allow setting host/port without specifying writer type. --- .../com/datadoghq/trace/DDTraceConfig.java | 16 ++++-- .../java/com/datadoghq/trace/DDTracer.java | 14 ++--- .../datadoghq/trace/sampling/AllSampler.java | 7 ++- .../com/datadoghq/trace/sampling/Sampler.java | 12 +++-- .../com/datadoghq/trace/writer/DDApi.java | 3 +- .../datadoghq/trace/writer/ListWriter.java | 5 ++ .../datadoghq/trace/writer/LoggingWriter.java | 5 ++ .../com/datadoghq/trace/writer/Writer.java | 16 ++++-- .../datadoghq/trace/DDTraceConfigTest.groovy | 42 +++++++++++---- .../com/datadoghq/trace/ServiceTest.groovy | 2 +- .../test/java/ExampleWithLoggingWriter.java | 37 ------------- .../com/datadoghq/trace/DDTracerTest.java | 2 +- .../sampling/ExampleWithDDAgentWriter.java | 52 ------------------- 13 files changed, 89 insertions(+), 124 deletions(-) delete mode 100644 dd-trace/src/test/java/ExampleWithLoggingWriter.java delete mode 100644 dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java diff --git a/dd-trace/src/main/java/com/datadoghq/trace/DDTraceConfig.java b/dd-trace/src/main/java/com/datadoghq/trace/DDTraceConfig.java index 9e45be4cd7..c104d5b412 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/DDTraceConfig.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/DDTraceConfig.java @@ -1,15 +1,19 @@ package com.datadoghq.trace; +import com.datadoghq.trace.sampling.Sampler; +import com.datadoghq.trace.writer.DDAgentWriter; +import com.datadoghq.trace.writer.Writer; import java.util.Properties; /** - * Config gives priority to system properties and falls back to environment variables. + * Config gives priority to system properties and falls back to environment variables. It also + * includes default values to ensure a valid config. * *

System properties are {@link DDTraceConfig#PREFIX}'ed. Environment variables are the same as * the system property, but uppercased with '.' -> '_'. */ public class DDTraceConfig extends Properties { - /** Config keys bel */ + /** Config keys below */ private static final String PREFIX = "dd."; public static final String SERVICE_NAME = "service.name"; @@ -31,9 +35,13 @@ public class DDTraceConfig extends Properties { final Properties defaults = new Properties(); defaults.setProperty(SERVICE_NAME, DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME); + defaults.setProperty(WRITER_TYPE, Writer.DD_AGENT_WRITER_TYPE); + defaults.setProperty(AGENT_HOST, DDAgentWriter.DEFAULT_HOSTNAME); + defaults.setProperty(AGENT_PORT, String.valueOf(DDAgentWriter.DEFAULT_PORT)); + defaults.setProperty(SAMPLER_TYPE, Sampler.ALL_SAMPLER_TYPE); + defaults.setProperty(SAMPLER_RATE, "1.0"); super.defaults = defaults; - final Properties baseValues = new Properties(defaults); setIfNotNull(SERVICE_NAME, serviceName); setIfNotNull(WRITER_TYPE, writerType); setIfNotNull(AGENT_HOST, agentHost); @@ -43,7 +51,7 @@ public class DDTraceConfig extends Properties { } public DDTraceConfig(final String serviceName) { - super(); + this(); put(SERVICE_NAME, serviceName); } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/DDTracer.java b/dd-trace/src/main/java/com/datadoghq/trace/DDTracer.java index 56d1a8a029..9a0a7ee286 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/DDTracer.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/DDTracer.java @@ -6,7 +6,6 @@ import com.datadoghq.trace.propagation.HTTPCodec; import com.datadoghq.trace.resolver.DDDecoratorsFactory; import com.datadoghq.trace.sampling.AllSampler; import com.datadoghq.trace.sampling.Sampler; -import com.datadoghq.trace.writer.DDAgentWriter; import com.datadoghq.trace.writer.Writer; import com.fasterxml.jackson.annotation.JsonIgnore; import io.opentracing.ActiveSpan; @@ -30,17 +29,14 @@ import lombok.extern.slf4j.Slf4j; public class DDTracer extends ThreadLocalActiveSpanSource implements io.opentracing.Tracer { public static final String UNASSIGNED_DEFAULT_SERVICE_NAME = "unnamed-java-app"; - public static final Writer UNASSIGNED_WRITER = new DDAgentWriter(); - public static final Sampler UNASSIGNED_SAMPLER = new AllSampler(); + /** Default service name if none provided on the trace or span */ + final String serviceName; /** Writer is an charge of reporting traces and spans to the desired endpoint */ final Writer writer; /** Sampler defines the sampling policy in order to reduce the number of traces for instance */ final Sampler sampler; - /** Default service name if none provided on the trace or span */ - final String serviceName; - /** Span context decorators */ private final Map> spanContextDecorators = new HashMap<>(); @@ -83,11 +79,7 @@ public class DDTracer extends ThreadLocalActiveSpanSource implements io.opentrac } public DDTracer(final Writer writer) { - this(writer, new AllSampler()); - } - - public DDTracer(final Writer writer, final Sampler sampler) { - this(UNASSIGNED_DEFAULT_SERVICE_NAME, writer, sampler); + this(UNASSIGNED_DEFAULT_SERVICE_NAME, writer, new AllSampler()); } /** diff --git a/dd-trace/src/main/java/com/datadoghq/trace/sampling/AllSampler.java b/dd-trace/src/main/java/com/datadoghq/trace/sampling/AllSampler.java index 967e9a2bc3..b4f25cfb9a 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/sampling/AllSampler.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/sampling/AllSampler.java @@ -6,7 +6,12 @@ import com.datadoghq.trace.DDBaseSpan; public class AllSampler extends AbstractSampler { @Override - public boolean doSample(DDBaseSpan span) { + public boolean doSample(final DDBaseSpan span) { return true; } + + @Override + public String toString() { + return "AllSampler { sample=true }"; + } } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/sampling/Sampler.java b/dd-trace/src/main/java/com/datadoghq/trace/sampling/Sampler.java index a811e1bd28..725b9c33b8 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/sampling/Sampler.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/sampling/Sampler.java @@ -2,8 +2,8 @@ package com.datadoghq.trace.sampling; import com.datadoghq.trace.DDBaseSpan; import com.datadoghq.trace.DDTraceConfig; -import com.datadoghq.trace.DDTracer; import java.util.Properties; +import lombok.extern.slf4j.Slf4j; /** Main interface to sample a collection of traces. */ public interface Sampler { @@ -18,6 +18,7 @@ public interface Sampler { */ boolean sample(DDBaseSpan span); + @Slf4j final class Builder { public static Sampler forConfig(final Properties config) { final Sampler sampler; @@ -29,10 +30,15 @@ public interface Sampler { } else if (ALL_SAMPLER_TYPE.equals(configuredType)) { sampler = new AllSampler(); } else { - sampler = DDTracer.UNASSIGNED_SAMPLER; + log.warn( + "Sampler type not configured correctly: Type {} not recognized. Defaulting to AllSampler.", + configuredType); + sampler = new AllSampler(); } } else { - sampler = DDTracer.UNASSIGNED_SAMPLER; + log.warn( + "Sampler type not configured correctly: No config provided! Defaulting to AllSampler."); + sampler = new AllSampler(); } return sampler; } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/writer/DDApi.java b/dd-trace/src/main/java/com/datadoghq/trace/writer/DDApi.java index 2d0c09b2cd..fe09121cc5 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/writer/DDApi.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/writer/DDApi.java @@ -103,9 +103,10 @@ public class DDApi { log.debug("Error while sending " + size + " " + type + " to the DD agent.", e); } else if (loggingRateLimiter.tryAcquire()) { log.warn( - "Error while sending {} {} to the DD agent. Message: {} (going silent for {} seconds)", + "Error while sending {} {} to the DD agent. {}: {} (going silent for {} seconds)", size, type, + e.getClass().getName(), e.getMessage(), SECONDS_BETWEEN_ERROR_LOG); } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/writer/ListWriter.java b/dd-trace/src/main/java/com/datadoghq/trace/writer/ListWriter.java index 9dc1f6b3f3..4d8e6fc6a6 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/writer/ListWriter.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/writer/ListWriter.java @@ -63,4 +63,9 @@ public class ListWriter extends CopyOnWriteArrayList>> implem latches.clear(); } } + + @Override + public String toString() { + return "ListWriter { size=" + this.size() + " }"; + } } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/writer/LoggingWriter.java b/dd-trace/src/main/java/com/datadoghq/trace/writer/LoggingWriter.java index 2cdbf1610e..4dd867b3b4 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/writer/LoggingWriter.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/writer/LoggingWriter.java @@ -30,4 +30,9 @@ public class LoggingWriter implements Writer { public void start() { log.info("start()"); } + + @Override + public String toString() { + return "LoggingWriter { }"; + } } diff --git a/dd-trace/src/main/java/com/datadoghq/trace/writer/Writer.java b/dd-trace/src/main/java/com/datadoghq/trace/writer/Writer.java index 38b9c29b33..6cd0893a05 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/writer/Writer.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/writer/Writer.java @@ -2,11 +2,11 @@ package com.datadoghq.trace.writer; import com.datadoghq.trace.DDBaseSpan; import com.datadoghq.trace.DDTraceConfig; -import com.datadoghq.trace.DDTracer; import com.datadoghq.trace.Service; import java.util.List; import java.util.Map; import java.util.Properties; +import lombok.extern.slf4j.Slf4j; /** A writer is responsible to send collected spans to some place */ public interface Writer { @@ -36,6 +36,7 @@ public interface Writer { */ void close(); + @Slf4j final class Builder { public static Writer forConfig(final Properties config) { final Writer writer; @@ -51,10 +52,19 @@ public interface Writer { } else if (LOGGING_WRITER_TYPE.equals(configuredType)) { writer = new LoggingWriter(); } else { - writer = DDTracer.UNASSIGNED_WRITER; + log.warn( + "Writer type not configured correctly: Type {} not recognized. Defaulting to DDAgentWriter.", + configuredType); + writer = + new DDAgentWriter( + new DDApi( + config.getProperty(DDTraceConfig.AGENT_HOST), + Integer.parseInt(config.getProperty(DDTraceConfig.AGENT_PORT)))); } } else { - writer = DDTracer.UNASSIGNED_WRITER; + log.warn( + "Writer type not configured correctly: No config provided! Defaulting to DDAgentWriter."); + writer = new DDAgentWriter(); } return writer; 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 2f0c2b1a31..2d3a3f2ec7 100644 --- a/dd-trace/src/test/groovy/com/datadoghq/trace/DDTraceConfigTest.groovy +++ b/dd-trace/src/test/groovy/com/datadoghq/trace/DDTraceConfigTest.groovy @@ -6,6 +6,7 @@ import com.datadoghq.trace.writer.DDAgentWriter import com.datadoghq.trace.writer.ListWriter import com.datadoghq.trace.writer.LoggingWriter import spock.lang.Specification +import spock.lang.Unroll import java.lang.reflect.Field import java.lang.reflect.Modifier @@ -67,22 +68,22 @@ class DDTraceConfigTest extends Specification { then: config.getProperty(SERVICE_NAME) == "unnamed-java-app" - config.getProperty(WRITER_TYPE) == null - config.getProperty(AGENT_HOST) == null - config.getProperty(AGENT_PORT) == null - config.getProperty(SAMPLER_TYPE) == null - config.getProperty(SAMPLER_RATE) == null + config.getProperty(WRITER_TYPE) == "DDAgentWriter" + config.getProperty(AGENT_HOST) == "localhost" + config.getProperty(AGENT_PORT) == "8126" + config.getProperty(SAMPLER_TYPE) == "AllSampler" + config.getProperty(SAMPLER_RATE) == "1.0" when: config = new DDTraceConfig("A different service name") then: config.getProperty(SERVICE_NAME) == "A different service name" - config.getProperty(WRITER_TYPE) == null - config.getProperty(AGENT_HOST) == null - config.getProperty(AGENT_PORT) == null - config.getProperty(SAMPLER_TYPE) == null - config.getProperty(SAMPLER_RATE) == null + config.getProperty(WRITER_TYPE) == "DDAgentWriter" + config.getProperty(AGENT_HOST) == "localhost" + config.getProperty(AGENT_PORT) == "8126" + config.getProperty(SAMPLER_TYPE) == "AllSampler" + config.getProperty(SAMPLER_RATE) == "1.0" } def "specify overrides via system properties"() { @@ -144,4 +145,25 @@ class DDTraceConfigTest extends Specification { tracer.spanContextDecorators.size() == 2 } + + @Unroll + def "verify single override on #source for #key"() { + when: + System.setProperty(PREFIX + key, value) + def tracer = new DDTracer() + + then: + tracer."$source".toString() == expected + + where: + + source | key | value | expected + "writer" | "default" | "default" | "DDAgentWriter { api=DDApi { tracesEndpoint=http://localhost:8126/v0.3/traces } }" + "writer" | "writer.type" | "LoggingWriter" | "LoggingWriter { }" + "writer" | "agent.host" | "somethingelse" | "DDAgentWriter { api=DDApi { tracesEndpoint=http://somethingelse:8126/v0.3/traces } }" + "writer" | "agent.port" | "9999" | "DDAgentWriter { api=DDApi { tracesEndpoint=http://localhost:9999/v0.3/traces } }" + "sampler" | "default" | "default" | "AllSampler { sample=true }" + "sampler" | "sampler.type" | "RateSampler" | "RateSampler { sampleRate=1.0 }" + "sampler" | "sampler.rate" | "100" | "AllSampler { sample=true }" + } } diff --git a/dd-trace/src/test/groovy/com/datadoghq/trace/ServiceTest.groovy b/dd-trace/src/test/groovy/com/datadoghq/trace/ServiceTest.groovy index cc6248bee0..b2799b30e5 100644 --- a/dd-trace/src/test/groovy/com/datadoghq/trace/ServiceTest.groovy +++ b/dd-trace/src/test/groovy/com/datadoghq/trace/ServiceTest.groovy @@ -54,7 +54,7 @@ class ServiceTest extends Specification { setup: def writer = spy(new DDAgentWriter()) - def tracer = new DDTracer(writer, new AllSampler()) + def tracer = new DDTracer(DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME, writer, new AllSampler()) when: diff --git a/dd-trace/src/test/java/ExampleWithLoggingWriter.java b/dd-trace/src/test/java/ExampleWithLoggingWriter.java deleted file mode 100644 index aed050d50c..0000000000 --- a/dd-trace/src/test/java/ExampleWithLoggingWriter.java +++ /dev/null @@ -1,37 +0,0 @@ -import com.datadoghq.trace.DDTracer; -import com.datadoghq.trace.Service; -import com.datadoghq.trace.sampling.AllSampler; -import com.datadoghq.trace.writer.LoggingWriter; -import io.opentracing.Span; - -public class ExampleWithLoggingWriter { - - public static void main(final String[] args) throws Exception { - - final DDTracer tracer = new DDTracer(new LoggingWriter(), new AllSampler()); - tracer.addServiceInfo(new Service("api-intake", "spark", Service.AppType.CACHE)); - - final Span parent = - tracer.buildSpan("fetch.backend").withServiceName("api-intake").startManual(); - - parent.setBaggageItem("scope-id", "a-1337"); - - Thread.sleep(100); - - final Span child = - tracer - .buildSpan("delete.resource") - .asChildOf(parent) - .withResourceName("delete") - .startManual(); - - Thread.sleep(100); - - child.finish(); - - Thread.sleep(100); - - parent.finish(); - tracer.close(); - } -} diff --git a/dd-trace/src/test/java/com/datadoghq/trace/DDTracerTest.java b/dd-trace/src/test/java/com/datadoghq/trace/DDTracerTest.java index c52574404c..bf507c65d2 100644 --- a/dd-trace/src/test/java/com/datadoghq/trace/DDTracerTest.java +++ b/dd-trace/src/test/java/com/datadoghq/trace/DDTracerTest.java @@ -30,7 +30,7 @@ public class DDTracerTest { spans.add(span); spans.add(span); - final DDTracer tracer = new DDTracer(writer, sampler); + final DDTracer tracer = new DDTracer(DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME, writer, sampler); tracer.write(spans); tracer.write(spans); diff --git a/dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java b/dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java deleted file mode 100644 index fb60275ce0..0000000000 --- a/dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java +++ /dev/null @@ -1,52 +0,0 @@ -package com.datadoghq.trace.sampling; - -import com.datadoghq.trace.DDTracer; -import com.datadoghq.trace.writer.DDAgentWriter; -import com.datadoghq.trace.writer.Writer; -import io.opentracing.Span; - -public class ExampleWithDDAgentWriter { - - public static void main(final String[] args) throws Exception { - - // Instantiate the DDWriter - // By default, traces are written to localhost:8126 (the ddagent) - final Writer writer = new DDAgentWriter(); - - // Instantiate the proper Sampler - // - RateSampler if you want to keep `ratio` traces - // - AllSampler to keep all traces - final Sampler sampler = new AllSampler(); - - // Create the tracer - final DDTracer tracer = new DDTracer(writer, sampler); - - final Span parent = - tracer - .buildSpan("hello-world") - .withServiceName("my-service-name") - .withSpanType("web") - .startManual(); - - Thread.sleep(100); - - parent.setBaggageItem("a-baggage", "value"); - - final Span child = - tracer - .buildSpan("hello-world") - .asChildOf(parent) - .withResourceName("my-resource-name") - .startManual(); - - Thread.sleep(100); - - child.finish(); - - Thread.sleep(100); - - parent.finish(); - - writer.close(); - } -}