diff --git a/tracing-jaxrs/src/test/java/com/palantir/tracing/jaxrs/JaxRsTracersTest.java b/tracing-jaxrs/src/test/java/com/palantir/tracing/jaxrs/JaxRsTracersTest.java index 081625f1a..4d29d58d9 100644 --- a/tracing-jaxrs/src/test/java/com/palantir/tracing/jaxrs/JaxRsTracersTest.java +++ b/tracing-jaxrs/src/test/java/com/palantir/tracing/jaxrs/JaxRsTracersTest.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; import com.palantir.tracing.Tracer; +import com.palantir.tracing.api.Span; import java.io.ByteArrayOutputStream; import javax.ws.rs.core.StreamingOutput; import org.junit.Test; @@ -32,14 +33,16 @@ public void testWrappingStreamingOutput_streamingOutputTraceIsIsolated() throws Tracer.startSpan("inside"); // never completed }); streamingOutput.write(new ByteArrayOutputStream()); - assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("outside"); + Tracer.completeSpan().map(Span::getOperation) + .ifPresent(op -> assertThat(op).isEqualTo("outside")); } @Test public void testWrappingStreamingOutput_traceStateIsCapturedAtConstructionTime() throws Exception { Tracer.startSpan("before-construction"); StreamingOutput streamingOutput = JaxRsTracers.wrap(os -> { - assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("streaming-output"); + Tracer.completeSpan().map(Span::getOperation) + .ifPresent(op -> assertThat(op).isEqualTo("streaming-output")); }); Tracer.startSpan("after-construction"); streamingOutput.write(new ByteArrayOutputStream()); diff --git a/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java b/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java index 5f81369a1..7a2a650bd 100644 --- a/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java +++ b/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java @@ -104,7 +104,7 @@ public T withTrace(Tracers.ThrowingCallable inner Optional originalTrace = Tracer.copyTrace(); - Tracer.setTrace(new Trace(isObservable, traceId)); + Tracer.setTrace(Trace.create(isObservable, traceId)); if (parentSpanId != null) { Tracer.startSpan(operation, parentSpanId, SpanType.LOCAL); } else { diff --git a/tracing/src/main/java/com/palantir/tracing/ImmutableEmptyDeque.java b/tracing/src/main/java/com/palantir/tracing/ImmutableEmptyDeque.java new file mode 100644 index 000000000..42373ab65 --- /dev/null +++ b/tracing/src/main/java/com/palantir/tracing/ImmutableEmptyDeque.java @@ -0,0 +1,232 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.tracing; + +import java.util.Collection; +import java.util.Collections; +import java.util.Deque; +import java.util.Iterator; +import java.util.NoSuchElementException; + +final class ImmutableEmptyDeque implements Deque { + + private static final Object[] EMPTY_ARRAY = new Object[0]; + private static final ImmutableEmptyDeque EMPTY_DEQUE = new ImmutableEmptyDeque<>(); + + ImmutableEmptyDeque() {} + + @SuppressWarnings("unchecked") + static Deque instance() { + return (Deque) EMPTY_DEQUE; + } + + private static UnsupportedOperationException cannotModify() { + return new UnsupportedOperationException("cannot modify immutable empty deque"); + } + + @Override + public boolean isEmpty() { + return true; + } + + @Override + public int size() { + return 0; + } + + @Override + public boolean equals(Object obj) { + return this == obj || this.getClass() == obj.getClass(); + } + + @Override + public int hashCode() { + return 0; + } + + @Override + public String toString() { + return "[]"; + } + + @Override + public Iterator iterator() { + return Collections.emptyIterator(); + } + + @Override + public Iterator descendingIterator() { + return Collections.emptyIterator(); + } + + @Override + public Object[] toArray() { + return EMPTY_ARRAY; + } + + @Override + @SuppressWarnings("unchecked") + public T[] toArray(T[] array) { + return (T[]) EMPTY_ARRAY; + } + + @Override + public boolean add(E element) { + throw cannotModify(); + } + + @Override + public boolean addAll(Collection collection) { + throw cannotModify(); + } + + @Override + public void addFirst(E element) { + throw cannotModify(); + } + + @Override + public void addLast(E element) { + throw cannotModify(); + } + + @Override + public void clear() { + throw cannotModify(); + } + + @Override + public boolean contains(Object obj) { + return false; + } + + @Override + public boolean containsAll(Collection collection) { + return false; + } + + @Override + public E element() { + throw new NoSuchElementException(); + } + + @Override + public E getFirst() { + throw new NoSuchElementException(); + } + + @Override + public E getLast() { + throw new NoSuchElementException(); + } + + @Override + public boolean offer(E element) { + throw cannotModify(); + } + + @Override + public boolean offerFirst(E element) { + throw cannotModify(); + } + + @Override + public boolean offerLast(E element) { + throw cannotModify(); + } + + @Override + public E peek() { + return null; + } + + @Override + public E peekFirst() { + return null; + } + + @Override + public E peekLast() { + return null; + } + + @Override + public E poll() { + return null; + } + + @Override + public E pollFirst() { + return null; + } + + @Override + public E pollLast() { + return null; + } + + @Override + public E pop() { + throw cannotModify(); + } + + @Override + public void push(E element) { + throw cannotModify(); + } + + @Override + public E remove() { + throw cannotModify(); + } + + @Override + public boolean remove(Object obj) { + throw cannotModify(); + } + + @Override + public boolean removeAll(Collection collection) { + throw cannotModify(); + } + + @Override + public E removeFirst() { + throw cannotModify(); + } + + @Override + public boolean removeFirstOccurrence(Object obj) { + throw cannotModify(); + } + + @Override + public E removeLast() { + throw cannotModify(); + } + + @Override + public boolean removeLastOccurrence(Object obj) { + throw cannotModify(); + } + + @Override + public boolean retainAll(Collection collection) { + throw cannotModify(); + } + +} diff --git a/tracing/src/main/java/com/palantir/tracing/Trace.java b/tracing/src/main/java/com/palantir/tracing/Trace.java index dbc8db4e3..a31c2b36d 100644 --- a/tracing/src/main/java/com/palantir/tracing/Trace.java +++ b/tracing/src/main/java/com/palantir/tracing/Trace.java @@ -34,7 +34,7 @@ public final class Trace { private final boolean isObservable; private final String traceId; - private Trace(ArrayDeque stack, boolean isObservable, String traceId) { + private Trace(Deque stack, boolean isObservable, String traceId) { checkArgument(!traceId.isEmpty(), "traceId must be non-empty"); this.stack = stack; @@ -42,12 +42,15 @@ private Trace(ArrayDeque stack, boolean isObservable, String traceId) this.traceId = traceId; } - Trace(boolean isObservable, String traceId) { - this(new ArrayDeque<>(), isObservable, traceId); + static Trace create(boolean isObservable, String traceId) { + Deque deque = isObservable ? new ArrayDeque<>() : ImmutableEmptyDeque.instance(); + return new Trace(deque, isObservable, traceId); } void push(OpenSpan span) { - stack.push(span); + if (isObservable) { + stack.push(span); + } } Optional top() { @@ -79,7 +82,8 @@ String getTraceId() { /** Returns a copy of this Trace which can be independently mutated. */ Trace deepCopy() { - return new Trace(new ArrayDeque<>(stack), isObservable, traceId); + Deque deque = isObservable ? new ArrayDeque<>(stack) : ImmutableEmptyDeque.instance(); + return new Trace(deque, isObservable, traceId); } @Override diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index 9364a5e23..0d92e732e 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -20,6 +20,7 @@ import static com.palantir.logsafe.Preconditions.checkNotNull; import static com.palantir.logsafe.Preconditions.checkState; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.errorprone.annotations.CheckReturnValue; import com.palantir.logsafe.SafeArg; @@ -48,6 +49,16 @@ public final class Tracer { private static final Logger log = LoggerFactory.getLogger(Tracer.class); + /** + * No-op span used internally to avoid allocating when trace is unobservable. + */ + private static final OpenSpan NOOP_SPAN = OpenSpan.builder() + .type(SpanType.LOCAL) + .operation("noop") + .parentSpanId(Optional.empty()) + .spanId("noop") + .build(); + private Tracer() {} // Thread-safe since thread-local @@ -68,7 +79,7 @@ private Tracer() {} private static Trace createTrace(Observability observability, String traceId) { checkArgument(!Strings.isNullOrEmpty(traceId), "traceId must be non-empty"); boolean observable = shouldObserve(observability); - return new Trace(observable, traceId); + return Trace.create(observable, traceId); } private static boolean shouldObserve(Observability observability) { @@ -94,7 +105,6 @@ public static void initTrace(Optional isObservable, String traceId) { Observability observability = isObservable .map(value -> Boolean.TRUE.equals(value) ? Observability.SAMPLE : Observability.DO_NOT_SAMPLE) .orElse(Observability.UNDECIDED); - setTrace(createTrace(observability, traceId)); } @@ -111,6 +121,10 @@ public static void initTrace(Observability observability, String traceId) { */ public static OpenSpan startSpan(String operation, String parentSpanId, SpanType type) { Trace current = getOrCreateCurrentTrace(); + if (!current.isObservable()) { + return NOOP_SPAN; + } + checkState(current.isEmpty(), "Cannot start a span with explicit parent if the current thread's trace is non-empty"); checkArgument(!Strings.isNullOrEmpty(parentSpanId), "parentSpanId must be non-empty"); @@ -139,12 +153,16 @@ public static OpenSpan startSpan(String operation) { } private static OpenSpan startSpanInternal(String operation, SpanType type) { + Trace trace = getOrCreateCurrentTrace(); + if (!trace.isObservable()) { + return NOOP_SPAN; + } + OpenSpan.Builder spanBuilder = OpenSpan.builder() .operation(operation) .spanId(Tracers.randomId()) .type(type); - Trace trace = getOrCreateCurrentTrace(); Optional prevState = trace.top(); // Avoid lambda allocation in hot paths if (prevState.isPresent()) { @@ -304,6 +322,11 @@ public static void setSampler(TraceSampler sampler) { Tracer.sampler = sampler; } + @VisibleForTesting + static TraceSampler getSampler() { + return Tracer.sampler; + } + /** Returns true if there is an active trace on this thread. */ public static boolean hasTraceId() { return currentTrace.get() != null; diff --git a/tracing/src/main/java/com/palantir/tracing/Tracers.java b/tracing/src/main/java/com/palantir/tracing/Tracers.java index 47dee7d7e..a66d272db 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracers.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracers.java @@ -234,7 +234,7 @@ public static Callable wrapWithNewTrace(String operation, Observability o Optional originalTrace = Tracer.getAndClearTraceIfPresent(); try { - Tracer.initTrace(observability, Tracers.randomId()); + Tracer.initTrace(observability, randomId()); Tracer.startSpan(operation); return delegate.call(); } finally { @@ -270,7 +270,7 @@ public static Runnable wrapWithNewTrace(String operation, Observability observab Optional originalTrace = Tracer.getAndClearTraceIfPresent(); try { - Tracer.initTrace(observability, Tracers.randomId()); + Tracer.initTrace(observability, randomId()); Tracer.startSpan(operation); delegate.run(); } finally { diff --git a/tracing/src/test/java/com/palantir/tracing/CloseableTracerTest.java b/tracing/src/test/java/com/palantir/tracing/CloseableTracerTest.java index 6a31cf72c..a5b35e30f 100644 --- a/tracing/src/test/java/com/palantir/tracing/CloseableTracerTest.java +++ b/tracing/src/test/java/com/palantir/tracing/CloseableTracerTest.java @@ -20,6 +20,7 @@ import com.palantir.tracing.api.OpenSpan; import com.palantir.tracing.api.SpanType; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -27,11 +28,21 @@ @RunWith(MockitoJUnitRunner.class) public final class CloseableTracerTest { + + private TraceSampler originalSampler; + @Before public void before() { + originalSampler = Tracer.getSampler(); + Tracer.setSampler(AlwaysSampler.INSTANCE); Tracer.getAndClearTrace(); } + @After + public void after() { + Tracer.setSampler(originalSampler); + } + @Test public void startsAndClosesSpan() { try (CloseableTracer tracer = CloseableTracer.startSpan("foo")) { diff --git a/tracing/src/test/java/com/palantir/tracing/TraceTest.java b/tracing/src/test/java/com/palantir/tracing/TraceTest.java index e2fa8c2f5..7c74c891e 100644 --- a/tracing/src/test/java/com/palantir/tracing/TraceTest.java +++ b/tracing/src/test/java/com/palantir/tracing/TraceTest.java @@ -27,13 +27,13 @@ public final class TraceTest { @Test public void constructTrace_emptyTraceId() { - assertThatThrownBy(() -> new Trace(false, "")) + assertThatThrownBy(() -> Trace.create(false, "")) .isInstanceOf(IllegalArgumentException.class); } @Test public void testToString() { - Trace trace = new Trace(true, "traceId"); + Trace trace = Trace.create(true, "traceId"); trace.push(OpenSpan.builder() .type(SpanType.LOCAL) .spanId("spanId") @@ -44,4 +44,36 @@ public void testToString() { assertThat(trace.toString()).isEqualTo("Trace{stack=[OpenSpan{operation=operation, startTimeMicroSeconds=0, " + "startClockNanoSeconds=0, spanId=spanId, type=LOCAL}], isObservable=true, traceId='traceId'}"); } + + @Test + public void unobservedTraces() { + Trace traceId = Trace.create(false, "testTraceId"); + assertThat(traceId.getTraceId()).isEqualTo("testTraceId"); + assertThat(traceId.isObservable()).isFalse(); + assertThat(traceId.isEmpty()).isTrue(); + assertThat(traceId.pop()).isNotPresent(); + assertThat(traceId.top()).isNotPresent(); + assertEquals(traceId, traceId.deepCopy()); + + traceId.push(OpenSpan.builder().spanId("spanId").operation("operation").type(SpanType.LOCAL).build()); + assertThat(traceId.getTraceId()).isEqualTo("testTraceId"); + assertThat(traceId.isObservable()).isFalse(); + assertThat(traceId.isEmpty()).isTrue(); + assertThat(traceId.pop()).isNotPresent(); + assertThat(traceId.top()).isNotPresent(); + assertEquals(traceId, traceId.deepCopy()); + + assertThat(traceId.toString()).isEqualTo("Trace{stack=[], isObservable=false, traceId='testTraceId'}"); + + assertThat(traceId).isNotEqualTo(Trace.create(false, "traceId2")); + } + + private static void assertEquals(Trace one, Trace two) { + assertThat(one.getTraceId()).isEqualTo(two.getTraceId()); + assertThat(one.isObservable()).isEqualTo(two.isObservable()); + assertThat(one.isEmpty()).isEqualTo(two.isEmpty()); + assertThat(one.top()).isEqualTo(two.top()); + assertThat(one.toString()).isEqualTo(two.toString()); + } + } diff --git a/tracing/src/test/java/com/palantir/tracing/TracerTest.java b/tracing/src/test/java/com/palantir/tracing/TracerTest.java index d9d63e473..cb6d6b717 100644 --- a/tracing/src/test/java/com/palantir/tracing/TracerTest.java +++ b/tracing/src/test/java/com/palantir/tracing/TracerTest.java @@ -32,6 +32,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import javax.annotation.Nullable; import org.assertj.core.util.Sets; import org.junit.After; import org.junit.Before; @@ -138,17 +139,20 @@ public void testObserversAreInvokedOnObservableTracesOnly() throws Exception { verifyNoMoreInteractions(observer1); Tracer.initTrace(Observability.DO_NOT_SAMPLE, Tracers.randomId()); - startAndCompleteSpan(); // not sampled, see above + assertThat(Tracer.isTraceObservable()).isFalse(); + assertThat(startAndCompleteSpan()).isNull(); // not sampled, see above verifyNoMoreInteractions(observer1); } @Test - public void testDerivesNewSpansWhenTraceIsNotObservable() throws Exception { + public void testIgnoresNewSpansWhenTraceIsNotObservable() throws Exception { Tracer.initTrace(Observability.DO_NOT_SAMPLE, Tracers.randomId()); + assertThat(Tracer.isTraceObservable()).isFalse(); Tracer.startSpan("foo"); Tracer.startSpan("bar"); - assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("bar"); - assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("foo"); + assertThat(Tracer.completeSpan()).isNotPresent(); + assertThat(Tracer.completeSpan()).isNotPresent(); + assertThat(Tracer.isTraceObservable()).isFalse(); } @Test @@ -163,7 +167,8 @@ public void testInitTraceCallsSampler() throws Exception { verifyNoMoreInteractions(observer1, sampler); Mockito.reset(observer1, sampler); - startAndCompleteSpan(); // not sampled, see above + assertThat(Tracer.isTraceObservable()).isFalse(); + assertThat(startAndCompleteSpan()).isNull(); // not sampled, see above verify(sampler).sample(); verifyNoMoreInteractions(observer1, sampler); } @@ -183,7 +188,7 @@ public void testTraceCopyIsIndependent() throws Exception { @Test public void testSetTraceSetsCurrentTraceAndMdcTraceIdKey() throws Exception { Tracer.startSpan("operation"); - Tracer.setTrace(new Trace(true, "newTraceId")); + Tracer.setTrace(Trace.create(true, "newTraceId")); assertThat(Tracer.getTraceId()).isEqualTo("newTraceId"); assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isEqualTo("newTraceId"); assertThat(Tracer.completeSpan()).isEmpty(); @@ -192,7 +197,7 @@ public void testSetTraceSetsCurrentTraceAndMdcTraceIdKey() throws Exception { @Test public void testSetTraceSetsMdcTraceSampledKeyWhenObserved() { - Tracer.setTrace(new Trace(true, "observedTraceId")); + Tracer.setTrace(Trace.create(true, "observedTraceId")); assertThat(MDC.get(Tracers.TRACE_SAMPLED_KEY)).isEqualTo("1"); assertThat(Tracer.completeSpan()).isEmpty(); assertThat(MDC.get(Tracers.TRACE_SAMPLED_KEY)).isNull(); @@ -200,7 +205,7 @@ public void testSetTraceSetsMdcTraceSampledKeyWhenObserved() { @Test public void testSetTraceMissingMdcTraceSampledKeyWhenNotObserved() { - Tracer.setTrace(new Trace(false, "notObservedTraceId")); + Tracer.setTrace(Trace.create(false, "notObservedTraceId")); assertThat(MDC.get(Tracers.TRACE_SAMPLED_KEY)).isNull(); assertThat(Tracer.completeSpan()).isEmpty(); assertThat(MDC.get(Tracers.TRACE_SAMPLED_KEY)).isNull(); @@ -275,7 +280,7 @@ public void testObserversThrow() { @Test public void testGetAndClearTraceIfPresent() { - Trace trace = new Trace(true, "newTraceId"); + Trace trace = Trace.create(true, "newTraceId"); Tracer.setTrace(trace); Optional nonEmptyTrace = Tracer.getAndClearTraceIfPresent(); @@ -329,8 +334,9 @@ public void testHasTraceId() { assertThat(Tracer.hasTraceId()).isEqualTo(false); } + @Nullable private static Span startAndCompleteSpan() { Tracer.startSpan("operation"); - return Tracer.completeSpan().get(); + return Tracer.completeSpan().orElse(null); } }