Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Open a new span when wrapping with a new trace #49

Merged
merged 3 commits into from
Jan 7, 2019
+62 −0
Merged

[fix] Open a new span when wrapping with a new trace #49

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6bc3ce8
Open a new span when creating a new trace
pkoenig10 Dec 20, 2018
78f3be4
Comments
pkoenig10 Dec 21, 2018
8455f06
Better tests
pkoenig10 Dec 21, 2018
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
7 changes: 7 additions & 0 deletions tracing/src/main/java/com/palantir/tracing/Tracers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
public final class Tracers {
/** The key under which trace ids are inserted into SLF4J {@link org.slf4j.MDC MDCs}. */
public static final String TRACE_ID_KEY = "traceId";
private static final String ROOT_SPAN_OPERATION = "root";
private static final char[] HEX_DIGITS =
{'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};

Expand Down Expand Up @@ -148,8 +149,10 @@ public static <V> Callable<V> wrapWithNewTrace(Callable<V> delegate) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there's an existing bug where wrapping a callable will leave an empty trace with no root span on the thread if there's not already an active trace.

Tracer.getAndClearTrace should probably return Optional<Trace>

Not related to this change though.

try {
Tracer.initTrace(Optional.empty(), Tracers.randomId());
Tracer.startSpan(ROOT_SPAN_OPERATION);
return delegate.call();
} finally {
Tracer.completeSpan();
Copy link
Contributor

Choose a reason for hiding this comment

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

fastCompleteSpan?

// restore the trace
Tracer.setTrace(originalTrace);
}
Expand All @@ -166,8 +169,10 @@ public static Runnable wrapWithNewTrace(Runnable delegate) {

try {
Tracer.initTrace(Optional.empty(), Tracers.randomId());
Tracer.startSpan(ROOT_SPAN_OPERATION);
delegate.run();
} finally {
Tracer.completeSpan();
Copy link
Contributor

Choose a reason for hiding this comment

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

fastCompleteSpan

// restore the trace
Tracer.setTrace(originalTrace);
}
Expand All @@ -187,8 +192,10 @@ public static Runnable wrapWithAlternateTraceId(String traceId, Runnable delegat

try {
Tracer.initTrace(Optional.empty(), traceId);
Tracer.startSpan(ROOT_SPAN_OPERATION);
delegate.run();
} finally {
Tracer.completeSpan();
// restore the trace
Tracer.setTrace(originalTrace);
}
Expand Down
38 changes: 38 additions & 0 deletions tracing/src/test/java/com/palantir/tracing/TracersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,15 @@ public void testWrapCallableWithNewTrace_traceStateInsideCallableIsIsolated() th
.isEqualTo(traceIdAfterCalls);
}

@Test
public void testWrapCallableWithNewTrace_traceStateInsideCallableHasSpan() throws Exception {
Callable<List<OpenSpan>> wrappedCallable = Tracers.wrapWithNewTrace(() -> {
return getCurrentFullTrace();
});

assertThat(wrappedCallable.call()).hasSize(1);
}

@Test
public void testWrapCallableWithNewTrace_traceStateRestoredWhenThrows() throws Exception {
String traceIdBeforeConstruction = Tracer.getTraceId();
Expand Down Expand Up @@ -258,6 +267,19 @@ public void testWrapRunnableWithNewTrace_traceStateInsideRunnableIsIsolated() th
.isEqualTo(traceIdAfterCalls);
}

@Test
public void testWrapRunnableWithNewTrace_traceStateInsideRunnableHasSpan() throws Exception {
List<List<OpenSpan>> spans = Lists.newArrayList();

Runnable wrappedRunnable = Tracers.wrapWithNewTrace(() -> {
spans.add(getCurrentFullTrace());
});

wrappedRunnable.run();

assertThat(spans.get(0)).hasSize(1);
}

@Test
public void testWrapRunnableWithNewTrace_traceStateRestoredWhenThrows() throws Exception {
String traceIdBeforeConstruction = Tracer.getTraceId();
Expand Down Expand Up @@ -293,6 +315,20 @@ public void testWrapRunnableWithAlternateTraceId_traceStateInsideRunnableUsesGiv
assertThat(traceIdBeforeConstruction).isEqualTo(traceIdAfterCall);
}

@Test
public void testWrapRunnableWithAlternateTraceId_traceStateInsideRunnableHasSpan() throws Exception {
List<List<OpenSpan>> spans = Lists.newArrayList();

String traceIdToUse = "someTraceId";
Runnable wrappedRunnable = Tracers.wrapWithAlternateTraceId(traceIdToUse, () -> {
spans.add(getCurrentFullTrace());
});

wrappedRunnable.run();

assertThat(spans.get(0)).hasSize(1);
}

@Test
public void testWrapRunnableWithAlternateTraceId_traceStateRestoredWhenThrows() {
String traceIdBeforeConstruction = Tracer.getTraceId();
Expand Down Expand Up @@ -325,6 +361,7 @@ public Void call() throws Exception {

assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isEqualTo(newTraceId);
assertThat(seenTraceIds).doesNotContain(newTraceId);
assertThat(getCurrentFullTrace()).hasSize(1);
seenTraceIds.add(newTraceId);
return null;
}
Expand All @@ -342,6 +379,7 @@ public void run() {

assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isEqualTo(newTraceId);
assertThat(seenTraceIds).doesNotContain(newTraceId);
assertThat(getCurrentFullTrace()).hasSize(1);
seenTraceIds.add(newTraceId);
}
};
Expand Down