-
Notifications
You must be signed in to change notification settings - Fork 232
Upgrade to OpenTracing java 0.30.0 with in-process propagation support #188
Changes from 4 commits
149ef6b
2b5a9fc
4c75232
05efbe9
6ae9e9b
7d9ce5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,9 +36,15 @@ | |
import com.uber.jaeger.utils.Clock; | ||
import com.uber.jaeger.utils.SystemClock; | ||
import com.uber.jaeger.utils.Utils; | ||
|
||
import io.opentracing.ActiveSpan; | ||
import io.opentracing.ActiveSpanSource; | ||
import io.opentracing.BaseSpan; | ||
import io.opentracing.References; | ||
import io.opentracing.propagation.Format; | ||
import io.opentracing.tag.Tags; | ||
import io.opentracing.util.ThreadLocalActiveSpanSource; | ||
|
||
import java.io.InputStream; | ||
import java.net.Inet4Address; | ||
import java.net.InetAddress; | ||
|
@@ -52,7 +58,7 @@ | |
import lombok.ToString; | ||
import lombok.extern.slf4j.Slf4j; | ||
|
||
@ToString(exclude = {"registry", "clock", "metrics"}) | ||
@ToString(exclude = {"registry", "clock", "metrics", "activeSpanSource"}) | ||
@Slf4j | ||
public class Tracer implements io.opentracing.Tracer { | ||
|
||
|
@@ -66,6 +72,7 @@ public class Tracer implements io.opentracing.Tracer { | |
private final int ip; | ||
private final Map<String, ?> tags; | ||
private final boolean zipkinSharedRpcSpan; | ||
private final ActiveSpanSource activeSpanSource; | ||
|
||
private Tracer( | ||
String serviceName, | ||
|
@@ -75,14 +82,16 @@ private Tracer( | |
Clock clock, | ||
Metrics metrics, | ||
Map<String, Object> tags, | ||
boolean zipkinSharedRpcSpan) { | ||
boolean zipkinSharedRpcSpan, | ||
ActiveSpanSource activeSpanSource) { | ||
this.serviceName = serviceName; | ||
this.reporter = reporter; | ||
this.sampler = sampler; | ||
this.registry = registry; | ||
this.clock = clock; | ||
this.metrics = metrics; | ||
this.zipkinSharedRpcSpan = zipkinSharedRpcSpan; | ||
this.activeSpanSource = activeSpanSource; | ||
|
||
int ip; | ||
try { | ||
|
@@ -158,6 +167,16 @@ public <T> io.opentracing.SpanContext extract(Format<T> format, T carrier) { | |
return extractor.extract(carrier); | ||
} | ||
|
||
@Override | ||
public ActiveSpan activeSpan() { | ||
return activeSpanSource.activeSpan(); | ||
} | ||
|
||
@Override | ||
public ActiveSpan makeActive(io.opentracing.Span span) { | ||
return activeSpanSource.makeActive(span); | ||
} | ||
|
||
/** | ||
* Shuts down the {@link Reporter} and {@link Sampler} | ||
*/ | ||
|
@@ -177,6 +196,7 @@ class SpanBuilder implements io.opentracing.Tracer.SpanBuilder { | |
*/ | ||
private List<Reference> references = Collections.emptyList(); | ||
private final Map<String, Object> tags = new HashMap<String, Object>(); | ||
private boolean ignoreActiveSpan = false; | ||
|
||
SpanBuilder(String operationName) { | ||
this.operationName = operationName; | ||
|
@@ -188,7 +208,7 @@ public io.opentracing.Tracer.SpanBuilder asChildOf(io.opentracing.SpanContext pa | |
} | ||
|
||
@Override | ||
public io.opentracing.Tracer.SpanBuilder asChildOf(io.opentracing.Span parent) { | ||
public io.opentracing.Tracer.SpanBuilder asChildOf(BaseSpan<?> parent) { | ||
return addReference(References.CHILD_OF, parent.context()); | ||
} | ||
|
||
|
@@ -349,9 +369,14 @@ private String debugId() { | |
} | ||
|
||
@Override | ||
public io.opentracing.Span start() { | ||
public io.opentracing.Span startManual() { | ||
SpanContext context; | ||
|
||
// Check if active span should be established as CHILD_OF relationship | ||
if (!ignoreActiveSpan && null != activeSpanSource.activeSpan()) { | ||
asChildOf(activeSpanSource.activeSpan()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lgtm. Just one more thing - reading the API docs, the active span should only be used as a parent if there are no other references added explicitly. So need to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And probably a respective unit test is missing for this scenario. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will add test soon - have to sign off for a while. |
||
|
||
String debugId = debugId(); | ||
if (references.isEmpty()) { | ||
context = createNewContext(null); | ||
|
@@ -395,6 +420,23 @@ public io.opentracing.Span start() { | |
metrics.spansStarted.inc(1); | ||
return span; | ||
} | ||
|
||
@Override | ||
public ActiveSpan startActive() { | ||
return activeSpanSource.makeActive(startManual()); | ||
} | ||
|
||
@Override | ||
public io.opentracing.Tracer.SpanBuilder ignoreActiveSpan() { | ||
ignoreActiveSpan = true; | ||
return this; | ||
} | ||
|
||
@Override | ||
@Deprecated | ||
public io.opentracing.Span start() { | ||
return startManual(); | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -409,6 +451,7 @@ public static final class Builder { | |
private Clock clock = new SystemClock(); | ||
private Map<String, Object> tags = new HashMap<String, Object>(); | ||
private boolean zipkinSharedRpcSpan; | ||
private ActiveSpanSource activeSpanSource = new ThreadLocalActiveSpanSource(); | ||
|
||
public Builder(String serviceName, Reporter reporter, Sampler sampler) { | ||
if (serviceName == null || serviceName.trim().length() == 0) { | ||
|
@@ -443,6 +486,11 @@ public Builder withStatsReporter(StatsReporter statsReporter) { | |
return this; | ||
} | ||
|
||
public Builder withActiveSpanSource(ActiveSpanSource activeSpanSource) { | ||
this.activeSpanSource = activeSpanSource; | ||
return this; | ||
} | ||
|
||
public Builder withClock(Clock clock) { | ||
this.clock = clock; | ||
return this; | ||
|
@@ -475,7 +523,7 @@ public Builder withTag(String key, Number value) { | |
|
||
public Tracer build() { | ||
return new Tracer(this.serviceName, reporter, sampler, registry, clock, metrics, tags, | ||
zipkinSharedRpcSpan); | ||
zipkinSharedRpcSpan, activeSpanSource); | ||
} | ||
} | ||
|
||
|
@@ -530,4 +578,5 @@ String getHostName() { | |
return null; | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,10 +23,16 @@ | |
package com.uber.jaeger; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertNotEquals; | ||
import static org.junit.Assert.assertTrue; | ||
import static org.mockito.Mockito.mock; | ||
|
||
import com.uber.jaeger.reporters.InMemoryReporter; | ||
import com.uber.jaeger.samplers.ConstSampler; | ||
|
||
import io.opentracing.ActiveSpan; | ||
import io.opentracing.ActiveSpanSource; | ||
import io.opentracing.References; | ||
import io.opentracing.propagation.Format; | ||
import io.opentracing.propagation.TextMap; | ||
import io.opentracing.propagation.TextMapExtractAdapter; | ||
|
@@ -45,10 +51,82 @@ public void testDebugCorrelationId() { | |
SpanContext spanContext = (SpanContext) tracer.extract(Format.Builtin.TEXT_MAP, carrier); | ||
assertTrue(spanContext.isDebugIdContainerOnly()); | ||
assertEquals("Coraline", spanContext.getDebugId()); | ||
Span span = (Span) tracer.buildSpan("span").asChildOf(spanContext).start(); | ||
Span span = (Span) tracer.buildSpan("span").asChildOf(spanContext).startManual(); | ||
spanContext = (SpanContext) span.context(); | ||
assertTrue(spanContext.isSampled()); | ||
assertTrue(spanContext.isDebug()); | ||
assertEquals("Coraline", span.getTags().get(Constants.DEBUG_ID_HEADER_KEY)); | ||
} | ||
|
||
@Test | ||
public void testActiveSpanPropagation() { | ||
Tracer tracer = | ||
new Tracer.Builder("test", new InMemoryReporter(), new ConstSampler(true)).build(); | ||
try (ActiveSpan parent = tracer.buildSpan("parent").startActive()) { | ||
assertEquals(parent, tracer.activeSpan()); | ||
} | ||
} | ||
|
||
@Test | ||
public void testActiveSpanAutoReference() { | ||
InMemoryReporter reporter = new InMemoryReporter(); | ||
Tracer tracer = | ||
new Tracer.Builder("test", reporter, new ConstSampler(true)).build(); | ||
try (ActiveSpan parent = tracer.buildSpan("parent").startActive()) { | ||
tracer.buildSpan("child").startActive().deactivate(); | ||
} | ||
assertEquals(2, reporter.getSpans().size()); | ||
|
||
Span childSpan = reporter.getSpans().get(0); | ||
Span parentSpan = reporter.getSpans().get(1); | ||
|
||
assertEquals("child", childSpan.getOperationName()); | ||
assertEquals(1, childSpan.getReferences().size()); | ||
assertEquals("parent", parentSpan.getOperationName()); | ||
assertTrue(parentSpan.getReferences().isEmpty()); | ||
assertEquals(References.CHILD_OF, childSpan.getReferences().get(0).getType()); | ||
assertEquals(parentSpan.context(), | ||
childSpan.getReferences().get(0).getSpanContext()); | ||
assertEquals(parentSpan.context().getTraceId(), childSpan.context().getTraceId()); | ||
assertEquals(parentSpan.context().getSpanId(), childSpan.context().getParentId()); | ||
} | ||
|
||
@Test | ||
public void testIgnoreActiveSpan() { | ||
InMemoryReporter reporter = new InMemoryReporter(); | ||
Tracer tracer = | ||
new Tracer.Builder("test", reporter, new ConstSampler(true)).build(); | ||
try (ActiveSpan parent = tracer.buildSpan("parent").startActive()) { | ||
tracer.buildSpan("child").ignoreActiveSpan().startActive().deactivate(); | ||
} | ||
assertEquals(2, reporter.getSpans().size()); | ||
|
||
Span childSpan = reporter.getSpans().get(0); | ||
Span parentSpan = reporter.getSpans().get(1); | ||
|
||
assertTrue(reporter.getSpans().get(0).getReferences().isEmpty()); | ||
assertTrue(reporter.getSpans().get(1).getReferences().isEmpty()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing:
|
||
assertNotEquals(parentSpan.context().getTraceId(), childSpan.context().getTraceId()); | ||
assertEquals(0, childSpan.context().getParentId()); | ||
} | ||
|
||
@Test | ||
public void testCustomActiveSpanSource() { | ||
ActiveSpan activeSpan = mock(ActiveSpan.class); | ||
Tracer tracer = | ||
new Tracer.Builder("test", new InMemoryReporter(), new ConstSampler(true)) | ||
.withActiveSpanSource(new ActiveSpanSource() { | ||
|
||
@Override | ||
public ActiveSpan activeSpan() { | ||
return activeSpan; | ||
} | ||
|
||
@Override | ||
public ActiveSpan makeActive(io.opentracing.Span span) { | ||
return activeSpan; | ||
} | ||
}).build(); | ||
assertEquals(activeSpan, tracer.activeSpan()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not
io.opentracing.BaseSpan
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is
io.opentracing.BaseSpan
.