From 588cfef4a4f1b27bf3c39731724d81883bebb9b1 Mon Sep 17 00:00:00 2001 From: Guillaume Polaert Date: Wed, 26 Apr 2017 15:42:55 +0200 Subject: [PATCH 1/2] fixing minor issues --- .../java/com/datadoghq/trace/impl/DDSpan.java | 8 ++-- .../datadoghq/trace/impl/DDSpanContext.java | 3 -- .../java/com/datadoghq/trace/impl/Tracer.java | 13 +++--- .../trace/impl/DDSpanBuilderTest.java | 40 +++++++++++++++---- 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/main/java/com/datadoghq/trace/impl/DDSpan.java b/src/main/java/com/datadoghq/trace/impl/DDSpan.java index e92ca34fe4..7f3e3d999d 100644 --- a/src/main/java/com/datadoghq/trace/impl/DDSpan.java +++ b/src/main/java/com/datadoghq/trace/impl/DDSpan.java @@ -1,14 +1,12 @@ package com.datadoghq.trace.impl; +import com.fasterxml.jackson.annotation.JsonGetter; +import io.opentracing.Span; import io.opentracing.SpanContext; import java.util.Map; import java.util.Optional; -import com.fasterxml.jackson.annotation.JsonGetter; - -import io.opentracing.Span; - public class DDSpan implements io.opentracing.Span { @@ -121,7 +119,7 @@ public class DDSpan implements io.opentracing.Span { @JsonGetter(value = "start") public long getStartTime() { - return startTime * 1000000; + return startTime; } @JsonGetter(value = "duration") diff --git a/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java b/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java index 820677a5ef..f8c5186cd8 100644 --- a/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java +++ b/src/main/java/com/datadoghq/trace/impl/DDSpanContext.java @@ -47,14 +47,11 @@ public class DDSpanContext implements io.opentracing.SpanContext { this.spanId = spanId; this.parentId = parentId; Optional> baggage = Optional.ofNullable(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; } diff --git a/src/main/java/com/datadoghq/trace/impl/Tracer.java b/src/main/java/com/datadoghq/trace/impl/Tracer.java index af84412299..864d690720 100644 --- a/src/main/java/com/datadoghq/trace/impl/Tracer.java +++ b/src/main/java/com/datadoghq/trace/impl/Tracer.java @@ -4,7 +4,7 @@ import io.opentracing.References; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.propagation.Format; -import io.opentracing.tag.Tags; +import sun.reflect.generics.reflectiveObjects.NotImplementedException; import java.util.*; @@ -27,7 +27,7 @@ public class Tracer implements io.opentracing.Tracer { public class SpanBuilder implements io.opentracing.Tracer.SpanBuilder { private final String operationName; - private Map tags = new HashMap(); + private Map tags = new HashMap(); private Long timestamp; private SpanContext parent; private String serviceName; @@ -54,8 +54,7 @@ public class Tracer implements io.opentracing.Tracer { // @todo: implements the notion of referenceType, currently only link a span to a parent one return asChildOf(spanContext); } else { - // do nothing - return this; + throw new IllegalArgumentException(); } } @@ -86,7 +85,6 @@ public class Tracer implements io.opentracing.Tracer { return this; } - public Tracer.SpanBuilder withResourceName(String resourceName) { this.resourceName = resourceName; return this; @@ -103,7 +101,6 @@ public class Tracer implements io.opentracing.Tracer { } - public Span start() { // build the context @@ -133,7 +130,7 @@ public class Tracer implements io.opentracing.Tracer { p.getBaggageItems(), errorFlag, null, - null, + this.spanType, true ); } else { @@ -146,7 +143,7 @@ public class Tracer implements io.opentracing.Tracer { null, errorFlag, null, - null, + this.spanType, true); } diff --git a/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java b/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java index 5d37192dc4..a2eca8993b 100644 --- a/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java +++ b/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java @@ -36,7 +36,7 @@ public class DDSpanBuilderTest { } @Test - public void shouldBuildTaggedSpan() { + public void shouldBuildMoreComplexSpan() { final String expectedName = "fakeName"; final Map tags = new HashMap() { @@ -66,13 +66,32 @@ public class DDSpanBuilderTest { assertThat(span.getTags()).isNotNull(); assertThat(span.getTags()).isEmpty(); + // with all custom fields provided + final String expectedResource = "fakeResource"; + final String expectedService = "fakeService"; + final String expectedType = "fakeType"; + + span = (DDSpan) tracer + .buildSpan(expectedName) + .withResourceName(expectedResource) + .withServiceName(expectedService) + .withErrorFlag() + .withSpanType(expectedType) + .start(); + + DDSpanContext actualContext = (DDSpanContext) span.context(); + + assertThat(actualContext.getResourceName()).isEqualTo(expectedResource); + assertThat(actualContext.getErrorFlag()).isTrue(); + assertThat(actualContext.getServiceName()).isEqualTo(expectedService); + assertThat(actualContext.getSpanType()).isEqualTo(expectedType); } @Test public void shouldBuildSpanTimestampInNano() { - final long expectedTimestamp = 487517802L; + final long expectedTimestamp = 4875178020000L; final String expectedName = "fakeName"; DDSpan span = (DDSpan) tracer @@ -152,15 +171,20 @@ public class DDSpanBuilderTest { actualContext = (DDSpanContext) span.context(); assertThat(actualContext.getParentId()).isEqualTo(expectedParentId); + + } + + @Test(expected = IllegalArgumentException.class) + public void shouldErrorUnknownReferenceType() { + + DDSpanContext mockedContext = mock(DDSpanContext.class); + when(mockedContext.getSpanId()).thenReturn(123L); + // case 2, using a WFT ref, should not be linked to the previous - span = (DDSpan) tracer - .buildSpan(expectedName) + DDSpan span = (DDSpan) tracer + .buildSpan("fakeName") .addReference("WTF", mockedContext) .start(); - - actualContext = (DDSpanContext) span.context(); - assertThat(actualContext.getParentId()).isEqualTo(0L); - } @Test From 52626f9dcb6262c7033beb6cd716de8b19a3a870 Mon Sep 17 00:00:00 2001 From: Guillaume Polaert Date: Wed, 26 Apr 2017 15:52:59 +0200 Subject: [PATCH 2/2] adding unsupported exceptions --- .../java/com/datadoghq/trace/impl/Tracer.java | 14 ++---- .../trace/impl/DDSpanBuilderTest.java | 48 ------------------- 2 files changed, 4 insertions(+), 58 deletions(-) diff --git a/src/main/java/com/datadoghq/trace/impl/Tracer.java b/src/main/java/com/datadoghq/trace/impl/Tracer.java index 864d690720..616b97bf16 100644 --- a/src/main/java/com/datadoghq/trace/impl/Tracer.java +++ b/src/main/java/com/datadoghq/trace/impl/Tracer.java @@ -15,19 +15,19 @@ public class Tracer implements io.opentracing.Tracer { return new SpanBuilder(operationName); } - public void inject(SpanContext spanContext, Format format, C c) { + throw new UnsupportedOperationException(); } public SpanContext extract(Format format, C c) { - return null; + throw new UnsupportedOperationException(); } public class SpanBuilder implements io.opentracing.Tracer.SpanBuilder { private final String operationName; - private Map tags = new HashMap(); + private Map tags = new HashMap<>(); private Long timestamp; private SpanContext parent; private String serviceName; @@ -49,13 +49,7 @@ public class Tracer implements io.opentracing.Tracer { } public Tracer.SpanBuilder addReference(String referenceType, SpanContext spanContext) { - - if (References.CHILD_OF.equals(referenceType) || References.FOLLOWS_FROM.equals(referenceType)) { - // @todo: implements the notion of referenceType, currently only link a span to a parent one - return asChildOf(spanContext); - } else { - throw new IllegalArgumentException(); - } + throw new UnsupportedOperationException(); } public Tracer.SpanBuilder withTag(String tag, Number number) { diff --git a/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java b/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java index a2eca8993b..bf2a456c7c 100644 --- a/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java +++ b/src/test/java/com/datadoghq/trace/impl/DDSpanBuilderTest.java @@ -136,56 +136,8 @@ public class DDSpanBuilderTest { assertThat(actualContext.getParentId()).isEqualTo(expectedParentId); - } - @Test - public void shouldLinkViaReferenceType() { - - - final long spanId = 223L; - final long expectedParentId = spanId; - - DDSpanContext mockedContext = mock(DDSpanContext.class); - when(mockedContext.getSpanId()).thenReturn(spanId); - - final String expectedName = "fakeName"; - - - // case 1, using a CHILD_OF ref - DDSpan span = (DDSpan) tracer - .buildSpan(expectedName) - .addReference(References.CHILD_OF, mockedContext) - .start(); - - DDSpanContext actualContext = (DDSpanContext) span.context(); - assertThat(actualContext.getParentId()).isEqualTo(expectedParentId); - - - // case 2, using a FOLLOW_FROM ref - span = (DDSpan) tracer - .buildSpan(expectedName) - .addReference(References.FOLLOWS_FROM, mockedContext) - .start(); - - actualContext = (DDSpanContext) span.context(); - assertThat(actualContext.getParentId()).isEqualTo(expectedParentId); - - - } - - @Test(expected = IllegalArgumentException.class) - public void shouldErrorUnknownReferenceType() { - - DDSpanContext mockedContext = mock(DDSpanContext.class); - when(mockedContext.getSpanId()).thenReturn(123L); - - // case 2, using a WFT ref, should not be linked to the previous - DDSpan span = (DDSpan) tracer - .buildSpan("fakeName") - .addReference("WTF", mockedContext) - .start(); - } @Test public void shouldInheritOfTheDDParentAttributes() {