Introduce topSpanId to disambiguate incoming requests that share traceid

changelog/@unreleased/pr-352.v2.yml

@@ -0,0 +1,7 @@
+type: feature
+feature:
+  description: Introduce a new property on a Trace, topSpanId, that can be read to
+    disambiguate span stacks that belong to the same traceid but had different entry
+    points.
+  links:
+  - https://github.com/palantir/tracing-java/pull/352

tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java

@@ -46,6 +46,7 @@ public final class TraceEnrichingFilter implements ContainerRequestFilter, Conta
      * This is the name of the trace id property we set on {@link ContainerRequestContext}.
      */
     public static final String TRACE_ID_PROPERTY_NAME = "com.palantir.tracing.traceId";
+    public static final String TOP_SPAN_ID_PROPERTY_NAME = "com.palantir.tracing.topSpanId";
     public static final String SAMPLED_PROPERTY_NAME = "com.palantir.tracing.sampled";
 
     @Context
@@ -82,6 +83,8 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
 
         // Give asynchronous downstream handlers access to the trace id
         requestContext.setProperty(TRACE_ID_PROPERTY_NAME, Tracer.getTraceId());
+        Tracer.getTopSpanId().ifPresent(localId ->
+                requestContext.setProperty(TOP_SPAN_ID_PROPERTY_NAME, localId));
         requestContext.setProperty(SAMPLED_PROPERTY_NAME, Tracer.isTraceObservable());
     }
 

tracing-jersey/src/test/java/com/palantir/tracing/jersey/TraceEnrichingFilterTest.java

@@ -117,6 +117,36 @@ public void testTraceState_withHeaderUsesTraceId() {
         assertThat(spanCaptor.getValue().getOperation()).isEqualTo("Jersey: GET /trace");
     }
 
