From 8fd82335564a9bebbca047b8a0bb55627c39bb46 Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Wed, 25 Aug 2021 14:09:21 +0100 Subject: [PATCH 01/11] Add first test --- .../tracing/api/TraceHttpHeaders.java | 2 ++ .../okhttp3/OkhttpTraceInterceptorTest.java | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java b/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java index 692e61059..8745b8cb4 100644 --- a/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java +++ b/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java @@ -34,4 +34,6 @@ public interface TraceHttpHeaders { * tracing, the typical trace logs (with sampling) are still required. */ String ORIGINATING_SPAN_ID = "X-OrigSpanId"; + + String ORIGIN_USER_AGENT = "Origin-User-Agent"; } diff --git a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java index 08812f304..5cde9a7d3 100644 --- a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java +++ b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java @@ -107,6 +107,27 @@ public void testPopulatesNewTrace_whenParentTraceIsPresent() throws IOException assertThat(intercepted.headers(TraceHttpHeaders.ORIGINATING_SPAN_ID)).containsOnly(originatingSpanId); } + @Test + public void testPopulatesNewTrace_whenOriginUserAgentIsPresent() throws IOException { + String originUserAgent = "originUserAgent"; + Tracer.initTraceWithSpan(Observability.SAMPLE, "id", "operation", SpanType.SERVER_INCOMING); + String traceId = Tracer.getTraceId(); + try { + OkhttpTraceInterceptor.INSTANCE.intercept(chain); + } finally { + Tracer.fastCompleteSpan(); + } + + verify(chain).request(); + verify(chain).proceed(requestCaptor.capture()); + verifyNoMoreInteractions(chain); + + Request intercepted = requestCaptor.getValue(); + assertThat(intercepted.headers(TraceHttpHeaders.SPAN_ID)).isNotNull(); + assertThat(intercepted.headers(TraceHttpHeaders.TRACE_ID)).containsOnly(traceId); + assertThat(intercepted.headers(TraceHttpHeaders.ORIGIN_USER_AGENT)).containsOnly(originUserAgent); + } + @Test public void testAddsIsSampledHeader_whenTraceIsObservable() throws IOException { Tracer.initTraceWithSpan(Observability.SAMPLE, Tracers.randomId(), "op", SpanType.LOCAL); From 91ffed75f20c32fbad32dabb58f38a12897ab020 Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Wed, 25 Aug 2021 16:27:01 +0100 Subject: [PATCH 02/11] Propagation of origin user agent works --- .../com/palantir/tracing/api/OpenSpan.java | 22 ++++++- .../okhttp3/OkhttpTraceInterceptor.java | 5 ++ .../okhttp3/OkhttpTraceInterceptorTest.java | 3 +- .../main/java/com/palantir/tracing/Trace.java | 58 ++++++++++++++----- .../java/com/palantir/tracing/Tracer.java | 30 ++++++++++ 5 files changed, 99 insertions(+), 19 deletions(-) diff --git a/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java b/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java index c32024529..3547991dc 100644 --- a/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java +++ b/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java @@ -64,6 +64,14 @@ public abstract class OpenSpan { @Value.Parameter public abstract Optional getOriginatingSpanId(); + /** + * Returns the identifier of the 'origin' User-Agent if one exists. + * + * @see TraceHttpHeaders + */ + @Value.Parameter + public abstract Optional getOriginUserAgent(); + /** Returns a globally unique identifier representing a single span within the call trace. */ @Value.Parameter public abstract String getSpanId(); @@ -89,9 +97,17 @@ public static OpenSpan of( String spanId, SpanType type, Optional parentSpanId, - Optional originatingSpanId) { + Optional originatingSpanId, + Optional originUserAgent) { return ImmutableOpenSpan.of( - operation, getNowInMicroSeconds(), System.nanoTime(), parentSpanId, originatingSpanId, spanId, type); + operation, + getNowInMicroSeconds(), + System.nanoTime(), + parentSpanId, + originatingSpanId, + originUserAgent, + spanId, + type); } /** @@ -102,7 +118,7 @@ public static OpenSpan of( @SuppressWarnings("InlineMeSuggester") @Deprecated public static OpenSpan of(String operation, String spanId, SpanType type, Optional parentSpanId) { - return of(operation, spanId, type, parentSpanId, Optional.empty()); + return of(operation, spanId, type, parentSpanId, Optional.empty(), Optional.empty()); } private static long getNowInMicroSeconds() { diff --git a/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java b/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java index 052c75f2a..59a225ba7 100644 --- a/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java +++ b/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java @@ -62,6 +62,11 @@ public Response intercept(Chain chain) throws IOException { TraceHttpHeaders.ORIGINATING_SPAN_ID, span.getOriginatingSpanId().get()); } + if (span.getOriginUserAgent().isPresent()) { + tracedRequest.header( + TraceHttpHeaders.ORIGIN_USER_AGENT, + span.getOriginUserAgent().get()); + } Response response; try { diff --git a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java index 5cde9a7d3..9adfaf783 100644 --- a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java +++ b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java @@ -110,7 +110,8 @@ public void testPopulatesNewTrace_whenParentTraceIsPresent() throws IOException @Test public void testPopulatesNewTrace_whenOriginUserAgentIsPresent() throws IOException { String originUserAgent = "originUserAgent"; - Tracer.initTraceWithSpan(Observability.SAMPLE, "id", "operation", SpanType.SERVER_INCOMING); + Tracer.initTraceWithSpan( + Observability.SAMPLE, "id", "operation", "parent", SpanType.SERVER_INCOMING, originUserAgent); String traceId = Tracer.getTraceId(); try { OkhttpTraceInterceptor.INSTANCE.intercept(chain); diff --git a/tracing/src/main/java/com/palantir/tracing/Trace.java b/tracing/src/main/java/com/palantir/tracing/Trace.java index f61c5fcf3..166026472 100644 --- a/tracing/src/main/java/com/palantir/tracing/Trace.java +++ b/tracing/src/main/java/com/palantir/tracing/Trace.java @@ -48,7 +48,10 @@ public abstract class Trace { private final Optional requestId; - private Trace(String traceId, Optional requestId) { + private final Optional originUserAgent; + + private Trace(String traceId, Optional requestId, Optional originUserAgent) { + this.originUserAgent = originUserAgent; checkArgument(!Strings.isNullOrEmpty(traceId), "traceId must be non-empty"); this.traceId = traceId; this.requestId = checkNotNull(requestId, "requestId"); @@ -68,7 +71,8 @@ final OpenSpan startSpan(String operation, String parentSpanId, SpanType type) { Tracers.randomId(), type, Optional.of(parentSpanId), - orElse(getOriginatingSpanId(), Optional.of(parentSpanId))); + orElse(getOriginatingSpanId(), Optional.of(parentSpanId)), + Optional.empty()); push(span); return span; } @@ -88,9 +92,16 @@ final OpenSpan startSpan(String operation, SpanType type) { Tracers.randomId(), type, Optional.of(prevState.get().getSpanId()), - orElse(getOriginatingSpanId(), prevState.get().getParentSpanId())); + orElse(getOriginatingSpanId(), prevState.get().getParentSpanId()), + orElse(getOriginUserAgent(), prevState.get().getOriginUserAgent())); } else { - span = OpenSpan.of(operation, Tracers.randomId(), type, Optional.empty(), getOriginatingSpanId()); + span = OpenSpan.of( + operation, + Tracers.randomId(), + type, + Optional.empty(), + getOriginatingSpanId(), + getOriginUserAgent()); } push(span); @@ -142,24 +153,40 @@ final Optional getRequestId() { abstract Optional getOriginatingSpanId(); + final Optional getOriginUserAgent() { + return originUserAgent; + } + /** Returns a copy of this Trace which can be independently mutated. */ abstract Trace deepCopy(); static Trace of(boolean isObservable, String traceId, Optional requestId) { - return isObservable ? new Sampled(traceId, requestId) : new Unsampled(traceId, requestId); + return isObservable + ? new Sampled(traceId, requestId, Optional.empty()) + : new Unsampled(traceId, requestId, Optional.empty()); + } + + static Trace of(boolean isObservable, String traceId, Optional requestId, String originUserAgent) { + return isObservable + ? new Sampled(traceId, requestId, Optional.of(originUserAgent)) + : new Unsampled(traceId, requestId, Optional.of(originUserAgent)); } private static final class Sampled extends Trace { private final Deque stack; - private Sampled(ArrayDeque stack, String traceId, Optional requestId) { - super(traceId, requestId); + private Sampled( + ArrayDeque stack, + String traceId, + Optional requestId, + Optional originUserAgent) { + super(traceId, requestId, originUserAgent); this.stack = stack; } - private Sampled(String traceId, Optional requestId) { - this(new ArrayDeque<>(), traceId, requestId); + private Sampled(String traceId, Optional requestId, Optional originUserAgent) { + this(new ArrayDeque<>(), traceId, requestId, originUserAgent); } @Override @@ -209,7 +236,7 @@ Optional getOriginatingSpanId() { @Override Trace deepCopy() { - return new Sampled(new ArrayDeque<>(stack), getTraceId(), getRequestId()); + return new Sampled(new ArrayDeque<>(stack), getTraceId(), getRequestId(), getOriginUserAgent()); } @Override @@ -227,14 +254,15 @@ private static final class Unsampled extends Trace { private Optional originatingSpanId = Optional.empty(); - private Unsampled(int numberOfSpans, String traceId, Optional requestId) { - super(traceId, requestId); + private Unsampled( + int numberOfSpans, String traceId, Optional requestId, Optional originUserAgent) { + super(traceId, requestId, originUserAgent); this.numberOfSpans = numberOfSpans; validateNumberOfSpans(); } - private Unsampled(String traceId, Optional requestId) { - this(0, traceId, requestId); + private Unsampled(String traceId, Optional requestId, Optional originUserAgent) { + this(0, traceId, requestId, originUserAgent); } @Override @@ -294,7 +322,7 @@ Optional getOriginatingSpanId() { @Override Trace deepCopy() { - return new Unsampled(numberOfSpans, getTraceId(), getRequestId()); + return new Unsampled(numberOfSpans, getTraceId(), getRequestId(), getOriginUserAgent()); } /** Internal validation, this should never fail because {@link #pop()} only decrements positive values. */ diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index 56e2509e3..b164991d1 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -69,6 +69,17 @@ private Tracer() {} // Thread-safe since stateless private static volatile TraceSampler sampler = RandomSampler.create(0.0005f); + /** + * Creates a new trace, but does not set it as the current trace. + */ + private static Trace createTrace( + Observability observability, String traceId, Optional requestId, String originUserAgent) { + checkArgument(!Strings.isNullOrEmpty(traceId), "traceId must be non-empty"); + checkArgument(!Strings.isNullOrEmpty(originUserAgent), "originUserAgent must ne non-empty"); + boolean observable = shouldObserve(observability); + return Trace.of(observable, traceId, requestId, originUserAgent); + } + /** * Creates a new trace, but does not set it as the current trace. */ @@ -157,6 +168,25 @@ public static void initTrace(Observability observability, String traceId) { setTrace(createTrace(observability, traceId, Optional.empty())); } + /** + * Initializes the current thread's trace with a root span, erasing any previously accrued open spans. + * The root span must eventually be completed using {@link #fastCompleteSpan()} or {@link #completeSpan()}. + */ + public static void initTraceWithSpan( + Observability observability, + String traceId, + @Safe String operation, + String parentSpanId, + SpanType type, + String originUserAgent) { + setTrace(createTrace( + observability, + traceId, + type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(), + originUserAgent)); + fastStartSpan(operation, parentSpanId, type); + } + /** * Initializes the current thread's trace with a root span, erasing any previously accrued open spans. * The root span must eventually be completed using {@link #fastCompleteSpan()} or {@link #completeSpan()}. From 36e81a6bd56a9b9c4a5668670b0085c5b7abfab4 Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Wed, 25 Aug 2021 16:43:56 +0100 Subject: [PATCH 03/11] Origin user agent in span --- .../com/palantir/tracing/api/OpenSpan.java | 22 +++---------------- .../tracing/OkhttpTraceInterceptor2.java | 6 +++++ .../okhttp3/OkhttpTraceInterceptor.java | 4 ++-- .../main/java/com/palantir/tracing/Trace.java | 14 +++--------- .../com/palantir/tracing/TraceMetadata.java | 3 +++ .../java/com/palantir/tracing/Tracer.java | 6 +++++ 6 files changed, 23 insertions(+), 32 deletions(-) diff --git a/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java b/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java index 3547991dc..c32024529 100644 --- a/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java +++ b/tracing-api/src/main/java/com/palantir/tracing/api/OpenSpan.java @@ -64,14 +64,6 @@ public abstract class OpenSpan { @Value.Parameter public abstract Optional getOriginatingSpanId(); - /** - * Returns the identifier of the 'origin' User-Agent if one exists. - * - * @see TraceHttpHeaders - */ - @Value.Parameter - public abstract Optional getOriginUserAgent(); - /** Returns a globally unique identifier representing a single span within the call trace. */ @Value.Parameter public abstract String getSpanId(); @@ -97,17 +89,9 @@ public static OpenSpan of( String spanId, SpanType type, Optional parentSpanId, - Optional originatingSpanId, - Optional originUserAgent) { + Optional originatingSpanId) { return ImmutableOpenSpan.of( - operation, - getNowInMicroSeconds(), - System.nanoTime(), - parentSpanId, - originatingSpanId, - originUserAgent, - spanId, - type); + operation, getNowInMicroSeconds(), System.nanoTime(), parentSpanId, originatingSpanId, spanId, type); } /** @@ -118,7 +102,7 @@ public static OpenSpan of( @SuppressWarnings("InlineMeSuggester") @Deprecated public static OpenSpan of(String operation, String spanId, SpanType type, Optional parentSpanId) { - return of(operation, spanId, type, parentSpanId, Optional.empty(), Optional.empty()); + return of(operation, spanId, type, parentSpanId, Optional.empty()); } private static long getNowInMicroSeconds() { diff --git a/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java b/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java index b2ddfa94a..d99be2ca4 100644 --- a/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java +++ b/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java @@ -64,6 +64,12 @@ public Response intercept(Chain chain) throws IOException { metadata.getOriginatingSpanId().get()); } + if (metadata.getOriginUserAgent().isPresent()) { + tracedRequest.header( + TraceHttpHeaders.ORIGIN_USER_AGENT, + metadata.getOriginUserAgent().get()); + } + return chain.proceed(tracedRequest.build()); } } diff --git a/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java b/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java index 59a225ba7..47cb26069 100644 --- a/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java +++ b/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java @@ -62,10 +62,10 @@ public Response intercept(Chain chain) throws IOException { TraceHttpHeaders.ORIGINATING_SPAN_ID, span.getOriginatingSpanId().get()); } - if (span.getOriginUserAgent().isPresent()) { + if (Tracer.getOriginUserAgent().isPresent()) { tracedRequest.header( TraceHttpHeaders.ORIGIN_USER_AGENT, - span.getOriginUserAgent().get()); + Tracer.getOriginUserAgent().get()); } Response response; diff --git a/tracing/src/main/java/com/palantir/tracing/Trace.java b/tracing/src/main/java/com/palantir/tracing/Trace.java index 166026472..dabe207ef 100644 --- a/tracing/src/main/java/com/palantir/tracing/Trace.java +++ b/tracing/src/main/java/com/palantir/tracing/Trace.java @@ -71,8 +71,7 @@ final OpenSpan startSpan(String operation, String parentSpanId, SpanType type) { Tracers.randomId(), type, Optional.of(parentSpanId), - orElse(getOriginatingSpanId(), Optional.of(parentSpanId)), - Optional.empty()); + orElse(getOriginatingSpanId(), Optional.of(parentSpanId))); push(span); return span; } @@ -92,16 +91,9 @@ final OpenSpan startSpan(String operation, SpanType type) { Tracers.randomId(), type, Optional.of(prevState.get().getSpanId()), - orElse(getOriginatingSpanId(), prevState.get().getParentSpanId()), - orElse(getOriginUserAgent(), prevState.get().getOriginUserAgent())); + orElse(getOriginatingSpanId(), prevState.get().getParentSpanId())); } else { - span = OpenSpan.of( - operation, - Tracers.randomId(), - type, - Optional.empty(), - getOriginatingSpanId(), - getOriginUserAgent()); + span = OpenSpan.of(operation, Tracers.randomId(), type, Optional.empty(), getOriginatingSpanId()); } push(span); diff --git a/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java b/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java index 8530b2ad8..d94292d28 100644 --- a/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java +++ b/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java @@ -42,6 +42,9 @@ public interface TraceMetadata { /** Corresponds to {@link com.palantir.tracing.api.TraceHttpHeaders#ORIGINATING_SPAN_ID}. */ Optional getOriginatingSpanId(); + /** Corresponds to {@link com.palantir.tracing.api.TraceHttpHeaders#ORIGIN_USER_AGENT}. */ + Optional getOriginUserAgent(); + static Builder builder() { return new Builder(); } diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index b164991d1..c22de6f86 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -130,6 +130,7 @@ public static Optional maybeGetTraceMetadata() { .spanId(openSpan.getSpanId()) .parentSpanId(openSpan.getParentSpanId()) .originatingSpanId(trace.getOriginatingSpanId()) + .originUserAgent(trace.getOriginUserAgent()) .traceId(trace.getTraceId()) .requestId(trace.getRequestId()) .build()); @@ -138,6 +139,7 @@ public static Optional maybeGetTraceMetadata() { .spanId(Tracers.randomId()) .parentSpanId(Optional.empty()) .originatingSpanId(trace.getOriginatingSpanId()) + .originUserAgent(trace.getOriginUserAgent()) .traceId(trace.getTraceId()) .requestId(trace.getRequestId()) .build()); @@ -721,6 +723,10 @@ public static String getTraceId() { return checkNotNull(currentTrace.get(), "There is no trace").getTraceId(); } + public static Optional getOriginUserAgent() { + return currentTrace.get().getOriginUserAgent(); + } + /** * Clears the current trace id and returns it if present. */ From 2c931da0892ebb284080389fb25fd1e083e11520 Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Wed, 25 Aug 2021 17:47:46 +0100 Subject: [PATCH 04/11] Basic forwarding --- .../tracing/jersey/TraceEnrichingFilter.java | 34 ++++++++++++++++--- .../okhttp3/OkhttpTraceInterceptorTest.java | 8 ++++- .../main/java/com/palantir/tracing/Trace.java | 7 ++-- .../java/com/palantir/tracing/Tracer.java | 23 +++++++++++-- 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java b/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java index ab51a60cf..4088a9d36 100644 --- a/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java +++ b/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java @@ -35,6 +35,7 @@ import javax.ws.rs.container.ContainerResponseContext; import javax.ws.rs.container.ContainerResponseFilter; import javax.ws.rs.core.Context; +import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.ext.Provider; import org.glassfish.jersey.server.ExtendedUriInfo; @@ -53,6 +54,8 @@ public final class TraceEnrichingFilter implements ContainerRequestFilter, Conta public static final String REQUEST_ID_PROPERTY_NAME = "com.palantir.tracing.requestId"; + public static final String ORIGIN_USER_AGENT_PROPERTY_NAME = "com.palantir.tracing.originUserAgent"; + public static final String SAMPLED_PROPERTY_NAME = "com.palantir.tracing.sampled"; @Context @@ -76,22 +79,37 @@ public void filter(ContainerRequestContext requestContext) throws IOException { getObservabilityFromHeader(requestContext), Tracers.randomId(), operation, - SpanType.SERVER_INCOMING); + SpanType.SERVER_INCOMING, + getOriginUserAgentFromHeader(requestContext)); } else if (spanId == null) { Tracer.initTraceWithSpan( - getObservabilityFromHeader(requestContext), traceId, operation, SpanType.SERVER_INCOMING); + getObservabilityFromHeader(requestContext), + traceId, + operation, + SpanType.SERVER_INCOMING, + getOriginUserAgentFromHeader(requestContext)); } else { // caller's span is this span's parent. Tracer.initTraceWithSpan( - getObservabilityFromHeader(requestContext), traceId, operation, spanId, SpanType.SERVER_INCOMING); + getObservabilityFromHeader(requestContext), + traceId, + operation, + spanId, + SpanType.SERVER_INCOMING, + getOriginUserAgentFromHeader(requestContext)); } // Give asynchronous downstream handlers access to the trace id requestContext.setProperty(TRACE_ID_PROPERTY_NAME, Tracer.getTraceId()); requestContext.setProperty(SAMPLED_PROPERTY_NAME, Tracer.isTraceObservable()); - Tracer.maybeGetTraceMetadata() + Optional traceMetadata = Tracer.maybeGetTraceMetadata(); + traceMetadata .flatMap(TraceMetadata::getRequestId) .ifPresent(requestId -> requestContext.setProperty(REQUEST_ID_PROPERTY_NAME, requestId)); + traceMetadata + .flatMap(TraceMetadata::getOriginUserAgent) + .ifPresent(originUserAgent -> + requestContext.setProperty(ORIGIN_USER_AGENT_PROPERTY_NAME, originUserAgent)); } // Handles outgoing response @@ -131,6 +149,14 @@ private static Observability getObservabilityFromHeader(ContainerRequestContext } } + private static Optional getOriginUserAgentFromHeader(ContainerRequestContext requestContext) { + String originUserAgent = requestContext.getHeaderString(TraceHttpHeaders.ORIGIN_USER_AGENT); + if (originUserAgent == null) { + return Optional.ofNullable(requestContext.getHeaderString(HttpHeaders.USER_AGENT)); + } + return Optional.of(originUserAgent); + } + private String getPathTemplate() { return Optional.ofNullable(uriInfo) .map(ExtendedUriInfo::getMatchedModelResource) diff --git a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java index 9adfaf783..14057c43f 100644 --- a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java +++ b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java @@ -31,6 +31,7 @@ import com.palantir.tracing.api.SpanType; import com.palantir.tracing.api.TraceHttpHeaders; import java.io.IOException; +import java.util.Optional; import okhttp3.Interceptor; import okhttp3.Request; import org.junit.After; @@ -111,7 +112,12 @@ public void testPopulatesNewTrace_whenParentTraceIsPresent() throws IOException public void testPopulatesNewTrace_whenOriginUserAgentIsPresent() throws IOException { String originUserAgent = "originUserAgent"; Tracer.initTraceWithSpan( - Observability.SAMPLE, "id", "operation", "parent", SpanType.SERVER_INCOMING, originUserAgent); + Observability.SAMPLE, + "id", + "operation", + "parent", + SpanType.SERVER_INCOMING, + Optional.of(originUserAgent)); String traceId = Tracer.getTraceId(); try { OkhttpTraceInterceptor.INSTANCE.intercept(chain); diff --git a/tracing/src/main/java/com/palantir/tracing/Trace.java b/tracing/src/main/java/com/palantir/tracing/Trace.java index dabe207ef..ca83100e2 100644 --- a/tracing/src/main/java/com/palantir/tracing/Trace.java +++ b/tracing/src/main/java/com/palantir/tracing/Trace.java @@ -158,10 +158,11 @@ static Trace of(boolean isObservable, String traceId, Optional requestId : new Unsampled(traceId, requestId, Optional.empty()); } - static Trace of(boolean isObservable, String traceId, Optional requestId, String originUserAgent) { + static Trace of( + boolean isObservable, String traceId, Optional requestId, Optional originUserAgent) { return isObservable - ? new Sampled(traceId, requestId, Optional.of(originUserAgent)) - : new Unsampled(traceId, requestId, Optional.of(originUserAgent)); + ? new Sampled(traceId, requestId, originUserAgent) + : new Unsampled(traceId, requestId, originUserAgent); } private static final class Sampled extends Trace { diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index c22de6f86..ead4d3c79 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -73,9 +73,8 @@ private Tracer() {} * Creates a new trace, but does not set it as the current trace. */ private static Trace createTrace( - Observability observability, String traceId, Optional requestId, String originUserAgent) { + Observability observability, String traceId, Optional requestId, Optional originUserAgent) { checkArgument(!Strings.isNullOrEmpty(traceId), "traceId must be non-empty"); - checkArgument(!Strings.isNullOrEmpty(originUserAgent), "originUserAgent must ne non-empty"); boolean observable = shouldObserve(observability); return Trace.of(observable, traceId, requestId, originUserAgent); } @@ -180,7 +179,7 @@ public static void initTraceWithSpan( @Safe String operation, String parentSpanId, SpanType type, - String originUserAgent) { + Optional originUserAgent) { setTrace(createTrace( observability, traceId, @@ -215,6 +214,24 @@ public static void initTraceWithSpan( fastStartSpan(operation, type); } + /** + * Initializes the current thread's trace with a root span, erasing any previously accrued open spans. + * The root span must eventually be completed using {@link #fastCompleteSpan()} or {@link #completeSpan()}. + */ + public static void initTraceWithSpan( + Observability observability, + String traceId, + @Safe String operation, + SpanType type, + Optional originUserAgent) { + setTrace(createTrace( + observability, + traceId, + type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(), + originUserAgent)); + fastStartSpan(operation, type); + } + /** * Opens a new span for this thread's call trace, labeled with the provided operation and parent span. Only allowed * when the current trace is empty. If the return value is not used, prefer {@link Tracer#fastStartSpan(String, From 3df060b0a2876ce683289939a4d834fdb76250c9 Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Thu, 16 Sep 2021 13:32:46 +0100 Subject: [PATCH 05/11] Rename header --- .../main/java/com/palantir/tracing/api/TraceHttpHeaders.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java b/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java index 31a22125f..d4aa446ba 100644 --- a/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java +++ b/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java @@ -38,5 +38,5 @@ public interface TraceHttpHeaders { @Deprecated String ORIGINATING_SPAN_ID = "X-OrigSpanId"; - String ORIGIN_USER_AGENT = "Origin-User-Agent"; + String ORIGIN_USER_AGENT = "X-Origin-User-Agent"; } From d30f67e66f8c9417e77aad356901609991d38a21 Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Thu, 16 Sep 2021 14:30:00 +0100 Subject: [PATCH 06/11] More user agent tracing --- .../tracing/undertow/UndertowTracing.java | 4 +- .../com/palantir/tracing/DetachedSpan.java | 5 +- .../java/com/palantir/tracing/Tracer.java | 57 ++++++++++++++----- .../java/com/palantir/tracing/TracerTest.java | 8 +-- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java b/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java index 3de412c64..72783e307 100644 --- a/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java +++ b/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java @@ -48,6 +48,7 @@ final class UndertowTracing { private static final HttpString TRACE_ID = HttpString.tryFromString(TraceHttpHeaders.TRACE_ID); private static final HttpString SPAN_ID = HttpString.tryFromString(TraceHttpHeaders.SPAN_ID); private static final HttpString IS_SAMPLED = HttpString.tryFromString(TraceHttpHeaders.IS_SAMPLED); + private static final HttpString ORIGIN_USER_AGENT = HttpString.tryFromString(TraceHttpHeaders.ORIGIN_USER_AGENT); // Consider moving this to TracingAttachments and making it public. For now it's well encapsulated // here because we expect the two handler implementations to be sufficient. @@ -108,7 +109,8 @@ private static DetachedSpan detachedSpan( traceId, newTrace ? Optional.empty() : Optional.ofNullable(requestHeaders.getFirst(SPAN_ID)), operationName, - SpanType.SERVER_INCOMING); + SpanType.SERVER_INCOMING, + Optional.ofNullable(requestHeaders.getFirst(ORIGIN_USER_AGENT))); } private enum DetachedTraceCompletionListener implements ExchangeCompletionListener { diff --git a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java index 066c02678..0b0e2008b 100644 --- a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java +++ b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java @@ -66,8 +66,9 @@ static DetachedSpan start( String traceId, Optional parentSpanId, @Safe String operation, - SpanType type) { - return Tracer.detachInternal(observability, traceId, parentSpanId, operation, type); + SpanType type, + Optional originUserAgent) { + return Tracer.detachInternal(observability, traceId, parentSpanId, operation, type, originUserAgent); } /** diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index 5b92caa37..e3da8d018 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -289,8 +289,8 @@ static DetachedSpan detachInternal(@Safe String operation, SpanType type) { Optional parentSpan = getParentSpanId(maybeCurrentTrace); Optional requestId = getRequestId(maybeCurrentTrace, type); return sampled - ? new SampledDetachedSpan(operation, type, traceId, requestId, parentSpan) - : new UnsampledDetachedSpan(traceId, requestId, Optional.empty()); + ? new SampledDetachedSpan(operation, type, traceId, requestId, parentSpan, Optional.empty()) + : new UnsampledDetachedSpan(traceId, requestId, Optional.empty(), Optional.empty()); } /** @@ -305,7 +305,23 @@ static DetachedSpan detachInternal( SpanType type) { Optional requestId = type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(); - return detachInternal(observability, traceId, requestId, parentSpanId, operation, type); + return detachInternal(observability, traceId, requestId, parentSpanId, operation, type, Optional.empty()); + } + + /** + * Like {@link #startSpan(String, SpanType)}, but does not set or modify tracing thread state. This is an internal + * utility that should not be called directly outside of {@link DetachedSpan}. + */ + static DetachedSpan detachInternal( + Observability observability, + String traceId, + Optional parentSpanId, + @Safe String operation, + SpanType type, + Optional originUserAgent) { + Optional requestId = + type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(); + return detachInternal(observability, traceId, requestId, parentSpanId, operation, type, originUserAgent); } /** @@ -318,12 +334,13 @@ static DetachedSpan detachInternal( Optional requestId, Optional parentSpanId, @Safe String operation, - SpanType type) { + SpanType type, + Optional originUserAgent) { // The current trace has no impact on this function, a new trace is spawned and existing thread state // is not modified. return shouldObserve(observability) - ? new SampledDetachedSpan(operation, type, traceId, requestId, parentSpanId) - : new UnsampledDetachedSpan(traceId, requestId, parentSpanId); + ? new SampledDetachedSpan(operation, type, traceId, requestId, parentSpanId, originUserAgent) + : new UnsampledDetachedSpan(traceId, requestId, parentSpanId, originUserAgent); } /** @@ -343,9 +360,9 @@ static Detached detachInternal() { if (maybeOpenSpan == null) { return NopDetached.INSTANCE; } - return new SampledDetached(traceId, requestId, maybeOpenSpan); + return new SampledDetached(traceId, requestId, maybeOpenSpan, trace.getOriginUserAgent()); } else { - return new UnsampledDetachedSpan(traceId, requestId, Optional.empty()); + return new UnsampledDetachedSpan(traceId, requestId, Optional.empty(), trace.getOriginUserAgent()); } } @@ -423,6 +440,7 @@ private static final class SampledDetachedSpan implements DetachedSpan { private final String traceId; private final Optional requestId; + private final Optional originUserAgent; private final OpenSpan openSpan; private volatile int completed; @@ -434,9 +452,11 @@ private static final class SampledDetachedSpan implements DetachedSpan { SpanType type, String traceId, Optional requestId, - Optional parentSpanId) { + Optional parentSpanId, + Optional originUserAgent) { this.traceId = traceId; this.requestId = requestId; + this.originUserAgent = originUserAgent; this.openSpan = OpenSpan.of(operation, Tracers.randomId(), type, parentSpanId); } @@ -464,7 +484,8 @@ public CloseableSpan childSpan( @Override public DetachedSpan childDetachedSpan(String operation, SpanType type) { - return new SampledDetachedSpan(operation, type, traceId, requestId, Optional.of(openSpan.getSpanId())); + return new SampledDetachedSpan( + operation, type, traceId, requestId, Optional.of(openSpan.getSpanId()), originUserAgent); } @MustBeClosed @@ -520,11 +541,14 @@ private static final class SampledDetached implements Detached { private final String traceId; private final Optional requestId; private final OpenSpan openSpan; + private final Optional originUserAgent; - SampledDetached(String traceId, Optional requestId, OpenSpan openSpan) { + SampledDetached( + String traceId, Optional requestId, OpenSpan openSpan, Optional originUserAgent) { this.traceId = traceId; this.requestId = requestId; this.openSpan = openSpan; + this.originUserAgent = originUserAgent; } @Override @@ -536,7 +560,8 @@ public CloseableSpan childSpan( @Override public DetachedSpan childDetachedSpan(String operation, SpanType type) { - return new SampledDetachedSpan(operation, type, traceId, requestId, Optional.of(openSpan.getSpanId())); + return new SampledDetachedSpan( + operation, type, traceId, requestId, Optional.of(openSpan.getSpanId()), originUserAgent); } @Override @@ -564,11 +589,17 @@ private static final class UnsampledDetachedSpan implements DetachedSpan { private final String traceId; private final Optional requestId; private final Optional parentSpanId; + private final Optional originUserAgent; - UnsampledDetachedSpan(String traceId, Optional requestId, Optional parentSpanId) { + UnsampledDetachedSpan( + String traceId, + Optional requestId, + Optional parentSpanId, + Optional originUserAgent) { this.traceId = traceId; this.requestId = requestId; this.parentSpanId = parentSpanId; + this.originUserAgent = originUserAgent; } @Override diff --git a/tracing/src/test/java/com/palantir/tracing/TracerTest.java b/tracing/src/test/java/com/palantir/tracing/TracerTest.java index 7a38d25fa..866eeaf6d 100644 --- a/tracing/src/test/java/com/palantir/tracing/TracerTest.java +++ b/tracing/src/test/java/com/palantir/tracing/TracerTest.java @@ -647,8 +647,8 @@ public void testDetachedTraceBuildsUponExistingTrace() { public void testNewDetachedTrace() { try (CloseableTracer ignored = CloseableTracer.startSpan("test")) { String currentTraceId = Tracer.getTraceId(); - DetachedSpan span = - DetachedSpan.start(Observability.SAMPLE, "12345", Optional.empty(), "op", SpanType.LOCAL); + DetachedSpan span = DetachedSpan.start( + Observability.SAMPLE, "12345", Optional.empty(), "op", SpanType.LOCAL, Optional.empty()); try (CloseableSpan ignored2 = span.completeAndStartChild("foo")) { assertThat(Tracer.getTraceId()).isEqualTo("12345"); } @@ -662,8 +662,8 @@ public void testNewDetachedTrace() { public void testNewDetachedTrace_doesNotModifyCurrentState() { try (CloseableTracer ignored = CloseableTracer.startSpan("test")) { String currentTraceId = Tracer.getTraceId(); - DetachedSpan span = - DetachedSpan.start(Observability.SAMPLE, "12345", Optional.empty(), "op", SpanType.LOCAL); + DetachedSpan span = DetachedSpan.start( + Observability.SAMPLE, "12345", Optional.empty(), "op", SpanType.LOCAL, Optional.empty()); assertThat(Tracer.getTraceId()) .as("Current thread state should not be modified") .isEqualTo(currentTraceId); From a2aaaa6e6c9dcb031a7ca379e07728d631d6493e Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Tue, 21 Sep 2021 16:04:28 +0100 Subject: [PATCH 07/11] Remove unused --- .../main/java/com/palantir/tracing/Tracer.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index e3da8d018..6b4065b90 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -293,21 +293,6 @@ static DetachedSpan detachInternal(@Safe String operation, SpanType type) { : new UnsampledDetachedSpan(traceId, requestId, Optional.empty(), Optional.empty()); } - /** - * Like {@link #startSpan(String, SpanType)}, but does not set or modify tracing thread state. This is an internal - * utility that should not be called directly outside of {@link DetachedSpan}. - */ - static DetachedSpan detachInternal( - Observability observability, - String traceId, - Optional parentSpanId, - @Safe String operation, - SpanType type) { - Optional requestId = - type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(); - return detachInternal(observability, traceId, requestId, parentSpanId, operation, type, Optional.empty()); - } - /** * Like {@link #startSpan(String, SpanType)}, but does not set or modify tracing thread state. This is an internal * utility that should not be called directly outside of {@link DetachedSpan}. From 3a52170c6b170187d16db93816dd0774cdf4e5fd Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Tue, 21 Sep 2021 18:36:10 +0100 Subject: [PATCH 08/11] Propagate origin user agent with attached --- .../com/palantir/tracing/DeferredTracer.java | 8 ++++- .../java/com/palantir/tracing/Tracer.java | 32 +++++++++++++------ .../java/com/palantir/tracing/Tracers.java | 5 +++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java b/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java index 44fdd65fa..9886d6132 100644 --- a/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java +++ b/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java @@ -74,6 +74,9 @@ public final class DeferredTracer implements Serializable { @Nullable private final transient String requestId; + @Nullable + private final String originUserAgent; + /** * Deprecated. * @@ -104,6 +107,7 @@ public DeferredTracer(@Safe String operation, @Safe Map metadata this.parentSpanId = trace.top().map(OpenSpan::getSpanId).orElse(null); this.operation = operation; this.metadata = metadata; + this.originUserAgent = trace.getOriginUserAgent().orElse(null); } else { this.traceId = null; this.requestId = null; @@ -111,6 +115,7 @@ public DeferredTracer(@Safe String operation, @Safe Map metadata this.parentSpanId = null; this.operation = null; this.metadata = null; + this.originUserAgent = null; } } @@ -134,7 +139,8 @@ CloseableTrace withTrace() { Optional originalTrace = Tracer.getAndClearTraceIfPresent(); - Tracer.setTrace(Trace.of(isObservable, traceId, Optional.ofNullable(requestId))); + Tracer.setTrace( + Trace.of(isObservable, traceId, Optional.ofNullable(requestId), Optional.ofNullable(originUserAgent))); if (parentSpanId != null) { Tracer.fastStartSpan(operation, parentSpanId, SpanType.LOCAL); } else { diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index 6b4065b90..f8b68b332 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -453,9 +453,10 @@ private static CloseableSpan childSpan( String operationName, TagTranslator translator, T data, - SpanType type) { + SpanType type, + Optional originUserAgent) { Trace maybeCurrentTrace = currentTrace.get(); - setTrace(Trace.of(true, traceId, requestId)); + setTrace(Trace.of(true, traceId, requestId, originUserAgent)); Tracer.fastStartSpan(operationName, openSpan.getSpanId(), type); return TraceRestoringCloseableSpanWithMetadata.of(maybeCurrentTrace, translator, data); } @@ -464,7 +465,7 @@ private static CloseableSpan childSpan( @MustBeClosed public CloseableSpan childSpan( String operationName, TagTranslator translator, T data, SpanType type) { - return childSpan(traceId, openSpan, requestId, operationName, translator, data, type); + return childSpan(traceId, openSpan, requestId, operationName, translator, data, type, originUserAgent); } @Override @@ -474,9 +475,10 @@ public DetachedSpan childDetachedSpan(String operation, SpanType type) { } @MustBeClosed - private static CloseableSpan attach(String traceId, OpenSpan openSpan, Optional requestId) { + private static CloseableSpan attach( + String traceId, OpenSpan openSpan, Optional requestId, Optional originUserAgent) { Trace maybeCurrentTrace = currentTrace.get(); - Trace newTrace = Trace.of(true, traceId, requestId); + Trace newTrace = Trace.of(true, traceId, requestId, originUserAgent); // 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. @@ -490,7 +492,7 @@ private static CloseableSpan attach(String traceId, OpenSpan openSpan, Optional< @Override @MustBeClosed public CloseableSpan attach() { - return attach(traceId, openSpan, requestId); + return attach(traceId, openSpan, requestId, originUserAgent); } @Override @@ -540,7 +542,8 @@ private static final class SampledDetached implements Detached { @MustBeClosed public CloseableSpan childSpan( String operationName, TagTranslator translator, T data, SpanType type) { - return SampledDetachedSpan.childSpan(traceId, openSpan, requestId, operationName, translator, data, type); + return SampledDetachedSpan.childSpan( + traceId, openSpan, requestId, operationName, translator, data, type, originUserAgent); } @Override @@ -552,7 +555,7 @@ public DetachedSpan childDetachedSpan(String operation, SpanType type) { @Override @MustBeClosed public CloseableSpan attach() { - return SampledDetachedSpan.attach(traceId, openSpan, requestId); + return SampledDetachedSpan.attach(traceId, openSpan, requestId, originUserAgent); } @Override @@ -591,7 +594,7 @@ private static final class UnsampledDetachedSpan implements DetachedSpan { public CloseableSpan childSpan( String operationName, TagTranslator _translator, T _data, SpanType type) { Trace maybeCurrentTrace = currentTrace.get(); - setTrace(Trace.of(false, traceId, requestId)); + setTrace(Trace.of(false, traceId, requestId, originUserAgent)); if (parentSpanId.isPresent()) { Tracer.fastStartSpan(operationName, parentSpanId.get(), type); } else { @@ -896,10 +899,20 @@ static void setTrace(Trace trace) { MDC.put(Tracers.TRACE_ID_KEY, trace.getTraceId()); setTraceSampledMdcIfObservable(trace.isObservable()); setTraceRequestId(trace.getRequestId()); + setOriginUserAgent(trace.getOriginUserAgent()); logSettingTrace(); } + private static void setOriginUserAgent(Optional originUserAgent) { + if (originUserAgent.isPresent()) { + MDC.put(Tracers.ORIGIN_USER_AGENT_KEY, originUserAgent.get()); + } else { + // Ensure MDC state is cleared when there is no origin user header + MDC.remove(Tracers.ORIGIN_USER_AGENT_KEY); + } + } + private static void setTraceSampledMdcIfObservable(boolean observable) { if (observable) { // Set to 1 to be consistent with values associated with http header key TraceHttpHeaders.IS_SAMPLED @@ -939,6 +952,7 @@ static void clearCurrentTrace() { MDC.remove(Tracers.TRACE_ID_KEY); MDC.remove(Tracers.TRACE_SAMPLED_KEY); MDC.remove(Tracers.REQUEST_ID_KEY); + MDC.remove(Tracers.ORIGIN_USER_AGENT_KEY); } private static void logClearingTrace() { diff --git a/tracing/src/main/java/com/palantir/tracing/Tracers.java b/tracing/src/main/java/com/palantir/tracing/Tracers.java index 97d10115d..522d0ee3a 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracers.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracers.java @@ -48,6 +48,11 @@ public final class Tracers { */ public static final String REQUEST_ID_KEY = "_requestId"; + /** + * The Key under which origin user agents are inserted into SLF4J {@link org.slf4j.MDC MDCs}. + */ + public static final String ORIGIN_USER_AGENT_KEY = "__originUserAgent"; + private static final String DEFAULT_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' From 54a32cf77f884a56aaf564fbeb39ab1c3eed8f1b Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Wed, 22 Sep 2021 15:49:32 +0100 Subject: [PATCH 09/11] Rename for user agent --- .../tracing/api/TraceHttpHeaders.java | 2 +- .../tracing/jersey/TraceEnrichingFilter.java | 21 +++-- .../tracing/OkhttpTraceInterceptor2.java | 6 +- .../okhttp3/OkhttpTraceInterceptor.java | 5 +- .../okhttp3/OkhttpTraceInterceptorTest.java | 13 +-- .../tracing/undertow/UndertowTracing.java | 4 +- .../com/palantir/tracing/DeferredTracer.java | 8 +- .../com/palantir/tracing/DetachedSpan.java | 4 +- .../main/java/com/palantir/tracing/Trace.java | 40 ++++----- .../com/palantir/tracing/TraceMetadata.java | 4 +- .../java/com/palantir/tracing/Tracer.java | 87 +++++++++---------- .../java/com/palantir/tracing/Tracers.java | 2 +- 12 files changed, 92 insertions(+), 104 deletions(-) diff --git a/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java b/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java index d4aa446ba..40c5eb01a 100644 --- a/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java +++ b/tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java @@ -38,5 +38,5 @@ public interface TraceHttpHeaders { @Deprecated String ORIGINATING_SPAN_ID = "X-OrigSpanId"; - String ORIGIN_USER_AGENT = "X-Origin-User-Agent"; + String FOR_USER_AGENT = "X-For-User-Agent"; } diff --git a/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java b/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java index 4088a9d36..dc7d89cc7 100644 --- a/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java +++ b/tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java @@ -54,7 +54,7 @@ public final class TraceEnrichingFilter implements ContainerRequestFilter, Conta public static final String REQUEST_ID_PROPERTY_NAME = "com.palantir.tracing.requestId"; - public static final String ORIGIN_USER_AGENT_PROPERTY_NAME = "com.palantir.tracing.originUserAgent"; + public static final String FOR_USER_AGENT_PROPERTY_NAME = "com.palantir.tracing.forUserAgent"; public static final String SAMPLED_PROPERTY_NAME = "com.palantir.tracing.sampled"; @@ -80,14 +80,14 @@ public void filter(ContainerRequestContext requestContext) throws IOException { Tracers.randomId(), operation, SpanType.SERVER_INCOMING, - getOriginUserAgentFromHeader(requestContext)); + getForUserAgentFromHeader(requestContext)); } else if (spanId == null) { Tracer.initTraceWithSpan( getObservabilityFromHeader(requestContext), traceId, operation, SpanType.SERVER_INCOMING, - getOriginUserAgentFromHeader(requestContext)); + getForUserAgentFromHeader(requestContext)); } else { // caller's span is this span's parent. Tracer.initTraceWithSpan( @@ -96,7 +96,7 @@ public void filter(ContainerRequestContext requestContext) throws IOException { operation, spanId, SpanType.SERVER_INCOMING, - getOriginUserAgentFromHeader(requestContext)); + getForUserAgentFromHeader(requestContext)); } // Give asynchronous downstream handlers access to the trace id @@ -107,9 +107,8 @@ public void filter(ContainerRequestContext requestContext) throws IOException { .flatMap(TraceMetadata::getRequestId) .ifPresent(requestId -> requestContext.setProperty(REQUEST_ID_PROPERTY_NAME, requestId)); traceMetadata - .flatMap(TraceMetadata::getOriginUserAgent) - .ifPresent(originUserAgent -> - requestContext.setProperty(ORIGIN_USER_AGENT_PROPERTY_NAME, originUserAgent)); + .flatMap(TraceMetadata::getForUserAgent) + .ifPresent(forUserAgent -> requestContext.setProperty(FOR_USER_AGENT_PROPERTY_NAME, forUserAgent)); } // Handles outgoing response @@ -149,12 +148,12 @@ private static Observability getObservabilityFromHeader(ContainerRequestContext } } - private static Optional getOriginUserAgentFromHeader(ContainerRequestContext requestContext) { - String originUserAgent = requestContext.getHeaderString(TraceHttpHeaders.ORIGIN_USER_AGENT); - if (originUserAgent == null) { + private static Optional getForUserAgentFromHeader(ContainerRequestContext requestContext) { + String forUserAgent = requestContext.getHeaderString(TraceHttpHeaders.FOR_USER_AGENT); + if (forUserAgent == null) { return Optional.ofNullable(requestContext.getHeaderString(HttpHeaders.USER_AGENT)); } - return Optional.of(originUserAgent); + return Optional.of(forUserAgent); } private String getPathTemplate() { diff --git a/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java b/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java index 3aceb59fc..ba2cdfb89 100644 --- a/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java +++ b/tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java @@ -51,10 +51,10 @@ public Response intercept(Chain chain) throws IOException { .header(TraceHttpHeaders.TRACE_ID, Tracer.getTraceId()) .header(TraceHttpHeaders.SPAN_ID, metadata.getSpanId()) .header(TraceHttpHeaders.IS_SAMPLED, Tracer.isTraceObservable() ? "1" : "0"); - if (metadata.getOriginUserAgent().isPresent()) { + if (metadata.getForUserAgent().isPresent()) { tracedRequest.header( - TraceHttpHeaders.ORIGIN_USER_AGENT, - metadata.getOriginUserAgent().get()); + TraceHttpHeaders.FOR_USER_AGENT, + metadata.getForUserAgent().get()); } return chain.proceed(tracedRequest.build()); diff --git a/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java b/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java index 2360d14ea..53d7bb5e8 100644 --- a/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java +++ b/tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java @@ -53,10 +53,9 @@ public Response intercept(Chain chain) throws IOException { .header(TraceHttpHeaders.TRACE_ID, Tracer.getTraceId()) .header(TraceHttpHeaders.SPAN_ID, span.getSpanId()) .header(TraceHttpHeaders.IS_SAMPLED, Tracer.isTraceObservable() ? "1" : "0"); - if (Tracer.getOriginUserAgent().isPresent()) { + if (Tracer.getForUserAgent().isPresent()) { tracedRequest.header( - TraceHttpHeaders.ORIGIN_USER_AGENT, - Tracer.getOriginUserAgent().get()); + TraceHttpHeaders.FOR_USER_AGENT, Tracer.getForUserAgent().get()); } Response response; diff --git a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java index ff8efc424..5fd681435 100644 --- a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java +++ b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java @@ -106,15 +106,10 @@ public void testPopulatesNewTrace_whenParentTraceIsPresent() throws IOException } @Test - public void testPopulatesNewTrace_whenOriginUserAgentIsPresent() throws IOException { - String originUserAgent = "originUserAgent"; + public void testPopulatesNewTrace_whenForUserAgentIsPresent() throws IOException { + String forUserAgent = "forUserAgent"; Tracer.initTraceWithSpan( - Observability.SAMPLE, - "id", - "operation", - "parent", - SpanType.SERVER_INCOMING, - Optional.of(originUserAgent)); + Observability.SAMPLE, "id", "operation", "parent", SpanType.SERVER_INCOMING, Optional.of(forUserAgent)); String traceId = Tracer.getTraceId(); try { OkhttpTraceInterceptor.INSTANCE.intercept(chain); @@ -129,7 +124,7 @@ public void testPopulatesNewTrace_whenOriginUserAgentIsPresent() throws IOExcept Request intercepted = requestCaptor.getValue(); assertThat(intercepted.headers(TraceHttpHeaders.SPAN_ID)).isNotNull(); assertThat(intercepted.headers(TraceHttpHeaders.TRACE_ID)).containsOnly(traceId); - assertThat(intercepted.headers(TraceHttpHeaders.ORIGIN_USER_AGENT)).containsOnly(originUserAgent); + assertThat(intercepted.headers(TraceHttpHeaders.FOR_USER_AGENT)).containsOnly(forUserAgent); } @Test diff --git a/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java b/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java index 72783e307..b23d5e8c3 100644 --- a/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java +++ b/tracing-undertow/src/main/java/com/palantir/tracing/undertow/UndertowTracing.java @@ -48,7 +48,7 @@ final class UndertowTracing { private static final HttpString TRACE_ID = HttpString.tryFromString(TraceHttpHeaders.TRACE_ID); private static final HttpString SPAN_ID = HttpString.tryFromString(TraceHttpHeaders.SPAN_ID); private static final HttpString IS_SAMPLED = HttpString.tryFromString(TraceHttpHeaders.IS_SAMPLED); - private static final HttpString ORIGIN_USER_AGENT = HttpString.tryFromString(TraceHttpHeaders.ORIGIN_USER_AGENT); + private static final HttpString FOR_USER_AGENT = HttpString.tryFromString(TraceHttpHeaders.FOR_USER_AGENT); // Consider moving this to TracingAttachments and making it public. For now it's well encapsulated // here because we expect the two handler implementations to be sufficient. @@ -110,7 +110,7 @@ private static DetachedSpan detachedSpan( newTrace ? Optional.empty() : Optional.ofNullable(requestHeaders.getFirst(SPAN_ID)), operationName, SpanType.SERVER_INCOMING, - Optional.ofNullable(requestHeaders.getFirst(ORIGIN_USER_AGENT))); + Optional.ofNullable(requestHeaders.getFirst(FOR_USER_AGENT))); } private enum DetachedTraceCompletionListener implements ExchangeCompletionListener { diff --git a/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java b/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java index 9886d6132..0eb0de32b 100644 --- a/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java +++ b/tracing/src/main/java/com/palantir/tracing/DeferredTracer.java @@ -75,7 +75,7 @@ public final class DeferredTracer implements Serializable { private final transient String requestId; @Nullable - private final String originUserAgent; + private final String forUserAgent; /** * Deprecated. @@ -107,7 +107,7 @@ public DeferredTracer(@Safe String operation, @Safe Map metadata this.parentSpanId = trace.top().map(OpenSpan::getSpanId).orElse(null); this.operation = operation; this.metadata = metadata; - this.originUserAgent = trace.getOriginUserAgent().orElse(null); + this.forUserAgent = trace.getForUserAgent().orElse(null); } else { this.traceId = null; this.requestId = null; @@ -115,7 +115,7 @@ public DeferredTracer(@Safe String operation, @Safe Map metadata this.parentSpanId = null; this.operation = null; this.metadata = null; - this.originUserAgent = null; + this.forUserAgent = null; } } @@ -140,7 +140,7 @@ CloseableTrace withTrace() { Optional originalTrace = Tracer.getAndClearTraceIfPresent(); Tracer.setTrace( - Trace.of(isObservable, traceId, Optional.ofNullable(requestId), Optional.ofNullable(originUserAgent))); + Trace.of(isObservable, traceId, Optional.ofNullable(requestId), Optional.ofNullable(forUserAgent))); if (parentSpanId != null) { Tracer.fastStartSpan(operation, parentSpanId, SpanType.LOCAL); } else { diff --git a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java index 0b0e2008b..7967d6f95 100644 --- a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java +++ b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java @@ -67,8 +67,8 @@ static DetachedSpan start( Optional parentSpanId, @Safe String operation, SpanType type, - Optional originUserAgent) { - return Tracer.detachInternal(observability, traceId, parentSpanId, operation, type, originUserAgent); + Optional forUserAgent) { + return Tracer.detachInternal(observability, traceId, parentSpanId, operation, type, forUserAgent); } /** diff --git a/tracing/src/main/java/com/palantir/tracing/Trace.java b/tracing/src/main/java/com/palantir/tracing/Trace.java index bd7f32401..fc7d0a54b 100644 --- a/tracing/src/main/java/com/palantir/tracing/Trace.java +++ b/tracing/src/main/java/com/palantir/tracing/Trace.java @@ -48,13 +48,13 @@ public abstract class Trace { private final Optional requestId; - private final Optional originUserAgent; + private final Optional forUserAgent; - private Trace(String traceId, Optional requestId, Optional originUserAgent) { - this.originUserAgent = originUserAgent; + private Trace(String traceId, Optional requestId, Optional forUserAgent) { checkArgument(!Strings.isNullOrEmpty(traceId), "traceId must be non-empty"); this.traceId = traceId; this.requestId = checkNotNull(requestId, "requestId"); + this.forUserAgent = forUserAgent; } /** @@ -130,8 +130,8 @@ final Optional getRequestId() { return requestId; } - final Optional getOriginUserAgent() { - return originUserAgent; + final Optional getForUserAgent() { + return forUserAgent; } /** Returns a copy of this Trace which can be independently mutated. */ @@ -143,11 +143,10 @@ static Trace of(boolean isObservable, String traceId, Optional requestId : new Unsampled(traceId, requestId, Optional.empty()); } - static Trace of( - boolean isObservable, String traceId, Optional requestId, Optional originUserAgent) { + static Trace of(boolean isObservable, String traceId, Optional requestId, Optional forUserAgent) { return isObservable - ? new Sampled(traceId, requestId, originUserAgent) - : new Unsampled(traceId, requestId, originUserAgent); + ? new Sampled(traceId, requestId, forUserAgent) + : new Unsampled(traceId, requestId, forUserAgent); } private static final class Sampled extends Trace { @@ -155,16 +154,13 @@ private static final class Sampled extends Trace { private final Deque stack; private Sampled( - ArrayDeque stack, - String traceId, - Optional requestId, - Optional originUserAgent) { - super(traceId, requestId, originUserAgent); + ArrayDeque stack, String traceId, Optional requestId, Optional forUserAgent) { + super(traceId, requestId, forUserAgent); this.stack = stack; } - private Sampled(String traceId, Optional requestId, Optional originUserAgent) { - this(new ArrayDeque<>(), traceId, requestId, originUserAgent); + private Sampled(String traceId, Optional requestId, Optional forUserAgent) { + this(new ArrayDeque<>(), traceId, requestId, forUserAgent); } @Override @@ -206,7 +202,7 @@ boolean isObservable() { @Override Trace deepCopy() { - return new Sampled(new ArrayDeque<>(stack), getTraceId(), getRequestId(), getOriginUserAgent()); + return new Sampled(new ArrayDeque<>(stack), getTraceId(), getRequestId(), getForUserAgent()); } @Override @@ -223,14 +219,14 @@ private static final class Unsampled extends Trace { private int numberOfSpans; private Unsampled( - int numberOfSpans, String traceId, Optional requestId, Optional originUserAgent) { - super(traceId, requestId, originUserAgent); + int numberOfSpans, String traceId, Optional requestId, Optional forUserAgent) { + super(traceId, requestId, forUserAgent); this.numberOfSpans = numberOfSpans; validateNumberOfSpans(); } - private Unsampled(String traceId, Optional requestId, Optional originUserAgent) { - this(0, traceId, requestId, originUserAgent); + private Unsampled(String traceId, Optional requestId, Optional forUserAgent) { + this(0, traceId, requestId, forUserAgent); } @Override @@ -275,7 +271,7 @@ boolean isObservable() { @Override Trace deepCopy() { - return new Unsampled(numberOfSpans, getTraceId(), getRequestId(), getOriginUserAgent()); + return new Unsampled(numberOfSpans, getTraceId(), getRequestId(), getForUserAgent()); } /** Internal validation, this should never fail because {@link #pop()} only decrements positive values. */ diff --git a/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java b/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java index ba90f01d5..f661612c1 100644 --- a/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java +++ b/tracing/src/main/java/com/palantir/tracing/TraceMetadata.java @@ -50,8 +50,8 @@ default Optional getOriginatingSpanId() { return Optional.empty(); } - /** Corresponds to {@link com.palantir.tracing.api.TraceHttpHeaders#ORIGIN_USER_AGENT}. */ - Optional getOriginUserAgent(); + /** Corresponds to {@link com.palantir.tracing.api.TraceHttpHeaders#FOR_USER_AGENT}. */ + Optional getForUserAgent(); static Builder builder() { return new Builder(); diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java index f8b68b332..8aa022775 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracer.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java @@ -72,10 +72,10 @@ private Tracer() {} * Creates a new trace, but does not set it as the current trace. */ private static Trace createTrace( - Observability observability, String traceId, Optional requestId, Optional originUserAgent) { + Observability observability, String traceId, Optional requestId, Optional forUserAgent) { checkArgument(!Strings.isNullOrEmpty(traceId), "traceId must be non-empty"); boolean observable = shouldObserve(observability); - return Trace.of(observable, traceId, requestId, originUserAgent); + return Trace.of(observable, traceId, requestId, forUserAgent); } /** @@ -127,7 +127,7 @@ public static Optional maybeGetTraceMetadata() { return trace.top().map(openSpan -> TraceMetadata.builder() .spanId(openSpan.getSpanId()) .parentSpanId(openSpan.getParentSpanId()) - .originUserAgent(trace.getOriginUserAgent()) + .forUserAgent(trace.getForUserAgent()) .traceId(trace.getTraceId()) .requestId(trace.getRequestId()) .build()); @@ -135,7 +135,7 @@ public static Optional maybeGetTraceMetadata() { return Optional.of(TraceMetadata.builder() .spanId(Tracers.randomId()) .parentSpanId(Optional.empty()) - .originUserAgent(trace.getOriginUserAgent()) + .forUserAgent(trace.getForUserAgent()) .traceId(trace.getTraceId()) .requestId(trace.getRequestId()) .build()); @@ -176,12 +176,12 @@ public static void initTraceWithSpan( @Safe String operation, String parentSpanId, SpanType type, - Optional originUserAgent) { + Optional forUserAgent) { setTrace(createTrace( observability, traceId, type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(), - originUserAgent)); + forUserAgent)); fastStartSpan(operation, parentSpanId, type); } @@ -220,12 +220,12 @@ public static void initTraceWithSpan( String traceId, @Safe String operation, SpanType type, - Optional originUserAgent) { + Optional forUserAgent) { setTrace(createTrace( observability, traceId, type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(), - originUserAgent)); + forUserAgent)); fastStartSpan(operation, type); } @@ -303,10 +303,10 @@ static DetachedSpan detachInternal( Optional parentSpanId, @Safe String operation, SpanType type, - Optional originUserAgent) { + Optional forUserAgent) { Optional requestId = type == SpanType.SERVER_INCOMING ? Optional.of(Tracers.randomId()) : Optional.empty(); - return detachInternal(observability, traceId, requestId, parentSpanId, operation, type, originUserAgent); + return detachInternal(observability, traceId, requestId, parentSpanId, operation, type, forUserAgent); } /** @@ -320,12 +320,12 @@ static DetachedSpan detachInternal( Optional parentSpanId, @Safe String operation, SpanType type, - Optional originUserAgent) { + Optional forUserAgent) { // The current trace has no impact on this function, a new trace is spawned and existing thread state // is not modified. return shouldObserve(observability) - ? new SampledDetachedSpan(operation, type, traceId, requestId, parentSpanId, originUserAgent) - : new UnsampledDetachedSpan(traceId, requestId, parentSpanId, originUserAgent); + ? new SampledDetachedSpan(operation, type, traceId, requestId, parentSpanId, forUserAgent) + : new UnsampledDetachedSpan(traceId, requestId, parentSpanId, forUserAgent); } /** @@ -345,9 +345,9 @@ static Detached detachInternal() { if (maybeOpenSpan == null) { return NopDetached.INSTANCE; } - return new SampledDetached(traceId, requestId, maybeOpenSpan, trace.getOriginUserAgent()); + return new SampledDetached(traceId, requestId, maybeOpenSpan, trace.getForUserAgent()); } else { - return new UnsampledDetachedSpan(traceId, requestId, Optional.empty(), trace.getOriginUserAgent()); + return new UnsampledDetachedSpan(traceId, requestId, Optional.empty(), trace.getForUserAgent()); } } @@ -425,7 +425,7 @@ private static final class SampledDetachedSpan implements DetachedSpan { private final String traceId; private final Optional requestId; - private final Optional originUserAgent; + private final Optional forUserAgent; private final OpenSpan openSpan; private volatile int completed; @@ -438,10 +438,10 @@ private static final class SampledDetachedSpan implements DetachedSpan { String traceId, Optional requestId, Optional parentSpanId, - Optional originUserAgent) { + Optional forUserAgent) { this.traceId = traceId; this.requestId = requestId; - this.originUserAgent = originUserAgent; + this.forUserAgent = forUserAgent; this.openSpan = OpenSpan.of(operation, Tracers.randomId(), type, parentSpanId); } @@ -454,9 +454,9 @@ private static CloseableSpan childSpan( TagTranslator translator, T data, SpanType type, - Optional originUserAgent) { + Optional forUserAgent) { Trace maybeCurrentTrace = currentTrace.get(); - setTrace(Trace.of(true, traceId, requestId, originUserAgent)); + setTrace(Trace.of(true, traceId, requestId, forUserAgent)); Tracer.fastStartSpan(operationName, openSpan.getSpanId(), type); return TraceRestoringCloseableSpanWithMetadata.of(maybeCurrentTrace, translator, data); } @@ -465,20 +465,20 @@ private static CloseableSpan childSpan( @MustBeClosed public CloseableSpan childSpan( String operationName, TagTranslator translator, T data, SpanType type) { - return childSpan(traceId, openSpan, requestId, operationName, translator, data, type, originUserAgent); + return childSpan(traceId, openSpan, requestId, operationName, translator, data, type, forUserAgent); } @Override public DetachedSpan childDetachedSpan(String operation, SpanType type) { return new SampledDetachedSpan( - operation, type, traceId, requestId, Optional.of(openSpan.getSpanId()), originUserAgent); + operation, type, traceId, requestId, Optional.of(openSpan.getSpanId()), forUserAgent); } @MustBeClosed private static CloseableSpan attach( - String traceId, OpenSpan openSpan, Optional requestId, Optional originUserAgent) { + String traceId, OpenSpan openSpan, Optional requestId, Optional forUserAgent) { Trace maybeCurrentTrace = currentTrace.get(); - Trace newTrace = Trace.of(true, traceId, requestId, originUserAgent); + Trace newTrace = Trace.of(true, traceId, requestId, forUserAgent); // 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. @@ -492,7 +492,7 @@ private static CloseableSpan attach( @Override @MustBeClosed public CloseableSpan attach() { - return attach(traceId, openSpan, requestId, originUserAgent); + return attach(traceId, openSpan, requestId, forUserAgent); } @Override @@ -528,14 +528,13 @@ private static final class SampledDetached implements Detached { private final String traceId; private final Optional requestId; private final OpenSpan openSpan; - private final Optional originUserAgent; + private final Optional forUserAgent; - SampledDetached( - String traceId, Optional requestId, OpenSpan openSpan, Optional originUserAgent) { + SampledDetached(String traceId, Optional requestId, OpenSpan openSpan, Optional forUserAgent) { this.traceId = traceId; this.requestId = requestId; this.openSpan = openSpan; - this.originUserAgent = originUserAgent; + this.forUserAgent = forUserAgent; } @Override @@ -543,19 +542,19 @@ private static final class SampledDetached implements Detached { public CloseableSpan childSpan( String operationName, TagTranslator translator, T data, SpanType type) { return SampledDetachedSpan.childSpan( - traceId, openSpan, requestId, operationName, translator, data, type, originUserAgent); + traceId, openSpan, requestId, operationName, translator, data, type, forUserAgent); } @Override public DetachedSpan childDetachedSpan(String operation, SpanType type) { return new SampledDetachedSpan( - operation, type, traceId, requestId, Optional.of(openSpan.getSpanId()), originUserAgent); + operation, type, traceId, requestId, Optional.of(openSpan.getSpanId()), forUserAgent); } @Override @MustBeClosed public CloseableSpan attach() { - return SampledDetachedSpan.attach(traceId, openSpan, requestId, originUserAgent); + return SampledDetachedSpan.attach(traceId, openSpan, requestId, forUserAgent); } @Override @@ -577,24 +576,24 @@ private static final class UnsampledDetachedSpan implements DetachedSpan { private final String traceId; private final Optional requestId; private final Optional parentSpanId; - private final Optional originUserAgent; + private final Optional forUserAgent; UnsampledDetachedSpan( String traceId, Optional requestId, Optional parentSpanId, - Optional originUserAgent) { + Optional forUserAgent) { this.traceId = traceId; this.requestId = requestId; this.parentSpanId = parentSpanId; - this.originUserAgent = originUserAgent; + this.forUserAgent = forUserAgent; } @Override public CloseableSpan childSpan( String operationName, TagTranslator _translator, T _data, SpanType type) { Trace maybeCurrentTrace = currentTrace.get(); - setTrace(Trace.of(false, traceId, requestId, originUserAgent)); + setTrace(Trace.of(false, traceId, requestId, forUserAgent)); if (parentSpanId.isPresent()) { Tracer.fastStartSpan(operationName, parentSpanId.get(), type); } else { @@ -837,8 +836,8 @@ public static String getTraceId() { return checkNotNull(currentTrace.get(), "There is no trace").getTraceId(); } - public static Optional getOriginUserAgent() { - return currentTrace.get().getOriginUserAgent(); + public static Optional getForUserAgent() { + return currentTrace.get().getForUserAgent(); } /** @@ -899,17 +898,17 @@ static void setTrace(Trace trace) { MDC.put(Tracers.TRACE_ID_KEY, trace.getTraceId()); setTraceSampledMdcIfObservable(trace.isObservable()); setTraceRequestId(trace.getRequestId()); - setOriginUserAgent(trace.getOriginUserAgent()); + setForUserAgent(trace.getForUserAgent()); logSettingTrace(); } - private static void setOriginUserAgent(Optional originUserAgent) { - if (originUserAgent.isPresent()) { - MDC.put(Tracers.ORIGIN_USER_AGENT_KEY, originUserAgent.get()); + private static void setForUserAgent(Optional forUserAgent) { + if (forUserAgent.isPresent()) { + MDC.put(Tracers.FOR_USER_AGENT_KEY, forUserAgent.get()); } else { // Ensure MDC state is cleared when there is no origin user header - MDC.remove(Tracers.ORIGIN_USER_AGENT_KEY); + MDC.remove(Tracers.FOR_USER_AGENT_KEY); } } @@ -952,7 +951,7 @@ static void clearCurrentTrace() { MDC.remove(Tracers.TRACE_ID_KEY); MDC.remove(Tracers.TRACE_SAMPLED_KEY); MDC.remove(Tracers.REQUEST_ID_KEY); - MDC.remove(Tracers.ORIGIN_USER_AGENT_KEY); + MDC.remove(Tracers.FOR_USER_AGENT_KEY); } private static void logClearingTrace() { diff --git a/tracing/src/main/java/com/palantir/tracing/Tracers.java b/tracing/src/main/java/com/palantir/tracing/Tracers.java index 522d0ee3a..690895bb9 100644 --- a/tracing/src/main/java/com/palantir/tracing/Tracers.java +++ b/tracing/src/main/java/com/palantir/tracing/Tracers.java @@ -51,7 +51,7 @@ public final class Tracers { /** * The Key under which origin user agents are inserted into SLF4J {@link org.slf4j.MDC MDCs}. */ - public static final String ORIGIN_USER_AGENT_KEY = "__originUserAgent"; + public static final String FOR_USER_AGENT_KEY = "__forUserAgent"; private static final String DEFAULT_ROOT_SPAN_OPERATION = "root"; private static final char[] HEX_DIGITS = { From ff640bd3e069991b96c2d6ad2bb9621551262bcb Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Mon, 27 Sep 2021 14:29:33 +0100 Subject: [PATCH 10/11] Prevent API breaks --- .../java/com/palantir/tracing/DetachedSpan.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java index 7967d6f95..5e11ed20f 100644 --- a/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java +++ b/tracing/src/main/java/com/palantir/tracing/DetachedSpan.java @@ -55,6 +55,21 @@ static DetachedSpan start(@Safe String operation, SpanType type) { return Tracer.detachInternal(operation, type); } + /** + * Marks the beginning of a span, which you can {@link #complete} on any other thread. + * + * @see DetachedSpan#start(String) + */ + @CheckReturnValue + static DetachedSpan start( + Observability observability, + String traceId, + Optional parentSpanId, + @Safe String operation, + SpanType type) { + return Tracer.detachInternal(observability, traceId, parentSpanId, operation, type, Optional.empty()); + } + /** * Marks the beginning of a span, which you can {@link #complete} on any other thread. * From 07f1e2e8ce97c73a32b49b36a2786aa5dbe1f6d6 Mon Sep 17 00:00:00 2001 From: Stevan Ognjanovic Date: Mon, 27 Sep 2021 14:44:03 +0100 Subject: [PATCH 11/11] Break --- .palantir/revapi.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml index f1212ff3a..60f43ad45 100644 --- a/.palantir/revapi.yml +++ b/.palantir/revapi.yml @@ -57,3 +57,8 @@ acceptedBreaks: old: "method com.palantir.tracing.api.OpenSpan.Builder com.palantir.tracing.api.ImmutableOpenSpan.Builder::originatingSpanId(java.util.Optional)\ \ @ com.palantir.tracing.api.OpenSpan.Builder" justification: "Type is not meant for external creation" + "6.4.0": + com.palantir.tracing:tracing: + - code: "java.method.addedToInterface" + new: "method java.util.Optional com.palantir.tracing.TraceMetadata::getForUserAgent()" + justification: "Adding a method will not break anyone"