diff --git a/dd-trace/src/main/java/datadog/trace/tracer/Tracer.java b/dd-trace/src/main/java/datadog/trace/tracer/Tracer.java index f22b2a2913..b82a39deb9 100644 --- a/dd-trace/src/main/java/datadog/trace/tracer/Tracer.java +++ b/dd-trace/src/main/java/datadog/trace/tracer/Tracer.java @@ -3,7 +3,6 @@ package datadog.trace.tracer; import datadog.trace.api.Config; import datadog.trace.tracer.sampling.AllSampler; import datadog.trace.tracer.sampling.Sampler; -import datadog.trace.tracer.writer.LoggingWriter; import datadog.trace.tracer.writer.Writer; import java.util.ArrayList; import java.util.Collections; @@ -40,7 +39,7 @@ public class Tracer { // TODO: implement and include "standard" interceptors this( config, - new LoggingWriter(), + Writer.Builder.forConfig(config), new AllSampler(), Collections.unmodifiableList(new ArrayList<>(interceptors))); } diff --git a/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentClient.java b/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentClient.java index a9b08770b5..bd11b15c04 100644 --- a/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentClient.java +++ b/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentClient.java @@ -16,6 +16,7 @@ import java.net.URL; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; +import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.msgpack.jackson.dataformat.MessagePackFactory; @@ -41,12 +42,12 @@ class AgentClient { private static final LogRateLimiter LOG_RATE_LIMITER = new LogRateLimiter(log, MILLISECONDS_BETWEEN_ERROR_LOG); - private final URL tracesUrl; + @Getter private final URL agentUrl; AgentClient(final String host, final int port) { final String url = "http://" + host + ":" + port + TRACES_ENDPOINT; try { - tracesUrl = new URL(url); + agentUrl = new URL(url); } catch (final MalformedURLException e) { // This should essentially mean agent should bail out from installing, we cannot meaningfully // recover from this. @@ -90,7 +91,7 @@ class AgentClient { } private HttpURLConnection createHttpConnection() throws IOException { - final HttpURLConnection connection = (HttpURLConnection) tracesUrl.openConnection(); + final HttpURLConnection connection = (HttpURLConnection) agentUrl.openConnection(); connection.setDoOutput(true); connection.setDoInput(true); diff --git a/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentWriter.java b/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentWriter.java index ea2f634cc8..4141f54686 100644 --- a/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentWriter.java +++ b/dd-trace/src/main/java/datadog/trace/tracer/writer/AgentWriter.java @@ -1,6 +1,7 @@ package datadog.trace.tracer.writer; import datadog.trace.tracer.Trace; +import java.net.URL; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ArrayBlockingQueue; @@ -51,6 +52,11 @@ public class AgentWriter implements Writer { shutdownCallback = new ShutdownCallback(executorService); } + /** @return Datadog agwent URL. Visible for testing. */ + URL getAgentUrl() { + return task.getClient().getAgentUrl(); + } + @Override public void write(final Trace trace) { if (trace.isValid()) { @@ -95,7 +101,7 @@ public class AgentWriter implements Writer { private static final class TracesSendingTask implements Runnable { /** The Datadog agent client */ - private final AgentClient client; + @Getter private final AgentClient client; /** Queue size */ private final int queueSize; /** In memory collection of traces waiting for departure */ diff --git a/dd-trace/src/main/java/datadog/trace/tracer/writer/Writer.java b/dd-trace/src/main/java/datadog/trace/tracer/writer/Writer.java index 923166eed6..e26b86b815 100644 --- a/dd-trace/src/main/java/datadog/trace/tracer/writer/Writer.java +++ b/dd-trace/src/main/java/datadog/trace/tracer/writer/Writer.java @@ -1,6 +1,9 @@ package datadog.trace.tracer.writer; +import datadog.trace.api.Config; import datadog.trace.tracer.Trace; +import java.util.Properties; +import lombok.extern.slf4j.Slf4j; /** A writer sends traces to some place. */ public interface Writer { @@ -32,4 +35,41 @@ public interface Writer { * connections and tasks */ void close(); + + @Slf4j + final class Builder { + + public static Writer forConfig(final Config config) { + if (config == null) { + // There is no way config is not create so getting here must be a code bug + throw new NullPointerException("Config is required to create writer"); + } + + final Writer writer; + + final String configuredType = config.getWriterType(); + if (Config.DD_AGENT_WRITER_TYPE.equals(configuredType)) { + writer = createAgentWriter(config); + } else if (Config.LOGGING_WRITER_TYPE.equals(configuredType)) { + writer = new LoggingWriter(); + } else { + log.warn( + "Writer type not configured correctly: Type {} not recognized. Defaulting to AgentWriter.", + configuredType); + writer = createAgentWriter(config); + } + + return writer; + } + + public static Writer forConfig(final Properties config) { + return forConfig(Config.get(config)); + } + + private static Writer createAgentWriter(final Config config) { + return new AgentWriter(new AgentClient(config.getAgentHost(), config.getAgentPort())); + } + + private Builder() {} + } } diff --git a/dd-trace/src/test/groovy/datadog/trace/tracer/TracerTest.groovy b/dd-trace/src/test/groovy/datadog/trace/tracer/TracerTest.groovy index a8a9906c4a..4824baa218 100644 --- a/dd-trace/src/test/groovy/datadog/trace/tracer/TracerTest.groovy +++ b/dd-trace/src/test/groovy/datadog/trace/tracer/TracerTest.groovy @@ -2,7 +2,7 @@ package datadog.trace.tracer import datadog.trace.api.Config import datadog.trace.tracer.sampling.AllSampler -import datadog.trace.tracer.writer.LoggingWriter +import datadog.trace.tracer.writer.AgentWriter import datadog.trace.tracer.writer.SampleRateByService import datadog.trace.tracer.writer.Writer import spock.lang.Shared @@ -25,7 +25,7 @@ class TracerTest extends Specification { def tracer = new Tracer(config) then: - tracer.getWriter() instanceof LoggingWriter + tracer.getWriter() instanceof AgentWriter tracer.getSampler() instanceof AllSampler tracer.getInterceptors() == [] tracer.getDefaultServiceName() == config.getServiceName() diff --git a/dd-trace/src/test/groovy/datadog/trace/tracer/writer/AgentWriterTest.groovy b/dd-trace/src/test/groovy/datadog/trace/tracer/writer/AgentWriterTest.groovy index d9e86f79ba..84ac01406e 100644 --- a/dd-trace/src/test/groovy/datadog/trace/tracer/writer/AgentWriterTest.groovy +++ b/dd-trace/src/test/groovy/datadog/trace/tracer/writer/AgentWriterTest.groovy @@ -13,8 +13,12 @@ class AgentWriterTest extends Specification { // We make this slightly longer than flush time. private static final int FLUSH_DELAY = TimeUnit.SECONDS.toMillis(AgentWriter.FLUSH_TIME_SECONDS * 2) + private static final AGENT_URL = new URL("http://example.com") + def sampleRateByService = Mock(SampleRateByService) - def client = Mock(AgentClient) + def client = Mock(AgentClient) { + getAgentUrl() >> AGENT_URL + } def "test happy path"() { setup: @@ -109,6 +113,16 @@ class AgentWriterTest extends Specification { writer.close() } + def "test agent url getter"() { + setup: + def writer = new AgentWriter(client) + + when: + def agentUrl = writer.getAgentUrl() + + then: + agentUrl == AGENT_URL + } def "test default sample rate by service"() { setup: diff --git a/dd-trace/src/test/groovy/datadog/trace/tracer/writer/WriterTest.groovy b/dd-trace/src/test/groovy/datadog/trace/tracer/writer/WriterTest.groovy new file mode 100644 index 0000000000..1ff4562e10 --- /dev/null +++ b/dd-trace/src/test/groovy/datadog/trace/tracer/writer/WriterTest.groovy @@ -0,0 +1,63 @@ +package datadog.trace.tracer.writer + +import datadog.trace.api.Config +import spock.lang.Specification + + +class WriterTest extends Specification { + + def "test builder logging writer"() { + setup: + def config = Mock(Config) { + getWriterType() >> Config.LOGGING_WRITER_TYPE + } + + when: + def writer = Writer.Builder.forConfig(config) + + then: + writer instanceof LoggingWriter + } + + def "test builder logging writer properties"() { + setup: + def properties = new Properties() + properties.setProperty(Config.WRITER_TYPE, Config.LOGGING_WRITER_TYPE) + + when: + def writer = Writer.Builder.forConfig(properties) + + then: + writer instanceof LoggingWriter + } + + def "test builder agent writer: '#writerType'"() { + setup: + def config = Mock(Config) { + getWriterType() >> writerType + getAgentHost() >> "test.host" + getAgentPort() >> 1234 + } + + when: + def writer = Writer.Builder.forConfig(config) + + then: + writer instanceof AgentWriter + ((AgentWriter) writer).getAgentUrl() == new URL("http://test.host:1234/v0.4/traces"); + + where: + writerType | _ + Config.DD_AGENT_WRITER_TYPE | _ + "some odd string" | _ + } + + def "test builder no config"() { + when: + Writer.Builder.forConfig(null) + + then: + thrown NullPointerException + } + +}