[PR Migrating all things to the span context (and add setters, and do not instantiate HashMap if it's not needed)
MODIFY
src/main/java/com/datadoghq/trace/impl/DDSpan.java
src/main/java/com/datadoghq/trace/impl/DDSpan.java
package com.datadoghq.trace.impl;
+import com.fasterxml.jackson.annotation.JsonGetter;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import io.opentracing.Span;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.TimeUnit;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.fasterxml.jackson.annotation.JsonGetter;
-import com.fasterxml.jackson.annotation.JsonIgnore;
-
-import io.opentracing.Span;
-
/**
* Represents an in-flight span in the opentracing system.
* <p>
@@ -22,14 +20,7 @@ import io.opentracing.Span;
*/
public class DDSpan implements io.opentracing.Span {
- /**
- * Each span have an operation name describing the current span
- */
- private String operationName;
- /**
- * Tags are associated to the current span, they will not propagate to the children span
- */
- private Map<String, Object> tags;
+
/**
* StartTime stores the creation time of the span in milliseconds
*/
@@ -53,19 +44,13 @@ public class DDSpan implements io.opentracing.Span {
* A simple constructor.
* Currently, users have
*
- * @param operationName the operation name associated to the span
- * @param tags Tags attached to the span
* @param timestampMicro if set, use this time instead of the auto-generated time
* @param context the context
*/
protected DDSpan(
- String operationName,
- Map<String, Object> tags,
long timestampMicro,
DDSpanContext context) {
- this.operationName = operationName;
- this.tags = tags;
this.context = context;
// record the start time in nano (current milli + nano delta)
@@ -79,11 +64,6 @@ public class DDSpan implements io.opentracing.Span {
// track each span of the trace
this.context.getTrace().add(this);
- // check DD attributes required
- // FIXME Remove IAE
- if (this.context.getServiceName() == null) {
- throw new IllegalArgumentException("No ServiceName provided");
- }
}
/* (non-Javadoc)
@@ -145,35 +125,27 @@ public class DDSpan implements io.opentracing.Span {
* @see io.opentracing.Span#setTag(java.lang.String, java.lang.String)
*/
public Span setTag(String tag, String value) {
- return setTag(tag, (Object) value);
+ this.context().setTag(tag, (Object) value);
+ return this;
}
/* (non-Javadoc)
* @see io.opentracing.Span#setTag(java.lang.String, boolean)
*/
public Span setTag(String tag, boolean value) {
- return setTag(tag, (Object) value);
+ this.context().setTag(tag, (Object) value);
+ return this;
}
/* (non-Javadoc)
* @see io.opentracing.Span#setTag(java.lang.String, java.lang.Number)
*/
public Span setTag(String tag, Number value) {
- return this.setTag(tag, (Object) value);
- }
-
- /**
- * 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);
+ this.context().setTag(tag, (Object) value);
return this;
}
+
/* (non-Javadoc)
* @see io.opentracing.Span#context()
*/
@@ -200,11 +172,7 @@ public class DDSpan implements io.opentracing.Span {
* @see io.opentracing.Span#setOperationName(java.lang.String)
*/
public Span setOperationName(String operationName) {
- // FIXME operationName is in each constructor --> always IAE
- if (this.operationName != null) {
- throw new IllegalArgumentException("The operationName is already assigned.");
- }
- this.operationName = operationName;
+ this.context().setOperationName(operationName);
return this;
}
@@ -258,15 +226,6 @@ public class DDSpan implements io.opentracing.Span {
//Getters and JSON serialisation instructions
- @JsonGetter("name")
- public String getOperationName() {
- return operationName;
- }
-
- @JsonIgnore
- public Map<String, Object> getTags() {
- return this.tags;
- }
/**
* Meta merges baggage and tags (stringified values)
@@ -295,8 +254,8 @@ public class DDSpan implements io.opentracing.Span {
return durationNano;
}
- @JsonGetter
- public String getService() {
+ @JsonGetter("service")
+ public String getServiceName() {
return context.getServiceName();
}
@@ -317,9 +276,18 @@ public class DDSpan implements io.opentracing.Span {
@JsonGetter("resource")
public String getResourceName() {
- return context.getResourceName() == null ? this.operationName : context.getResourceName();
+ return context.getResourceName() == null ? context.getOperationName() : context.getResourceName();
+ }
+
+ @JsonGetter("name")
+ public String getOperationName() {
+ return this.context().getOperationName();
}
+ @JsonIgnore
+ public Map<String, Object> getTags() {
+ return this.context().getTags();
+ }
@JsonGetter
public String getType() {
return context.getSpanType();
@@ -335,4 +303,19 @@ public class DDSpan implements io.opentracing.Span {
return context.toString();
}
+
+ public Span setServiceName(String serviceName) {
+ this.context().setServiceName(serviceName);
+ return this;
+ }
+
+ public Span setResourceName(String resourceName) {
+ this.context().setResourceName(resourceName);
+ return this;
+ }
+
+ public Span setType(String type) {
+ this.context().setType(type);
+ return this;
+ }
}

MODIFY
src/main/java/com/datadoghq/trace/impl/DDSpanContext.java
src/main/java/com/datadoghq/trace/impl/DDSpanContext.java
package com.datadoghq.trace.impl;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
import com.fasterxml.jackson.annotation.JsonIgnore;
@@ -23,17 +20,17 @@ public class DDSpanContext implements io.opentracing.SpanContext {
private final long traceId;
private final long spanId;
private final long parentId;
- private final Map<String, String> baggageItems;
+ private Map<String, String> baggageItems;
// DD attributes
/**
* The service name is required, otherwise the span are dropped by the agent
*/
- private final String serviceName;
+ private String serviceName;
/**
* The resource associated to the service (server_web, database, etc.)
*/
- private final String resourceName;
+ private String resourceName;
/**
* True indicates that the span reports an error
*/
@@ -41,11 +38,19 @@ public class DDSpanContext implements io.opentracing.SpanContext {
/**
* The type of the span. If null, the Datadog Agent will report as a custom
*/
- private final String spanType;
+ private String spanType;
/**
* The collection of all span related to this one
*/
private final List<Span> trace;
+ /**
+ * Each span have an operation name describing the current span
+ */
+ private String operationName;
+ /**
+ * Tags are associated to the current span, they will not propagate to the children span
+ */
+ private Map<String, Object> tags;
// Others attributes
/**
* For technical reasons, the ref to the original tracer
@@ -57,10 +62,12 @@ public class DDSpanContext implements io.opentracing.SpanContext {
long spanId,
long parentId,
String serviceName,
+ String operationName,
String resourceName,
Map<String, String> baggageItems,
boolean errorFlag,
String spanType,
+ Map<String, Object> tags,
List<Span> trace,
DDTracer tracer) {
@@ -69,15 +76,19 @@ public class DDSpanContext implements io.opentracing.SpanContext {
this.parentId = parentId;
if (baggageItems == null) {
- this.baggageItems = new HashMap<String, String>();
+ this.baggageItems = Collections.emptyMap();
} else {
this.baggageItems = baggageItems;
}
+
this.serviceName = serviceName;
+ this.operationName = operationName;
this.resourceName = resourceName;
this.errorFlag = errorFlag;
this.spanType = spanType;
+ this.tags = tags;
+
if (trace == null) {
this.trace = new ArrayList<Span>();
} else {
@@ -117,6 +128,9 @@ public class DDSpanContext implements io.opentracing.SpanContext {
}
public void setBaggageItem(String key, String value) {
+ if (this.baggageItems.isEmpty()) {
+ this.baggageItems = new HashMap<String, String>();
+ }
this.baggageItems.put(key, value);
}
@@ -145,6 +159,20 @@ public class DDSpanContext implements io.opentracing.SpanContext {
return this.tracer;
}
+ /**
+ * 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
+ */
+ public void setTag(String tag, Object value) {
+ if (this.tags.isEmpty()) {
+ this.tags = new HashMap<String, Object>();
+ }
+ this.tags.put(tag, value);
+ }
+
@Override
public String toString() {
return "Span [traceId=" + traceId
@@ -152,4 +180,27 @@ public class DDSpanContext implements io.opentracing.SpanContext {
+ ", parentId=" + parentId + "]";
}
+ public void setOperationName(String operationName) {
+ this.operationName = operationName;
+ }
+
+ public String getOperationName() {
+ return operationName;
+ }
+
+ public Map<String, Object> getTags() {
+ return tags;
+ }
+
+ public void setServiceName(String serviceName) {
+ this.serviceName = serviceName;
+ }
+
+ public void setResourceName(String resourceName) {
+ this.resourceName = resourceName;
+ }
+
+ public void setType(String type) {
+ this.spanType = type;
+ }
}

MODIFY
src/main/java/com/datadoghq/trace/impl/DDTracer.java
src/main/java/com/datadoghq/trace/impl/DDTracer.java
@@ -9,10 +9,7 @@ import io.opentracing.propagation.Format;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
/**
* DDTracer makes it easy to send traces and span to DD using the OpenTracing instrumentation.
@@ -83,10 +80,10 @@ public class DDTracer implements io.opentracing.Tracer {
/**
* Each span must have an operationName according to the opentracing specification
*/
- private final String operationName;
+ private String operationName;
// Builder attributes
- private Map<String, Object> tags = new HashMap<String, Object>();
+ private Map<String, Object> tags = Collections.emptyMap();
private long timestamp;
private SpanContext parent;
private String serviceName;
@@ -104,7 +101,7 @@ public class DDTracer implements io.opentracing.Tracer {
// build the context
DDSpanContext context = buildSpanContext();
- DDSpan span = new DDSpan(this.operationName, this.tags, this.timestamp, context);
+ DDSpan span = new DDSpan(this.timestamp, context);
logger.debug("{} - Starting a new span.", span);
@@ -177,6 +174,9 @@ public class DDTracer implements io.opentracing.Tracer {
// Private methods
private DDTracer.DDSpanBuilder withTag(String tag, Object value) {
+ if (this.tags.isEmpty()){
+ this.tags = new HashMap<String, Object>();
+ }
this.tags.put(tag, value);
return this;
}
@@ -195,7 +195,6 @@ public class DDTracer implements io.opentracing.Tracer {
* @return the context
*/
private DDSpanContext buildSpanContext() {
-
long generatedId = generateNewId();
DDSpanContext context;
DDSpanContext p = this.parent != null ? (DDSpanContext) this.parent : null;
@@ -210,16 +209,20 @@ public class DDTracer implements io.opentracing.Tracer {
serviceName = p.getServiceName();
}
+ //this.operationName, this.tags,
+
// some attributes are inherited from the parent
context = new DDSpanContext(
this.parent == null ? generatedId : p.getTraceId(),
generatedId,
this.parent == null ? 0L : p.getSpanId(),
serviceName,
+ this.operationName,
this.resourceName,
- this.parent == null ? null : p.getBaggageItems(),
+ this.parent == null ? Collections.<String, String>emptyMap() : p.getBaggageItems(),
errorFlag,
spanType,
+ this.tags,
this.parent == null ? null : p.getTrace(),
DDTracer.this
);

MODIFY
src/main/java/com/datadoghq/trace/writer/impl/DDAgentWriter.java
src/main/java/com/datadoghq/trace/writer/impl/DDAgentWriter.java
@@ -75,7 +75,6 @@ public class DDAgentWriter implements Writer {
*/
public void write(List<Span> trace) {
//Try to add a new span in the queue
- //FIXME oldest?
boolean proceed = tokens.tryAcquire(trace.size());
if (proceed) {

MODIFY
src/test/java/com/datadoghq/trace/impl/DDSpanSerializerTest.java
src/test/java/com/datadoghq/trace/impl/DDSpanSerializerTest.java
@@ -28,16 +28,16 @@ public class DDSpanSerializerTest {
2L,
0L,
"service",
+ "operation",
"resource",
baggage,
false,
"type",
+ tags,
null,
null);
span = new DDSpan(
- "operation",
- tags,
100L,
context);
@@ -48,12 +48,14 @@ public class DDSpanSerializerTest {
@Test
public void test() throws Exception {
+
String expected = "{\"meta\":{\"a-baggage\":\"value\",\"k1\":\"v1\"},\"service\":\"service\",\"error\":0,\"type\":\"type\",\"name\":\"operation\",\"duration\":33000,\"resource\":\"resource\",\"start\":100000,\"span_id\":2,\"parent_id\":0,\"trace_id\":1}";
- //FIXME attributes order is not maintained I disabled the test for now
- //assertEquals("
- // , serializer.serialize(span));
// FIXME At the moment, just compare the string sizes
+ try {
assertThat(serializer.serialize(span).length()).isEqualTo(expected.length());
+ } catch (AssertionError e) {
+ assertThat(serializer.serialize(span)).isEqualTo(expected);
+ }
}
}

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 io.opentracing.Span;
import org.junit.Test;
+import java.util.Collections;
+
import static org.assertj.core.api.Assertions.assertThat;
public class DDSpanTest {
- @Test(expected = IllegalArgumentException.class)
- public void shouldHaveServiceName() {
- new DDTracer().buildSpan("operationName").start();
- }
+ @Test
+ public void testGetterSetter() {
+
+ DDSpanContext context = new DDSpanContext(
+ 1L,
+ 1L,
+ 0L,
+ "fakeService",
+ "fakeOperation",
+ "fakeResource",
+ Collections.<String, String>emptyMap(),
+ false,
+ "fakeType",
+ null,
+ null,
+ null);
+
+
+ String expected;
+ DDSpan span = new DDSpan(1L, context);
+
+ expected = "service";
+ span.setServiceName(expected);
+ assertThat(span.getServiceName()).isEqualTo(expected);
+
+ expected = "operation";
+ span.setOperationName(expected);
+ assertThat(span.getOperationName()).isEqualTo(expected);
+
+ expected = "resource";
+ span.setResourceName(expected);
+ assertThat(span.getResourceName()).isEqualTo(expected);
+
+ expected = "type";
+ span.setType(expected);
+ assertThat(span.getType()).isEqualTo(expected);
- @Test(expected = IllegalArgumentException.class)
- public void shouldOperationNameImmutable() {
- Span span = new DDTracer().buildSpan("foo").withServiceName("foo").start();
- span.setOperationName("boom");
}
+
@Test
public void shouldResourceNameEqualsOperationNameIfNull() {
@@ -39,4 +69,6 @@ public class DDSpanTest {
assertThat(span.getResourceName()).isEqualTo(expectedResourceName);
}
+
+
}
\ No newline at end of file

MODIFY
src/test/java/com/datadoghq/trace/impl/RateSamplerTest.java
src/test/java/com/datadoghq/trace/impl/RateSamplerTest.java
package com.datadoghq.trace.impl;
import com.datadoghq.trace.Sampler;
-import com.datadoghq.trace.impl.DDSpan;
-import com.datadoghq.trace.impl.RateSampler;
import org.junit.Test;
import static org.assertj.core.api.Assertions.assertThat;
@@ -14,7 +12,6 @@ public class RateSamplerTest {
@Test
public void testRateSampler() {
- //FIXME test has to be more predictable
DDSpan mockSpan = mock(DDSpan.class);
final double sampleRate = 0.35;
@@ -28,7 +25,7 @@ public class RateSamplerTest {
kept++;
}
}
-
+ //FIXME test has to be more predictable
//assertThat(((double) kept / iterations)).isBetween(sampleRate - 0.02, sampleRate + 0.02);
}

[PR #1] Migrating all things to the span context (and add setters, and do not instantiate HashMap if it's not needed)