From a6bcea9b3d4d0d03ce508ab8a0fcfab2ffb7d916 Mon Sep 17 00:00:00 2001 From: Guillaume Polaert Date: Thu, 27 Apr 2017 16:00:25 +0200 Subject: [PATCH] cleaning and moving all att to the context --- pom.xml | 4 +- .../datadoghq/trace/Utils/TracerLogger.java | 19 -- .../java/com/datadoghq/trace/impl/DDSpan.java | 111 +++++++---- .../datadoghq/trace/impl/DDSpanContext.java | 86 ++++++--- .../com/datadoghq/trace/impl/DDTracer.java | 172 +++++++++--------- .../datadoghq/trace/writer/impl/DDApi.java | 3 +- src/test/java/Example.java | 23 ++- 7 files changed, 240 insertions(+), 178 deletions(-) delete mode 100644 src/main/java/com/datadoghq/trace/Utils/TracerLogger.java diff --git a/pom.xml b/pom.xml index 4c68604ca9..48b1b6d67a 100644 --- a/pom.xml +++ b/pom.xml @@ -61,8 +61,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.8 - 1.8 + 1.6 + 1.6 diff --git a/src/main/java/com/datadoghq/trace/Utils/TracerLogger.java b/src/main/java/com/datadoghq/trace/Utils/TracerLogger.java deleted file mode 100644 index 9e34d94092..0000000000 --- a/src/main/java/com/datadoghq/trace/Utils/TracerLogger.java +++ /dev/null @@ -1,19 +0,0 @@ -package com.datadoghq.trace.Utils; - - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class TracerLogger { - - private Logger logger = LoggerFactory.getLogger("com.datadoghq.trace"); - - private final String startNewSpan = "Starting new span - %s [%s]"; - - public void startNewSpan(String operationName, long spanId) { - - if (!logger.isTraceEnabled()) return; - logger.trace(String.format(startNewSpan, operationName, String.valueOf(spanId))); - } - -} diff --git a/src/main/java/com/datadoghq/trace/impl/DDSpan.java b/src/main/java/com/datadoghq/trace/impl/DDSpan.java index f5cfc3da6e..1351cdcfe5 100644 --- a/src/main/java/com/datadoghq/trace/impl/DDSpan.java +++ b/src/main/java/com/datadoghq/trace/impl/DDSpan.java @@ -1,67 +1,83 @@ package com.datadoghq.trace.impl; -import java.time.Clock; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Optional; - import com.fasterxml.jackson.annotation.JsonGetter; -import com.fasterxml.jackson.annotation.JsonIgnore; - import io.opentracing.Span; import io.opentracing.SpanContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.Map; public class DDSpan implements io.opentracing.Span { - protected final DDTracer tracer; protected String operationName; protected Map tags; + protected final long startTime; protected long startTimeNano; protected long durationNano; protected final DDSpanContext context; - protected final List trace; + + private final static Logger logger = LoggerFactory.getLogger(DDSpan.class); DDSpan( - DDTracer tracer, String operationName, - List trace, Map tags, Long timestampMilliseconds, DDSpanContext context) { - this.tracer = tracer; this.operationName = operationName; - this.trace = Optional.ofNullable(trace).orElse(new ArrayList<>()); this.tags = tags; - this.startTimeNano = Optional.ofNullable(timestampMilliseconds).orElse(Clock.systemUTC().millis()) * 1000000L; this.context = context; + // record the start time in nano (current milli + nano delta) + if (timestampMilliseconds == 0L) { + this.startTime = System.currentTimeMillis(); + } else { + this.startTime = timestampMilliseconds; + } + this.startTimeNano = System.nanoTime(); + // track each span of the trace - this.trace.add(this); + this.context.getTrace().add(this); - } + // check DD attributes required + if (this.context.getServiceName() == null) { + throw new IllegalArgumentException("No ServiceName provided"); + } - public SpanContext context() { - return this.context; - } + logger.debug("Starting a new span. " + this.toString()); - public void finish() { - finish(Clock.systemUTC().millis()); } public void finish(long stopTimeMillis) { - this.durationNano = (stopTimeMillis * 1000000L - startTimeNano) ; + + // formula: millis(stop - start) * 1000 * 1000 + nano(stop - start) + long stopTimeNano = System.nanoTime(); + this.durationNano = (stopTimeMillis - startTime) * 1000000L + (stopTimeNano - this.startTimeNano); + + logger.debug("Finishing the span." + this.toString()); + + // warn if one of the parent's children is not finished if (this.isRootSpan()) { - this.trace.stream() - .filter(s -> { - boolean isSelf = ((DDSpanContext) s.context()).getSpanId() == ((DDSpanContext) this.context()).getSpanId(); - boolean isFinished = ((DDSpan) s).getDurationNano() != 0L; - return !isSelf && !isFinished; - }) - .forEach(Span::finish); + logger.debug("Checking all children attached to the current root span"); + List spans = this.context.getTrace(); + for (Span span : spans) { + if (((DDSpan) span).getDurationNano() == 0L) { + // FIXME + logger.warn("The parent span is marked as finished but this span isn't. You have to close each children." + this.toString()); + } + } + this.context.getTracer().write(this.context.getTrace()); + logger.debug("Sending the trace to the writer"); + } + + } + + public void finish() { + finish(System.currentTimeMillis()); } public void close() { @@ -105,6 +121,10 @@ public class DDSpan implements io.opentracing.Span { return null; } + public SpanContext context() { + return this.context; + } + public Span setBaggageItem(String key, String value) { this.context.setBaggageItem(key, value); return this; @@ -115,8 +135,7 @@ public class DDSpan implements io.opentracing.Span { } public Span setOperationName(String operationName) { - // FIXME: @renaud, the operationName (mandatory) is always set by the constructor - // FIXME: should be an UnsupportedOperation if we don't want to update the operationName + final + // FIXME operationName is in each constructor --> always IAE if (this.operationName != null) { throw new IllegalArgumentException("The operationName is already assigned."); } @@ -133,7 +152,6 @@ public class DDSpan implements io.opentracing.Span { } //Getters and JSON serialisation instructions - @JsonGetter(value = "name") public String getOperationName() { return operationName; @@ -175,7 +193,7 @@ public class DDSpan implements io.opentracing.Span { @JsonGetter(value = "resource") public String getResourceName() { - return context.getResourceName() == null ? getOperationName() : context.getResourceName(); + return context.getResourceName() == null ? this.operationName : context.getResourceName(); } public String getType() { @@ -186,8 +204,27 @@ public class DDSpan implements io.opentracing.Span { return context.getErrorFlag() ? 1 : 0; } - @JsonIgnore - public List getTrace() { - return trace; + @Override + public String toString() { + return "Span{" + + "'operationName='" + operationName + '\'' + + ", tags=" + tags + + ", startTime=" + startTime + + ", startTimeNano=" + startTimeNano + + ", durationNano=" + durationNano + + ", context=" + context + + '}'; + } + + + @Override + public int hashCode() { + int result = operationName != null ? operationName.hashCode() : 0; + result = 31 * result + (tags != null ? tags.hashCode() : 0); + result = 31 * result + (int) (startTime ^ (startTime >>> 32)); + result = 31 * result + (int) (startTimeNano ^ (startTimeNano >>> 32)); + result = 31 * result + (int) (durationNano ^ (durationNano >>> 32)); + result = 31 * result + (context != null ? context.hashCode() : 0); + return result; } } diff --git a/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java b/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java index f8c5186cd8..044d864e3e 100644 --- a/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java +++ b/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java @@ -1,37 +1,33 @@ package com.datadoghq.trace.impl; +import com.fasterxml.jackson.annotation.JsonIgnore; +import io.opentracing.Span; + +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; -import java.util.Optional; public class DDSpanContext implements io.opentracing.SpanContext { - // Public span attributes - private final String serviceName; - private final String resourceName; + private static final String SPAN_TYPE_DEFAULT = "custom"; + // Opentracing attributes private final long traceId; private final long spanId; private final long parentId; - private final Map baggageItems; // know as 'meta' in dd-trace-py + private final Map baggageItems; + // DD attributes + private final String serviceName; + private final String resourceName; private final boolean errorFlag; private final Map metrics; private final String spanType; - // Sampler attributes + private final List trace; + // Others attributes private boolean sampled; + private DDTracer tracer; - // Testing purpose, @todo better mock - DDSpanContext() { - serviceName = null; - resourceName = null; - traceId = 0; - spanId = 0; - parentId = 0; - baggageItems = new HashMap<>(); - errorFlag = false; - metrics = null; - spanType = null; - } public DDSpanContext( long traceId, @@ -43,29 +39,55 @@ public class DDSpanContext implements io.opentracing.SpanContext { boolean errorFlag, Map metrics, String spanType, - boolean sampled) { this.traceId = traceId; + boolean sampled, + List trace, + DDTracer tracer) { + + this.traceId = traceId; this.spanId = spanId; this.parentId = parentId; - Optional> baggage = Optional.ofNullable(baggageItems); + + if (baggageItems == null) { + this.baggageItems = new HashMap(); + } else { + this.baggageItems = baggageItems; + } this.serviceName = serviceName; this.resourceName = resourceName; - this.baggageItems = baggage.orElse(new HashMap<>()); this.errorFlag = errorFlag; this.metrics = metrics; this.spanType = spanType; this.sampled = sampled; + + if (trace == null) { + this.trace = new ArrayList(); + } else { + this.trace = trace; + } + + this.tracer = tracer; } + protected static DDSpanContext newContext(long generateId, String serviceName, String resourceName) { + DDSpanContext context = new DDSpanContext( + // Opentracing attributes + generateId, generateId, 0L, + // DD attributes + serviceName, resourceName, + // Other stuff + null, false, null, + DDSpanContext.SPAN_TYPE_DEFAULT, true, + null, null - public Iterable> baggageItems() { - return this.baggageItems.entrySet(); + ); + return context; } + public long getTraceId() { return this.traceId; } - public long getParentId() { return this.parentId; } @@ -74,7 +96,6 @@ public class DDSpanContext implements io.opentracing.SpanContext { return this.spanId; } - public String getServiceName() { return serviceName; } @@ -110,4 +131,19 @@ public class DDSpanContext implements io.opentracing.SpanContext { public Map getBaggageItems() { return baggageItems; } + + public Iterable> baggageItems() { + return this.baggageItems.entrySet(); + } + + @JsonIgnore + public List getTrace() { + return this.trace; + } + + @JsonIgnore + public DDTracer getTracer() { + return this.tracer; + } + } diff --git a/src/main/java/com/datadoghq/trace/impl/DDTracer.java b/src/main/java/com/datadoghq/trace/impl/DDTracer.java index a061eb2848..1a4782ece4 100644 --- a/src/main/java/com/datadoghq/trace/impl/DDTracer.java +++ b/src/main/java/com/datadoghq/trace/impl/DDTracer.java @@ -1,17 +1,31 @@ package com.datadoghq.trace.impl; -import java.util.*; - -import com.datadoghq.trace.Utils.TracerLogger; - +import com.datadoghq.trace.Sampler; +import com.datadoghq.trace.Writer; import io.opentracing.Span; import io.opentracing.SpanContext; 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; public class DDTracer implements io.opentracing.Tracer { - private TracerLogger logger = new TracerLogger(); + + private Writer writer; + private final Sampler sampler; + + private final static Logger logger = LoggerFactory.getLogger(DDTracer.class); + + public DDTracer(Writer writer, Sampler sampler) { + this.writer = writer; + this.sampler = sampler; + } public DDSpanBuilder buildSpan(String operationName) { return new DDSpanBuilder(operationName); @@ -19,13 +33,16 @@ public class DDTracer implements io.opentracing.Tracer { public void inject(SpanContext spanContext, Format format, C c) { throw new UnsupportedOperationException(); - } public SpanContext extract(Format format, C c) { throw new UnsupportedOperationException(); } + public void write(List trace) { + this.writer.write(trace); + } + public class DDSpanBuilder implements SpanBuilder { private final String operationName; @@ -37,23 +54,19 @@ public class DDTracer implements io.opentracing.Tracer { private boolean errorFlag; private String spanType; - public DDSpanBuilder(String operationName) { - this.operationName = operationName; - } + public Span start() { - public DDTracer.DDSpanBuilder asChildOf(SpanContext spanContext) { - throw new UnsupportedOperationException("Should be a complete span"); - //this.parent = spanContext; - //return this; - } + // build the context + DDSpanContext context = buildTheSpanContext(); - public DDTracer.DDSpanBuilder asChildOf(Span span) { - this.parent = (DDSpan) span; - return this; - } + // FIXME + logger.debug("Starting new span " + this.operationName); - public DDTracer.DDSpanBuilder addReference(String referenceType, SpanContext spanContext) { - throw new UnsupportedOperationException(); + return new DDSpan( + this.operationName, + this.tags, + this.timestamp, + context); } public DDTracer.DDSpanBuilder withTag(String tag, Number number) { @@ -68,8 +81,12 @@ public class DDTracer implements io.opentracing.Tracer { return withTag(tag, (Object) bool); } - private DDTracer.DDSpanBuilder withTag(String tag, Object value) { - this.tags.put(tag, value); + public DDSpanBuilder(String operationName) { + this.operationName = operationName; + } + + public DDTracer.DDSpanBuilder asChildOf(Span span) { + this.parent = (DDSpan) span; return this; } @@ -98,72 +115,63 @@ public class DDTracer implements io.opentracing.Tracer { return this; } - - public Span start() { - - // build the context - DDSpanContext context = buildTheSpanContext(); - logger.startNewSpan(this.operationName, context.getSpanId()); - - List trace = null; - if (this.parent != null) { - trace = parent.getTrace(); - } - - return new DDSpan( - DDTracer.this, - this.operationName, - trace, - this.tags, - this.timestamp, - context); - } - - private DDSpanContext buildTheSpanContext() { - - DDSpanContext context; - - long generatedId = generateNewId(); - if (this.parent != null) { - DDSpanContext p = (DDSpanContext) this.parent.context(); - context = new DDSpanContext( - p.getTraceId(), - generatedId, - p.getSpanId(), - Optional.ofNullable(this.serviceName).orElse(p.getServiceName()), - Optional.ofNullable(this.resourceName).orElse(this.operationName), - p.getBaggageItems(), - errorFlag, - null, - Optional.ofNullable(this.spanType).orElse(p.getSpanType()), - true - ); - } else { - context = new DDSpanContext( - generatedId, - generatedId, - 0L, - this.serviceName, - Optional.ofNullable(this.resourceName).orElse(this.operationName), - null, - errorFlag, - null, - this.spanType, - true); - } - - return context; - } - public Iterable> baggageItems() { if (parent == null) { return Collections.emptyList(); } return parent.context().baggageItems(); } + + // UnsupportedOperation + public DDTracer.DDSpanBuilder asChildOf(SpanContext spanContext) { + throw new UnsupportedOperationException("Should be a Span instead of a context due to DD implementation"); + } + + public DDTracer.DDSpanBuilder addReference(String referenceType, SpanContext spanContext) { + throw new UnsupportedOperationException(); + } + + + // Private methods + private DDTracer.DDSpanBuilder withTag(String tag, Object value) { + this.tags.put(tag, value); + return this; + } + + private long generateNewId() { + return System.nanoTime(); + } + + + private DDSpanContext buildTheSpanContext() { + + long generatedId = generateNewId(); + DDSpanContext context; + DDSpanContext p = this.parent != null ? (DDSpanContext) this.parent.context() : null; + + // some attributes are inherited from the parent + context = new DDSpanContext( + this.parent == null ? generatedId : p.getTraceId(), + generatedId, + this.parent == null ? 0L : p.getSpanId(), + this.parent == null ? this.serviceName : p.getServiceName(), + this.resourceName, + this.parent == null ? null : p.getBaggageItems(), + errorFlag, + null, + this.spanType, + true, + this.parent == null ? null : p.getTrace(), + DDTracer.this + ); + + logger.debug("Building a new span context." + context.toString()); + + return context; + } + + } - long generateNewId() { - return System.nanoTime(); - } + } diff --git a/src/main/java/com/datadoghq/trace/writer/impl/DDApi.java b/src/main/java/com/datadoghq/trace/writer/impl/DDApi.java index 6b022eb38d..c6b0db2fee 100644 --- a/src/main/java/com/datadoghq/trace/writer/impl/DDApi.java +++ b/src/main/java/com/datadoghq/trace/writer/impl/DDApi.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.List; import com.datadoghq.trace.SpanSerializer; +import com.datadoghq.trace.impl.DDSpan; import com.datadoghq.trace.impl.DDSpanSerializer; import com.datadoghq.trace.impl.DDTracer; @@ -115,7 +116,7 @@ public class DDApi { parent.finish(); DDAgentWriter writer = new DDAgentWriter(); - writer.write(array); + writer.write(((DDSpan)parent.getTrace()); Thread.sleep(1000); diff --git a/src/test/java/Example.java b/src/test/java/Example.java index d560494947..582113a985 100644 --- a/src/test/java/Example.java +++ b/src/test/java/Example.java @@ -4,17 +4,17 @@ import java.util.List; import com.datadoghq.trace.Writer; import com.datadoghq.trace.impl.DDTracer; import com.datadoghq.trace.writer.impl.DDAgentWriter; -import com.fasterxml.jackson.databind.ObjectMapper; import io.opentracing.Span; public class Example { - public static void main(String[] args) throws Exception{ - List trace = new ArrayList(); - - DDTracer tracer = new DDTracer(); + public static void main(String[] args) throws Exception { + + Writer writer = new DDAgentWriter(); + DDTracer tracer = new DDTracer(writer, null); + Span parent = tracer .buildSpan("hello-world") @@ -23,20 +23,19 @@ public class Example { .start(); parent.setBaggageItem("a-baggage", "value"); - trace.add(parent); - parent.finish(); + Span child = tracer .buildSpan("hello-world") .asChildOf(parent) .withResourceName("resource-name") - .start(); + .start(); + child.finish(); - trace.add(child); + parent.finish(); - - writer.write(trace); - + + writer.close(); }