Skip to content

Commit 7c4ca0d

Browse files
tylerbensontylerbenson
authored
Merge pull request #185 from DataDog/tyler/fix-config
Allow setting host/port without specifying writer type.
2 parents cca7a73 + d95309b commit 7c4ca0d

File tree

13 files changed

+89
-124
lines changed

13 files changed

+89
-124
lines changed

‎dd-trace/src/main/java/com/datadoghq/trace/DDTraceConfig.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
package com.datadoghq.trace;
22

3+
import com.datadoghq.trace.sampling.Sampler;
4+
import com.datadoghq.trace.writer.DDAgentWriter;
5+
import com.datadoghq.trace.writer.Writer;
36
import java.util.Properties;
47

58
/**
6-
* Config gives priority to system properties and falls back to environment variables.
9+
* Config gives priority to system properties and falls back to environment variables. It also
10+
* includes default values to ensure a valid config.
711
*
812
* <p>System properties are {@link DDTraceConfig#PREFIX}'ed. Environment variables are the same as
913
* the system property, but uppercased with '.' -> '_'.
1014
*/
1115
public class DDTraceConfig extends Properties {
12-
/** Config keys bel */
16+
/** Config keys below */
1317
private static final String PREFIX = "dd.";
1418

1519
public static final String SERVICE_NAME = "service.name";
@@ -31,9 +35,13 @@ public DDTraceConfig() {
3135

3236
final Properties defaults = new Properties();
3337
defaults.setProperty(SERVICE_NAME, DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME);
38+
defaults.setProperty(WRITER_TYPE, Writer.DD_AGENT_WRITER_TYPE);
39+
defaults.setProperty(AGENT_HOST, DDAgentWriter.DEFAULT_HOSTNAME);
40+
defaults.setProperty(AGENT_PORT, String.valueOf(DDAgentWriter.DEFAULT_PORT));
41+
defaults.setProperty(SAMPLER_TYPE, Sampler.ALL_SAMPLER_TYPE);
42+
defaults.setProperty(SAMPLER_RATE, "1.0");
3443
super.defaults = defaults;
3544

36-
final Properties baseValues = new Properties(defaults);
3745
setIfNotNull(SERVICE_NAME, serviceName);
3846
setIfNotNull(WRITER_TYPE, writerType);
3947
setIfNotNull(AGENT_HOST, agentHost);
@@ -43,7 +51,7 @@ public DDTraceConfig() {
4351
}
4452

4553
public DDTraceConfig(final String serviceName) {
46-
super();
54+
this();
4755
put(SERVICE_NAME, serviceName);
4856
}
4957

‎dd-trace/src/main/java/com/datadoghq/trace/DDTracer.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import com.datadoghq.trace.resolver.DDDecoratorsFactory;
77
import com.datadoghq.trace.sampling.AllSampler;
88
import com.datadoghq.trace.sampling.Sampler;
9-
import com.datadoghq.trace.writer.DDAgentWriter;
109
import com.datadoghq.trace.writer.Writer;
1110
import com.fasterxml.jackson.annotation.JsonIgnore;
1211
import io.opentracing.ActiveSpan;
@@ -30,17 +29,14 @@
3029
public class DDTracer extends ThreadLocalActiveSpanSource implements io.opentracing.Tracer {
3130

3231
public static final String UNASSIGNED_DEFAULT_SERVICE_NAME = "unnamed-java-app";
33-
public static final Writer UNASSIGNED_WRITER = new DDAgentWriter();
34-
public static final Sampler UNASSIGNED_SAMPLER = new AllSampler();
3532

33+
/** Default service name if none provided on the trace or span */
34+
final String serviceName;
3635
/** Writer is an charge of reporting traces and spans to the desired endpoint */
3736
final Writer writer;
3837
/** Sampler defines the sampling policy in order to reduce the number of traces for instance */
3938
final Sampler sampler;
4039

41-
/** Default service name if none provided on the trace or span */
42-
final String serviceName;
43-
4440
/** Span context decorators */
4541
private final Map<String, List<AbstractDecorator>> spanContextDecorators = new HashMap<>();
4642

@@ -83,11 +79,7 @@ public DDTracer(final String serviceName, final Writer writer, final Sampler sam
8379
}
8480

8581
public DDTracer(final Writer writer) {
86-
this(writer, new AllSampler());
87-
}
88-
89-
public DDTracer(final Writer writer, final Sampler sampler) {
90-
this(UNASSIGNED_DEFAULT_SERVICE_NAME, writer, sampler);
82+
this(UNASSIGNED_DEFAULT_SERVICE_NAME, writer, new AllSampler());
9183
}
9284

9385
/**

‎dd-trace/src/main/java/com/datadoghq/trace/sampling/AllSampler.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@
66
public class AllSampler extends AbstractSampler {
77

88
@Override
9-
public boolean doSample(DDBaseSpan<?> span) {
9+
public boolean doSample(final DDBaseSpan<?> span) {
1010
return true;
1111
}
12+
13+
@Override
14+
public String toString() {
15+
return "AllSampler { sample=true }";
16+
}
1217
}

‎dd-trace/src/main/java/com/datadoghq/trace/sampling/Sampler.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
import com.datadoghq.trace.DDBaseSpan;
44
import com.datadoghq.trace.DDTraceConfig;
5-
import com.datadoghq.trace.DDTracer;
65
import java.util.Properties;
6+
import lombok.extern.slf4j.Slf4j;
77

88
/** Main interface to sample a collection of traces. */
99
public interface Sampler {
@@ -18,6 +18,7 @@ public interface Sampler {
1818
*/
1919
boolean sample(DDBaseSpan<?> span);
2020

21+
@Slf4j
2122
final class Builder {
2223
public static Sampler forConfig(final Properties config) {
2324
final Sampler sampler;
@@ -29,10 +30,15 @@ public static Sampler forConfig(final Properties config) {
2930
} else if (ALL_SAMPLER_TYPE.equals(configuredType)) {
3031
sampler = new AllSampler();
3132
} else {
32-
sampler = DDTracer.UNASSIGNED_SAMPLER;
33+
log.warn(
34+
"Sampler type not configured correctly: Type {} not recognized. Defaulting to AllSampler.",
35+
configuredType);
36+
sampler = new AllSampler();
3337
}
3438
} else {
35-
sampler = DDTracer.UNASSIGNED_SAMPLER;
39+
log.warn(
40+
"Sampler type not configured correctly: No config provided! Defaulting to AllSampler.");
41+
sampler = new AllSampler();
3642
}
3743
return sampler;
3844
}

‎dd-trace/src/main/java/com/datadoghq/trace/writer/DDApi.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,10 @@ private boolean putContent(
103103
log.debug("Error while sending " + size + " " + type + " to the DD agent.", e);
104104
} else if (loggingRateLimiter.tryAcquire()) {
105105
log.warn(
106-
"Error while sending {} {} to the DD agent. Message: {} (going silent for {} seconds)",
106+
"Error while sending {} {} to the DD agent. {}: {} (going silent for {} seconds)",
107107
size,
108108
type,
109+
e.getClass().getName(),
109110
e.getMessage(),
110111
SECONDS_BETWEEN_ERROR_LOG);
111112
}

‎dd-trace/src/main/java/com/datadoghq/trace/writer/ListWriter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,9 @@ public void close() {
6363
latches.clear();
6464
}
6565
}
66+
67+
@Override
68+
public String toString() {
69+
return "ListWriter { size=" + this.size() + " }";
70+
}
6671
}

‎dd-trace/src/main/java/com/datadoghq/trace/writer/LoggingWriter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,9 @@ public void close() {
3030
public void start() {
3131
log.info("start()");
3232
}
33+
34+
@Override
35+
public String toString() {
36+
return "LoggingWriter { }";
37+
}
3338
}

‎dd-trace/src/main/java/com/datadoghq/trace/writer/Writer.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33
import com.datadoghq.trace.DDBaseSpan;
44
import com.datadoghq.trace.DDTraceConfig;
5-
import com.datadoghq.trace.DDTracer;
65
import com.datadoghq.trace.Service;
76
import java.util.List;
87
import java.util.Map;
98
import java.util.Properties;
9+
import lombok.extern.slf4j.Slf4j;
1010

1111
/** A writer is responsible to send collected spans to some place */
1212
public interface Writer {
@@ -36,6 +36,7 @@ public interface Writer {
3636
*/
3737
void close();
3838

39+
@Slf4j
3940
final class Builder {
4041
public static Writer forConfig(final Properties config) {
4142
final Writer writer;
@@ -51,10 +52,19 @@ public static Writer forConfig(final Properties config) {
5152
} else if (LOGGING_WRITER_TYPE.equals(configuredType)) {
5253
writer = new LoggingWriter();
5354
} else {
54-
writer = DDTracer.UNASSIGNED_WRITER;
55+
log.warn(
56+
"Writer type not configured correctly: Type {} not recognized. Defaulting to DDAgentWriter.",
57+
configuredType);
58+
writer =
59+
new DDAgentWriter(
60+
new DDApi(
61+
config.getProperty(DDTraceConfig.AGENT_HOST),
62+
Integer.parseInt(config.getProperty(DDTraceConfig.AGENT_PORT))));
5563
}
5664
} else {
57-
writer = DDTracer.UNASSIGNED_WRITER;
65+
log.warn(
66+
"Writer type not configured correctly: No config provided! Defaulting to DDAgentWriter.");
67+
writer = new DDAgentWriter();
5868
}
5969

6070
return writer;

‎dd-trace/src/test/groovy/com/datadoghq/trace/DDTraceConfigTest.groovy‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.datadoghq.trace.writer.DDAgentWriter
66
import com.datadoghq.trace.writer.ListWriter
77
import com.datadoghq.trace.writer.LoggingWriter
88
import spock.lang.Specification
9+
import spock.lang.Unroll
910

1011
import java.lang.reflect.Field
1112
import java.lang.reflect.Modifier
@@ -67,22 +68,22 @@ class DDTraceConfigTest extends Specification {
6768

6869
then:
6970
config.getProperty(SERVICE_NAME) == "unnamed-java-app"
70-
config.getProperty(WRITER_TYPE) == null
71-
config.getProperty(AGENT_HOST) == null
72-
config.getProperty(AGENT_PORT) == null
73-
config.getProperty(SAMPLER_TYPE) == null
74-
config.getProperty(SAMPLER_RATE) == null
71+
config.getProperty(WRITER_TYPE) == "DDAgentWriter"
72+
config.getProperty(AGENT_HOST) == "localhost"
73+
config.getProperty(AGENT_PORT) == "8126"
74+
config.getProperty(SAMPLER_TYPE) == "AllSampler"
75+
config.getProperty(SAMPLER_RATE) == "1.0"
7576

7677
when:
7778
config = new DDTraceConfig("A different service name")
7879

7980
then:
8081
config.getProperty(SERVICE_NAME) == "A different service name"
81-
config.getProperty(WRITER_TYPE) == null
82-
config.getProperty(AGENT_HOST) == null
83-
config.getProperty(AGENT_PORT) == null
84-
config.getProperty(SAMPLER_TYPE) == null
85-
config.getProperty(SAMPLER_RATE) == null
82+
config.getProperty(WRITER_TYPE) == "DDAgentWriter"
83+
config.getProperty(AGENT_HOST) == "localhost"
84+
config.getProperty(AGENT_PORT) == "8126"
85+
config.getProperty(SAMPLER_TYPE) == "AllSampler"
86+
config.getProperty(SAMPLER_RATE) == "1.0"
8687
}
8788

8889
def "specify overrides via system properties"() {
@@ -144,4 +145,25 @@ class DDTraceConfigTest extends Specification {
144145

145146
tracer.spanContextDecorators.size() == 2
146147
}
148+
149+
@Unroll
150+
def "verify single override on #source for #key"() {
151+
when:
152+
System.setProperty(PREFIX + key, value)
153+
def tracer = new DDTracer()
154+
155+
then:
156+
tracer."$source".toString() == expected
157+
158+
where:
159+
160+
source | key | value | expected
161+
"writer" | "default" | "default" | "DDAgentWriter { api=DDApi { tracesEndpoint=http://localhost:8126/v0.3/traces } }"
162+
"writer" | "writer.type" | "LoggingWriter" | "LoggingWriter { }"
163+
"writer" | "agent.host" | "somethingelse" | "DDAgentWriter { api=DDApi { tracesEndpoint=http://somethingelse:8126/v0.3/traces } }"
164+
"writer" | "agent.port" | "9999" | "DDAgentWriter { api=DDApi { tracesEndpoint=http://localhost:9999/v0.3/traces } }"
165+
"sampler" | "default" | "default" | "AllSampler { sample=true }"
166+
"sampler" | "sampler.type" | "RateSampler" | "RateSampler { sampleRate=1.0 }"
167+
"sampler" | "sampler.rate" | "100" | "AllSampler { sample=true }"
168+
}
147169
}

‎dd-trace/src/test/groovy/com/datadoghq/trace/ServiceTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class ServiceTest extends Specification {
5454

5555
setup:
5656
def writer = spy(new DDAgentWriter())
57-
def tracer = new DDTracer(writer, new AllSampler())
57+
def tracer = new DDTracer(DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME, writer, new AllSampler())
5858

5959

6060
when:

0 commit comments

Comments
 (0)