cleaning code for the PR
MODIFY
src/main/java/com/datadoghq/trace/Sampler.java
src/main/java/com/datadoghq/trace/Sampler.java
@@ -3,8 +3,17 @@ package com.datadoghq.trace;
import com.datadoghq.trace.impl.DDSpan;
+/**
+ * Main interface to sample a collection of traces.
+ */
public interface Sampler {
+ /**
+ * Sample a collection of traces based on the parent span
+ *
+ * @param span the parent span with its context
+ * @return true when the trace/spans has to be reported/written
+ */
boolean sample(DDSpan span);
}

MODIFY
src/main/java/com/datadoghq/trace/impl/DDSpan.java
src/main/java/com/datadoghq/trace/impl/DDSpan.java
@@ -13,19 +13,51 @@ import com.fasterxml.jackson.annotation.JsonIgnore;
import io.opentracing.Span;
-
+/**
+ * Represents an in-flight span in the opentracing system.
+ * <p>
+ * <p>Spans are created by the {@link DDTracer#buildSpan}.
+ * This implementation adds some features according to the DD agent.
+ */
public class DDSpan implements io.opentracing.Span {
+ /**
+ * Each span have an operation name describing the current span
+ */
protected String operationName;
+ /**
+ * Tags are associated to the current span, they will not propagate to the children span
+ */
protected Map<String, Object> tags;
+ /**
+ * StartTime stores the creation time of the span in milliseconds
+ */
protected long startTime;
+ /**
+ * StartTimeNano stores the only the nanoseconds for more accuracy
+ */
protected long startTimeNano;
+ /**
+ * The duration in nanoseconds computed using the startTime and startTimeNano
+ */
protected long durationNano;
+ /**
+ * The context attached to the span
+ */
protected final DDSpanContext context;
private final static Logger logger = LoggerFactory.getLogger(DDSpan.class);
- DDSpan(
+ /**
+ * A simple constructor.
+ * Currently, users have
+ *
+ * @param operationName the operation name associated to the span
+ * @param tags Tags attached to the span
+ * @param timestampMilliseconds if set, use this time instead of the auto-generated time
+ * @param context the context
+ */
+ protected DDSpan(
String operationName,
Map<String, Object> tags,
long timestampMilliseconds,
@@ -52,10 +84,23 @@ public class DDSpan implements io.opentracing.Span {
}
}
+ /**
+ * Same as finish, see {@link DDSpan#finish}
+ */
public void finish(long stopTimeMillis) {
+ this.finish(stopTimeMillis, 0L);
+
+ }
+
+ /**
+ * Close the span. If the current span is the parent, check if each child has also been closed
+ * If not, warned it
+ *
+ * @param stopTimeMillis The stopTime in milliseconds
+ */
+ private void finish(long stopTimeMillis, long stopTimeNano) {
// formula: millis(stop - start) * 1000 * 1000 + keepNano(nano(stop - start))
- long stopTimeNano = System.nanoTime();
this.durationNano = (stopTimeMillis - startTime) * 1000000L + ((stopTimeNano - this.startTimeNano) % 1000000L);
logger.debug(this + " - Closing the span." + this.toString());
@@ -68,7 +113,6 @@ public class DDSpan implements io.opentracing.Span {
for (Span span : spans) {
if (((DDSpan) span).getDurationNano() == 0L) {
- // FIXME
logger.warn(this + " - The parent span is marked as finished but this span isn't. You have to close each children." + this.toString());
}
}
@@ -76,17 +120,27 @@ public class DDSpan implements io.opentracing.Span {
logger.debug(this + " - Sending the trace to the writer");
}
-
}
+ /**
+ * Same as finish, see {@link DDSpan#finish}
+ */
public void finish() {
- finish(System.currentTimeMillis());
+ finish(System.currentTimeMillis(), System.nanoTime());
}
+ /**
+ * Same as finish, see {@link DDSpan#finish}
+ */
public void close() {
this.finish();
}
+ /**
+ * Check if the span is a parent. It means that the traceId is the same as the spanId
+ *
+ * @return
+ */
private boolean isRootSpan() {
return context.getTraceId() == context.getSpanId();
}
@@ -113,9 +167,11 @@ public class DDSpan implements io.opentracing.Span {
}
/**
- * @param tag
- * @param value
- * @return
+ * Add a tag to the span. Tags are not propagated to the children
+ *
+ * @param tag the tag-name
+ * @param value the value of the value
+ * @return the builder instance
*/
private Span setTag(String tag, Object value) {
tags.put(tag, value);
@@ -180,9 +236,9 @@ public class DDSpan implements io.opentracing.Span {
}
/**
- * Meta merges bagage and tags (stringified values)
+ * Meta merges baggage and tags (stringified values)
*
- * @return merged context baggages and tags
+ * @return merged context baggage and tags
*/
@JsonGetter
public Map<String, String> getMeta() {

MODIFY
src/main/java/com/datadoghq/trace/impl/DDSpanContext.java
src/main/java/com/datadoghq/trace/impl/DDSpanContext.java
@@ -10,6 +10,13 @@ import com.fasterxml.jackson.annotation.JsonIgnore;
import io.opentracing.Span;
+/**
+ * SpanContext represents Span state that must propagate to descendant Spans and across process boundaries.
+ * <p>
+ * SpanContext is logically divided into two pieces: (1) the user-level "Baggage" that propagates across Span
+ * boundaries and (2) any Datadog fields that are needed to identify or contextualize
+ * the associated Span instance
+ */
public class DDSpanContext implements io.opentracing.SpanContext {
// Opentracing attributes
@@ -17,15 +24,32 @@ public class DDSpanContext implements io.opentracing.SpanContext {
protected long spanId;
protected long parentId;
protected Map<String, String> baggageItems;
- // DD attributes
+ // DD attributes
+ /**
+ * The service name is required, otherwise the span are dropped by the agent
+ */
protected String serviceName;
+ /**
+ * The resource associated to the service (server_web, database, etc.)
+ */
protected String resourceName;
+ /**
+ * True indicates that the span reports an error
+ */
protected boolean errorFlag;
+ /**
+ * The type of the span. If null, the Datadog Agent will report as a custom
+ */
protected String spanType;
+ /**
+ * The collection of all span related to this one
+ */
protected final List<Span> trace;
// Others attributes
- protected boolean sampled;
+ /**
+ * For technical reasons, the ref to the original tracer
+ */
protected DDTracer tracer;
public DDSpanContext(

MODIFY
src/main/java/com/datadoghq/trace/impl/DDTracer.java
src/main/java/com/datadoghq/trace/impl/DDTracer.java
@@ -14,13 +14,26 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
-
+/**
+ * DDTracer makes it easy to send traces and span to DD using the OpenTracing instrumentation.
+ */
public class DDTracer implements io.opentracing.Tracer {
+
+ /**
+ * Writer is an charge of reporting traces and spans to the desired endpoint
+ */
private Writer writer;
+ /**
+ * Sampler defines the sampling policy in order to reduce the number of traces for instance
+ */
private final Sampler sampler;
+
private final static Logger logger = LoggerFactory.getLogger(DDTracer.class);
+ /**
+ * Default constructor, trace/spans are logged, no trace/span dropped
+ */
public DDTracer() {
this(new LoggingWritter(), new AllSampler());
}
@@ -42,6 +55,14 @@ public class DDTracer implements io.opentracing.Tracer {
throw new UnsupportedOperationException();
}
+
+ /**
+ * We use the sampler to know if the trace has to be reported/written.
+ * The sampler is called on the first span (root span) of the trace.
+ * If the trace is marked as a sample, we report it.
+ *
+ * @param trace a list of the spans related to the same trace
+ */
public void write(List<Span> trace) {
if (trace.size() == 0) return;
if (this.sampler.sample((DDSpan) trace.get(0))) {
@@ -49,17 +70,31 @@ public class DDTracer implements io.opentracing.Tracer {
}
}
+ /**
+ * Spans are built using this builder
+ */
public class DDSpanBuilder implements SpanBuilder {
+ /**
+ * Each span must have an operationName according to the opentracing specification
+ */
private final String operationName;
+
+ // Builder attributes
private Map<String, Object> tags = new HashMap<String, Object>();
private long timestamp;
- private DDSpan parent;
+ private SpanContext parent;
private String serviceName;
private String resourceName;
private boolean errorFlag;
private String spanType;
+ /**
+ * This method actually build the span according to the builder settings
+ * DD-Agent requires a serviceName. If it has not been provided, the method will throw a RuntimeException
+ *
+ * @return An fresh span
+ */
public DDSpan start() {
// build the context
@@ -71,6 +106,7 @@ public class DDTracer implements io.opentracing.Tracer {
return span;
}
+
public DDTracer.DDSpanBuilder withTag(String tag, Number number) {
return withTag(tag, (Object) number);
}
@@ -87,10 +123,6 @@ public class DDTracer implements io.opentracing.Tracer {
this.operationName = operationName;
}
- public DDTracer.DDSpanBuilder asChildOf(Span span) {
- this.parent = (DDSpan) span;
- return this;
- }
public DDTracer.DDSpanBuilder withStartTimestamp(long timestampMillis) {
this.timestamp = timestampMillis;
@@ -121,12 +153,16 @@ public class DDTracer implements io.opentracing.Tracer {
if (parent == null) {
return Collections.emptyList();
}
- return parent.context().baggageItems();
+ return parent.baggageItems();
+ }
+
+ public DDTracer.DDSpanBuilder asChildOf(Span span) {
+ return asChildOf(span.context());
}
- // UnsupportedOperation
public DDTracer.DDSpanBuilder asChildOf(SpanContext spanContext) {
- throw new UnsupportedOperationException("Should be a Span instead of a context due to DD implementation");
+ this.parent = spanContext;
+ return this;
}
public DDTracer.DDSpanBuilder addReference(String referenceType, SpanContext spanContext) {
@@ -143,12 +179,19 @@ public class DDTracer implements io.opentracing.Tracer {
return System.nanoTime();
}
-
+ /**
+ * Build the SpanContext, if the actual span has a parent, the following attributes must be propagated:
+ * - ServiceName
+ * - Baggage
+ * - Trace (a list of all spans related)
+ *
+ * @return
+ */
private DDSpanContext buildTheSpanContext() {
long generatedId = generateNewId();
DDSpanContext context;
- DDSpanContext p = this.parent != null ? this.parent.context() : null;
+ DDSpanContext p = this.parent != null ? (DDSpanContext) this.parent : null;
// some attributes are inherited from the parent
context = new DDSpanContext(
@@ -159,7 +202,7 @@ public class DDTracer implements io.opentracing.Tracer {
this.resourceName,
this.parent == null ? null : p.getBaggageItems(),
errorFlag,
- this.parent == null ? this.spanType : p.getSpanType(),
+ this.spanType,
this.parent == null ? null : p.getTrace(),
DDTracer.this
);

MODIFY
src/main/java/com/datadoghq/trace/impl/RateSampler.java
src/main/java/com/datadoghq/trace/impl/RateSampler.java
@@ -5,11 +5,27 @@ import com.datadoghq.trace.Sampler;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+
+/**
+ * This sampler sample the traces at a predefined rate.
+ * <p>
+ * Keep (100 * `sample_rate`)% of the traces.
+ * It samples randomly, its main purpose is to reduce the instrumentation footprint.
+ */
public class RateSampler implements Sampler {
+
private final static Logger logger = LoggerFactory.getLogger(RateSampler.class);
+ /**
+ * The sample rate used
+ */
private final double sampleRate;
+ /**
+ * Build an instance of the sampler. The Sample rate is fixed for each instance.
+ *
+ * @param sampleRate a number [0,1] representing the rate ratio.
+ */
public RateSampler(double sampleRate) {
if (sampleRate <= 0) {

MODIFY
src/main/java/com/datadoghq/trace/writer/impl/DDApi.java
src/main/java/com/datadoghq/trace/writer/impl/DDApi.java
@@ -112,8 +112,7 @@ public class DDApi {
public static void main(String[] args) throws Exception {
- DDAgentWriter writer = new DDAgentWriter();
- DDTracer tracer = new DDTracer(writer, null);
+ DDTracer tracer = new DDTracer();
Span parent = tracer
.buildSpan("hello-world")
@@ -140,7 +139,6 @@ public class DDApi {
Thread.sleep(1000);
- writer.close();
}
}

MODIFY
src/test/java/Example.java
src/test/java/Example.java
@@ -12,8 +12,7 @@ public class Example {
public static void main(String[] args) throws Exception {
- Writer writer = new DDAgentWriter();
- DDTracer tracer = new DDTracer(writer, null);
+ DDTracer tracer = new DDTracer();
Span parent = tracer
@@ -36,7 +35,6 @@ public class Example {
parent.finish();
- writer.close();
}
}
\ No newline at end of file

MODIFY
src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java
src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java
@@ -184,31 +184,25 @@ public class DDSpanBuilderTest {
final int nbSamples = 10;
// root (aka spans[0]) is the parent
- // spans[1] has a predictable duration
// others are just for fun
DDSpan root = tracer.buildSpan("fake_O").withServiceName("foo").start();
spans.add(root);
- long tickStart = System.currentTimeMillis();
- spans.add(tracer.buildSpan("fake_" + 1).withServiceName("foo").asChildOf(spans.get(0)).withStartTimestamp(tickStart).start());
- for (int i = 2; i <= 10; i++) {
- spans.add(tracer.buildSpan("fake_" + i).withServiceName("foo").asChildOf(spans.get(i - 1)).start());
- }
- Thread.sleep(300);
+ Thread.sleep(200);
long tickEnd = System.currentTimeMillis();
+
+ for (int i = 1; i <= 10; i++) {
+ spans.add(tracer.buildSpan("fake_" + i).withServiceName("foo").asChildOf(spans.get(i - 1)).start());
+ }
spans.get(1).finish(tickEnd);
assertThat(root.context.getTrace()).hasSize(nbSamples + 1);
assertThat(root.context.getTrace()).containsAll(spans);
assertThat(spans.get((int) (Math.random() * nbSamples)).context.getTrace()).containsAll(spans);
- root.finish();
-
- // not comparing the nano
- assertThat(spans.get(1).durationNano / 1000000L).isEqualTo((tickEnd - tickStart));
}

MODIFY
src/test/java/com/datadoghq/trace/impl/DDSpanTest.java
src/test/java/com/datadoghq/trace/impl/DDSpanTest.java
package com.datadoghq.trace.impl;
+import com.datadoghq.trace.SpanSerializer;
+import io.opentracing.Span;
import org.junit.Test;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
public class DDSpanTest {
@@ -13,4 +16,29 @@ public class DDSpanTest {
new DDTracer().buildSpan("operationName").start();
}
+ @Test(expected = IllegalArgumentException.class)
+ public void shouldOperationNameImmutable() {
+ Span span = new DDTracer().buildSpan("foo").withServiceName("foo").start();
+ span.setOperationName("boom");
+ }
+
+ @Test
+ public void shouldResourceNameEqualsOperationNameIfNull() {
+
+ final String expectedName = "operationName";
+
+ DDSpan span = new DDTracer().buildSpan(expectedName).withServiceName("foo").start();
+ // ResourceName = expectedName
+ assertThat(span.getResourceName()).isEqualTo(expectedName);
+
+ // ResourceName = expectedResourceName
+ final String expectedResourceName = "fake";
+ span = new DDTracer()
+ .buildSpan(expectedName)
+ .withResourceName(expectedResourceName)
+ .withServiceName("foo").start();
+
+ assertThat(span.getResourceName()).isEqualTo(expectedResourceName);
+
+ }
}
\ No newline at end of file

cleaning code for the PR