Replace DDAgentWriter.Spec with a proper Builder.

Also rename the builder class on DDTracer to default name generated by Lombok.
This commit is contained in:
Tyler Benson 2020-01-16 16:43:01 -08:00
parent 3aea763769
commit 5cce4cb783
5 changed files with 57 additions and 54 deletions

View File

@ -12,8 +12,7 @@ minimumInstructionCoverage = 0.6
excludedClassesCoverage += [ excludedClassesCoverage += [
'datadog.trace.common.writer.ListWriter', 'datadog.trace.common.writer.ListWriter',
'datadog.trace.common.writer.LoggingWriter', 'datadog.trace.common.writer.LoggingWriter',
'datadog.trace.common.writer.DDAgentWriter.Spec', 'datadog.trace.common.writer.DDAgentWriter.DDAgentWriterBuilder',
'datadog.trace.common.writer.DDAgentWriter.Spec.SpecBuilder',
'datadog.trace.common.sampling.PrioritySampling', 'datadog.trace.common.sampling.PrioritySampling',
// This code is copied from okHttp samples and we have integration tests to verify that it works. // This code is copied from okHttp samples and we have integration tests to verify that it works.
'datadog.trace.common.writer.unixdomainsockets.TunnelingUnixSocket', 'datadog.trace.common.writer.unixdomainsockets.TunnelingUnixSocket',

View File

@ -44,6 +44,7 @@ import java.util.SortedSet;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.ConcurrentSkipListSet;
import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.ThreadLocalRandom;
import lombok.Builder;
import lombok.Getter; import lombok.Getter;
import lombok.extern.slf4j.Slf4j; 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.Injector injector;
private final HttpCodec.Extractor extractor; private final HttpCodec.Extractor extractor;
public static class Builder { public static class DDTracerBuilder {
public Builder() { public DDTracerBuilder() {
// Apply the default values from config. // Apply the default values from config.
config(Config.get()); config(Config.get());
} }
public Builder withProperties(final Properties properties) { public DDTracerBuilder withProperties(final Properties properties) {
return config(Config.get(properties)); return config(Config.get(properties));
} }
public Builder config(final Config config) { public DDTracerBuilder config(final Config config) {
this.config = config; this.config = config;
serviceName(config.getServiceName()); serviceName(config.getServiceName());
// Explicitly skip setting writer to avoid allocating resources prematurely. // Explicitly skip setting writer to avoid allocating resources prematurely.
@ -265,7 +266,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace
partialFlushMinSpans); partialFlushMinSpans);
} }
@lombok.Builder(builderClassName = "Builder") @Builder
// These field names must be stable to ensure the builder api is stable. // These field names must be stable to ensure the builder api is stable.
private DDTracer( private DDTracer(
final Config config, final Config config,

View File

@ -12,7 +12,6 @@ import datadog.trace.common.writer.ddagent.Monitor;
import datadog.trace.common.writer.ddagent.TraceProcessingDisruptor; import datadog.trace.common.writer.ddagent.TraceProcessingDisruptor;
import java.util.List; import java.util.List;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import lombok.Value;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
/** /**
@ -31,16 +30,6 @@ import lombok.extern.slf4j.Slf4j;
*/ */
@Slf4j @Slf4j
public class DDAgentWriter implements Writer { 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; private static final int DISRUPTOR_BUFFER_SIZE = 1024;
@ -52,20 +41,22 @@ public class DDAgentWriter implements Writer {
public final Monitor monitor; public final Monitor monitor;
public DDAgentWriter() { // Apply defaults to the class generated by lombok.
this(Spec.builder().build()); 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) { @Deprecated
api = new DDAgentApi(spec.agentHost, spec.traceAgentPort, spec.unixDomainSocket); public DDAgentWriter() {
monitor = spec.monitor; this(
new DDAgentApi(
batchWritingDisruptor = DEFAULT_AGENT_HOST, DEFAULT_TRACE_AGENT_PORT, DEFAULT_AGENT_UNIX_DOMAIN_SOCKET),
new BatchWritingDisruptor( new Monitor.Noop());
spec.traceBufferSize, spec.flushFrequencySeconds, api, monitor, this);
traceProcessingDisruptor =
new TraceProcessingDisruptor(
spec.traceBufferSize, api, batchWritingDisruptor, monitor, this);
} }
@Deprecated @Deprecated
@ -79,17 +70,27 @@ public class DDAgentWriter implements Writer {
DISRUPTOR_BUFFER_SIZE, api, batchWritingDisruptor, monitor, this); DISRUPTOR_BUFFER_SIZE, api, batchWritingDisruptor, monitor, this);
} }
@Deprecated @lombok.Builder
// DQH - TODO - Update the tests & remove this // These field names must be stable to ensure the builder api is stable.
private DDAgentWriter( private DDAgentWriter(
final DDAgentApi api, final int disruptorSize, final int flushFrequencySeconds) { final DDAgentApi agentApi,
this.api = api; final String agentHost,
monitor = new Monitor.Noop(); 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 = batchWritingDisruptor =
new BatchWritingDisruptor(disruptorSize, flushFrequencySeconds, api, monitor, this); new BatchWritingDisruptor(traceBufferSize, flushFrequencySeconds, api, monitor, this);
traceProcessingDisruptor = traceProcessingDisruptor =
new TraceProcessingDisruptor(disruptorSize, api, batchWritingDisruptor, monitor, this); new TraceProcessingDisruptor(traceBufferSize, api, batchWritingDisruptor, monitor, this);
} }
public void addResponseListener(final DDAgentResponseListener listener) { public void addResponseListener(final DDAgentResponseListener listener) {

View File

@ -53,7 +53,7 @@ public interface Writer extends Closeable {
} else { } else {
log.warn( log.warn(
"Writer type not configured correctly: No config provided! Defaulting to DDAgentWriter."); "Writer type not configured correctly: No config provided! Defaulting to DDAgentWriter.");
writer = new DDAgentWriter(); writer = DDAgentWriter.builder().build();
} }
return writer; return writer;
@ -64,8 +64,10 @@ public interface Writer extends Closeable {
} }
private static Writer createAgentWriter(final Config config) { private static Writer createAgentWriter(final Config config) {
// TODO: switch to using DDAgentWriter.Spec constructor... return DDAgentWriter.builder()
return new DDAgentWriter(createApi(config), createMonitor(config)); .agentApi(createApi(config))
.monitor(createMonitor(config))
.build();
} }
private static DDAgentApi createApi(final Config config) { private static DDAgentApi createApi(final Config config) {

View File

@ -44,7 +44,7 @@ class DDAgentWriterTest extends DDSpecification {
def "test happy path"() { def "test happy path"() {
setup: setup:
def writer = new DDAgentWriter(api, 2, -1) def writer = DDAgentWriter.builder().agentApi(api).traceBufferSize(2).flushFrequencySeconds(-1).build()
writer.start() writer.start()
when: when:
@ -72,7 +72,7 @@ class DDAgentWriterTest extends DDSpecification {
def "test flood of traces"() { def "test flood of traces"() {
setup: setup:
def writer = new DDAgentWriter(api, disruptorSize, -1) def writer = DDAgentWriter.builder().agentApi(api).traceBufferSize(disruptorSize).flushFrequencySeconds(-1).build()
writer.start() writer.start()
when: when:
@ -97,7 +97,7 @@ class DDAgentWriterTest extends DDSpecification {
def "test flush by size"() { def "test flush by size"() {
setup: 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() writer.start()
when: when:
@ -137,7 +137,7 @@ class DDAgentWriterTest extends DDSpecification {
def "test flush by time"() { def "test flush by time"() {
setup: setup:
def writer = new DDAgentWriter(api, monitor) def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build()
writer.start() writer.start()
when: when:
@ -167,7 +167,7 @@ class DDAgentWriterTest extends DDSpecification {
def "test default buffer size"() { def "test default buffer size"() {
setup: 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() writer.start()
when: when:
@ -212,7 +212,7 @@ class DDAgentWriterTest extends DDSpecification {
def "check that are no interactions after close"() { def "check that are no interactions after close"() {
setup: setup:
def writer = new DDAgentWriter(api, monitor) def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build()
writer.start() writer.start()
when: when:
@ -230,7 +230,7 @@ class DDAgentWriterTest extends DDSpecification {
def "check shutdown if batchWritingDisruptor stopped first"() { def "check shutdown if batchWritingDisruptor stopped first"() {
setup: setup:
def writer = new DDAgentWriter(api, monitor) def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build()
writer.start() writer.start()
writer.batchWritingDisruptor.close() 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: when:
writer.start() 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: when:
writer.start() writer.start()
@ -371,7 +371,7 @@ class DDAgentWriterTest extends DDSpecification {
return DDAgentApi.Response.failed(new IOException("comm error")) return DDAgentApi.Response.failed(new IOException("comm error"))
} }
} }
def writer = new DDAgentWriter(api, monitor) def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build()
when: when:
writer.start() 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() writer.start()
// gate responses // 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() writer.start()
when: when:
@ -604,7 +604,7 @@ class DDAgentWriterTest extends DDSpecification {
} }
def monitor = new Monitor.StatsD(statsd) 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() writer.start()
when: when:
@ -652,7 +652,7 @@ class DDAgentWriterTest extends DDSpecification {
} }
def monitor = new Monitor.StatsD(statsd) def monitor = new Monitor.StatsD(statsd)
def writer = new DDAgentWriter(api, monitor) def writer = DDAgentWriter.builder().agentApi(api).monitor(monitor).build()
writer.start() writer.start()
when: when: