diff --git a/dd-trace-ot/dd-trace-ot.gradle b/dd-trace-ot/dd-trace-ot.gradle index 96f9a4bd6d..cf92790fcb 100644 --- a/dd-trace-ot/dd-trace-ot.gradle +++ b/dd-trace-ot/dd-trace-ot.gradle @@ -12,8 +12,7 @@ minimumInstructionCoverage = 0.6 excludedClassesCoverage += [ 'datadog.trace.common.writer.ListWriter', 'datadog.trace.common.writer.LoggingWriter', - 'datadog.trace.common.writer.DDAgentWriter.Spec', - 'datadog.trace.common.writer.DDAgentWriter.Spec.SpecBuilder', + 'datadog.trace.common.writer.DDAgentWriter.DDAgentWriterBuilder', 'datadog.trace.common.sampling.PrioritySampling', // This code is copied from okHttp samples and we have integration tests to verify that it works. 'datadog.trace.common.writer.unixdomainsockets.TunnelingUnixSocket', diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 7f182b5aec..7312fa398c 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -44,6 +44,7 @@ import java.util.SortedSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.ThreadLocalRandom; +import lombok.Builder; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -96,18 +97,18 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace private final HttpCodec.Injector injector; private final HttpCodec.Extractor extractor; - public static class Builder { + public static class DDTracerBuilder { - public Builder() { + public DDTracerBuilder() { // Apply the default values from config. config(Config.get()); } - public Builder withProperties(final Properties properties) { + public DDTracerBuilder withProperties(final Properties properties) { return config(Config.get(properties)); } - public Builder config(final Config config) { + public DDTracerBuilder config(final Config config) { this.config = config; serviceName(config.getServiceName()); // Explicitly skip setting writer to avoid allocating resources prematurely. @@ -265,7 +266,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace partialFlushMinSpans); } - @lombok.Builder(builderClassName = "Builder") + @Builder // These field names must be stable to ensure the builder api is stable. private DDTracer( final Config config, diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index 81b884e43d..44e28da917 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -12,7 +12,6 @@ import datadog.trace.common.writer.ddagent.Monitor; import datadog.trace.common.writer.ddagent.TraceProcessingDisruptor; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; -import lombok.Value; import lombok.extern.slf4j.Slf4j; /** @@ -31,16 +30,6 @@ import lombok.extern.slf4j.Slf4j; */ @Slf4j public class DDAgentWriter implements Writer { - @Value - @lombok.Builder - public static class Spec { - @lombok.Builder.Default public String agentHost = DEFAULT_AGENT_HOST; - @lombok.Builder.Default public int traceAgentPort = DEFAULT_TRACE_AGENT_PORT; - @lombok.Builder.Default public String unixDomainSocket = DEFAULT_AGENT_UNIX_DOMAIN_SOCKET; - @lombok.Builder.Default public int traceBufferSize = DISRUPTOR_BUFFER_SIZE; - @lombok.Builder.Default public Monitor monitor = new Monitor.Noop(); - @lombok.Builder.Default public int flushFrequencySeconds = 1; - } private static final int DISRUPTOR_BUFFER_SIZE = 1024; @@ -52,20 +41,22 @@ public class DDAgentWriter implements Writer { public final Monitor monitor; - public DDAgentWriter() { - this(Spec.builder().build()); + // Apply defaults to the class generated by lombok. + public static class DDAgentWriterBuilder { + String agentHost = DEFAULT_AGENT_HOST; + int traceAgentPort = DEFAULT_TRACE_AGENT_PORT; + String unixDomainSocket = DEFAULT_AGENT_UNIX_DOMAIN_SOCKET; + int traceBufferSize = DISRUPTOR_BUFFER_SIZE; + Monitor monitor = new Monitor.Noop(); + int flushFrequencySeconds = 1; } - public DDAgentWriter(final Spec spec) { - api = new DDAgentApi(spec.agentHost, spec.traceAgentPort, spec.unixDomainSocket); - monitor = spec.monitor; - - batchWritingDisruptor = - new BatchWritingDisruptor( - spec.traceBufferSize, spec.flushFrequencySeconds, api, monitor, this); - traceProcessingDisruptor = - new TraceProcessingDisruptor( - spec.traceBufferSize, api, batchWritingDisruptor, monitor, this); + @Deprecated + public DDAgentWriter() { + this( + new DDAgentApi( + DEFAULT_AGENT_HOST, DEFAULT_TRACE_AGENT_PORT, DEFAULT_AGENT_UNIX_DOMAIN_SOCKET), + new Monitor.Noop()); } @Deprecated @@ -79,17 +70,27 @@ public class DDAgentWriter implements Writer { DISRUPTOR_BUFFER_SIZE, api, batchWritingDisruptor, monitor, this); } - @Deprecated - // DQH - TODO - Update the tests & remove this + @lombok.Builder + // These field names must be stable to ensure the builder api is stable. private DDAgentWriter( - final DDAgentApi api, final int disruptorSize, final int flushFrequencySeconds) { - this.api = api; - monitor = new Monitor.Noop(); + final DDAgentApi agentApi, + final String agentHost, + final int traceAgentPort, + final String unixDomainSocket, + final int traceBufferSize, + final Monitor monitor, + final int flushFrequencySeconds) { + if (agentApi != null) { + api = agentApi; + } else { + api = new DDAgentApi(agentHost, traceAgentPort, unixDomainSocket); + } + this.monitor = monitor; batchWritingDisruptor = - new BatchWritingDisruptor(disruptorSize, flushFrequencySeconds, api, monitor, this); + new BatchWritingDisruptor(traceBufferSize, flushFrequencySeconds, api, monitor, this); traceProcessingDisruptor = - new TraceProcessingDisruptor(disruptorSize, api, batchWritingDisruptor, monitor, this); + new TraceProcessingDisruptor(traceBufferSize, api, batchWritingDisruptor, monitor, this); } public void addResponseListener(final DDAgentResponseListener listener) { diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java index 51e0e85c4b..190c75d0e4 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java @@ -53,7 +53,7 @@ public interface Writer extends Closeable { } else { log.warn( "Writer type not configured correctly: No config provided! Defaulting to DDAgentWriter."); - writer = new DDAgentWriter(); + writer = DDAgentWriter.builder().build(); } return writer; @@ -64,8 +64,10 @@ public interface Writer extends Closeable { } private static Writer createAgentWriter(final Config config) { - // TODO: switch to using DDAgentWriter.Spec constructor... - return new DDAgentWriter(createApi(config), createMonitor(config)); + return DDAgentWriter.builder() + .agentApi(createApi(config)) + .monitor(createMonitor(config)) + .build(); } private static DDAgentApi createApi(final Config config) { diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy index 23e3e691ef..46f65e7c0e 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy @@ -44,7 +44,7 @@ class DDAgentWriterTest extends DDSpecification { def "test happy path"() { setup: - def writer = new DDAgentWriter(api, 2, -1) + def writer = DDAgentWriter.builder().agentApi(api).traceBufferSize(2).flushFrequencySeconds(-1).build() writer.start() when: @@ -72,7 +72,7 @@ class DDAgentWriterTest extends DDSpecification { def "test flood of traces"() { setup: - def writer = new DDAgentWriter(api, disruptorSize, -1) + def writer = DDAgentWriter.builder().agentApi(api).traceBufferSize(disruptorSize).flushFrequencySeconds(-1).build() writer.start() when: @@ -97,7 +97,7 @@ class DDAgentWriterTest extends DDSpecification { def "test flush by size"() { setup: - def writer = new DDAgentWriter(api, DISRUPTOR_BUFFER_SIZE, -1) + def writer = DDAgentWriter.builder().agentApi(api).traceBufferSize(DISRUPTOR_BUFFER_SIZE).flushFrequencySeconds(-1).build() writer.start() when: @@ -137,7 +137,7 @@ class DDAgentWriterTest extends DDSpecification { def "test flush by time"() { setup: - def writer = new DDAgentWriter(api, monitor) + def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build() writer.start() when: @@ -167,7 +167,7 @@ class DDAgentWriterTest extends DDSpecification { def "test default buffer size"() { setup: - def writer = new DDAgentWriter(api, DISRUPTOR_BUFFER_SIZE, -1) + def writer = DDAgentWriter.builder().agentApi(api).traceBufferSize(DISRUPTOR_BUFFER_SIZE).flushFrequencySeconds(-1).build() writer.start() when: @@ -212,7 +212,7 @@ class DDAgentWriterTest extends DDSpecification { def "check that are no interactions after close"() { setup: - def writer = new DDAgentWriter(api, monitor) + def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build() writer.start() when: @@ -230,7 +230,7 @@ class DDAgentWriterTest extends DDSpecification { def "check shutdown if batchWritingDisruptor stopped first"() { setup: - def writer = new DDAgentWriter(api, monitor) + def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build() writer.start() writer.batchWritingDisruptor.close() @@ -283,7 +283,7 @@ class DDAgentWriterTest extends DDSpecification { } } } - def writer = new DDAgentWriter(DDAgentWriter.Spec.builder().traceAgentPort(agent.address.port).monitor(monitor).build()) + def writer = DDAgentWriter.builder().traceAgentPort(agent.address.port).monitor(monitor).build() when: writer.start() @@ -330,7 +330,7 @@ class DDAgentWriterTest extends DDSpecification { } } } - def writer = new DDAgentWriter(DDAgentWriter.Spec.builder().traceAgentPort(agent.address.port).monitor(monitor).build()) + def writer = DDAgentWriter.builder().traceAgentPort(agent.address.port).monitor(monitor).build() when: writer.start() @@ -371,7 +371,7 @@ class DDAgentWriterTest extends DDSpecification { return DDAgentApi.Response.failed(new IOException("comm error")) } } - def writer = new DDAgentWriter(api, monitor) + def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build() when: writer.start() @@ -446,7 +446,7 @@ class DDAgentWriterTest extends DDSpecification { } } - def writer = new DDAgentWriter(DDAgentWriter.Spec.builder().traceAgentPort(agent.address.port).monitor(monitor).traceBufferSize(bufferSize).build()) + def writer = DDAgentWriter.builder().traceAgentPort(agent.address.port).monitor(monitor).traceBufferSize(bufferSize).build() writer.start() // gate responses @@ -544,7 +544,7 @@ class DDAgentWriterTest extends DDSpecification { } } - def writer = new DDAgentWriter(DDAgentWriter.Spec.builder().traceAgentPort(agent.address.port).monitor(monitor).build()) + def writer = DDAgentWriter.builder().traceAgentPort(agent.address.port).monitor(monitor).build() writer.start() when: @@ -604,7 +604,7 @@ class DDAgentWriterTest extends DDSpecification { } def monitor = new Monitor.StatsD(statsd) - def writer = new DDAgentWriter(DDAgentWriter.Spec.builder().traceAgentPort(agent.address.port).monitor(monitor).build()) + def writer = DDAgentWriter.builder().traceAgentPort(agent.address.port).monitor(monitor).build() writer.start() when: @@ -652,7 +652,7 @@ class DDAgentWriterTest extends DDSpecification { } def monitor = new Monitor.StatsD(statsd) - def writer = new DDAgentWriter(api, monitor) + def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build() writer.start() when: