From 43eb14c7d40add33cf7c6d42dd7a5161d9872c54 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 31 Aug 2021 16:09:55 -0400 Subject: [PATCH 1/8] initial implementation bits --- .../com/palantir/tracing/DetachedSpan.java | 11 ++ .../com/palantir/tracing/NopDetachedSpan.java | 104 ++++++++++++++ .../java/com/palantir/tracing/Tracer.java | 136 ++++++++++++++---- 3 files changed, 227 insertions(+), 24 deletions(-) create mode 100644 tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java diff --git a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java index f00326aac..203ff53ba 100644 --- a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java +++ b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java @@ -68,6 +68,17 @@ static DetachedSpan start( return Tracer.detachInternal(observability, traceId, parentSpanId, operation, type); } + /** + * Creates a {@link DetachedSpan} instance based on the current tracing state without adding a new span. + * If there is no tracing state present a no-op instance is returned. This is equivalent to {@link #attach()}. + * + * @see DetachedSpan#attach() + */ + @CheckReturnValue + static DetachedSpan detach() { + return Tracer.detachInternal(); + } + /** * Equivalent to {@link Tracer#startSpan(String, SpanType)}, but using this {@link DetachedSpan} as the parent * instead of thread state. diff --git a/tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java new file mode 100644 index 000000000..1eff26ebb --- /dev/null +++ b/tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java @@ -0,0 +1,104 @@ +/* + * (c) Copyright 2021 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 com.palantir.tracing.api.SpanType; +import java.util.Map; + +enum NopDetachedSpan implements DetachedSpan { + INSTANCE; + + @Override + public CloseableSpan childSpan(String _operationName, SpanType _type) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public CloseableSpan childSpan(String _operationName, Map _metadata) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public CloseableSpan childSpan(String _operationName, TagTranslator _translator, T _data) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public CloseableSpan childSpan(String _operationName, Map _metadata, SpanType _type) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public CloseableSpan childSpan( + String _operationName, TagTranslator _translator, T _data, SpanType _type) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public CloseableSpan childSpan(String _operationName) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public CloseableSpan completeAndStartChild(String _operationName, SpanType _type) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public CloseableSpan completeAndStartChild(String _operationName) { + return NopCloseableSpan.INSTANCE; + } + + @Override + public DetachedSpan childDetachedSpan(String _operation, SpanType _type) { + return NopDetachedSpan.INSTANCE; + } + + @Override + public DetachedSpan childDetachedSpan(String _operation) { + return NopDetachedSpan.INSTANCE; + } + + @Override + public CloseableSpan attach() { + return NopCloseableSpan.INSTANCE; + } + + @Override + public void complete() { + // nop + } + + @Override + public void complete(Map _metadata) { + // nop + } + + @Override + public void complete(TagTranslator _tagTranslator, T _data) { + // nop + } + + private enum NopCloseableSpan implements CloseableSpan { + INSTANCE; + + @Override + public void close() { + // nop + } + } +} diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index e3d2ab8b2..78bb57835 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -244,7 +244,7 @@ static DetachedSpan detachInternal(@Safe String operation, SpanType type) { Optional requestId = getRequestId(maybeCurrentTrace, type); return sampled ? new SampledDetachedSpan(operation, type, traceId, requestId, parentSpan) - : new UnsampledDetachedSpan(traceId, requestId, parentSpan); + : new UnsampledDetachedSpan(traceId, requestId, Optional.empty()); } /** @@ -280,6 +280,29 @@ static DetachedSpan detachInternal( : new UnsampledDetachedSpan(traceId, requestId, parentSpanId); } + /** + * Like {@link #detachInternal(String, SpanType)}, but does not create a new span, and may return a + * no-op implementation if no tracing state is currently set. + */ + static DetachedSpan detachInternal() { + Trace trace = currentTrace.get(); + if (trace == null) { + return NopDetachedSpan.INSTANCE; + } + + String traceId = trace.getTraceId(); + Optional requestId = trace.getRequestId(); + if (trace.isObservable()) { + OpenSpan maybeOpenSpan = trace.top().orElse(null); + if (maybeOpenSpan == null) { + return NopDetachedSpan.INSTANCE; + } + return new SampledUnnamedDetachedSpan(traceId, requestId, maybeOpenSpan); + } else { + return new UnsampledDetachedSpan(traceId, requestId, Optional.empty()); + } + } + private static Optional getParentSpanId(@Nullable Trace trace) { if (trace != null) { Optional maybeOpenSpan = trace.top(); @@ -413,40 +436,105 @@ private void warnIfCompleted(String feature) { SafeArg.of("detachedSpan", this)); } } + } - private static final class TraceRestoringCloseableSpanWithMetadata implements CloseableSpan { + private static final class TraceRestoringCloseableSpanWithMetadata implements CloseableSpan { - @Nullable - private final Trace original; + @Nullable + private final Trace original; - private final TagTranslator translator; - private final T data; + private final TagTranslator translator; + private final T data; - static CloseableSpan of(@Nullable Trace original, TagTranslator translator, T data) { - if (original != null || !translator.isEmpty(data)) { - return new TraceRestoringCloseableSpanWithMetadata<>(original, translator, data); - } - return DEFAULT_CLOSEABLE_SPAN; + static CloseableSpan of(@Nullable Trace original, TagTranslator translator, T data) { + if (original != null || !translator.isEmpty(data)) { + return new TraceRestoringCloseableSpanWithMetadata<>(original, translator, data); } + return DEFAULT_CLOSEABLE_SPAN; + } - TraceRestoringCloseableSpanWithMetadata( - @Nullable Trace original, TagTranslator translator, T data) { - this.original = original; - this.translator = translator; - this.data = data; - } + TraceRestoringCloseableSpanWithMetadata(@Nullable Trace original, TagTranslator translator, T data) { + this.original = original; + this.translator = translator; + this.data = data; + } - @Override - public void close() { - Tracer.fastCompleteSpan(translator, data); - Trace originalTrace = original; - if (originalTrace != null) { - Tracer.setTrace(originalTrace); - } + @Override + public void close() { + Tracer.fastCompleteSpan(translator, data); + Trace originalTrace = original; + if (originalTrace != null) { + Tracer.setTrace(originalTrace); } } } + private static final class SampledUnnamedDetachedSpan implements DetachedSpan { + + private final String traceId; + private final Optional requestId; + private final OpenSpan openSpan; + + SampledUnnamedDetachedSpan(String traceId, Optional requestId, OpenSpan openSpan) { + this.traceId = traceId; + this.requestId = requestId; + this.openSpan = openSpan; + } + + @Override + @MustBeClosed + public CloseableSpan childSpan( + String operationName, TagTranslator translator, T data, SpanType type) { + Trace maybeCurrentTrace = currentTrace.get(); + setTrace(Trace.of(true, traceId, requestId)); + Tracer.fastStartSpan(operationName, openSpan.getSpanId(), type); + return TraceRestoringCloseableSpanWithMetadata.of(maybeCurrentTrace, translator, data); + } + + @Override + public DetachedSpan childDetachedSpan(String operation, SpanType type) { + return new SampledDetachedSpan(operation, type, traceId, requestId, Optional.of(openSpan.getSpanId())); + } + + @Override + @MustBeClosed + public CloseableSpan attach() { + Trace maybeCurrentTrace = currentTrace.get(); + Trace newTrace = Trace.of(true, traceId, requestId); + // Push the DetachedSpan OpenSpan to provide the correct parent information + // to child spans created within the context of this attach. + // It is VITAL that this span is never completed, it exists only for attribution. + newTrace.push(openSpan); + setTrace(newTrace); + // Do not complete the synthetic root span, it simply prevents nested spans from removing trace state, and + // allows + return maybeCurrentTrace == null ? REMOVE_TRACE : () -> Tracer.setTrace(maybeCurrentTrace); + } + + @Override + public void complete() { + // nop -- unnamed span cannot be completed + } + + @Override + public void complete(TagTranslator _tagTranslator, T _data) { + // nop -- unnamed span cannot be completed + } + + @Override + public String toString() { + return "SampledUnnamedDetachedSpan{traceId='" + + traceId + + '\'' + + ", requestId='" + + requestId.orElse(null) + + '\'' + + ", openSpan=" + + openSpan + + '}'; + } + } + private static final class UnsampledDetachedSpan implements DetachedSpan { private final String traceId; From 345983224c923c467e4e33b3d7d1b6c9ce350fd5 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 31 Aug 2021 16:10:04 -0400 Subject: [PATCH 2/8] runnables --- .../java/com/palantir/tracing/Tracers.java | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/tracing/src/main/java/com/palantir/tracing/Tracers.java b/tracing/src/main/java/com/palantir/tracing/Tracers.java index d3483d525..e0b2a2234 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracers.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracers.java @@ -507,34 +507,42 @@ private static void restoreTrace(Optional trace) { */ private static class TracingAwareCallable implements Callable { private final Callable delegate; - private final DeferredTracer deferredTracer; + private final DetachedSpan detachedSpan; + private final String operation; + private final Map metadata; TracingAwareCallable(String operation, Map metadata, Callable delegate) { this.delegate = delegate; - this.deferredTracer = new DeferredTracer(operation, metadata); + this.detachedSpan = DetachedSpan.detach(); + this.operation = operation; + this.metadata = metadata; } @Override public V call() throws Exception { - try (DeferredTracer.CloseableTrace ignored = deferredTracer.withTrace()) { + try (CloseableSpan ignored = detachedSpan.childSpan(operation, metadata)) { return delegate.call(); + } finally { + detachedSpan.complete(); } } } private static class AnonymousTracingAwareCallable implements Callable { private final Callable delegate; - private final DeferredTracer deferredTracer; + private final DetachedSpan detachedSpan; AnonymousTracingAwareCallable(Callable delegate) { this.delegate = delegate; - this.deferredTracer = new DeferredTracer("DeferredTracer(unnamed operation)"); + this.detachedSpan = DetachedSpan.detach(); } @Override public V call() throws Exception { - try (DeferredTracer.CloseableTrace ignored = deferredTracer.withTrace()) { + try (CloseableSpan ignored = detachedSpan.childSpan("DeferredTracer(unnamed operation)")) { return delegate.call(); + } finally { + detachedSpan.complete(); } } } @@ -545,34 +553,42 @@ public V call() throws Exception { */ private static class TracingAwareRunnable implements Runnable { private final Runnable delegate; - private final DeferredTracer deferredTracer; + private final DetachedSpan detachedSpan; + private final String operation; + private final Map metadata; TracingAwareRunnable(String operation, Map metadata, Runnable delegate) { this.delegate = delegate; - this.deferredTracer = new DeferredTracer(operation, metadata); + this.detachedSpan = DetachedSpan.detach(); + this.operation = operation; + this.metadata = metadata; } @Override public void run() { - try (DeferredTracer.CloseableTrace ignored = deferredTracer.withTrace()) { + try (CloseableSpan ignored = detachedSpan.childSpan(operation, metadata)) { delegate.run(); + } finally { + detachedSpan.complete(); } } } private static class AnonymousTracingAwareRunnable implements Runnable { private final Runnable delegate; - private final DeferredTracer deferredTracer; + private final DetachedSpan detachedSpan; AnonymousTracingAwareRunnable(Runnable delegate) { this.delegate = delegate; - this.deferredTracer = new DeferredTracer("DeferredTracer(unnamed operation)"); + this.detachedSpan = DetachedSpan.detach(); } @Override public void run() { - try (DeferredTracer.CloseableTrace ignored = deferredTracer.withTrace()) { + try (CloseableSpan ignored = detachedSpan.childSpan("DeferredTracer(unnamed operation)")) { delegate.run(); + } finally { + detachedSpan.complete(); } } } From 503ead803970d703373db700b7529bce351a14da Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 31 Aug 2021 16:37:14 -0400 Subject: [PATCH 3/8] Fix tracersTest --- tracing/src/test/java/com/palantir/tracing/TracersTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracing/src/test/java/com/palantir/tracing/TracersTest.java b/tracing/src/test/java/com/palantir/tracing/TracersTest.java index 78815ad52..6c2bb250d 100644 --- a/tracing/src/test/java/com/palantir/tracing/TracersTest.java +++ b/tracing/src/test/java/com/palantir/tracing/TracersTest.java @@ -59,13 +59,13 @@ public void before() { Tracer.setSampler(AlwaysSampler.INSTANCE); // Initialize a new trace for each test - Tracer.setTrace(Trace.of(true, "defaultTraceId", Optional.empty())); + Tracer.initTraceWithSpan(Observability.SAMPLE, "defaultTraceId", "rootOperation", SpanType.LOCAL); } @After public void after() { // Clear out the old trace from each test - Tracer.getAndClearTraceIfPresent(); + Tracer.clearCurrentTrace(); } @Test From 3a2a8df0253f5ff8add5b655539c8100477a0173 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 31 Aug 2021 17:13:02 -0400 Subject: [PATCH 4/8] also JaxRsTracers --- .../com/palantir/tracing/jaxrs/JaxRsTracers.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java b/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java index 0db91b904..03d272831 100644 --- a/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java +++ b/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java @@ -16,7 +16,8 @@ package com.palantir.tracing.jaxrs; -import com.palantir.tracing.DeferredTracer; +import com.palantir.tracing.CloseableSpan; +import com.palantir.tracing.DetachedSpan; import com.palantir.tracing.Tracers; import java.io.IOException; import java.io.OutputStream; @@ -36,19 +37,18 @@ public static StreamingOutput wrap(StreamingOutput delegate) { private static class TracingAwareStreamingOutput implements StreamingOutput { private final StreamingOutput delegate; - private DeferredTracer deferredTracer; + private final DetachedSpan detachedSpan; TracingAwareStreamingOutput(StreamingOutput delegate) { this.delegate = delegate; - this.deferredTracer = new DeferredTracer("streaming-output"); + this.detachedSpan = DetachedSpan.detach(); } @Override public void write(OutputStream output) throws IOException, WebApplicationException { - deferredTracer.withTrace(() -> { + try (CloseableSpan ignored = detachedSpan.childSpan("streaming-output")) { delegate.write(output); - return true; - }); + } } } } From 0efeae6c8f7835cc1d5ece176f1f65d0dbab9677 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Wed, 1 Sep 2021 14:29:29 -0400 Subject: [PATCH 5/8] self-nits --- .../palantir/tracing/jaxrs/JaxRsTracers.java | 7 +- .../java/com/palantir/tracing/Detached.java | 99 +++++++++++++++++ .../com/palantir/tracing/DetachedSpan.java | 80 ++------------ .../com/palantir/tracing/NopDetached.java | 65 +++++++++++ .../com/palantir/tracing/NopDetachedSpan.java | 104 ------------------ .../java/com/palantir/tracing/Tracer.java | 26 ++--- .../java/com/palantir/tracing/Tracers.java | 32 ++---- .../com/palantir/tracing/TracersTest.java | 91 +++++++-------- 8 files changed, 242 insertions(+), 262 deletions(-) create mode 100644 tracing/src/main/java/com/palantir/tracing/Detached.java create mode 100644 tracing/src/main/java/com/palantir/tracing/NopDetached.java delete mode 100644 tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java diff --git a/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java b/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java index 03d272831..5d988f7cb 100644 --- a/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java +++ b/tracing-jaxrs/src/main/java/com/palantir/tracing/jaxrs/JaxRsTracers.java @@ -17,6 +17,7 @@ package com.palantir.tracing.jaxrs; import com.palantir.tracing.CloseableSpan; +import com.palantir.tracing.Detached; import com.palantir.tracing.DetachedSpan; import com.palantir.tracing.Tracers; import java.io.IOException; @@ -37,16 +38,16 @@ public static StreamingOutput wrap(StreamingOutput delegate) { private static class TracingAwareStreamingOutput implements StreamingOutput { private final StreamingOutput delegate; - private final DetachedSpan detachedSpan; + private final Detached detached; TracingAwareStreamingOutput(StreamingOutput delegate) { this.delegate = delegate; - this.detachedSpan = DetachedSpan.detach(); + this.detached = DetachedSpan.detach(); } @Override public void write(OutputStream output) throws IOException, WebApplicationException { - try (CloseableSpan ignored = detachedSpan.childSpan("streaming-output")) { + try (CloseableSpan ignored = detached.childSpan("streaming-output")) { delegate.write(output); } } diff --git a/tracing/src/main/java/com/palantir/tracing/Detached.java b/tracing/src/main/java/com/palantir/tracing/Detached.java new file mode 100644 index 000000000..344c5adfa --- /dev/null +++ b/tracing/src/main/java/com/palantir/tracing/Detached.java @@ -0,0 +1,99 @@ +/* + * (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 com.google.errorprone.annotations.MustBeClosed; +import com.palantir.logsafe.Safe; +import com.palantir.tracing.api.SpanType; +import java.util.Map; +import javax.annotation.CheckReturnValue; + +/** + * Detached tracing component which is not bound to thread state, and can be used on any thread. + * + * This class must not be implemented externally. + */ +public interface Detached { + + /** + * Equivalent to {@link Tracer#startSpan(String, SpanType)}, but using this {@link DetachedSpan} as the parent + * instead of thread state. + */ + @MustBeClosed + default CloseableSpan childSpan(@Safe String operationName, SpanType type) { + return childSpan(operationName, NoTagTranslator.INSTANCE, NoTagTranslator.INSTANCE, type); + } + + /** + * Equivalent to {@link #childSpan(String, Map, SpanType)} using a {@link SpanType#LOCAL span}. + */ + @MustBeClosed + default CloseableSpan childSpan(@Safe String operationName, @Safe Map metadata) { + return childSpan(operationName, metadata, SpanType.LOCAL); + } + + @MustBeClosed + default CloseableSpan childSpan(@Safe String operationName, TagTranslator translator, T data) { + return childSpan(operationName, translator, data, SpanType.LOCAL); + } + + /** + * Equivalent to {@link #childSpan(String, SpanType)}, but using {@link Map metadata} tags. + */ + @MustBeClosed + default CloseableSpan childSpan(@Safe String operationName, @Safe Map metadata, SpanType type) { + return childSpan(operationName, MapTagTranslator.INSTANCE, metadata, type); + } + + @MustBeClosed + CloseableSpan childSpan(@Safe String operationName, TagTranslator translator, T data, SpanType type); + + /** + * Equivalent to {@link Tracer#startSpan(String)}, but using this {@link DetachedSpan} as the parent instead of + * thread state. + */ + @MustBeClosed + default CloseableSpan childSpan(@Safe String operationName) { + return childSpan(operationName, SpanType.LOCAL); + } + + /** + * Starts a child {@link DetachedSpan} using this instance as the parent. + */ + @CheckReturnValue + DetachedSpan childDetachedSpan(String operation, SpanType type); + + /** + * Starts a child {@link DetachedSpan} using this instance as the parent. Equivalent to + * {@link #childDetachedSpan(String, SpanType)} using {@link SpanType#LOCAL}. + */ + @CheckReturnValue + default DetachedSpan childDetachedSpan(@Safe String operation) { + return childDetachedSpan(operation, SpanType.LOCAL); + } + + /** + * Attaches the current {@link DetachedSpan} state to the current thread without creating additional spans. + * This is useful when a long-lived {@link DetachedSpan} measures many smaller operations (like async-io) + * in which we don't want to produce spans for each task, but do need tracing state associated for logging + * and potential child traces. + * @apiNote This must be executed within a try-with-resources block, and the parent detached span must still be + * completed separately. + */ + @MustBeClosed + CloseableSpan attach(); +} diff --git a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java index 203ff53ba..a84752a21 100644 --- a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java +++ b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java @@ -25,8 +25,10 @@ /** * Span which is not bound to thread state, and can be completed on any other thread. + * + * This class must not be implemented externally. */ -public interface DetachedSpan { +public interface DetachedSpan extends Detached { /** * Marks the beginning of a span, which you can {@link #complete} on any other thread. Further work on this @@ -69,58 +71,18 @@ static DetachedSpan start( } /** - * Creates a {@link DetachedSpan} instance based on the current tracing state without adding a new span. - * If there is no tracing state present a no-op instance is returned. This is equivalent to {@link #attach()}. + * Creates a {@link Detached} instance based on the current tracing state without adding a new span. + * Note that + * + * If there is no tracing state present a no-op instance is returned. This is the inverse of {@link #attach()}. * * @see DetachedSpan#attach() */ @CheckReturnValue - static DetachedSpan detach() { + static Detached detach() { return Tracer.detachInternal(); } - /** - * Equivalent to {@link Tracer#startSpan(String, SpanType)}, but using this {@link DetachedSpan} as the parent - * instead of thread state. - */ - @MustBeClosed - default CloseableSpan childSpan(@Safe String operationName, SpanType type) { - return childSpan(operationName, NoTagTranslator.INSTANCE, NoTagTranslator.INSTANCE, type); - } - - /** - * Equivalent to {@link #childSpan(String, Map, SpanType)} using a {@link SpanType#LOCAL span}. - */ - @MustBeClosed - default CloseableSpan childSpan(@Safe String operationName, @Safe Map metadata) { - return childSpan(operationName, metadata, SpanType.LOCAL); - } - - @MustBeClosed - default CloseableSpan childSpan(@Safe String operationName, TagTranslator translator, T data) { - return childSpan(operationName, translator, data, SpanType.LOCAL); - } - - /** - * Equivalent to {@link #childSpan(String, SpanType)}, but using {@link Map metadata} tags. - */ - @MustBeClosed - default CloseableSpan childSpan(@Safe String operationName, @Safe Map metadata, SpanType type) { - return childSpan(operationName, MapTagTranslator.INSTANCE, metadata, type); - } - - @MustBeClosed - CloseableSpan childSpan(@Safe String operationName, TagTranslator translator, T data, SpanType type); - - /** - * Equivalent to {@link Tracer#startSpan(String)}, but using this {@link DetachedSpan} as the parent instead of - * thread state. - */ - @MustBeClosed - default CloseableSpan childSpan(@Safe String operationName) { - return childSpan(operationName, SpanType.LOCAL); - } - @MustBeClosed @SuppressWarnings("MustBeClosedChecker") default CloseableSpan completeAndStartChild(@Safe String operationName, SpanType type) { @@ -134,32 +96,6 @@ default CloseableSpan completeAndStartChild(String operationName) { return completeAndStartChild(operationName, SpanType.LOCAL); } - /** - * Starts a child {@link DetachedSpan} using this instance as the parent. - */ - @CheckReturnValue - DetachedSpan childDetachedSpan(String operation, SpanType type); - - /** - * Starts a child {@link DetachedSpan} using this instance as the parent. Equivalent to - * {@link #childDetachedSpan(String, SpanType)} using {@link SpanType#LOCAL}. - */ - @CheckReturnValue - default DetachedSpan childDetachedSpan(@Safe String operation) { - return childDetachedSpan(operation, SpanType.LOCAL); - } - - /** - * Attaches the current {@link DetachedSpan} state to the current thread without creating additional spans. - * This is useful when a long-lived {@link DetachedSpan} measures many smaller operations (like async-io) - * in which we don't want to produce spans for each task, but do need tracing state associated for logging - * and potential child traces. - * @apiNote This must be executed within a try-with-resources block, and the parent detached span must still be - * completed separately. - */ - @MustBeClosed - CloseableSpan attach(); - /** * Completes this span. After complete is invoked, other methods are not expected to produce spans, but they must * not throw either in order to avoid confusing failures. diff --git a/tracing/src/main/java/com/palantir/tracing/NopDetached.java b/tracing/src/main/java/com/palantir/tracing/NopDetached.java new file mode 100644 index 000000000..bb0855c87 --- /dev/null +++ b/tracing/src/main/java/com/palantir/tracing/NopDetached.java @@ -0,0 +1,65 @@ +/* + * (c) Copyright 2021 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 com.google.errorprone.annotations.MustBeClosed; +import com.palantir.logsafe.Safe; +import com.palantir.tracing.api.SpanType; + +/** + * {@link Detached} implementation which represents state with no trace. + * Operations which create child traces will each generate a unique traceId and sample independently. + */ +enum NopDetached implements Detached { + INSTANCE; + + private static final CloseableSpan COMPLETE_SPAN = Tracer::fastCompleteSpan; + + @Override + @MustBeClosed + public CloseableSpan childSpan( + String operationName, TagTranslator translator, T data, SpanType type) { + return CloseableTracer.startSpan(operationName, translator, data, type)::close; + } + + @Override + @MustBeClosed + public CloseableSpan childSpan(@Safe String operationName, SpanType type) { + Tracer.fastStartSpan(operationName, type); + return COMPLETE_SPAN; + } + + @Override + @MustBeClosed + public CloseableSpan attach() { + return NopCloseableSpan.INSTANCE; + } + + @Override + public DetachedSpan childDetachedSpan(String operation, SpanType type) { + return DetachedSpan.start(operation, type); + } + + private enum NopCloseableSpan implements CloseableSpan { + INSTANCE; + + @Override + public void close() { + // nop + } + } +} diff --git a/tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java deleted file mode 100644 index 1eff26ebb..000000000 --- a/tracing/src/main/java/com/palantir/tracing/NopDetachedSpan.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * (c) Copyright 2021 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 com.palantir.tracing.api.SpanType; -import java.util.Map; - -enum NopDetachedSpan implements DetachedSpan { - INSTANCE; - - @Override - public CloseableSpan childSpan(String _operationName, SpanType _type) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public CloseableSpan childSpan(String _operationName, Map _metadata) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public CloseableSpan childSpan(String _operationName, TagTranslator _translator, T _data) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public CloseableSpan childSpan(String _operationName, Map _metadata, SpanType _type) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public CloseableSpan childSpan( - String _operationName, TagTranslator _translator, T _data, SpanType _type) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public CloseableSpan childSpan(String _operationName) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public CloseableSpan completeAndStartChild(String _operationName, SpanType _type) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public CloseableSpan completeAndStartChild(String _operationName) { - return NopCloseableSpan.INSTANCE; - } - - @Override - public DetachedSpan childDetachedSpan(String _operation, SpanType _type) { - return NopDetachedSpan.INSTANCE; - } - - @Override - public DetachedSpan childDetachedSpan(String _operation) { - return NopDetachedSpan.INSTANCE; - } - - @Override - public CloseableSpan attach() { - return NopCloseableSpan.INSTANCE; - } - - @Override - public void complete() { - // nop - } - - @Override - public void complete(Map _metadata) { - // nop - } - - @Override - public void complete(TagTranslator _tagTranslator, T _data) { - // nop - } - - private enum NopCloseableSpan implements CloseableSpan { - INSTANCE; - - @Override - public void close() { - // nop - } - } -} diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index 78bb57835..d6d8357a9 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -281,13 +281,13 @@ static DetachedSpan detachInternal( } /** - * Like {@link #detachInternal(String, SpanType)}, but does not create a new span, and may return a + * Like {@link #detachInternal(String, SpanType)} but does not create a new span and may return a * no-op implementation if no tracing state is currently set. */ - static DetachedSpan detachInternal() { + static Detached detachInternal() { Trace trace = currentTrace.get(); if (trace == null) { - return NopDetachedSpan.INSTANCE; + return NopDetached.INSTANCE; } String traceId = trace.getTraceId(); @@ -295,9 +295,9 @@ static DetachedSpan detachInternal() { if (trace.isObservable()) { OpenSpan maybeOpenSpan = trace.top().orElse(null); if (maybeOpenSpan == null) { - return NopDetachedSpan.INSTANCE; + return NopDetached.INSTANCE; } - return new SampledUnnamedDetachedSpan(traceId, requestId, maybeOpenSpan); + return new SampledDetached(traceId, requestId, maybeOpenSpan); } else { return new UnsampledDetachedSpan(traceId, requestId, Optional.empty()); } @@ -469,13 +469,13 @@ public void close() { } } - private static final class SampledUnnamedDetachedSpan implements DetachedSpan { + private static final class SampledDetached implements Detached { private final String traceId; private final Optional requestId; private final OpenSpan openSpan; - SampledUnnamedDetachedSpan(String traceId, Optional requestId, OpenSpan openSpan) { + SampledDetached(String traceId, Optional requestId, OpenSpan openSpan) { this.traceId = traceId; this.requestId = requestId; this.openSpan = openSpan; @@ -511,19 +511,9 @@ public CloseableSpan attach() { return maybeCurrentTrace == null ? REMOVE_TRACE : () -> Tracer.setTrace(maybeCurrentTrace); } - @Override - public void complete() { - // nop -- unnamed span cannot be completed - } - - @Override - public void complete(TagTranslator _tagTranslator, T _data) { - // nop -- unnamed span cannot be completed - } - @Override public String toString() { - return "SampledUnnamedDetachedSpan{traceId='" + return "SampledUnnamedDetached{traceId='" + traceId + '\'' + ", requestId='" diff --git a/tracing/src/main/java/com/palantir/tracing/Tracers.java b/tracing/src/main/java/com/palantir/tracing/Tracers.java index e0b2a2234..97d10115d 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracers.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracers.java @@ -507,42 +507,38 @@ private static void restoreTrace(Optional trace) { */ private static class TracingAwareCallable implements Callable { private final Callable delegate; - private final DetachedSpan detachedSpan; + private final Detached detached; private final String operation; private final Map metadata; TracingAwareCallable(String operation, Map metadata, Callable delegate) { this.delegate = delegate; - this.detachedSpan = DetachedSpan.detach(); + this.detached = DetachedSpan.detach(); this.operation = operation; this.metadata = metadata; } @Override public V call() throws Exception { - try (CloseableSpan ignored = detachedSpan.childSpan(operation, metadata)) { + try (CloseableSpan ignored = detached.childSpan(operation, metadata)) { return delegate.call(); - } finally { - detachedSpan.complete(); } } } private static class AnonymousTracingAwareCallable implements Callable { private final Callable delegate; - private final DetachedSpan detachedSpan; + private final Detached detached; AnonymousTracingAwareCallable(Callable delegate) { this.delegate = delegate; - this.detachedSpan = DetachedSpan.detach(); + this.detached = DetachedSpan.detach(); } @Override public V call() throws Exception { - try (CloseableSpan ignored = detachedSpan.childSpan("DeferredTracer(unnamed operation)")) { + try (CloseableSpan ignored = detached.attach()) { return delegate.call(); - } finally { - detachedSpan.complete(); } } } @@ -553,42 +549,38 @@ public V call() throws Exception { */ private static class TracingAwareRunnable implements Runnable { private final Runnable delegate; - private final DetachedSpan detachedSpan; + private final Detached detached; private final String operation; private final Map metadata; TracingAwareRunnable(String operation, Map metadata, Runnable delegate) { this.delegate = delegate; - this.detachedSpan = DetachedSpan.detach(); + this.detached = DetachedSpan.detach(); this.operation = operation; this.metadata = metadata; } @Override public void run() { - try (CloseableSpan ignored = detachedSpan.childSpan(operation, metadata)) { + try (CloseableSpan ignored = detached.childSpan(operation, metadata)) { delegate.run(); - } finally { - detachedSpan.complete(); } } } private static class AnonymousTracingAwareRunnable implements Runnable { private final Runnable delegate; - private final DetachedSpan detachedSpan; + private final Detached detached; AnonymousTracingAwareRunnable(Runnable delegate) { this.delegate = delegate; - this.detachedSpan = DetachedSpan.detach(); + this.detached = DetachedSpan.detach(); } @Override public void run() { - try (CloseableSpan ignored = detachedSpan.childSpan("DeferredTracer(unnamed operation)")) { + try (CloseableSpan ignored = detached.attach()) { delegate.run(); - } finally { - detachedSpan.complete(); } } } diff --git a/tracing/src/test/java/com/palantir/tracing/TracersTest.java b/tracing/src/test/java/com/palantir/tracing/TracersTest.java index 6c2bb250d..7122b5e83 100644 --- a/tracing/src/test/java/com/palantir/tracing/TracersTest.java +++ b/tracing/src/test/java/com/palantir/tracing/TracersTest.java @@ -35,7 +35,9 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.concurrent.Callable; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -69,30 +71,37 @@ public void after() { } @Test - public void testWrapExecutorService() throws Exception { - ExecutorService wrappedService = Tracers.wrap(Executors.newSingleThreadExecutor()); - - // Empty trace - wrappedService - .submit(traceExpectingCallableWithSingleSpan("DeferredTracer(unnamed operation)")) - .get(); - wrappedService - .submit(traceExpectingCallableWithSingleSpan("DeferredTracer(unnamed operation)")) - .get(); - - // Non-empty trace - Tracer.fastStartSpan("foo"); - Tracer.fastStartSpan("bar"); - Tracer.fastStartSpan("baz"); - wrappedService - .submit(traceExpectingCallableWithSingleSpan("DeferredTracer(unnamed operation)")) - .get(); - wrappedService - .submit(traceExpectingCallableWithSingleSpan("DeferredTracer(unnamed operation)")) - .get(); - Tracer.fastCompleteSpan(); - Tracer.fastCompleteSpan(); - Tracer.fastCompleteSpan(); + public void testWrapExecutorService() { + withExecutor(() -> Tracers.wrap(Executors.newSingleThreadExecutor()), wrappedService -> { + String subscription = UUID.randomUUID().toString(); + List operations = new CopyOnWriteArrayList<>(); + Tracer.subscribe(subscription, span -> operations.add(span.getOperation())); + try { + String traceId = Tracer.getTraceId(); + wrappedService + .submit(() -> assertThat(Tracer.getTraceId()).isEqualTo(traceId)) + .get(); + wrappedService + .submit(() -> assertThat(Tracer.getTraceId()).isEqualTo(traceId)) + .get(); + + Tracer.fastStartSpan("foo"); + Tracer.fastStartSpan("bar"); + Tracer.fastStartSpan("baz"); + wrappedService + .submit(() -> assertThat(Tracer.getTraceId()).isEqualTo(traceId)) + .get(); + wrappedService + .submit(() -> assertThat(Tracer.getTraceId()).isEqualTo(traceId)) + .get(); + Tracer.fastCompleteSpan(); + Tracer.fastCompleteSpan(); + Tracer.fastCompleteSpan(); + assertThat(operations).containsExactly("baz", "bar", "foo"); + } finally { + Tracer.unsubscribe(subscription); + } + }); } @Test @@ -112,6 +121,8 @@ public void testWrapExecutorService_withSpan() throws Exception { Tracer.fastCompleteSpan(); Tracer.fastCompleteSpan(); Tracer.fastCompleteSpan(); + + wrappedService.shutdownNow(); } @Test @@ -130,23 +141,18 @@ public void testWrapExecutorService_withRequestId() throws Exception { .submit(traceExpectingRunnableWithSingleSpan("operation", Optional.of(requestId))) .get(); Tracer.fastCompleteSpan(); + wrappedService.shutdownNow(); } @Test public void testWrapScheduledExecutorService() { - withExecutor(() -> Tracers.wrap(Executors.newSingleThreadScheduledExecutor()), wrappedService -> { + withExecutor(() -> Tracers.wrap("scheduler", Executors.newSingleThreadScheduledExecutor()), wrappedService -> { // Empty trace wrappedService - .schedule( - traceExpectingCallableWithSingleSpan("DeferredTracer(unnamed operation)"), - 0, - TimeUnit.SECONDS) + .schedule(traceExpectingCallableWithSingleSpan("scheduler"), 0, TimeUnit.SECONDS) .get(); wrappedService - .schedule( - traceExpectingRunnableWithSingleSpan("DeferredTracer(unnamed operation)"), - 0, - TimeUnit.SECONDS) + .schedule(traceExpectingRunnableWithSingleSpan("scheduler"), 0, TimeUnit.SECONDS) .get(); // Non-empty trace @@ -154,20 +160,15 @@ public void testWrapScheduledExecutorService() { Tracer.fastStartSpan("bar"); Tracer.fastStartSpan("baz"); wrappedService - .schedule( - traceExpectingCallableWithSingleSpan("DeferredTracer(unnamed operation)"), - 0, - TimeUnit.SECONDS) + .schedule(traceExpectingCallableWithSingleSpan("scheduler"), 0, TimeUnit.SECONDS) .get(); wrappedService - .schedule( - traceExpectingRunnableWithSingleSpan("DeferredTracer(unnamed operation)"), - 0, - TimeUnit.SECONDS) + .schedule(traceExpectingRunnableWithSingleSpan("scheduler"), 0, TimeUnit.SECONDS) .get(); Tracer.fastCompleteSpan(); Tracer.fastCompleteSpan(); Tracer.fastCompleteSpan(); + wrappedService.shutdownNow(); }); } @@ -324,8 +325,8 @@ public void testWrapCallable_callableTraceIsIsolated() throws Exception { @Test public void testWrapCallable_traceStateIsCapturedAtConstructionTime() throws Exception { Tracer.fastStartSpan("before-construction"); - Callable callable = Tracers.wrap(() -> { - assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("DeferredTracer(unnamed operation)"); + Callable callable = Tracers.wrap("callable", () -> { + assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("callable"); return null; }); Tracer.fastStartSpan("after-construction"); @@ -355,8 +356,8 @@ public void testWrapRunnable_runnableTraceIsIsolated() throws Exception { @Test public void testWrapRunnable_traceStateIsCapturedAtConstructionTime() throws Exception { Tracer.fastStartSpan("before-construction"); - Runnable runnable = Tracers.wrap(() -> { - assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("DeferredTracer(unnamed operation)"); + Runnable runnable = Tracers.wrap("runnable", () -> { + assertThat(Tracer.completeSpan().get().getOperation()).isEqualTo("runnable"); }); Tracer.fastStartSpan("after-construction"); runnable.run(); From f09dc76083a2e727c42898a996a10d45b16d2a37 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Wed, 1 Sep 2021 18:30:50 +0000 Subject: [PATCH 6/8] Add generated changelog entries --- changelog/@unreleased/pr-770.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-770.v2.yml diff --git a/changelog/@unreleased/pr-770.v2.yml b/changelog/@unreleased/pr-770.v2.yml new file mode 100644 index 000000000..49faf33d4 --- /dev/null +++ b/changelog/@unreleased/pr-770.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Allow detachment from thread state without creating a new span. + links: + - https://github.com/palantir/tracing-java/pull/770 From fee8834c1ee91c86469031f2fd28b8cb2a339e80 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Wed, 1 Sep 2021 15:24:11 -0400 Subject: [PATCH 7/8] deduplicate --- .../java/com/palantir/tracing/Tracer.java | 111 +++++++++--------- 1 file changed, 58 insertions(+), 53 deletions(-) diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index d6d8357a9..859beb41f 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -337,6 +337,37 @@ static boolean isSampled(DetachedSpan detachedSpan) { return detachedSpan instanceof SampledDetachedSpan; } + private static final class TraceRestoringCloseableSpanWithMetadata implements CloseableSpan { + + @Nullable + private final Trace original; + + private final TagTranslator translator; + private final T data; + + static CloseableSpan of(@Nullable Trace original, TagTranslator translator, T data) { + if (original != null || !translator.isEmpty(data)) { + return new TraceRestoringCloseableSpanWithMetadata<>(original, translator, data); + } + return DEFAULT_CLOSEABLE_SPAN; + } + + TraceRestoringCloseableSpanWithMetadata(@Nullable Trace original, TagTranslator translator, T data) { + this.original = original; + this.translator = translator; + this.data = data; + } + + @Override + public void close() { + Tracer.fastCompleteSpan(translator, data); + Trace originalTrace = original; + if (originalTrace != null) { + Tracer.setTrace(originalTrace); + } + } + } + private static final class SampledDetachedSpan implements DetachedSpan { private static final int NOT_COMPLETE = 0; private static final int COMPLETE = 1; @@ -368,27 +399,37 @@ private static final class SampledDetachedSpan implements DetachedSpan { .build(); } - @Override @MustBeClosed - public CloseableSpan childSpan( - String operationName, TagTranslator translator, T data, SpanType type) { - warnIfCompleted("startSpanOnCurrentThread"); + private static CloseableSpan childSpan( + String traceId, + OpenSpan openSpan, + Optional requestId, + String operationName, + TagTranslator translator, + T data, + SpanType type) { Trace maybeCurrentTrace = currentTrace.get(); setTrace(Trace.of(true, traceId, requestId)); Tracer.fastStartSpan(operationName, openSpan.getSpanId(), type); return TraceRestoringCloseableSpanWithMetadata.of(maybeCurrentTrace, translator, data); } + @Override + @MustBeClosed + public CloseableSpan childSpan( + String operationName, TagTranslator translator, T data, SpanType type) { + warnIfCompleted("startSpanOnCurrentThread"); + return childSpan(traceId, openSpan, requestId, operationName, translator, data, type); + } + @Override public DetachedSpan childDetachedSpan(String operation, SpanType type) { warnIfCompleted("startDetachedSpan"); return new SampledDetachedSpan(operation, type, traceId, requestId, Optional.of(openSpan.getSpanId())); } - @Override @MustBeClosed - public CloseableSpan attach() { - warnIfCompleted("startSpanOnCurrentThread"); + private static CloseableSpan attach(String traceId, OpenSpan openSpan, Optional requestId) { Trace maybeCurrentTrace = currentTrace.get(); Trace newTrace = Trace.of(true, traceId, requestId); // Push the DetachedSpan OpenSpan to provide the correct parent information @@ -401,6 +442,13 @@ public CloseableSpan attach() { return maybeCurrentTrace == null ? REMOVE_TRACE : () -> Tracer.setTrace(maybeCurrentTrace); } + @Override + @MustBeClosed + public CloseableSpan attach() { + warnIfCompleted("startSpanOnCurrentThread"); + return attach(traceId, openSpan, requestId); + } + @Override public void complete() { complete(NoTagTranslator.INSTANCE, NoTagTranslator.INSTANCE); @@ -438,37 +486,6 @@ private void warnIfCompleted(String feature) { } } - private static final class TraceRestoringCloseableSpanWithMetadata implements CloseableSpan { - - @Nullable - private final Trace original; - - private final TagTranslator translator; - private final T data; - - static CloseableSpan of(@Nullable Trace original, TagTranslator translator, T data) { - if (original != null || !translator.isEmpty(data)) { - return new TraceRestoringCloseableSpanWithMetadata<>(original, translator, data); - } - return DEFAULT_CLOSEABLE_SPAN; - } - - TraceRestoringCloseableSpanWithMetadata(@Nullable Trace original, TagTranslator translator, T data) { - this.original = original; - this.translator = translator; - this.data = data; - } - - @Override - public void close() { - Tracer.fastCompleteSpan(translator, data); - Trace originalTrace = original; - if (originalTrace != null) { - Tracer.setTrace(originalTrace); - } - } - } - private static final class SampledDetached implements Detached { private final String traceId; @@ -485,10 +502,7 @@ private static final class SampledDetached implements Detached { @MustBeClosed public CloseableSpan childSpan( String operationName, TagTranslator translator, T data, SpanType type) { - Trace maybeCurrentTrace = currentTrace.get(); - setTrace(Trace.of(true, traceId, requestId)); - Tracer.fastStartSpan(operationName, openSpan.getSpanId(), type); - return TraceRestoringCloseableSpanWithMetadata.of(maybeCurrentTrace, translator, data); + return SampledDetachedSpan.childSpan(traceId, openSpan, requestId, operationName, translator, data, type); } @Override @@ -499,21 +513,12 @@ public DetachedSpan childDetachedSpan(String operation, SpanType type) { @Override @MustBeClosed public CloseableSpan attach() { - Trace maybeCurrentTrace = currentTrace.get(); - Trace newTrace = Trace.of(true, traceId, requestId); - // Push the DetachedSpan OpenSpan to provide the correct parent information - // to child spans created within the context of this attach. - // It is VITAL that this span is never completed, it exists only for attribution. - newTrace.push(openSpan); - setTrace(newTrace); - // Do not complete the synthetic root span, it simply prevents nested spans from removing trace state, and - // allows - return maybeCurrentTrace == null ? REMOVE_TRACE : () -> Tracer.setTrace(maybeCurrentTrace); + return SampledDetachedSpan.attach(traceId, openSpan, requestId); } @Override public String toString() { - return "SampledUnnamedDetached{traceId='" + return "SampledDetached{traceId='" + traceId + '\'' + ", requestId='" From ec74259eb900c485774d9ba9b8d521f28f9554b1 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 7 Sep 2021 11:05:07 -0400 Subject: [PATCH 8/8] Update tracing/src/main/java/com/palantir/tracing/DetachedSpan.java Co-authored-by: Tom Petracca --- tracing/src/main/java/com/palantir/tracing/DetachedSpan.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java index a84752a21..066c02678 100644 --- a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java +++ b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java @@ -72,9 +72,8 @@ static DetachedSpan start( /** * Creates a {@link Detached} instance based on the current tracing state without adding a new span. - * Note that - * - * If there is no tracing state present a no-op instance is returned. This is the inverse of {@link #attach()}. + * Note that if there is no tracing state present a no-op instance is returned. This is the inverse of + * {@link #attach()}. * * @see DetachedSpan#attach() */