+    @Test
+    public void testTraceState_withHeaderUsesTraceIdWithDifferentLocalIds() {
+        Response response = target.path("/top-span").request()
+                .header(TraceHttpHeaders.TRACE_ID, "traceId")
+                .header(TraceHttpHeaders.PARENT_SPAN_ID, "parentSpanId")
+                .header(TraceHttpHeaders.SPAN_ID, "spanId")
+                .get();
+        assertThat(response.getHeaderString(TraceHttpHeaders.TRACE_ID)).isEqualTo("traceId");
+        assertThat(response.getHeaderString(TraceHttpHeaders.PARENT_SPAN_ID)).isNull();
+        assertThat(response.getHeaderString(TraceHttpHeaders.SPAN_ID)).isNull();
+        verify(observer).consume(spanCaptor.capture());
+        assertThat(spanCaptor.getValue().getOperation()).isEqualTo("Jersey: GET /top-span");
+
+        String firstLocalTrace = response.readEntity(String.class);
+        assertThat(firstLocalTrace).isNotEmpty();
+
+        // make the query again
+        Response otherResponse = target.path("/top-span").request()
+                .header(TraceHttpHeaders.TRACE_ID, "traceId")
+                .header(TraceHttpHeaders.PARENT_SPAN_ID, "parentSpanId")
+                .header(TraceHttpHeaders.SPAN_ID, "spanId")
+                .get();
+        assertThat(otherResponse.getHeaderString(TraceHttpHeaders.TRACE_ID)).isEqualTo("traceId");
+        assertThat(otherResponse.getHeaderString(TraceHttpHeaders.PARENT_SPAN_ID)).isNull();
+        assertThat(otherResponse.getHeaderString(TraceHttpHeaders.SPAN_ID)).isNull();
+        String otherLocalTrace = otherResponse.readEntity(String.class);
+        assertThat(otherLocalTrace).isNotEmpty();
+        assertThat(otherLocalTrace).isNotEqualTo(firstLocalTrace);
+    }
+
     @Test
     public void testTraceState_respectsMethod() {
         Response response = target.path("/trace").request()
@@ -218,6 +248,8 @@ public void testFilter_setsMdcIfTraceIdHeaderIsPresent() throws Exception {
         TraceEnrichingFilter.INSTANCE.filter(request);
         assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isEqualTo("traceId");
         verify(request).setProperty(TraceEnrichingFilter.TRACE_ID_PROPERTY_NAME, "traceId");
+        // the top span id is randomly generated
+        verify(request).setProperty(eq(TraceEnrichingFilter.TOP_SPAN_ID_PROPERTY_NAME), anyString());
         // Note: this will be set to a random value; we want to check whether the value is being set
         verify(request).setProperty(eq(TraceEnrichingFilter.SAMPLED_PROPERTY_NAME), anyBoolean());
     }
@@ -262,6 +294,11 @@ public void getFailingTraceOperation() {
             throw new RuntimeException();
         }
 
+        @Override
+        public String getTopSpanId() {
+            return Tracer.getTopSpanId().get();
+        }
+
         @Override
         public StreamingOutput getFailingStreamingTraceOperation() {
             return os -> {
@@ -296,6 +333,10 @@ public interface TracingTestService {
         @Path("/failing-trace")
         void getFailingTraceOperation();
 
+        @GET
+        @Path("/top-span")
+        String getTopSpanId();
+
         @GET
         @Path("/failing-streaming-trace")
         @Produces(MediaType.APPLICATION_OCTET_STREAM)

tracing-undertow/src/test/java/com/palantir/tracing/undertow/TracedOperationHandlerTest.java

@@ -102,6 +102,29 @@ public void whenTraceIsInHeader_usesGivenTraceId() throws Exception {
         assertThat(exchange.getResponseHeaders().getFirst(TraceHttpHeaders.TRACE_ID)).isEqualTo(traceId);
     }
 
+    @Test
+    public void whenTraceIsInHeader_usesGivenTraceIdwithDifferentLocalIds() throws Exception {
+        setRequestTraceId(traceId);
+        AtomicReference<String> traceIdValue = new AtomicReference<>();
+        AtomicReference<String> localTraceIdValue = new AtomicReference<>();
+        TracedOperationHandler traceSettingHandler =
+                new TracedOperationHandler(exc -> {
+                    traceIdValue.set(Tracer.getTraceId());
+                    localTraceIdValue.set(Tracer.getTopSpanId().orElse(null));
+
+                }, "GET /traced");
+        traceSettingHandler.handleRequest(exchange);
+
+        assertThat(traceIdValue.get()).isEqualTo(traceId);
+        assertThat(localTraceIdValue).isNotNull();
+        String firstLocalId = localTraceIdValue.get();
+
+        traceSettingHandler.handleRequest(exchange);
+        assertThat(traceIdValue.get()).isEqualTo(traceId);
+        assertThat(localTraceIdValue.get()).isNotNull();
+        assertThat(localTraceIdValue.get()).isNotEqualTo(firstLocalId);
+    }
+
     @Test
     public void whenParentSpanIsGiven_usesParentSpan() throws Exception {
         setRequestTraceId(traceId);

tracing/src/main/java/com/palantir/tracing/Trace.java

@@ -136,6 +136,14 @@ final String getTraceId() {
         return traceId;
     }
 
+    /**
+     * The globally unique non-empty identifier for this call trace within a service (or another locality context).
+     *
+     * While {@link #getTraceId()} is expected to be propagated across RPC calls, the top span id distinguishes
+     * between two concurrent RPC calls made to a service with the same traceid.
+     */
+    abstract Optional<String> getTopSpanId();
+
     abstract Optional<String> getOriginatingSpanId();
 
     /** Returns a copy of this Trace which can be independently mutated. */
@@ -195,6 +203,14 @@ boolean isObservable() {
             return true;
         }
 
+        @Override
+        Optional<String> getTopSpanId() {
+            if (stack.isEmpty()) {
+                return Optional.empty();
+            }
+            return Optional.of(stack.peekLast().getSpanId());
+        }
+
         @Override
         Optional<String> getOriginatingSpanId() {
             if (stack.isEmpty()) {
@@ -220,36 +236,47 @@ private static final class Unsampled extends Trace {
          * This allows thread trace state to be cleared when all "started" spans have been "removed".
          */
         private int numberOfSpans;
-        private Optional<String> originatingSpanId = Optional.empty();
-
-        private Unsampled(int numberOfSpans, String traceId) {
+        private Optional<String> originatingSpanId;
+        private Optional<String> topSpanId;
+
+        private Unsampled(
+                int numberOfSpans,
+                String traceId,
+                Optional<String> originatingSpanId,
+                Optional<String> topSpanId) {
             super(traceId);
             this.numberOfSpans = numberOfSpans;
+            this.originatingSpanId = originatingSpanId;
+            this.topSpanId = topSpanId;
             validateNumberOfSpans();
         }
 
         private Unsampled(String traceId) {
-            this(0, traceId);
+            this(0, traceId, Optional.empty(), Optional.empty());
         }
 
         @Override
         void fastStartSpan(String _operation, String parentSpanId, SpanType _type) {
-            startSpan(Optional.of(parentSpanId));
+            if (numberOfSpans == 0) {
+                originatingSpanId = Optional.of(parentSpanId);
+                topSpanId = Optional.of(Tracers.randomId());
+            }
+            numberOfSpans++;
         }
 
         @Override
         void fastStartSpan(String _operation, SpanType _type) {
+            if (numberOfSpans == 0) {
+                topSpanId = Optional.of(Tracers.randomId());
+            }
             numberOfSpans++;
         }
 
         @Override
         protected void push(OpenSpan span) {
-            startSpan(span.getParentSpanId());
-        }
-
-        private void startSpan(Optional<String> parentSpanId) {
             if (numberOfSpans == 0) {
-                originatingSpanId = parentSpanId;
+                originatingSpanId = span.getParentSpanId();
+                topSpanId = Optional.of(span.getSpanId());
             }
             numberOfSpans++;
         }
@@ -267,6 +294,7 @@ Optional<OpenSpan> pop() {
             }
             if (numberOfSpans == 0) {
                 originatingSpanId = Optional.empty();
+                topSpanId = Optional.empty();
             }
             return Optional.empty();
         }
@@ -282,14 +310,19 @@ boolean isObservable() {
             return false;
         }
 
+        @Override
+        Optional<String> getTopSpanId() {
+            return topSpanId;
+        }
+
         @Override
         Optional<String> getOriginatingSpanId() {
             return originatingSpanId;
         }
 
         @Override
         Trace deepCopy() {
-            return new Unsampled(numberOfSpans, getTraceId());
+            return new Unsampled(numberOfSpans, getTraceId(), getOriginatingSpanId(), getTopSpanId());
         }
 
         /** Internal validation, this should never fail because {@link #pop()} only decrements positive values. */

tracing/src/main/java/com/palantir/tracing/Tracer.java

@@ -483,6 +483,11 @@ public static String getTraceId() {
         return checkNotNull(currentTrace.get(), "There is no trace").getTraceId();
     }
 
+    /** Returns the globally unique identifier for this thread's trace specific to this call. */
+    public static Optional<String> getTopSpanId() {
+        return checkNotNull(currentTrace.get(), "There is no trace").getTopSpanId();
+    }
+
     /** Clears the current trace id and returns it if present. */
     static Optional<Trace> getAndClearTraceIfPresent() {
         Optional<Trace> trace = Optional.ofNullable(currentTrace.get());