From 108a26ecb5a4d21ac53b27249e2d326f3a772d1c Mon Sep 17 00:00:00 2001 From: Doug Chimento Date: Thu, 28 Mar 2019 18:40:28 +0200 Subject: [PATCH 1/5] Opentracing 0.32.0 support Signed-off-by: Doug Chimento --- build.gradle | 2 +- .../io/jaegertracing/internal/JaegerSpan.java | 10 +++++ .../internal/JaegerSpanContext.java | 20 +++++++++- .../jaegertracing/internal/JaegerTracer.java | 32 ++++++++++++++-- .../io/jaegertracing/ConfigurationTest.java | 17 +++++++-- .../internal/ActiveSpanTest.java | 18 +++++++++ .../internal/AdhocHeadersTest.java | 30 +++++++++++++-- .../internal/JaegerSpanTest.java | 33 +++++++++++++++++ .../internal/JaegerTracerTest.java | 27 ++++++++++++++ .../propagation/TextMapCodecTest.java | 37 ++++++++++++++++--- .../internal/ThriftSpanConverterTest.java | 33 +++++++++++++++-- .../zipkin/internal/V2SpanConverterTest.java | 27 +++++++++++++- 12 files changed, 262 insertions(+), 24 deletions(-) diff --git a/build.gradle b/build.gradle index 0c23b66db..2d0212560 100644 --- a/build.gradle +++ b/build.gradle @@ -17,7 +17,7 @@ plugins { ext.developmentVersion = getProperty('developmentVersion','0.34.1-SNAPSHOT') -ext.opentracingVersion = getProperty('opentracingVersion','0.31.0') +ext.opentracingVersion = getProperty('opentracingVersion','0.32.0') ext.guavaVersion = getProperty('guavaVersion','18.0') ext.apacheThriftVersion = getProperty('apacheThriftVersion','0.12.0') ext.jerseyVersion = getProperty('jerseyVersion','2.22.2') diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java index 695a76e8e..1e6dc2ea8 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java @@ -17,6 +17,7 @@ import io.opentracing.Span; import io.opentracing.log.Fields; +import io.opentracing.tag.Tag; import io.opentracing.tag.Tags; import java.io.PrintWriter; import java.io.StringWriter; @@ -74,6 +75,10 @@ public long getStart() { return startTimeMicroseconds; } + public boolean isFinished() { + return finished; + } + public long getDuration() { synchronized (this) { return durationMicroseconds; @@ -206,6 +211,11 @@ public synchronized JaegerSpan setTag(String key, Number value) { return setTagAsObject(key, value); } + @Override + public synchronized Span setTag(Tag tag, T value) { + return setTagAsObject(tag.getKey(), value); + } + private JaegerSpan setTagAsObject(String key, Object value) { if (key.equals(Tags.SAMPLING_PRIORITY.getKey()) && (value instanceof Number)) { int priority = ((Number) value).intValue(); diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java index 2cae01d86..7b24fa688 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java @@ -34,6 +34,8 @@ public class JaegerSpanContext implements SpanContext { private final Map baggage; private final String debugId; private final JaegerObjectFactory objectFactory; + private final String traceIdTextMapCodec; + private final String spanIdTextMapCodec; public JaegerSpanContext(long traceIdHigh, long traceIdLow, long spanId, long parentId, byte flags) { this( @@ -67,6 +69,8 @@ protected JaegerSpanContext( this.baggage = baggage; this.debugId = debugId; this.objectFactory = objectFactory; + this.traceIdTextMapCodec = convertTraceId(); + this.spanIdTextMapCodec = Long.toHexString(spanId); } @Override @@ -82,7 +86,7 @@ Map baggage() { return this.baggage; } - public String getTraceId() { + private String convertTraceId() { if (traceIdHigh == 0L) { return Long.toHexString(traceIdLow); } @@ -97,6 +101,10 @@ public String getTraceId() { return hexStringHigh + hexStringLow; } + public String getTraceId() { + return this.traceIdTextMapCodec; + } + public long getTraceIdLow() { return traceIdLow; } @@ -170,4 +178,14 @@ boolean hasTrace() { String getDebugId() { return debugId; } + + @Override + public String toTraceId() { + return traceIdTextMapCodec; + } + + @Override + public String toSpanId() { + return spanIdTextMapCodec; + } } diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java index fd49d3b03..6e2e3fcf7 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java @@ -42,6 +42,7 @@ import io.opentracing.SpanContext; import io.opentracing.Tracer; import io.opentracing.propagation.Format; +import io.opentracing.tag.Tag; import io.opentracing.tag.Tags; import io.opentracing.util.ThreadLocalScopeManager; import java.io.Closeable; @@ -189,8 +190,7 @@ public ScopeManager scopeManager() { public Span activeSpan() { // the active scope might have been added there through an API extension, similar to what the OT java-metrics // library does -- therefore, we can't guarantee that we are returning a JaegerSpan here. - Scope scope = this.scopeManager.active(); - return scope == null ? null : scope.span(); + return this.scopeManager.activeSpan(); } @Override @@ -301,6 +301,24 @@ public JaegerTracer.SpanBuilder withTag(String key, Number value) { return this; } + @Override + public Tracer.SpanBuilder withTag(Tag tag, T value) { + if (value instanceof Number) { + return this.withTag(tag.getKey(), (Number) value); + } + + if (value instanceof String) { + return this.withTag(tag.getKey(), (String) value); + } + + if (value instanceof Boolean) { + return this.withTag(tag.getKey(), (Boolean) value); + } + + this.tags.put(tag.getKey(), value); + return this; + } + @Override public JaegerTracer.SpanBuilder withStartTimestamp(long microseconds) { this.startTimeMicroseconds = microseconds; @@ -428,8 +446,8 @@ public JaegerSpan start() { JaegerSpanContext context; // Check if active span should be established as CHILD_OF relationship - if (references.isEmpty() && !ignoreActiveSpan && null != scopeManager.active()) { - asChildOf(scopeManager.active().span()); + if (references.isEmpty() && !ignoreActiveSpan && null != scopeManager.activeSpan()) { + asChildOf(scopeManager.activeSpan()); } if (references.isEmpty() || !references.get(0).getSpanContext().hasTrace()) { @@ -486,6 +504,7 @@ public JaegerSpan startManual() { private JaegerObjectFactory getObjectFactory() { return JaegerTracer.this.objectFactory; } + } /** @@ -700,4 +719,9 @@ boolean isExpandExceptionLogs() { public boolean isUseTraceId128Bit() { return this.useTraceId128Bit; } + + @Override + public Scope activateSpan(Span span) { + return scopeManager().activate(span); + } } diff --git a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java index 0d61541c1..abf849e89 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java @@ -479,14 +479,15 @@ public void testCodecWithPropagationB3() { @SuppressWarnings("unchecked") private void assertInjectExtract(JaegerTracer tracer, Format format, JaegerSpanContext contextToInject, boolean injectMapIsEmpty) { - HashMap injectMap = new HashMap<>(); - tracer.inject(contextToInject, format, (C) new TextMapInjectAdapter(injectMap)); + Map injectMap = new HashMap<>(); + TextMap textMap = new TestTextMap(injectMap); + tracer.inject(contextToInject, format, (C) textMap); assertEquals(injectMapIsEmpty, injectMap.isEmpty()); if (injectMapIsEmpty) { return; } - JaegerSpanContext extractedContext = tracer.extract(format, (C) new TextMapExtractAdapter(injectMap)); + JaegerSpanContext extractedContext = tracer.extract(format, (C) textMap); assertEquals(contextToInject.getTraceId(), extractedContext.getTraceId()); assertEquals(contextToInject.getSpanId(), extractedContext.getSpanId()); } @@ -502,7 +503,15 @@ public void testMetricsFactoryFromServiceLoader() { static class TestTextMap implements TextMap { - private Map values = new HashMap<>(); + private final Map values; + + public TestTextMap() { + this(new HashMap<>()); + } + + public TestTextMap(Map values) { + this.values = values; + } @Override public Iterator> iterator() { diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/ActiveSpanTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/ActiveSpanTest.java index f57898dc3..319ba45bf 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/ActiveSpanTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/ActiveSpanTest.java @@ -51,6 +51,13 @@ public void testActiveSpan() { assertEquals(mockSpan, tracer.activeSpan()); } + @Test + public void testActivateSpan() { + JaegerSpan mockSpan = Mockito.mock(JaegerSpan.class); + tracer.activateSpan(mockSpan); + assertEquals(mockSpan, tracer.activeSpan()); + } + @Test public void testActiveSpanPropagation() { try (Scope parent = tracer.buildSpan("parent").startActive(true)) { @@ -158,12 +165,23 @@ public Scope activate(Span span, boolean finishSpanOnClose) { return scope; } + @Override + public Scope activate(Span span) { + return activate(span, false); + } + @Override public Scope active() { return scope; } + + @Override + public Span activeSpan() { + return null; + } }).build(); assertEquals(scope, tracer.scopeManager().active()); + assertEquals(scope, tracer.scopeManager().activate(mock(Span.class))); } diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java index f24fd6e99..a72f5cffd 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; import org.junit.Before; @@ -50,7 +51,7 @@ public void setUp() { @Test public void testDebugCorrelationId() { Map headers = Collections.singletonMap(Constants.DEBUG_ID_HEADER_KEY, "Coraline"); - TextMap carrier = new TextMapExtractAdapter(headers); + TextMap carrier = new TestTextMap(headers); JaegerSpanContext inboundSpanContext = tracer.extract(Format.Builtin.TEXT_MAP, carrier); assertNotNull(inboundSpanContext); @@ -74,7 +75,7 @@ public void testStartTraceWithAdhocBaggage() { public void testJoinTraceWithAdhocBaggage() { Span span = tracer.buildSpan("test").start(); Map headers = new HashMap(); - tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new TextMapInjectAdapter(headers)); + tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new TestTextMap(headers)); assertEquals(1, headers.size()); traceWithAdhocBaggage(headers); @@ -83,11 +84,34 @@ public void testJoinTraceWithAdhocBaggage() { private void traceWithAdhocBaggage(Map headers) { headers.put("jaeger-baggage", "k1=v1, k2 = v2"); - JaegerSpanContext parent = tracer.extract(Format.Builtin.HTTP_HEADERS, new TextMapExtractAdapter(headers)); + JaegerSpanContext parent = tracer.extract(Format.Builtin.HTTP_HEADERS, new TestTextMap(headers)); JaegerSpan span = tracer.buildSpan("test").asChildOf(parent).start(); assertTrue(span.context().isSampled()); assertEquals("must have baggage", "v1", span.getBaggageItem("k1")); assertEquals("must have baggage", "v2", span.getBaggageItem("k2")); } + + private static class TestTextMap implements TextMap { + + private final Map values; + + public TestTextMap(Map values) { + this.values = values; + } + + @Override + public Iterator> iterator() { + return values.entrySet().iterator(); + } + + @Override + public void put(String key, String value) { + values.put(key, value); + } + + public String get(String key) { + return values.get(key); + } + } } diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerSpanTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerSpanTest.java index 843c0fc5e..acb4f9f8d 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerSpanTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerSpanTest.java @@ -37,6 +37,10 @@ import io.opentracing.SpanContext; import io.opentracing.log.Fields; import io.opentracing.noop.NoopSpan; +import io.opentracing.tag.AbstractTag; +import io.opentracing.tag.BooleanTag; +import io.opentracing.tag.IntTag; +import io.opentracing.tag.StringTag; import io.opentracing.tag.Tags; import java.io.PrintWriter; import java.io.StringWriter; @@ -154,6 +158,35 @@ public void testSetNumberTag() { assertEquals(expected, jaegerSpan.getTags().get(key)); } + @Test + public void testSetTag() { + jaegerSpan.setTag(new StringTag("stringTag"), "stringTagValue") + .setTag(new IntTag("numberTag"), 1) + .setTag(new BooleanTag("booleanTag"), true) + .setTag(new AbstractTag("objectTag") { + @Override + public void set(Span span, Object tagValue) { + } + }, this); + + Map tags = jaegerSpan.getTags(); + assertEquals("stringTagValue", tags.get("stringTag")); + assertEquals(1, tags.get("numberTag")); + assertEquals(true, tags.get("booleanTag")); + assertEquals(this, tags.get("objectTag")); + } + + @Test + public void testToTraceId() { + assertEquals(jaegerSpan.context().getTraceId(), jaegerSpan.context().toTraceId()); + } + + @Test + public void testToSpanId() { + assertEquals(Long.toHexString(jaegerSpan.context().getSpanId()), jaegerSpan.context().toSpanId()); + } + + @Test public void testWithTimestampAccurateClock() { testWithTimestamp(true); diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerTracerTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerTracerTest.java index 9a58fffc1..c4ac91983 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerTracerTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/JaegerTracerTest.java @@ -34,9 +34,14 @@ import io.opentracing.Span; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; +import io.opentracing.tag.AbstractTag; +import io.opentracing.tag.BooleanTag; +import io.opentracing.tag.IntTag; +import io.opentracing.tag.StringTag; import io.opentracing.tag.Tags; import java.io.Closeable; +import java.util.Map; import org.junit.Before; import org.junit.Test; @@ -184,4 +189,26 @@ public void testSpanContextNotSampled() { assertEquals(1, metricsFactory.getCounter("jaeger_tracer_traces", "sampled=y,state=started")); assertEquals(0, metricsFactory.getCounter("jaeger_tracer_traces", "sampled=n,state=started")); } + + @Test + public void testWithTagObject() { + JaegerTracer.SpanBuilder spanBuilder = tracer.buildSpan("ndnd"); + spanBuilder.withTag(new StringTag("stringTag"), "stringTagValue") + .withTag(new IntTag("numberTag"), 1) + .withTag(new BooleanTag("booleanTag"), true) + .withTag(new AbstractTag("objectTag") { + @Override + public void set(Span span, Object tagValue) { + } + }, this); + + Span span = spanBuilder.start(); + Map tags = ((JaegerSpan) span).getTags(); + assertEquals("stringTagValue", tags.get("stringTag")); + assertEquals(1, tags.get("numberTag")); + assertEquals(true, tags.get("booleanTag")); + assertEquals(this, tags.get("objectTag")); + span.finish(); + } + } diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java index 567df8e80..b6ec0fa79 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java @@ -22,10 +22,12 @@ import io.jaegertracing.internal.exceptions.EmptyTracerStateStringException; import io.jaegertracing.internal.exceptions.MalformedTracerStateStringException; import io.jaegertracing.internal.exceptions.TraceIdOutOfBoundException; +import io.opentracing.propagation.TextMap; import io.opentracing.propagation.TextMapExtractAdapter; import io.opentracing.propagation.TextMapInjectAdapter; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; import org.junit.Test; @@ -132,19 +134,20 @@ public void testContextAsStringFormatsPositiveFields() { @Test public void testAdhocBaggageWithTraceId() { TextMapCodec codec = new TextMapCodec(false); - Map headers = new HashMap(); + Map headers = new HashMap<>(); long traceIdLow = 42; long spanId = 1; long parentId = 0; - codec.inject(new JaegerSpanContext(0L, traceIdLow, spanId, parentId, (byte)1), new TextMapInjectAdapter(headers)); + TextMap textMap = new TestTextMap(headers); + codec.inject(new JaegerSpanContext(0L, traceIdLow, spanId, parentId, (byte)1), textMap); headers.put("jaeger-baggage", "k1=v1, k2 = v2"); - JaegerSpanContext context = codec.extract(new TextMapExtractAdapter(headers)); + JaegerSpanContext context = codec.extract(textMap); assertEquals("must have trace ID", 42, context.getTraceIdLow()); assertEquals("must have trace ID", 0L, context.getTraceIdHigh()); assertEquals("must have bagggae", "v1", context.getBaggageItem("k1")); assertEquals("must have bagggae", "v2", context.getBaggageItem("k2")); } - + /** * Tests that the codec will return non-null SpanContext even if the only header * present is "jaeger-baggage". @@ -152,11 +155,35 @@ public void testAdhocBaggageWithTraceId() { @Test public void testAdhocBaggageWithoutTraceId() { Map headers = new HashMap(); + TextMap textMap = new TestTextMap(headers); headers.put("jaeger-baggage", "k1=v1, k2 = v2, k3=v3=d3"); TextMapCodec codec = new TextMapCodec(false); - JaegerSpanContext context = codec.extract(new TextMapExtractAdapter(headers)); + JaegerSpanContext context = codec.extract(textMap); assertEquals("v1", context.getBaggageItem("k1")); assertEquals("v2", context.getBaggageItem("k2")); assertEquals(null, context.getBaggageItem("k3")); } + + private static class TestTextMap implements TextMap { + + private final Map values; + + public TestTextMap(Map values) { + this.values = values; + } + + @Override + public Iterator> iterator() { + return values.entrySet().iterator(); + } + + @Override + public void put(String key, String value) { + values.put(key, value); + } + + public String get(String key) { + return values.get(key); + } + } } diff --git a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java index 0959de512..e0467f3c1 100644 --- a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java +++ b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java @@ -35,12 +35,11 @@ import io.jaegertracing.zipkin.internal.ThriftSpanConverter; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; -import io.opentracing.propagation.TextMapExtractAdapter; -import io.opentracing.propagation.TextMapInjectAdapter; import io.opentracing.tag.Tags; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -264,10 +263,9 @@ public void testRpcChildSpanHasTheSameId() { .start(); Map map = new HashMap<>(); - TextMap carrier = new TextMapInjectAdapter(map); + TextMap carrier = new TestTextMap(map); tracer.inject(client.context(), Format.Builtin.TEXT_MAP, carrier); - carrier = new TextMapExtractAdapter(map); JaegerSpanContext ctx = tracer.extract(Format.Builtin.TEXT_MAP, carrier); JaegerSpanContext clientCtx = client.context(); assertEquals(clientCtx.getSpanId(), ctx.getSpanId()); @@ -318,4 +316,31 @@ public void testConvertSpanWith128BitTraceId() { assertEquals(span.context().getTraceIdLow(), zipkinSpan.getTrace_id()); assertEquals(span.context().getTraceIdHigh(), zipkinSpan.getTrace_id_high()); } + + static class TestTextMap implements TextMap { + + private final Map values; + + public TestTextMap() { + this(new HashMap<>()); + } + + public TestTextMap(Map values) { + this.values = values; + } + + @Override + public Iterator> iterator() { + return values.entrySet().iterator(); + } + + @Override + public void put(String key, String value) { + values.put(key, value); + } + + public String get(String key) { + return values.get(key); + } + } } diff --git a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java index 891d94f89..36dae6c01 100644 --- a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java +++ b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java @@ -39,6 +39,7 @@ import io.opentracing.tag.Tags; import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -257,10 +258,9 @@ public void testRpcChildSpanHasTheSameId() { .start(); Map map = new HashMap<>(); - TextMap carrier = new TextMapInjectAdapter(map); + TextMap carrier = new TestTextMap(map); tracer.inject(client.context(), Format.Builtin.TEXT_MAP, carrier); - carrier = new TextMapExtractAdapter(map); JaegerSpanContext ctx = tracer.extract(Format.Builtin.TEXT_MAP, carrier); JaegerSpanContext clientCtx = client.context(); @@ -311,4 +311,27 @@ public void testConvertSpanWith128BitTraceId() { assertNotEquals(0, span.context().getTraceIdHigh()); assertEquals(span.context().getTraceId(), zipkinSpan.traceId()); } + + static class TestTextMap implements TextMap { + + private final Map values; + + public TestTextMap() { + this(new HashMap<>()); + } + + public TestTextMap(Map values) { + this.values = values; + } + + @Override + public Iterator> iterator() { + return values.entrySet().iterator(); + } + + @Override + public void put(String key, String value) { + values.put(key, value); + } + } } From a80e31ad57abcb1c24a1057a412bc8f801d67cd4 Mon Sep 17 00:00:00 2001 From: Doug Chimento Date: Thu, 28 Mar 2019 22:08:18 +0200 Subject: [PATCH 2/5] Use TextMapAdaptor in tests Rename traceIdTextMapCodec -> traceIdAsString Remove checking value type in withTag(Tag tag, T value) Signed-off-by: Doug Chimento --- .../internal/JaegerSpanContext.java | 14 +++---- .../jaegertracing/internal/JaegerTracer.java | 14 +------ .../io/jaegertracing/ConfigurationTest.java | 20 +++------- .../internal/AdhocHeadersTest.java | 33 ++-------------- .../propagation/TextMapCodecTest.java | 38 +++---------------- 5 files changed, 23 insertions(+), 96 deletions(-) diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java index 7b24fa688..806a05420 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java @@ -34,8 +34,8 @@ public class JaegerSpanContext implements SpanContext { private final Map baggage; private final String debugId; private final JaegerObjectFactory objectFactory; - private final String traceIdTextMapCodec; - private final String spanIdTextMapCodec; + private final String traceIdAsString; + private final String spanIdAsString; public JaegerSpanContext(long traceIdHigh, long traceIdLow, long spanId, long parentId, byte flags) { this( @@ -69,8 +69,8 @@ protected JaegerSpanContext( this.baggage = baggage; this.debugId = debugId; this.objectFactory = objectFactory; - this.traceIdTextMapCodec = convertTraceId(); - this.spanIdTextMapCodec = Long.toHexString(spanId); + this.traceIdAsString = convertTraceId(); + this.spanIdAsString = Long.toHexString(spanId); } @Override @@ -102,7 +102,7 @@ private String convertTraceId() { } public String getTraceId() { - return this.traceIdTextMapCodec; + return this.traceIdAsString; } public long getTraceIdLow() { @@ -181,11 +181,11 @@ String getDebugId() { @Override public String toTraceId() { - return traceIdTextMapCodec; + return traceIdAsString; } @Override public String toSpanId() { - return spanIdTextMapCodec; + return spanIdAsString; } } diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java index 6e2e3fcf7..c4f8392c3 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java @@ -303,19 +303,9 @@ public JaegerTracer.SpanBuilder withTag(String key, Number value) { @Override public Tracer.SpanBuilder withTag(Tag tag, T value) { - if (value instanceof Number) { - return this.withTag(tag.getKey(), (Number) value); + if (tag != null && tag.getKey() != null) { + this.tags.put(tag.getKey(), value); } - - if (value instanceof String) { - return this.withTag(tag.getKey(), (String) value); - } - - if (value instanceof Boolean) { - return this.withTag(tag.getKey(), (Boolean) value); - } - - this.tags.put(tag.getKey(), value); return this; } diff --git a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java index abf849e89..4b4cf8c51 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java @@ -40,8 +40,7 @@ import io.opentracing.propagation.Format; import io.opentracing.propagation.Format.Builtin; import io.opentracing.propagation.TextMap; -import io.opentracing.propagation.TextMapExtractAdapter; -import io.opentracing.propagation.TextMapInjectAdapter; +import io.opentracing.propagation.TextMapAdapter; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -479,15 +478,14 @@ public void testCodecWithPropagationB3() { @SuppressWarnings("unchecked") private void assertInjectExtract(JaegerTracer tracer, Format format, JaegerSpanContext contextToInject, boolean injectMapIsEmpty) { - Map injectMap = new HashMap<>(); - TextMap textMap = new TestTextMap(injectMap); - tracer.inject(contextToInject, format, (C) textMap); + HashMap injectMap = new HashMap<>(); + tracer.inject(contextToInject, format, (C) new TextMapAdapter(injectMap)); assertEquals(injectMapIsEmpty, injectMap.isEmpty()); if (injectMapIsEmpty) { return; } - JaegerSpanContext extractedContext = tracer.extract(format, (C) textMap); + JaegerSpanContext extractedContext = tracer.extract(format, (C) new TextMapAdapter(injectMap)); assertEquals(contextToInject.getTraceId(), extractedContext.getTraceId()); assertEquals(contextToInject.getSpanId(), extractedContext.getSpanId()); } @@ -503,15 +501,7 @@ public void testMetricsFactoryFromServiceLoader() { static class TestTextMap implements TextMap { - private final Map values; - - public TestTextMap() { - this(new HashMap<>()); - } - - public TestTextMap(Map values) { - this.values = values; - } + private Map values = new HashMap<>(); @Override public Iterator> iterator() { diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java index a72f5cffd..5bcf4814e 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/AdhocHeadersTest.java @@ -25,12 +25,10 @@ import io.opentracing.Span; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; -import io.opentracing.propagation.TextMapExtractAdapter; -import io.opentracing.propagation.TextMapInjectAdapter; +import io.opentracing.propagation.TextMapAdapter; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.Map; import org.junit.Before; @@ -51,7 +49,7 @@ public void setUp() { @Test public void testDebugCorrelationId() { Map headers = Collections.singletonMap(Constants.DEBUG_ID_HEADER_KEY, "Coraline"); - TextMap carrier = new TestTextMap(headers); + TextMap carrier = new TextMapAdapter(headers); JaegerSpanContext inboundSpanContext = tracer.extract(Format.Builtin.TEXT_MAP, carrier); assertNotNull(inboundSpanContext); @@ -75,7 +73,7 @@ public void testStartTraceWithAdhocBaggage() { public void testJoinTraceWithAdhocBaggage() { Span span = tracer.buildSpan("test").start(); Map headers = new HashMap(); - tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new TestTextMap(headers)); + tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new TextMapAdapter(headers)); assertEquals(1, headers.size()); traceWithAdhocBaggage(headers); @@ -84,34 +82,11 @@ public void testJoinTraceWithAdhocBaggage() { private void traceWithAdhocBaggage(Map headers) { headers.put("jaeger-baggage", "k1=v1, k2 = v2"); - JaegerSpanContext parent = tracer.extract(Format.Builtin.HTTP_HEADERS, new TestTextMap(headers)); + JaegerSpanContext parent = tracer.extract(Format.Builtin.HTTP_HEADERS, new TextMapAdapter(headers)); JaegerSpan span = tracer.buildSpan("test").asChildOf(parent).start(); assertTrue(span.context().isSampled()); assertEquals("must have baggage", "v1", span.getBaggageItem("k1")); assertEquals("must have baggage", "v2", span.getBaggageItem("k2")); } - - private static class TestTextMap implements TextMap { - - private final Map values; - - public TestTextMap(Map values) { - this.values = values; - } - - @Override - public Iterator> iterator() { - return values.entrySet().iterator(); - } - - @Override - public void put(String key, String value) { - values.put(key, value); - } - - public String get(String key) { - return values.get(key); - } - } } diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java index b6ec0fa79..036fad2fa 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java @@ -22,12 +22,9 @@ import io.jaegertracing.internal.exceptions.EmptyTracerStateStringException; import io.jaegertracing.internal.exceptions.MalformedTracerStateStringException; import io.jaegertracing.internal.exceptions.TraceIdOutOfBoundException; -import io.opentracing.propagation.TextMap; -import io.opentracing.propagation.TextMapExtractAdapter; -import io.opentracing.propagation.TextMapInjectAdapter; +import io.opentracing.propagation.TextMapAdapter; import java.util.HashMap; -import java.util.Iterator; import java.util.Map; import org.junit.Test; @@ -134,14 +131,13 @@ public void testContextAsStringFormatsPositiveFields() { @Test public void testAdhocBaggageWithTraceId() { TextMapCodec codec = new TextMapCodec(false); - Map headers = new HashMap<>(); + Map headers = new HashMap(); long traceIdLow = 42; long spanId = 1; long parentId = 0; - TextMap textMap = new TestTextMap(headers); - codec.inject(new JaegerSpanContext(0L, traceIdLow, spanId, parentId, (byte)1), textMap); + codec.inject(new JaegerSpanContext(0L, traceIdLow, spanId, parentId, (byte)1), new TextMapAdapter(headers)); headers.put("jaeger-baggage", "k1=v1, k2 = v2"); - JaegerSpanContext context = codec.extract(textMap); + JaegerSpanContext context = codec.extract(new TextMapAdapter(headers)); assertEquals("must have trace ID", 42, context.getTraceIdLow()); assertEquals("must have trace ID", 0L, context.getTraceIdHigh()); assertEquals("must have bagggae", "v1", context.getBaggageItem("k1")); @@ -155,35 +151,11 @@ public void testAdhocBaggageWithTraceId() { @Test public void testAdhocBaggageWithoutTraceId() { Map headers = new HashMap(); - TextMap textMap = new TestTextMap(headers); headers.put("jaeger-baggage", "k1=v1, k2 = v2, k3=v3=d3"); TextMapCodec codec = new TextMapCodec(false); - JaegerSpanContext context = codec.extract(textMap); + JaegerSpanContext context = codec.extract(new TextMapAdapter(headers)); assertEquals("v1", context.getBaggageItem("k1")); assertEquals("v2", context.getBaggageItem("k2")); assertEquals(null, context.getBaggageItem("k3")); } - - private static class TestTextMap implements TextMap { - - private final Map values; - - public TestTextMap(Map values) { - this.values = values; - } - - @Override - public Iterator> iterator() { - return values.entrySet().iterator(); - } - - @Override - public void put(String key, String value) { - values.put(key, value); - } - - public String get(String key) { - return values.get(key); - } - } } From d2f3eb465427054cb8ee8e7669469823bfe60f9c Mon Sep 17 00:00:00 2001 From: Doug Chimento Date: Thu, 28 Mar 2019 22:57:29 +0200 Subject: [PATCH 3/5] Use TextMapAdaptor in thrift tests Signed-off-by: Doug Chimento --- .../internal/ThriftSpanConverterTest.java | 30 ++----------------- .../zipkin/internal/V2SpanConverterTest.java | 26 ++-------------- 2 files changed, 4 insertions(+), 52 deletions(-) diff --git a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java index e0467f3c1..eaf9e5c9f 100644 --- a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java +++ b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java @@ -35,6 +35,7 @@ import io.jaegertracing.zipkin.internal.ThriftSpanConverter; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; +import io.opentracing.propagation.TextMapAdapter; import io.opentracing.tag.Tags; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -263,7 +264,7 @@ public void testRpcChildSpanHasTheSameId() { .start(); Map map = new HashMap<>(); - TextMap carrier = new TestTextMap(map); + TextMap carrier = new TextMapAdapter(map); tracer.inject(client.context(), Format.Builtin.TEXT_MAP, carrier); JaegerSpanContext ctx = tracer.extract(Format.Builtin.TEXT_MAP, carrier); @@ -316,31 +317,4 @@ public void testConvertSpanWith128BitTraceId() { assertEquals(span.context().getTraceIdLow(), zipkinSpan.getTrace_id()); assertEquals(span.context().getTraceIdHigh(), zipkinSpan.getTrace_id_high()); } - - static class TestTextMap implements TextMap { - - private final Map values; - - public TestTextMap() { - this(new HashMap<>()); - } - - public TestTextMap(Map values) { - this.values = values; - } - - @Override - public Iterator> iterator() { - return values.entrySet().iterator(); - } - - @Override - public void put(String key, String value) { - values.put(key, value); - } - - public String get(String key) { - return values.get(key); - } - } } diff --git a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java index 36dae6c01..cf17af064 100644 --- a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java +++ b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java @@ -34,6 +34,7 @@ import io.jaegertracing.zipkin.internal.V2SpanConverter; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; +import io.opentracing.propagation.TextMapAdapter; import io.opentracing.propagation.TextMapExtractAdapter; import io.opentracing.propagation.TextMapInjectAdapter; import io.opentracing.tag.Tags; @@ -258,7 +259,7 @@ public void testRpcChildSpanHasTheSameId() { .start(); Map map = new HashMap<>(); - TextMap carrier = new TestTextMap(map); + TextMap carrier = new TextMapAdapter(map); tracer.inject(client.context(), Format.Builtin.TEXT_MAP, carrier); JaegerSpanContext ctx = tracer.extract(Format.Builtin.TEXT_MAP, carrier); @@ -311,27 +312,4 @@ public void testConvertSpanWith128BitTraceId() { assertNotEquals(0, span.context().getTraceIdHigh()); assertEquals(span.context().getTraceId(), zipkinSpan.traceId()); } - - static class TestTextMap implements TextMap { - - private final Map values; - - public TestTextMap() { - this(new HashMap<>()); - } - - public TestTextMap(Map values) { - this.values = values; - } - - @Override - public Iterator> iterator() { - return values.entrySet().iterator(); - } - - @Override - public void put(String key, String value) { - values.put(key, value); - } - } } From 5b9fcd32b6614fc0afc59329714e2f0687596ec2 Mon Sep 17 00:00:00 2001 From: Doug Chimento Date: Thu, 28 Mar 2019 23:26:51 +0200 Subject: [PATCH 4/5] Remove unused imports in thrift tests Signed-off-by: Doug Chimento --- .../jaegertracing/zipkin/internal/ThriftSpanConverterTest.java | 1 - .../io/jaegertracing/zipkin/internal/V2SpanConverterTest.java | 3 --- 2 files changed, 4 deletions(-) diff --git a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java index eaf9e5c9f..ffe2ec4d0 100644 --- a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java +++ b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/ThriftSpanConverterTest.java @@ -40,7 +40,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.TreeMap; diff --git a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java index cf17af064..4c77b63c4 100644 --- a/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java +++ b/jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java @@ -35,12 +35,9 @@ import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; import io.opentracing.propagation.TextMapAdapter; -import io.opentracing.propagation.TextMapExtractAdapter; -import io.opentracing.propagation.TextMapInjectAdapter; import io.opentracing.tag.Tags; import java.util.ArrayList; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.TreeMap; From e4f396c297d82e4ee92598ffc66f80328994f62a Mon Sep 17 00:00:00 2001 From: Doug Chimento Date: Thu, 28 Mar 2019 23:43:04 +0200 Subject: [PATCH 5/5] Remove isFinished() in span Signed-off-by: Doug Chimento --- .../src/main/java/io/jaegertracing/internal/JaegerSpan.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java index 1e6dc2ea8..f53d5a7ea 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpan.java @@ -75,10 +75,6 @@ public long getStart() { return startTimeMicroseconds; } - public boolean isFinished() { - return finished; - } - public long getDuration() { synchronized (this) { return durationMicroseconds;