Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Upgrade to OpenTracing java 0.30.0 with in-process propagation support #188

Merged
merged 6 commits into from
Jun 9, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ plugins {
id 'ru.vyarus.animalsniffer' version '1.2.0'
}

ext.opentracingVersion = '0.22.0'
ext.opentracingVersion = '0.30.0'
ext.guavaVersion = '18.0'
ext.apacheThriftVersion = '0.9.2'
ext.jerseyVersion = '2.22.2'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void process(HttpRequest httpRequest, HttpContext httpContext)
clientSpanBuilder.asChildOf(parentContext.getCurrentSpan());
}

Span clientSpan = clientSpanBuilder.start();
Span clientSpan = clientSpanBuilder.startManual();
Tags.SPAN_KIND.set(clientSpan, Tags.SPAN_KIND_CLIENT);
Tags.HTTP_URL.set(clientSpan, requestLine.getUri());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void setUp() {
Sampler sampler = new ConstSampler(true);
tracer = new Tracer.Builder("test_service", reporter, sampler).build();

parentSpan = (Span) tracer.buildSpan("parent_operation").start();
parentSpan = (Span) tracer.buildSpan("parent_operation").startManual();
parentSpan.setBaggageItem(BAGGAGE_KEY, BAGGAGE_VALUE);
parentSpan.finish();
//Set up a parent span context
Expand Down
1 change: 1 addition & 0 deletions jaeger-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ description = 'Core library for jaeger-client'
dependencies {
compile project(':jaeger-thrift')
compile group: 'io.opentracing', name: 'opentracing-api', version: opentracingVersion
compile group: 'io.opentracing', name: 'opentracing-util', version: opentracingVersion
compile group: 'com.google.code.gson', name: 'gson', version: gsonVersion
compile group: 'org.slf4j', name: 'slf4j-api', version: slf4jVersion

Expand Down
1 change: 0 additions & 1 deletion jaeger-core/src/main/java/com/uber/jaeger/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ private void finishWithDuration(long durationMicros) {
}
}

@Override
public void close() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we even have this method? IIRC the OT Span is no longer closable

finish();
}
Expand Down
59 changes: 54 additions & 5 deletions jaeger-core/src/main/java/com/uber/jaeger/Tracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand All @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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}
*/
Expand All @@ -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;
Expand All @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not io.opentracing.BaseSpan?

Copy link
Contributor Author

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.

return addReference(References.CHILD_OF, parent.context());
}

Expand Down Expand Up @@ -263,6 +283,11 @@ private SpanContext createNewContext(String debugId) {
}
}

// Check if active span should be established as CHILD_OF relationship
if (!ignoreActiveSpan && null != activeSpanSource.activeSpan()) {
asChildOf(activeSpanSource.activeSpan());
}

return new SpanContext(id, id, 0, flags);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentID == 0 and traceID == random() are incorrect if we have a parent span

}

Expand Down Expand Up @@ -349,7 +374,7 @@ private String debugId() {
}

@Override
public io.opentracing.Span start() {
public io.opentracing.Span startManual() {
SpanContext context;

String debugId = debugId();
Expand Down Expand Up @@ -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();
}
}

/**
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -530,4 +578,5 @@ String getHostName() {
return null;
}
}

}
67 changes: 66 additions & 1 deletion jaeger-core/src/test/java/com/uber/jaeger/PropagationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@

import static org.junit.Assert.assertEquals;
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;
Expand All @@ -45,10 +50,70 @@ 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());
assertEquals("child", reporter.getSpans().get(0).getOperationName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extract to vars to make the code easier to read

Span childSpan = reporter.getSpans().get(0);
Span parentSpan = reporter.getSpans().get(1);

assertEquals(1, reporter.getSpans().get(0).getReferences().size());
assertEquals("parent", reporter.getSpans().get(1).getOperationName());
assertTrue(reporter.getSpans().get(1).getReferences().isEmpty());
assertEquals(References.CHILD_OF, reporter.getSpans().get(0).getReferences().get(0).getType());
assertEquals(reporter.getSpans().get(1).context(),
reporter.getSpans().get(0).getReferences().get(0).getSpanContext());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing checks:

  • parentSpan.traceID == childSpan.traceID
  • parentSpan.spanID == childSpan.parentID

}

@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());
assertTrue(reporter.getSpans().get(0).getReferences().isEmpty());
assertTrue(reporter.getSpans().get(1).getReferences().isEmpty());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing:

  • outerSpan.traceID != inner.traceID
  • innerSpan.parentID == 0

}

@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());
}
}
Loading